reva update bring #419

2025-11-13 17:46:50 +01:00
parent b73353ef9b
commit c685d280f5
27 changed files with 669 additions and 82 deletions
@@ -455,6 +455,36 @@ func (tb *Trashbin) PurgeRecycleItem(ctx context.Context, spaceID, key, relative
 						if err := os.Remove(it.path); err != nil && !os.IsNotExist(err) {
 							tb.log.Error().Err(err).Str("path", it.path).Msg("Failed to delete file")
 						}
+
+						// delete revisions
+						globPattern := tb.lu.InternalPath(spaceID, id+node.RevisionIDDelimiter) + "*"
+						revs, err := filepath.Glob(globPattern)
+						if err != nil {
+							tb.log.Error().Err(err).Str("path", globPattern).Msg("glob failed badly")
+						}
+						for _, rev := range revs {
+							if tb.lu.MetadataBackend().IsMetaFile(rev) {
+								continue
+							}
+
+							if err := os.Remove(rev); err != nil {
+								tb.log.Error().Err(err).Str("revision", rev).Msg("error removing revision file")
+							}
+
+							// delete the metadata mlock file
+							parts := strings.SplitN(filepath.Base(rev), node.RevisionIDDelimiter, 2)
+							if len(parts) != 2 {
+								tb.log.Error().Str("path", rev).Msg("could not split revision")
+								continue
+							}
+							revId := id + node.RevisionIDDelimiter + parts[1]
+							mlockFile := tb.lu.MetadataBackend().LockfilePath(&trashNode{spaceID: spaceID, id: revId})
+							if err := os.Remove(mlockFile); err != nil {
+								tb.log.Error().Err(err).Str("path", mlockFile).Msg("error removing metadata mlock file")
+							}
+
+						}
+
 					}
 				}
 			}
@@ -581,6 +581,9 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) error {
 		}
 	}

+	// Remove metadata mlock file if it exists
+	_ = os.Remove(t.lookup.MetadataBackend().LockfilePath(n))
+
 	err := t.trashbin.MoveToTrash(ctx, n, path)
 	if err != nil {
 		return err
@@ -125,7 +125,8 @@ func (b HybridBackend) list(ctx context.Context, n MetadataNode, acquireLock boo
 		if err != nil {
 			return nil, err
 		}
-		defer cleanupLockfile(ctx, f)
+		// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
+		defer f.Close()

 	}
 	return xattr.List(filePath)
@@ -378,7 +379,8 @@ func (b HybridBackend) Remove(ctx context.Context, n MetadataNode, key string, a
 		if err != nil {
 			return err
 		}
-		defer cleanupLockfile(ctx, lockedFile)
+		// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
+		defer lockedFile.Close()
 	}

 	if isOffloadingAttribute(key) {
@@ -467,9 +469,6 @@ func (b HybridBackend) Purge(ctx context.Context, n MetadataNode) error {
 		}
 	}

-	// delete the metadata lockfile
-	_ = os.Remove(b.LockfilePath(n))
-
 	return b.metaCache.RemoveMetadata(b.cacheKey(n))
 }

@@ -285,15 +285,6 @@ func (b MessagePackBackend) Purge(_ context.Context, n MetadataNode) error {
 		return err
 	}

-	internalPath := n.InternalPath()
-	// for trash files always use the path without the timestamp
-	parts := strings.SplitN(n.GetID(), ".T.", 2)
-	if len(parts) > 1 {
-		internalPath = strings.TrimSuffix(internalPath, ".T."+parts[1])
-	}
-
-	_ = os.Remove(internalPath + ".mlock")
-
 	return os.Remove(b.MetadataPath(n))
 }

@@ -100,7 +100,8 @@ func (b XattrsBackend) list(ctx context.Context, n MetadataNode, acquireLock boo
 		if err != nil {
 			return nil, err
 		}
-		defer cleanupLockfile(ctx, f)
+		// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
+		defer f.Close()

 	}
 	return xattr.List(filePath)
@@ -177,7 +178,8 @@ func (b XattrsBackend) SetMultiple(ctx context.Context, n MetadataNode, attribs
 		if err != nil {
 			return err
 		}
-		defer cleanupLockfile(ctx, lockedFile)
+		// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
+		defer lockedFile.Close()
 	}

 	// error handling: Count if there are errors while setting the attribs.
@@ -211,7 +213,8 @@ func (b XattrsBackend) Remove(ctx context.Context, n MetadataNode, key string, a
 		if err != nil {
 			return err
 		}
-		defer cleanupLockfile(ctx, lockedFile)
+		// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
+		defer lockedFile.Close()
 	}

 	err := xattr.Remove(path, key)
@@ -248,9 +251,6 @@ func (b XattrsBackend) Purge(ctx context.Context, n MetadataNode) error {
 		}
 	}

-	// delete the metadata lockfile
-	_ = os.Remove(b.LockfilePath(n))
-
 	return b.metaCache.RemoveMetadata(b.cacheKey(n))
 }

@@ -286,11 +286,6 @@ func (b XattrsBackend) Lock(n MetadataNode) (UnlockFunc, error) {
 	}, nil
 }

-func cleanupLockfile(_ context.Context, f *lockedfile.File) {
-	_ = f.Close()
-	// Warning: do not remove the lockfile or we may lock the same file more than once, https://github.com/opencloud-eu/opencloud/issues/1793
-}
-
 // AllWithLockedSource reads all extended attributes from the given reader.
 // The path argument is used for storing the data in the cache
 func (b XattrsBackend) AllWithLockedSource(ctx context.Context, n MetadataNode, _ io.Reader) (map[string][]byte, error) {
@@ -204,7 +204,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
 	err = fs.updateIndexes(ctx, &provider.Grantee{
 		Type: provider.GranteeType_GRANTEE_TYPE_USER,
 		Id:   &provider.Grantee_UserId{UserId: req.GetOwner().GetId()},
-	}, req.Type, root.ID, root.ID)
+	}, req.Type, root.GetID(), root.GetID())
 	if err != nil {
 		return nil, err
 	}
@@ -221,7 +221,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr
 			Grantee: &provider.Grantee{
 				Type: provider.GranteeType_GRANTEE_TYPE_USER,
 				Id: &provider.Grantee_UserId{
-					UserId: u.Id,
+					UserId: u.GetId(),
 				},
 			},
 			Permissions: ocsconv.NewManagerRole().CS3ResourcePermissions(),
@@ -295,7 +295,8 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 		spaceTypes[spaceTypeAny] = struct{}{}
 	}

-	authenticatedUserID := ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId()
+	authenticatedUser := ctxpkg.ContextMustGetUser(ctx)
+	authenticatedUserID := authenticatedUser.GetId().GetOpaqueId()

 	if !fs.p.ListSpacesOfUser(ctx, requestedUserID) {
 		return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list spaces of other users", authenticatedUserID))
@@ -500,10 +501,14 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide
 	}()

 	for r := range results {
+		if authenticatedUser.GetId().GetTenantId() != r.GetOwner().GetId().GetTenantId() {
+			appctx.GetLogger(ctx).Warn().Str("id", r.GetId().GetOpaqueId()).Msg("space belongs to a different tenant")
+			continue
+		}
 		r.HasTrashedItems = true
 		resourceID, err := storagespace.ParseID(r.GetId().GetOpaqueId())
 		if err != nil {
-			appctx.GetLogger(ctx).Error().Err(err).Str("id", r.Id.GetOpaqueId()).Msg("could not parse space id")
+			appctx.GetLogger(ctx).Error().Err(err).Str("id", r.GetId().GetOpaqueId()).Msg("could not parse space id")
 			r.HasTrashedItems = false
 			continue
 		}
@@ -577,7 +582,7 @@ func (fs *Decomposedfs) UpdateStorageSpace(ctx context.Context, req *provider.Up
 	}

 	space := req.StorageSpace
-	_, spaceID, _, _ := storagespace.SplitID(space.Id.OpaqueId)
+	_, spaceID, _, _ := storagespace.SplitID(space.GetId().GetOpaqueId())

 	metadata := make(node.Attributes, 5)
 	if space.Name != "" {
@@ -723,7 +728,7 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De
 		_, purge = opaque.Map["purge"]
 	}

-	_, spaceID, _, err := storagespace.SplitID(req.Id.GetOpaqueId())
+	_, spaceID, _, err := storagespace.SplitID(req.GetId().GetOpaqueId())
 	if err != nil {
 		return err
 	}
@@ -735,7 +740,7 @@ func (fs *Decomposedfs) DeleteStorageSpace(ctx context.Context, req *provider.De

 	st, err := n.SpaceRoot.XattrString(ctx, prefixes.SpaceTypeAttr)
 	if err != nil {
-		return errtypes.InternalError(fmt.Sprintf("space %s does not have a spacetype, possible corrupt decompsedfs", n.ID))
+		return errtypes.InternalError(fmt.Sprintf("space %s does not have a spacetype, possible corrupt decompsedfs", n.GetID()))
 	}

 	if err := canDeleteSpace(ctx, spaceID, st, purge, n, fs.p); err != nil {
@@ -877,18 +882,18 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 		case err != nil:
 			return nil, err
 		case !rp.Stat:
-			return nil, errtypes.NotFound(fmt.Sprintf("space %s not found", n.ID))
+			return nil, errtypes.NotFound(fmt.Sprintf("space %s not found", n.GetID()))
 		}

 		if n.SpaceRoot.IsDisabled(ctx) {
 			rp, err := fs.p.AssemblePermissions(ctx, n)
 			if err != nil || !permissions.IsManager(rp) {
-				return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list deleted spaces %s", user.Username, n.ID))
+				return nil, errtypes.PermissionDenied(fmt.Sprintf("user %s is not allowed to list deleted spaces %s", user.Username, n.GetID()))
 			}
 		}
 	}

-	sublog := appctx.GetLogger(ctx).With().Str("spaceid", n.SpaceID).Logger()
+	sublog := appctx.GetLogger(ctx).With().Str("spaceid", n.GetSpaceID()).Logger()

 	var err error
 	// TODO apply more filters
@@ -939,13 +944,13 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 					switch {
 					case g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_USER:
 						// remove from user index
-						if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), n.SpaceID); err != nil {
+						if err := fs.userSpaceIndex.Remove(g.Grantee.GetUserId().GetOpaqueId(), n.GetSpaceID()); err != nil {
 							sublog.Error().Err(err).Str("grantee", id).
 								Msg("failed to delete expired user space index")
 						}
 					case g.Grantee.Type == provider.GranteeType_GRANTEE_TYPE_GROUP:
 						// remove from group index
-						if err := fs.groupSpaceIndex.Remove(g.Grantee.GetGroupId().GetOpaqueId(), n.SpaceID); err != nil {
+						if err := fs.groupSpaceIndex.Remove(g.Grantee.GetGroupId().GetOpaqueId(), n.GetSpaceID()); err != nil {
 							sublog.Error().Err(err).Str("grantee", id).
 								Msg("failed to delete expired group space index")
 						}
@@ -977,8 +982,8 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 	ssID, err := storagespace.FormatReference(
 		&provider.Reference{
 			ResourceId: &provider.ResourceId{
-				SpaceId:  n.SpaceRoot.SpaceID,
-				OpaqueId: n.SpaceRoot.ID},
+				SpaceId:  n.SpaceRoot.GetSpaceID(),
+				OpaqueId: n.SpaceRoot.GetID()},
 		},
 	)
 	if err != nil {
@@ -1003,8 +1008,8 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 		},
 		Id: &provider.StorageSpaceId{OpaqueId: ssID},
 		Root: &provider.ResourceId{
-			SpaceId:  n.SpaceRoot.SpaceID,
-			OpaqueId: n.SpaceRoot.ID,
+			SpaceId:  n.SpaceRoot.GetSpaceID(),
+			OpaqueId: n.SpaceRoot.GetID(),
 		},
 		Name: sname,
 		// SpaceType is read from xattr below
@@ -1046,7 +1051,7 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 		}
 	}

-	etag, err := node.CalculateEtag(n.ID, tmtime)
+	etag, err := node.CalculateEtag(n.GetID(), tmtime)
 	if err != nil {
 		return nil, err
 	}
@@ -1075,7 +1080,7 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 	}
 	if si := spaceAttributes.String(prefixes.SpaceImageAttr); si != "" {
 		space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "image", storagespace.FormatResourceID(
-			&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.Root.SpaceId, OpaqueId: si},
+			&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.GetRoot().GetSpaceId(), OpaqueId: si},
 		))
 	}
 	if sd := spaceAttributes.String(prefixes.SpaceDescriptionAttr); sd != "" {
@@ -1083,7 +1088,7 @@ func (fs *Decomposedfs) StorageSpaceFromNode(ctx context.Context, n *node.Node,
 	}
 	if sr := spaceAttributes.String(prefixes.SpaceReadmeAttr); sr != "" {
 		space.Opaque = utils.AppendPlainToOpaque(space.Opaque, "readme", storagespace.FormatResourceID(
-			&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.Root.SpaceId, OpaqueId: sr},
+			&provider.ResourceId{StorageId: space.Root.StorageId, SpaceId: space.GetRoot().GetSpaceId(), OpaqueId: sr},
 		))
 	}
 	if sa := spaceAttributes.String(prefixes.SpaceAliasAttr); sa != "" {
@@ -1157,5 +1162,5 @@ func canDeleteSpace(ctx context.Context, spaceID string, typ string, purge bool,
 		return nil
 	}

-	return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete space %s", n.ID))
+	return errtypes.PermissionDenied(fmt.Sprintf("user is not allowed to delete space %s", n.GetID()))
 }
@@ -503,6 +503,9 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) {
 	// Remove lock file if it exists
 	_ = os.Remove(n.LockFilePaths()[0])

+	// Remove metadata mlock file if it exists
+	_ = os.Remove(t.lookup.MetadataBackend().LockfilePath(n))
+
 	// finally remove the entry from the parent dir
 	if err = os.Remove(path); err != nil {
 		// To roll back changes
@@ -704,7 +707,7 @@ func (t *Tree) InitNewNode(ctx context.Context, n *node.Node, fsize uint64) (met

 func (t *Tree) removeNode(ctx context.Context, n *node.Node) error {
 	path := n.InternalPath()
-	logger := appctx.GetLogger(ctx)
+	logger := appctx.GetLogger(ctx).With().Str("spaceid", n.SpaceID).Str("nodeid", n.ID).Logger()

 	if n.IsDir(ctx) {
 		item, err := t.ListFolder(ctx, n)
@@ -719,12 +722,7 @@ func (t *Tree) removeNode(ctx context.Context, n *node.Node) error {
 		}
 	}

-	// delete the actual node
-	if err := utils.RemoveItem(path); err != nil {
-		logger.Error().Err(err).Str("path", path).Msg("error purging node")
-		return err
-	}
-
+	// delete any metadata from disk and cache
 	if err := t.lookup.MetadataBackend().Purge(ctx, n); err != nil {
 		logger.Error().Err(err).Str("path", t.lookup.MetadataBackend().MetadataPath(n)).Msg("error purging node metadata")
 		return err
@@ -733,7 +731,7 @@ func (t *Tree) removeNode(ctx context.Context, n *node.Node) error {
 	// delete blob from blobstore
 	if n.BlobID != "" {
 		if err := t.DeleteBlob(n); err != nil {
-			logger.Error().Err(err).Str("blobID", n.BlobID).Msg("error purging nodes blob")
+			logger.Error().Err(err).Str("blobID", n.BlobID).Msg("error purging node blob")
 			return err
 		}
 	}
@@ -760,7 +758,7 @@ func (t *Tree) removeNode(ctx context.Context, n *node.Node) error {
 			return err
 		}

-		if err := utils.RemoveItem(rev); err != nil {
+		if err := os.Remove(rev); err != nil {
 			logger.Error().Err(err).Str("revision", rev).Msg("error removing revision node")
 			return err
 		}
@@ -777,6 +775,12 @@ func (t *Tree) removeNode(ctx context.Context, n *node.Node) error {

 	}

+	// delete the actual node and empty parent dirs
+	if err := utils.RemoveItem(path); err != nil {
+		logger.Error().Err(err).Str("path", path).Msg("error purging node")
+		return err
+	}
+
 	return nil
 }