From 065a0335d4337b6422f9b9f24e648a37e2e0f634 Mon Sep 17 00:00:00 2001 From: Anatoly Milkov Date: Tue, 19 Jun 2018 23:46:42 -0700 Subject: [PATCH] add avatar caching #85 --- app/rest/proxy/avatar.go | 4 +-- app/rest/proxy/avatar_store.go | 16 ++++++++- app/rest/proxy/avatar_store_test.go | 51 ++++++++++++++++++++++++++--- app/rest/proxy/avatar_test.go | 31 +++++++++++++++++- 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/app/rest/proxy/avatar.go b/app/rest/proxy/avatar.go index c07205f1..97358838 100644 --- a/app/rest/proxy/avatar.go +++ b/app/rest/proxy/avatar.go @@ -75,9 +75,9 @@ func (p *Avatar) Routes(middlewares ...func(http.Handler) http.Handler) (string, avatar := chi.URLParam(r, "avatar") // enforce client-side caching - etag := `"` + avatar + `"` + etag := `"` + p.Store.ID(avatar) + `"` w.Header().Set("Etag", etag) - w.Header().Set("Cache-Control", "max-age=2592000") // 30 days + w.Header().Set("Cache-Control", "max-age=604800") // 7 days if match := r.Header.Get("If-None-Match"); match != "" { if strings.Contains(match, etag) { w.WriteHeader(http.StatusNotModified) diff --git a/app/rest/proxy/avatar_store.go b/app/rest/proxy/avatar_store.go index 22b3bc63..a5cef33f 100644 --- a/app/rest/proxy/avatar_store.go +++ b/app/rest/proxy/avatar_store.go @@ -12,6 +12,7 @@ import ( "log" "os" "path" + "strconv" "strings" "sync" @@ -29,6 +30,7 @@ import ( type AvatarStore interface { Put(userID string, reader io.Reader) (avatar string, err error) Get(avatar string) (reader io.ReadCloser, size int, err error) + ID(avatar string) (id string) } // FSAvatarStore implements AvatarStore for local file system @@ -83,7 +85,7 @@ func (fs *FSAvatarStore) Get(avatar string) (reader io.ReadCloser, size int, err avFile := path.Join(location, avatar) fh, err := os.Open(avFile) if err != nil { - return nil, 0, errors.Wrapf(err, "can't load avatar %s, id") + return nil, 0, errors.Wrapf(err, "can't load avatar %s, id", avatar) } if fi, e := fh.Stat(); e == nil { size = int(fi.Size()) @@ -91,6 +93,18 @@ func (fs *FSAvatarStore) Get(avatar string) (reader io.ReadCloser, size int, err return fh, size, nil } +// ID returns a fingerprint of the avatar content. +func (fs *FSAvatarStore) ID(avatar string) (id string) { + location := fs.location(strings.TrimSuffix(avatar, imgSfx)) + avFile := path.Join(location, avatar) + fi, err := os.Stat(avFile) + if err != nil { + log.Printf("[DEBUG] can't get file info '%s', %s", avFile, err) + return store.EncodeID(avatar) + } + return store.EncodeID(avatar + strconv.FormatInt(fi.ModTime().Unix(), 10)) +} + // get location (directory) for user id by adding partition to final path in order to keep files // in different subdirectories and avoid too many files in a single place. // the end result is a full path like this - /tmp/avatars.test/92 diff --git a/app/rest/proxy/avatar_store_test.go b/app/rest/proxy/avatar_store_test.go index 702c881b..34c8291a 100644 --- a/app/rest/proxy/avatar_store_test.go +++ b/app/rest/proxy/avatar_store_test.go @@ -8,6 +8,7 @@ import ( "os" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -15,7 +16,8 @@ import ( func TestAvatarStore_Put(t *testing.T) { p := NewFSAvatarStore("/tmp/avatars.test", 300) - os.MkdirAll("/tmp/avatars.test", 0700) + err := os.MkdirAll("/tmp/avatars.test", 0700) + require.NoError(t, err) defer os.RemoveAll("/tmp/avatars.test") avatar, err := p.Put("user1", nil) @@ -53,11 +55,21 @@ func TestAvatarStore_Put(t *testing.T) { func TestAvatarStore_Get(t *testing.T) { p := NewFSAvatarStore("/tmp/avatars.test", 300) - os.MkdirAll("/tmp/avatars.test/30", 0700) + err := os.MkdirAll("/tmp/avatars.test/30", 0700) + require.NoError(t, err) defer os.RemoveAll("/tmp/avatars.test") - err := ioutil.WriteFile("/tmp/avatars.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image", []byte("something"), 0666) + + // file not exists + r, size, err := p.Get("some_random_name.image") + // nil, 0, errors.Wrapf(err, "can't load avatar %s, id") + assert.Nil(t, r) + assert.Equal(t, 0, size) + assert.EqualError(t, err, "can't load avatar some_random_name.image, id: open /tmp/avatars.test/91/some_random_name.image: no such file or directory") + // file exists + err = ioutil.WriteFile("/tmp/avatars.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image", []byte("something"), 0666) assert.Nil(t, err) - r, size, err := p.Get("b3daa77b4c04a9551b8781d03191fe098f325e67.image") + r, size, err = p.Get("b3daa77b4c04a9551b8781d03191fe098f325e67.image") + assert.Nil(t, err) assert.Equal(t, 9, size) data, err := ioutil.ReadAll(r) @@ -133,3 +145,34 @@ func TestAvatarStore_resize(t *testing.T) { assert.Equalf(t, c.hr, bounds.Dy(), "file %s", c.file) } } + +func TestAvatarStore_ID(t *testing.T) { + p := NewFSAvatarStore("/tmp/avatars.test", 300) + err := os.MkdirAll("/tmp/avatars.test/30", 0700) + require.NoError(t, err) + defer os.RemoveAll("/tmp/avatars.test") + + // file not exists + id := p.ID("some_random_name.image") + assert.Equal(t, "a008de0a2ccb3308b5d99ffff66436e15538f701", id) // store.EncodeID("some_random_name.image") + // file exists + err = ioutil.WriteFile("/tmp/avatars.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image", []byte("something"), 0666) + require.NoError(t, err) + touch := time.Date(2017, 7, 14, 2, 40, 0, 0, time.UTC) // 1500000000 + err = os.Chtimes("/tmp/avatars.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image", touch, touch) + require.NoError(t, err) + id = p.ID("b3daa77b4c04a9551b8781d03191fe098f325e67.image") + assert.Equal(t, "325d5b451f32c2f8e7f30a9fd65bff6a42954d9a", id) // store.EncodeID("b3daa77b4c04a9551b8781d03191fe098f325e67.image1500000000") +} +func BenchmarkAvatarStore_ID(b *testing.B) { + p := NewFSAvatarStore("/tmp/avatars.test", 300) + os.MkdirAll("/tmp/avatars.test/30", 0700) + defer os.RemoveAll("/tmp/avatars.test") + err := ioutil.WriteFile("/tmp/avatars.test/30/b3daa77b4c04a9551b8781d03191fe098f325e67.image", []byte("something"), 0666) + require.NoError(b, err) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + p.ID("b3daa77b4c04a9551b8781d03191fe098f325e67.image") + } +} diff --git a/app/rest/proxy/avatar_test.go b/app/rest/proxy/avatar_test.go index 5d1a523a..6bc24414 100644 --- a/app/rest/proxy/avatar_test.go +++ b/app/rest/proxy/avatar_test.go @@ -97,7 +97,8 @@ func TestAvatar_Routes(t *testing.T) { _, err := p.Put(u) assert.NoError(t, err) - req, err := http.NewRequest("GET", "/b3daa77b4c04a9551b8781d03191fe098f325e67.image", nil) + // status 400 + req, err := http.NewRequest("GET", "/some_random_name.image", nil) if err != nil { t.Fatal(err) } @@ -107,6 +108,19 @@ func TestAvatar_Routes(t *testing.T) { handler := http.Handler(routes) handler.ServeHTTP(rr, req) + assert.Equal(t, http.StatusBadRequest, rr.Code) + + // status 200 + req, err = http.NewRequest("GET", "/b3daa77b4c04a9551b8781d03191fe098f325e67.image", nil) + if err != nil { + t.Fatal(err) + } + + rr = httptest.NewRecorder() + _, routes = p.Routes() + handler = http.Handler(routes) + handler.ServeHTTP(rr, req) + assert.Equal(t, http.StatusOK, rr.Code) assert.Equal(t, []string{"image/*"}, rr.HeaderMap["Content-Type"]) @@ -119,6 +133,21 @@ func TestAvatar_Routes(t *testing.T) { assert.NoError(t, err) assert.Equal(t, int64(21), sz) assert.Equal(t, "some picture bin data", bb.String()) + + // status 304 + req, err = http.NewRequest("GET", "/some_random_name.image", nil) + if err != nil { + t.Fatal(err) + } + req.Header.Add("If-None-Match", `"a008de0a2ccb3308b5d99ffff66436e15538f701"`) // hash of `some_random_name.image` since the file doesn't exist + + rr = httptest.NewRecorder() + _, routes = p.Routes() + handler = http.Handler(routes) + handler.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusNotModified, rr.Code) + assert.Equal(t, []string{`"a008de0a2ccb3308b5d99ffff66436e15538f701"`}, rr.HeaderMap["Etag"]) } func TestAvatar_Retry(t *testing.T) {