jsoncs3 cache fixes

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
cs3org · Feb 21, 2024 · d76e4c4 · d76e4c4
1 parent bdd0495
commit d76e4c4
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 4 deletions.
diff --git a/changelog/unreleased/jsoncs3-cache-fixes.md b/changelog/unreleased/jsoncs3-cache-fixes.md
@@ -0,0 +1,5 @@
+Bugfix: jsoncs3 cache fixes
+
+The jsoncs3 share manager now retries persisting if the file already existed and picks up the etag of the upload response in all cases.
+
+https://github.com/cs3org/reva/pull/4532
diff --git a/pkg/share/manager/jsoncs3/providercache/providercache.go b/pkg/share/manager/jsoncs3/providercache/providercache.go
@@ -202,6 +202,11 @@ func (c *Cache) Add(ctx context.Context, storageID, spaceID, shareID string, sha
 			log.Debug().Msg("precondition failed when persisting added provider share: etag changed. retrying...")
 			// actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side
 			// continue with sync below
+		case errtypes.AlreadyExists:
+			log.Debug().Msg("already exists when persisting added provider share. retrying...")
+			// CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call.
+			// Thas happens when the cache thinks there is no file.
+			// continue with sync below
 		default:
 			span.SetStatus(codes.Error, fmt.Sprintf("persisting added provider share failed. giving up: %s", err.Error()))
 			log.Error().Err(err).Msg("persisting added provider share failed")
@@ -393,10 +398,8 @@ func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error {
 	// > If the field value is "*", the condition is false if the origin server has a current representation for the target resource.
 	if space.Etag == "" {
 		ur.IfNoneMatch = []string{"*"}
-		log.Debug().Msg("setting IfNoneMatch to *")
-	} else {
-		log.Debug().Msg("setting IfMatchEtag")
 	}
+
 	res, err := c.storage.Upload(ctx, ur)
 	if err != nil {
 		span.RecordError(err)
@@ -405,6 +408,7 @@ func (c *Cache) Persist(ctx context.Context, storageID, spaceID string) error {
 		return err
 	}
 	space.Etag = res.Etag
+
 	span.SetStatus(codes.Ok, "")
 	shares := []string{}
 	for _, s := range space.Shares {

diff --git a/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go b/pkg/share/manager/jsoncs3/receivedsharecache/receivedsharecache.go
@@ -146,6 +146,11 @@ func (c *Cache) Add(ctx context.Context, userID, spaceID string, rs *collaborati
 			log.Debug().Msg("precondition failed when persisting added received share: etag changed. retrying...")
 			// actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side
 			// continue with sync below
+		case errtypes.AlreadyExists:
+			log.Debug().Msg("already exists when persisting added received share. retrying...")
+			// CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call.
+			// Thas happens when the cache thinks there is no file.
+			// continue with sync below
 		default:
 			span.SetStatus(codes.Error, fmt.Sprintf("persisting added received share failed. giving up: %s", err.Error()))
 			log.Error().Err(err).Msg("persisting added received share failed")
@@ -294,12 +299,14 @@ func (c *Cache) persist(ctx context.Context, userID string) error {
 		ur.IfNoneMatch = []string{"*"}
 	}
 
-	_, err = c.storage.Upload(ctx, ur)
+	res, err := c.storage.Upload(ctx, ur)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())
 		return err
 	}
+	rss.etag = res.Etag
+
 	span.SetStatus(codes.Ok, "")
 	return nil
 }

diff --git a/pkg/share/manager/jsoncs3/sharecache/sharecache.go b/pkg/share/manager/jsoncs3/sharecache/sharecache.go
@@ -137,6 +137,11 @@ func (c *Cache) Add(ctx context.Context, userid, shareID string) error {
 			log.Debug().Msg("precondition failed when persisting added share: etag changed. retrying...")
 			// actually, this is the wrong status code and we treat it like errtypes.Aborted because of inconsistencies on the server side
 			// continue with sync below
+		case errtypes.AlreadyExists:
+			log.Debug().Msg("already exists when persisting added share. retrying...")
+			// CS3 uses an already exists error instead of precondition failed when using an If-None-Match=* header / IfExists flag in the InitiateFileUpload call.
+			// Thas happens when the cache thinks there is no file.
+			// continue with sync below
 		default:
 			span.SetStatus(codes.Error, fmt.Sprintf("persisting added share failed. giving up: %s", err.Error()))
 			log.Error().Err(err).Msg("persisting added share failed")
@@ -330,13 +335,15 @@ func (c *Cache) Persist(ctx context.Context, userid string) error {
 	if us.Etag == "" {
 		ur.IfNoneMatch = []string{"*"}
 	}
+
 	res, err := c.storage.Upload(ctx, ur)
 	if err != nil {
 		span.RecordError(err)
 		span.SetStatus(codes.Error, err.Error())
 		return err
 	}
 	us.Etag = res.Etag
+
 	span.SetStatus(codes.Ok, "")
 	return nil
 }