revert mtime change on error

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Aug 22, 2022 · 18f2d66 · 18f2d66
1 parent 285e35c
commit 18f2d66
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 12 deletions.
diff --git a/pkg/share/manager/jsoncs3/providercache/providercache.go b/pkg/share/manager/jsoncs3/providercache/providercache.go
@@ -135,27 +135,41 @@ func (c *Cache) ListSpace(storageID, spaceID string) *Shares {
 	return c.Providers[storageID].Spaces[spaceID]
 }
 
-// Persist persists the data of one space
-func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error {
+// PersistWithTime persists the data of one space if it has not been modified since the given mtime
+func (c *Cache) PersistWithTime(ctx context.Context, storageID, spaceID string, mtime time.Time) error {
 	if c.Providers[storageID] == nil || c.Providers[storageID].Spaces[spaceID] == nil {
 		return nil
 	}
 
-	c.Providers[storageID].Spaces[spaceID].Mtime = time.Now()
+	oldMtime := c.Providers[storageID].Spaces[spaceID].Mtime
+	c.Providers[storageID].Spaces[spaceID].Mtime = mtime
+
+	// FIXME there is a race when between this time now and the below Uploed another process also updates the file -> we need a lock
 	createdBytes, err := json.Marshal(c.Providers[storageID].Spaces[spaceID])
 	if err != nil {
+		c.Providers[storageID].Spaces[spaceID].Mtime = oldMtime
 		return err
 	}
 	jsonPath := spaceJSONPath(storageID, spaceID)
 	if err := c.storage.MakeDirIfNotExist(ctx, path.Dir(jsonPath)); err != nil {
+		c.Providers[storageID].Spaces[spaceID].Mtime = oldMtime
 		return err
 	}
 
-	return c.storage.Upload(ctx, metadata.UploadRequest{
+	if err = c.storage.Upload(ctx, metadata.UploadRequest{
 		Path:              jsonPath,
 		Content:           createdBytes,
 		IfUnmodifiedSince: c.Providers[storageID].Spaces[spaceID].Mtime,
-	})
+	}); err != nil {
+		c.Providers[storageID].Spaces[spaceID].Mtime = oldMtime
+		return err
+	}
+	return nil
+}
+
+// Persist persists the data of one space
+func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error {
+	return c.PersistWithTime(ctx, storageID, spaceID, time.Now())
 }
 
 // Sync updates the in-memory data with the data from the storage if it is outdated

diff --git a/pkg/share/manager/jsoncs3/providercache/providercache_test.go b/pkg/share/manager/jsoncs3/providercache/providercache_test.go
@@ -130,10 +130,11 @@ var _ = Describe("Cache", func() {
 				Expect(c.Providers[storageID].Spaces[spaceID].Mtime).ToNot(Equal(oldMtime))
 			})
 
-			It("does not persist if the etag changed on disk", func() {
-				c.Providers[storageID].Spaces[spaceID].Mtime = time.Now().Add(-3 * time.Hour)
+		})
 
-				Expect(c.Persist(ctx, storageID, spaceID)).ToNot(Succeed())
+		Describe("PersistWithTime", func() {
+			It("does not persist if the mtime on disk is more recent", func() {
+				Expect(c.PersistWithTime(ctx, storageID, spaceID, time.Now().Add(-3*time.Hour))).ToNot(Succeed())
 			})
 		})
 

diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go
@@ -148,21 +148,29 @@ func (c *Cache) Persist(ctx context.Context, userID string) error {
 		return nil
 	}
 
+	oldMtime := c.ReceivedSpaces[userID].Mtime
 	c.ReceivedSpaces[userID].Mtime = time.Now()
+
 	createdBytes, err := json.Marshal(c.ReceivedSpaces[userID])
 	if err != nil {
+		c.ReceivedSpaces[userID].Mtime = oldMtime
 		return err
 	}
 	jsonPath := userJSONPath(userID)
 	if err := c.storage.MakeDirIfNotExist(ctx, path.Dir(jsonPath)); err != nil {
+		c.ReceivedSpaces[userID].Mtime = oldMtime
 		return err
 	}
 
-	return c.storage.Upload(ctx, metadata.UploadRequest{
+	if err = c.storage.Upload(ctx, metadata.UploadRequest{
 		Path:              jsonPath,
 		Content:           createdBytes,
 		IfUnmodifiedSince: c.ReceivedSpaces[userID].Mtime,
-	})
+	}); err != nil {
+		c.ReceivedSpaces[userID].Mtime = oldMtime
+		return err
+	}
+	return nil
 }
 
 func userJSONPath(userID string) string {

diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache.go b/pkg/share/manager/jsoncs3/sharecache/sharecache.go
@@ -168,22 +168,29 @@ func (c *Cache) Sync(ctx context.Context, userID string) error {
 
 // Persist persists the data for one user/group to the storage
 func (c *Cache) Persist(ctx context.Context, userid string) error {
+	oldMtime := c.UserShares[userid].Mtime
 	c.UserShares[userid].Mtime = time.Now()
 
 	createdBytes, err := json.Marshal(c.UserShares[userid])
 	if err != nil {
+		c.UserShares[userid].Mtime = oldMtime
 		return err
 	}
 	jsonPath := c.userCreatedPath(userid)
 	if err := c.storage.MakeDirIfNotExist(ctx, path.Dir(jsonPath)); err != nil {
+		c.UserShares[userid].Mtime = oldMtime
 		return err
 	}
 
-	return c.storage.Upload(ctx, metadata.UploadRequest{
+	if err = c.storage.Upload(ctx, metadata.UploadRequest{
 		Path:              jsonPath,
 		Content:           createdBytes,
 		IfUnmodifiedSince: c.UserShares[userid].Mtime,
-	})
+	}); err != nil {
+		c.UserShares[userid].Mtime = oldMtime
+		return err
+	}
+	return nil
 }
 
 func (c *Cache) userCreatedPath(userid string) string {