diff --git a/blob.go b/blob.go index d0ace9e80..4a63f31ca 100644 --- a/blob.go +++ b/blob.go @@ -9,6 +9,7 @@ import ( "net/http" "os" "sync" + "time" ) type BlobType int @@ -41,6 +42,15 @@ type Blob struct { filepath string contentType string memory *memory + + Stat *Stat +} + +// Stat blob stat attributes +type Stat struct { + ModifiedTime time.Time + ETag string + Size int64 } func NewBlob(newReader func() (reader io.ReadCloser, size int64, err error)) *Blob { @@ -62,7 +72,7 @@ func NewBlobFromFile(filepath string, checks ...func(os.FileInfo) error) *Blob { } } } - return &Blob{ + blob := &Blob{ err: err, filepath: filepath, fanout: true, @@ -74,6 +84,15 @@ func NewBlobFromFile(filepath string, checks ...func(os.FileInfo) error) *Blob { return reader, stat.Size(), err }, } + if err == nil && stat != nil { + size := stat.Size() + modTime := stat.ModTime() + blob.Stat = &Stat{ + Size: size, + ModifiedTime: modTime, + } + } + return blob } func NewBlobFromJsonMarshal(v any) *Blob { diff --git a/context.go b/context.go index f97765781..20a06a156 100644 --- a/context.go +++ b/context.go @@ -11,7 +11,6 @@ type imagorContextKey struct{} type imagorContextRef struct { funcs []func() l sync.Mutex - Cache sync.Map } func (r *imagorContextRef) Defer(fn func()) { @@ -51,18 +50,3 @@ func mustContextValue(ctx context.Context) *imagorContextRef { func Defer(ctx context.Context, fn func()) { mustContextValue(ctx).Defer(fn) } - -// ContextCachePut put cache within the imagor request context lifetime -func ContextCachePut(ctx context.Context, key any, val any) { - if r, ok := ctx.Value(imagorContextKey{}).(*imagorContextRef); ok && r != nil { - r.Cache.Store(key, val) - } -} - -// ContextCacheGet get cache within the imagor request context lifetime -func ContextCacheGet(ctx context.Context, key any) (any, bool) { - if r, ok := ctx.Value(imagorContextKey{}).(*imagorContextRef); ok && r != nil { - return r.Cache.Load(key) - } - return nil, false -} diff --git a/context_test.go b/context_test.go index dd4a30a97..2f4e80f4d 100644 --- a/context_test.go +++ b/context_test.go @@ -30,18 +30,3 @@ func TestDefer(t *testing.T) { }) assert.Equal(t, 2, called, "should count all defers before cancel") } - -func TestContextCache(t *testing.T) { - ctx := context.Background() - assert.NotPanics(t, func() { - ContextCachePut(ctx, "foo", "bar") - }) - ctx = WithContext(ctx) - s, ok := ContextCacheGet(ctx, "foo") - assert.False(t, ok) - assert.Nil(t, s) - ContextCachePut(ctx, "foo", "bar") - s, ok = ContextCacheGet(ctx, "foo") - assert.True(t, ok) - assert.Equal(t, "bar", s) -} diff --git a/imagor.go b/imagor.go index eb7842a16..1f3845269 100644 --- a/imagor.go +++ b/imagor.go @@ -44,12 +44,6 @@ type Processor interface { Shutdown(ctx context.Context) error } -// Stat image attributes -type Stat struct { - ModifiedTime time.Time - Size int64 -} - // Imagor image resize HTTP handler type Imagor struct { Unsafe bool @@ -191,10 +185,14 @@ func (app *Imagor) ServeHTTP(w http.ResponseWriter, r *http.Request) { if isBlobEmpty(blob) { return } - reader, size, _ := blob.NewReader() w.Header().Set("Content-Type", blob.ContentType()) w.Header().Set("Content-Disposition", getContentDisposition(p, blob)) - setCacheHeaders(w, app.CacheHeaderTTL, app.CacheHeaderSWR, app.Debug) + setCacheHeaders(w, r, app.CacheHeaderTTL, app.CacheHeaderSWR) + if checkStatNotModified(w, r, blob.Stat) { + w.WriteHeader(http.StatusNotModified) + return + } + reader, size, _ := blob.NewReader() writeBody(w, r, reader, size) return } @@ -383,12 +381,10 @@ func (app *Imagor) loadResult(r *http.Request, resultKey, imageKey string) *Blob ctx := r.Context() blob, origin, err := fromStorages(r, app.ResultStorages, resultKey) if err == nil && !isBlobEmpty(blob) { - if app.ModifiedTimeCheck && origin != nil { - if resStat, err1 := origin.Stat(ctx, resultKey); resStat != nil && err1 == nil { - if sourceStat, err2 := app.storageStat(ctx, imageKey); sourceStat != nil && err2 == nil { - if !resStat.ModifiedTime.Before(sourceStat.ModifiedTime) { - return blob - } + if app.ModifiedTimeCheck && origin != nil && blob.Stat != nil { + if sourceStat, err2 := app.storageStat(ctx, imageKey); sourceStat != nil && err2 == nil { + if !blob.Stat.ModifiedTime.Before(sourceStat.ModifiedTime) { + return blob } } } else { @@ -595,8 +591,42 @@ func (app *Imagor) debugLog() { ) } -func setCacheHeaders(w http.ResponseWriter, ttl, swr time.Duration, isDebug bool) { - if isDebug { +func checkStatNotModified(w http.ResponseWriter, r *http.Request, stat *Stat) bool { + if stat == nil || strings.Contains(r.Header.Get("Cache-Control"), "no-cache") { + return false + } + var isETagMatch, isNotModified bool + var etag = stat.ETag + if etag == "" && stat.Size > 0 && !stat.ModifiedTime.IsZero() { + etag = fmt.Sprintf( + "%x-%x", int(stat.ModifiedTime.Unix()), int(stat.Size)) + } + if etag != "" { + w.Header().Set("ETag", etag) + if inm := r.Header.Get("If-None-Match"); inm == etag { + isETagMatch = true + } + } + if mTime := stat.ModifiedTime; !mTime.IsZero() { + w.Header().Set("Last-Modified", mTime.Format(http.TimeFormat)) + if ims := r.Header.Get("If-Modified-Since"); ims != "" { + if imsTime, err := time.Parse(http.TimeFormat, ims); err == nil { + isNotModified = mTime.Before(imsTime) + } + } + if !isNotModified { + if ius := r.Header.Get("If-Unmodified-Since"); ius != "" { + if iusTime, err := time.Parse(http.TimeFormat, ius); err == nil { + isNotModified = mTime.After(iusTime) + } + } + } + } + return isETagMatch || isNotModified +} + +func setCacheHeaders(w http.ResponseWriter, r *http.Request, ttl, swr time.Duration) { + if strings.Contains(r.Header.Get("Cache-Control"), "no-cache") { ttl = 0 } expires := time.Now().Add(ttl) diff --git a/imagor_test.go b/imagor_test.go index ed3b78f66..efeb2335f 100644 --- a/imagor_test.go +++ b/imagor_test.go @@ -328,21 +328,6 @@ func TestWithCacheHeaderTTL(t *testing.T) { assert.Equal(t, 200, w.Code) assert.Equal(t, "public, s-maxage=169, max-age=169, no-transform", w.Header().Get("Cache-Control")) }) - t.Run("custom ttl debug no cache", func(t *testing.T) { - app := New( - WithDebug(true), - WithLogger(zap.NewExample()), - WithCacheHeaderSWR(time.Second*169), - WithCacheHeaderTTL(time.Second*169), - WithLoaders(loader), - WithUnsafe(true)) - w := httptest.NewRecorder() - app.ServeHTTP(w, httptest.NewRequest( - http.MethodGet, "https://example.com/unsafe/foo.jpg", nil)) - assert.Equal(t, 200, w.Code) - assert.NotEmpty(t, w.Header().Get("Expires")) - assert.Equal(t, "private, no-cache, no-store, must-revalidate", w.Header().Get("Cache-Control")) - }) t.Run("no cache", func(t *testing.T) { app := New( WithLoaders(loader), @@ -423,7 +408,7 @@ func TestParams(t *testing.T) { var clock time.Time type mapStore struct { - l sync.Mutex + l sync.RWMutex Map map[string]*Blob ModTime map[string]time.Time LoadCnt map[string]int @@ -439,12 +424,13 @@ func newMapStore() *mapStore { } func (s *mapStore) Get(r *http.Request, image string) (*Blob, error) { - s.l.Lock() - defer s.l.Unlock() + s.l.RLock() + defer s.l.RUnlock() buf, ok := s.Map[image] if !ok { return nil, ErrNotFound } + buf.Stat, _ = s.Stat(r.Context(), image) s.LoadCnt[image] = s.LoadCnt[image] + 1 return buf, nil } @@ -468,14 +454,19 @@ func (s *mapStore) Delete(ctx context.Context, image string) error { } func (s *mapStore) Stat(ctx context.Context, image string) (*Stat, error) { - s.l.Lock() - defer s.l.Unlock() + s.l.RLock() + defer s.l.RUnlock() t, ok := s.ModTime[image] if !ok { return nil, ErrNotFound } + b, ok := s.Map[image] + if !ok { + return nil, ErrNotFound + } return &Stat{ ModifiedTime: t, + Size: b.Size(), }, nil } @@ -681,6 +672,87 @@ func TestWithLoadersStoragesProcessors(t *testing.T) { } } +func TestWithResultStorageNotModified(t *testing.T) { + resultStore := newMapStore() + app := New( + WithDebug(true), + WithLogger(zap.NewExample()), + WithResultStorages(resultStore), + WithLoaders(loaderFunc(func(r *http.Request, image string) (*Blob, error) { + return NewBlobFromBytes([]byte(image)), nil + })), + WithUnsafe(true), + ) + w := httptest.NewRecorder() + r := httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + app.ServeHTTP(w, r) + time.Sleep(time.Millisecond * 10) // make sure storage reached + assert.Equal(t, 200, w.Code) + assert.Equal(t, "foo", w.Body.String()) + assert.Empty(t, w.Header().Get("ETag")) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + app.ServeHTTP(w, r) + assert.Equal(t, 200, w.Code) + assert.Equal(t, "foo", w.Body.String()) + etag := w.Header().Get("ETag") + lastModified := w.Header().Get("Last-Modified") + assert.NotEmpty(t, etag) + assert.NotEmpty(t, lastModified) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-None-Match", etag) + app.ServeHTTP(w, r) + assert.Equal(t, 304, w.Code) + assert.Empty(t, w.Body.String()) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-None-Match", etag) + r.Header.Set("Cache-Control", "no-cache") + app.ServeHTTP(w, r) + assert.Equal(t, 200, w.Code) + assert.Equal(t, "foo", w.Body.String()) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-None-Match", "abcd") + app.ServeHTTP(w, r) + assert.Equal(t, 200, w.Code) + assert.Equal(t, "foo", w.Body.String()) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-Modified-Since", clock.Add(time.Hour).Format(http.TimeFormat)) + app.ServeHTTP(w, r) + assert.Equal(t, 304, w.Code) + assert.Empty(t, w.Body.String()) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-Modified-Since", time.Time{}.Format(http.TimeFormat)) + app.ServeHTTP(w, r) + assert.Equal(t, 200, w.Code) + assert.Equal(t, "foo", w.Body.String()) + + w = httptest.NewRecorder() + r = httptest.NewRequest( + http.MethodGet, "https://example.com/unsafe/foo", nil) + r.Header.Set("If-Unmodified-Since", time.Time{}.Format(http.TimeFormat)) + app.ServeHTTP(w, r) + assert.Equal(t, 304, w.Code) + assert.Empty(t, w.Body.String()) +} + type storageKeyFunc func(img string) string func (fn storageKeyFunc) Hash(img string) string { diff --git a/storage/filestorage/filestorage.go b/storage/filestorage/filestorage.go index 1e224406e..77d36d187 100644 --- a/storage/filestorage/filestorage.go +++ b/storage/filestorage/filestorage.go @@ -15,10 +15,6 @@ import ( var dotFileRegex = regexp.MustCompile("/\\.") -type statKey struct { - Key string -} - type FileStorage struct { BaseDir string PathPrefix string @@ -60,16 +56,12 @@ func (s *FileStorage) Path(image string) (string, bool) { return filepath.Join(s.BaseDir, strings.TrimPrefix(image, s.PathPrefix)), true } -func (s *FileStorage) Get(r *http.Request, image string) (*imagor.Blob, error) { +func (s *FileStorage) Get(_ *http.Request, image string) (*imagor.Blob, error) { image, ok := s.Path(image) if !ok { return nil, imagor.ErrInvalid } return imagor.NewBlobFromFile(image, func(stat os.FileInfo) error { - imagor.ContextCachePut(r.Context(), statKey{image}, imagor.Stat{ - Size: stat.Size(), - ModifiedTime: stat.ModTime(), - }) if s.Expiration > 0 && time.Now().Sub(stat.ModTime()) > s.Expiration { return imagor.ErrExpired } @@ -117,16 +109,11 @@ func (s *FileStorage) Delete(_ context.Context, image string) error { return os.Remove(image) } -func (s *FileStorage) Stat(ctx context.Context, image string) (stat *imagor.Stat, err error) { +func (s *FileStorage) Stat(_ context.Context, image string) (stat *imagor.Stat, err error) { image, ok := s.Path(image) if !ok { return nil, imagor.ErrInvalid } - if s, ok := imagor.ContextCacheGet(ctx, statKey{image}); ok && s != nil { - if stat, ok2 := s.(imagor.Stat); ok2 { - return &stat, nil - } - } osStat, err := os.Stat(image) if err != nil { if os.IsNotExist(err) { @@ -134,8 +121,10 @@ func (s *FileStorage) Stat(ctx context.Context, image string) (stat *imagor.Stat } return nil, err } + size := osStat.Size() + modTime := osStat.ModTime() return &imagor.Stat{ - Size: osStat.Size(), - ModifiedTime: osStat.ModTime(), + Size: size, + ModifiedTime: modTime, }, nil } diff --git a/storage/gcloudstorage/gcloudstorage.go b/storage/gcloudstorage/gcloudstorage.go index 104bfad98..f33d4d871 100644 --- a/storage/gcloudstorage/gcloudstorage.go +++ b/storage/gcloudstorage/gcloudstorage.go @@ -25,10 +25,6 @@ type GCloudStorage struct { safeChars imagorpath.SafeChars } -type statKey struct { - Key string -} - func New(client *storage.Client, bucket string, options ...Option) *GCloudStorage { s := &GCloudStorage{client: client, Bucket: bucket} for _, option := range options { @@ -52,24 +48,26 @@ func (s *GCloudStorage) Get(r *http.Request, image string) (imageData *imagor.Bl } return nil, err } - if attrs != nil { - imagor.ContextCachePut(ctx, statKey{image}, imagor.Stat{ - Size: attrs.Size, - ModifiedTime: attrs.Updated, - }) - } if s.Expiration > 0 { if attrs != nil && time.Now().Sub(attrs.Updated) > s.Expiration { return nil, imagor.ErrExpired } } - return imagor.NewBlob(func() (reader io.ReadCloser, size int64, err error) { + blob := imagor.NewBlob(func() (reader io.ReadCloser, size int64, err error) { if attrs != nil { size = attrs.Size } reader, err = object.NewReader(ctx) return - }), err + }) + if attrs != nil { + blob.Stat = &imagor.Stat{ + Size: attrs.Size, + ETag: attrs.Etag, + ModifiedTime: attrs.Updated, + } + } + return blob, err } func (s *GCloudStorage) Put(ctx context.Context, image string, blob *imagor.Blob) (err error) { @@ -121,11 +119,6 @@ func (s *GCloudStorage) Stat(ctx context.Context, image string) (stat *imagor.St if !ok { return nil, imagor.ErrInvalid } - if s, ok := imagor.ContextCacheGet(ctx, statKey{image}); ok && s != nil { - if stat, ok2 := s.(imagor.Stat); ok2 { - return &stat, nil - } - } object := s.client.Bucket(s.Bucket).Object(image) attrs, err := object.Attrs(ctx) if err != nil { @@ -136,6 +129,7 @@ func (s *GCloudStorage) Stat(ctx context.Context, image string) (stat *imagor.St } return &imagor.Stat{ Size: attrs.Size, + ETag: attrs.Etag, ModifiedTime: attrs.Updated, }, nil } diff --git a/storage/gcloudstorage/gcloudstorage_test.go b/storage/gcloudstorage/gcloudstorage_test.go index f156afcf9..dc7201c50 100644 --- a/storage/gcloudstorage/gcloudstorage_test.go +++ b/storage/gcloudstorage/gcloudstorage_test.go @@ -123,16 +123,16 @@ func TestCRUD(t *testing.T) { stat, err := s.Stat(ctx, "/foo/fooo/asdf") require.NoError(t, err) assert.True(t, stat.ModifiedTime.Before(time.Now())) + assert.NotEmpty(t, stat.ETag) b, err = s.Get(r, "/foo/fooo/asdf") require.NoError(t, err) buf, err := b.ReadAll() require.NoError(t, err) assert.Equal(t, "bar", string(buf)) - - stat, err = s.Stat(ctx, "/foo/fooo/asdf") - require.NoError(t, err) - assert.True(t, stat.ModifiedTime.Before(time.Now())) + assert.NotEmpty(t, b.Stat) + assert.Equal(t, stat.ModifiedTime, b.Stat.ModifiedTime) + assert.Equal(t, stat.ETag, b.Stat.ETag) err = s.Delete(ctx, "/foo/fooo/asdf") require.NoError(t, err) diff --git a/storage/s3storage/s3storage.go b/storage/s3storage/s3storage.go index 30f90f7fa..e8fddfb7f 100644 --- a/storage/s3storage/s3storage.go +++ b/storage/s3storage/s3storage.go @@ -13,6 +13,7 @@ import ( "net/http" "path/filepath" "strings" + "sync" "time" ) @@ -31,10 +32,6 @@ type S3Storage struct { safeChars imagorpath.SafeChars } -type statKey struct { - Key string -} - func New(sess *session.Session, bucket string, options ...Option) *S3Storage { baseDir := "/" if idx := strings.Index(bucket, "/"); idx > -1 { @@ -73,7 +70,9 @@ func (s *S3Storage) Get(r *http.Request, image string) (*imagor.Blob, error) { if !ok { return nil, imagor.ErrInvalid } - return imagor.NewBlob(func() (io.ReadCloser, int64, error) { + var blob *imagor.Blob + var once sync.Once + blob = imagor.NewBlob(func() (io.ReadCloser, int64, error) { input := &s3.GetObjectInput{ Bucket: aws.String(s.Bucket), Key: aws.String(image), @@ -84,9 +83,12 @@ func (s *S3Storage) Get(r *http.Request, image string) (*imagor.Blob, error) { } else if err != nil { return nil, 0, err } - imagor.ContextCachePut(ctx, statKey{image}, imagor.Stat{ - Size: *out.ContentLength, - ModifiedTime: *out.LastModified, + once.Do(func() { + blob.Stat = &imagor.Stat{ + Size: *out.ContentLength, + ETag: *out.ETag, + ModifiedTime: *out.LastModified, + } }) if s.Expiration > 0 && out.LastModified != nil { if time.Now().Sub(*out.LastModified) > s.Expiration { @@ -98,7 +100,8 @@ func (s *S3Storage) Get(r *http.Request, image string) (*imagor.Blob, error) { size = *out.ContentLength } return out.Body, size, nil - }), nil + }) + return blob, nil } func (s *S3Storage) Put(ctx context.Context, image string, blob *imagor.Blob) error { @@ -143,11 +146,6 @@ func (s *S3Storage) Stat(ctx context.Context, image string) (stat *imagor.Stat, if !ok { return nil, imagor.ErrInvalid } - if s, ok := imagor.ContextCacheGet(ctx, statKey{image}); ok && s != nil { - if stat, ok2 := s.(imagor.Stat); ok2 { - return &stat, nil - } - } input := &s3.HeadObjectInput{ Bucket: aws.String(s.Bucket), Key: aws.String(image), @@ -160,6 +158,7 @@ func (s *S3Storage) Stat(ctx context.Context, image string) (stat *imagor.Stat, } return &imagor.Stat{ Size: *head.ContentLength, + ETag: *head.ETag, ModifiedTime: *head.LastModified, }, nil } diff --git a/storage/s3storage/s3storage_test.go b/storage/s3storage/s3storage_test.go index 44a8161d7..09df84170 100644 --- a/storage/s3storage/s3storage_test.go +++ b/storage/s3storage/s3storage_test.go @@ -175,16 +175,16 @@ func TestCRUD(t *testing.T) { stat, err := s.Stat(ctx, "/foo/fooo/asdf") require.NoError(t, err) assert.True(t, stat.ModifiedTime.Before(time.Now())) + assert.NotEmpty(t, stat.ETag) b, err = s.Get(r, "/foo/fooo/asdf") require.NoError(t, err) buf, err := b.ReadAll() require.NoError(t, err) assert.Equal(t, "bar", string(buf)) - - stat, err = s.Stat(ctx, "/foo/fooo/asdf") - require.NoError(t, err) - assert.True(t, stat.ModifiedTime.Before(time.Now())) + assert.NotEmpty(t, b.Stat) + assert.Equal(t, stat.ModifiedTime, b.Stat.ModifiedTime) + assert.NotEmpty(t, stat.ETag, b.Stat.ETag) err = s.Delete(ctx, "/foo/fooo/asdf") require.NoError(t, err)