Use updated etag of home directory even if it is cached (#1416)

cs3org · Jan 21, 2021 · 44870ec · 44870ec
1 parent 7fe671a
commit 44870ec
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 6 deletions.
diff --git a/changelog/unreleased/etag-cache-fix.md b/changelog/unreleased/etag-cache-fix.md
@@ -0,0 +1,8 @@
+Enhancement: Use updated etag of home directory even if it is cached
+
+We cache the home directory and shares folder etags as calculating these is an
+expensive process. But if these directories were updated after the previously
+calculated etag was cached, we can ignore this calculation and directly return
+the new one.
+
+https://github.com/cs3org/reva/pull/1416
diff --git a/internal/grpc/services/gateway/storageprovider.go b/internal/grpc/services/gateway/storageprovider.go
@@ -37,6 +37,7 @@ import (
 	"github.com/cs3org/reva/pkg/storage/utils/etag"
 	"github.com/cs3org/reva/pkg/storage/utils/templates"
 	"github.com/cs3org/reva/pkg/user"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/dgrijalva/jwt-go"
 	"github.com/pkg/errors"
 )
@@ -985,12 +986,17 @@ func (s *svc) statHome(ctx context.Context) (*provider.StatResponse, error) {
 		}, nil
 	}
 
-	if resEtag, err := s.etagCache.Get(statRes.Info.Owner.OpaqueId + ":" + statRes.Info.Path); err == nil {
-		statRes.Info.Etag = resEtag.(string)
+	if etagIface, err := s.etagCache.Get(statRes.Info.Owner.OpaqueId + ":" + statRes.Info.Path); err == nil {
+		resMtime := utils.TSToTime(statRes.Info.Mtime)
+		resEtag := etagIface.(etagWithTS)
+		// Use the updated etag if the home folder has been modified
+		if resMtime.Before(resEtag.Timestamp) {
+			statRes.Info.Etag = resEtag.Etag
+		}
 	} else {
 		statRes.Info.Etag = etag.GenerateEtagFromResources(statRes.Info, []*provider.ResourceInfo{statSharedFolder.Info})
 		if s.c.EtagCacheTTL > 0 {
-			_ = s.etagCache.Set(statRes.Info.Owner.OpaqueId+":"+statRes.Info.Path, statRes.Info.Etag)
+			_ = s.etagCache.Set(statRes.Info.Owner.OpaqueId+":"+statRes.Info.Path, etagWithTS{statRes.Info.Etag, time.Now()})
 		}
 	}
 
@@ -1029,12 +1035,18 @@ func (s *svc) statSharesFolder(ctx context.Context) (*provider.StatResponse, err
 		}, nil
 	}
 
-	if resEtag, err := s.etagCache.Get(statRes.Info.Owner.OpaqueId + ":" + statRes.Info.Path); err == nil {
-		statRes.Info.Etag = resEtag.(string)
+	if etagIface, err := s.etagCache.Get(statRes.Info.Owner.OpaqueId + ":" + statRes.Info.Path); err == nil {
+		resMtime := utils.TSToTime(statRes.Info.Mtime)
+		resEtag := etagIface.(etagWithTS)
+		// Use the updated etag if the shares folder has been modified, i.e., a new
+		// reference has been created.
+		if resMtime.Before(resEtag.Timestamp) {
+			statRes.Info.Etag = resEtag.Etag
+		}
 	} else {
 		statRes.Info.Etag = etag.GenerateEtagFromResources(statRes.Info, lsRes.Infos)
 		if s.c.EtagCacheTTL > 0 {
-			_ = s.etagCache.Set(statRes.Info.Owner.OpaqueId+":"+statRes.Info.Path, statRes.Info.Etag)
+			_ = s.etagCache.Set(statRes.Info.Owner.OpaqueId+":"+statRes.Info.Path, etagWithTS{statRes.Info.Etag, time.Now()})
 		}
 	}
 	return statRes, nil
@@ -1852,3 +1864,8 @@ func (s *svc) getStorageProvider(ctx context.Context, ref *provider.Reference) (
 
 	return res.Provider, nil
 }
+
+type etagWithTS struct {
+	Etag      string
+	Timestamp time.Time
+}