Consolidate all metadata Get's and Set's to central functions. (#2512)

* Consolidate all metadata Get's and Set's to central functions.

In the decomposed FS, access to xattr was spread all over. This
patch consolidates that to use either the Node.SetMetadata() or
xattrs.Set(). This allows us to hook in indexing for example.

* hound and typos

* Add changelog.

* Some more fixes to use xattrs functions.

* Fix function name in tests.

* Fix some linting hints.

* Even more linter warning fixes

* Even more linting issues

* And another iteration

* Linter I hate you

* Use proper Int formatting

Co-authored-by: David Christofas <[email protected]>

* Update pkg/storage/utils/decomposedfs/node/node.go

Co-authored-by: David Christofas <[email protected]>

* Update pkg/storage/utils/decomposedfs/node/node.go

Co-authored-by: David Christofas <[email protected]>

* Update pkg/storage/utils/decomposedfs/xattrs/xattrs.go

Co-authored-by: David Christofas <[email protected]>

* again linting

* use correct variable in decomposedfs

Co-authored-by: Jörn Friedrich Dreyer <[email protected]>
Co-authored-by: David Christofas <[email protected]>

changelog/unreleased/consolidate-xattr.md

@@ -0,0 +1,7 @@
+Enhancement: Consolidate xattr setter and getter
+
+- Consolidate all metadata Get's and Set's to central functions.
+- Cleaner code by reduction of casts
+- Easier to hook functionality like indexing
+
+https://github.com/cs3org/reva/pull/2512

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -50,7 +50,6 @@ import (
 	rtrace "github.com/cs3org/reva/pkg/trace"
 	"github.com/cs3org/reva/pkg/utils"
 	"github.com/pkg/errors"
-	"github.com/pkg/xattr"
 	"go.opentelemetry.io/otel/codes"
 )
 
@@ -234,14 +233,13 @@ func (fs *Decomposedfs) CreateHome(ctx context.Context) (err error) {
 
 	// update the owner
 	u := ctxpkg.ContextMustGetUser(ctx)
-	if err = h.WriteMetadata(u.Id); err != nil {
+	if err = h.WriteAllNodeMetadata(u.Id); err != nil {
 		return
 	}
 
 	if fs.o.TreeTimeAccounting || fs.o.TreeSizeAccounting {
-		homePath := h.InternalPath()
 		// mark the home node as the end of propagation
-		if err = xattr.Set(homePath, xattrs.PropagationAttr, []byte("1")); err != nil {
+		if err = h.SetMetadata(xattrs.PropagationAttr, "1"); err != nil {
 			appctx.GetLogger(ctx).Error().Err(err).Interface("node", h).Msg("could not mark home as propagation root")
 			return
 		}
@@ -340,9 +338,8 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)
 	}
 
 	if fs.o.TreeTimeAccounting || fs.o.TreeSizeAccounting {
-		nodePath := n.InternalPath()
 		// mark the home node as the end of propagation
-		if err = xattr.Set(nodePath, xattrs.PropagationAttr, []byte("1")); err != nil {
+		if err = n.SetMetadata(xattrs.PropagationAttr, "1"); err != nil {
 			appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not mark node to propagate")
 
 			// FIXME: This does not return an error at all, but results in a severe situation that the
@@ -433,12 +430,11 @@ func (fs *Decomposedfs) CreateReference(ctx context.Context, p string, targetURI
 	}
 	childCreated = true
 
-	internalPath := childNode.InternalPath()
-	if err := xattr.Set(internalPath, xattrs.ReferenceAttr, []byte(targetURI.String())); err != nil {
+	if err := childNode.SetMetadata(xattrs.ReferenceAttr, targetURI.String()); err != nil {
 		// the reference could not be set - that would result in an lost reference?
 		err := errors.Wrapf(err, "Decomposedfs: error setting the target %s on the reference file %s",
 			targetURI.String(),
-			internalPath,
+			childNode.InternalPath(),
 		)
 		span.SetStatus(codes.Error, err.Error())
 		return err

pkg/storage/utils/decomposedfs/grants.go

@@ -62,9 +62,14 @@ func (fs *Decomposedfs) AddGrant(ctx context.Context, ref *provider.Reference, g
 		return errtypes.PermissionDenied(filepath.Join(node.ParentID, node.Name))
 	}
 
+	// check lock
+	if err := node.CheckLock(ctx); err != nil {
+		return err
+	}
+
 	e := ace.FromGrant(g)
 	principal, value := e.Marshal()
-	if err := xattr.Set(node.InternalPath(), xattrs.GrantPrefix+principal, value); err != nil {
+	if err := node.SetMetadata(xattrs.GrantPrefix+principal, string(value)); err != nil {
 		return err
 	}
 
@@ -172,15 +177,15 @@ func extractACEsFromAttrs(ctx context.Context, fsfn string, attrs []string) (ent
 	entries = []*ace.ACE{}
 	for i := range attrs {
 		if strings.HasPrefix(attrs[i], xattrs.GrantPrefix) {
-			var value []byte
+			var value string
 			var err error
-			if value, err = xattr.Get(fsfn, attrs[i]); err != nil {
+			if value, err = xattrs.Get(fsfn, attrs[i]); err != nil {
 				log.Error().Err(err).Str("attr", attrs[i]).Msg("could not read attribute")
 				continue
 			}
 			var e *ace.ACE
 			principal := attrs[i][len(xattrs.GrantPrefix):]
-			if e, err = ace.Unmarshal(principal, value); err != nil {
+			if e, err = ace.Unmarshal(principal, []byte(value)); err != nil {
 				log.Error().Err(err).Str("principal", principal).Str("attr", attrs[i]).Msg("could not unmarshal ace")
 				continue
 			}

pkg/storage/utils/decomposedfs/lookup.go

@@ -33,7 +33,6 @@ import (
 	"github.com/cs3org/reva/pkg/storage/utils/decomposedfs/options"
 	"github.com/cs3org/reva/pkg/storage/utils/decomposedfs/xattrs"
 	"github.com/cs3org/reva/pkg/storage/utils/templates"
-	"github.com/pkg/xattr"
 )
 
 // Lookup implements transformations from filepath to node and back
@@ -157,9 +156,9 @@ func (lu *Lookup) WalkPath(ctx context.Context, r *node.Node, p string, followRe
 		}
 
 		if followReferences {
-			if attrBytes, err := xattr.Get(r.InternalPath(), xattrs.ReferenceAttr); err == nil {
+			if attrBytes, err := r.GetMetadata(xattrs.ReferenceAttr); err == nil {
 				realNodeID := attrBytes
-				ref, err := xattrs.ReferenceFromAttr(realNodeID)
+				ref, err := xattrs.ReferenceFromAttr([]byte(realNodeID))
 				if err != nil {
 					return nil, err
 				}