From 18f2d669d470ac9df7035601f0b1dc2851476be9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 22 Aug 2022 12:05:09 +0000 Subject: [PATCH] revert mtime change on error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../jsoncs3/providercache/providercache.go | 24 +++++++++++++++---- .../providercache/providercache_test.go | 7 +++--- .../receivedsharecache/receivedsharecache.go | 12 ++++++++-- .../manager/jsoncs3/sharecache/sharecache.go | 11 +++++++-- 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/pkg/share/manager/jsoncs3/providercache/providercache.go b/pkg/share/manager/jsoncs3/providercache/providercache.go index 13879271e9..83210d270b 100644 --- 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 index 106c72e796..7f31b6efd8 100644 --- 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 index 8eac0df917..d2302f8ae1 100644 --- 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 index 40470d3d90..82cb19b27e 100644 --- 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 {