From d10f4de88cc618ad28e0c67f818b299a6bd641eb Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 7 Feb 2022 12:45:37 +0100 Subject: [PATCH 01/16] 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. --- .../utils/decomposedfs/decomposedfs.go | 15 +- pkg/storage/utils/decomposedfs/grants.go | 9 +- pkg/storage/utils/decomposedfs/lookup.go | 5 +- pkg/storage/utils/decomposedfs/node/node.go | 140 +++++++++--------- pkg/storage/utils/decomposedfs/spaces.go | 15 +- pkg/storage/utils/decomposedfs/tree/tree.go | 56 +++---- .../utils/decomposedfs/xattrs/xattrs.go | 53 +++++++ 7 files changed, 165 insertions(+), 128 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 501d3f77fd..3e6064475d 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/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" ) @@ -223,14 +222,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.WriteNodeMetadata(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 } @@ -329,9 +327,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") return } @@ -396,11 +393,11 @@ func (fs *Decomposedfs) CreateReference(ctx context.Context, p string, targetURI return err } - internal := n.InternalPath() - if err := xattr.Set(internal, xattrs.ReferenceAttr, []byte(targetURI.String())); err != nil { + if err := n.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(), - internal, + n.InternalPath(), ) span.SetStatus(codes.Error, err.Error()) return err diff --git a/pkg/storage/utils/decomposedfs/grants.go b/pkg/storage/utils/decomposedfs/grants.go index 2920ddd175..71a11aef1c 100644 --- a/pkg/storage/utils/decomposedfs/grants.go +++ b/pkg/storage/utils/decomposedfs/grants.go @@ -67,10 +67,9 @@ func (fs *Decomposedfs) AddGrant(ctx context.Context, ref *provider.Reference, g return err } - np := fs.lu.InternalPath(node.ID) e := ace.FromGrant(g) principal, value := e.Marshal() - if err := xattr.Set(np, xattrs.GrantPrefix+principal, value); err != nil { + if err := node.SetMetadata(xattrs.GrantPrefix+principal, string(value)); err != nil { return err } @@ -179,15 +178,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 } diff --git a/pkg/storage/utils/decomposedfs/lookup.go b/pkg/storage/utils/decomposedfs/lookup.go index 64c3ee76c0..70a8daa596 100644 --- a/pkg/storage/utils/decomposedfs/lookup.go +++ b/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 @@ -148,9 +147,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 } diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index f165daaf9c..c94d326e8b 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -110,14 +110,13 @@ func New(id, parentID, name string, blobsize int64, blobID string, owner *userpb func (n *Node) ChangeOwner(new *userpb.UserId) (err error) { nodePath := n.InternalPath() n.owner = new - if err = xattr.Set(nodePath, xattrs.OwnerIDAttr, []byte(new.OpaqueId)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not reset owner id attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerIDPAttr, []byte(new.Idp)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not reset owner idp attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerTypeAttr, []byte(utils.UserTypeToString(new.Type))); err != nil { - return errors.Wrap(err, "Decomposedfs: could not reset owner idp attribute") + + var attribs = map[string]string{xattrs.OwnerIDAttr: new.OpaqueId, + xattrs.OwnerIDPAttr: new.Idp, + xattrs.OwnerTypeAttr: utils.UserTypeToString(new.Type)} + + if err := xattrs.SetMutltiple(nodePath, attribs); err != nil { + return err } return @@ -127,47 +126,41 @@ func (n *Node) ChangeOwner(new *userpb.UserId) (err error) { // Note that consumers should be aware of the metadata options on xattrs.go. func (n *Node) SetMetadata(key string, val string) (err error) { nodePath := n.InternalPath() - if err := xattr.Set(nodePath, key, []byte(val)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set parentid attribute") + if err := xattrs.Set(nodePath, key, val); err != nil { + return errors.Wrap(err, "Decomposedfs: could not set extended attribute") } return nil } +func (n *Node) GetMetadata(key string) (val string, err error) { + nodePath := n.InternalPath() + if val, err := xattrs.Get(nodePath, key); err != nil { + return errors.Wrap(err, "Decomposedfs: could not get extended attribute") + } + return val +} + // WriteMetadata writes the Node metadata to disk func (n *Node) WriteMetadata(owner *userpb.UserId) (err error) { + attribs := make(map[string]string) + + attribs[xattrs.ParentidAttr] = n.ParentID + attribs[xattrs.NameAttr] = n.Name + attribs[xattrs.BlobIDAttr] = n.BlobID + attribs[xattrs.BlobsizeAttr] = fmt.Sprintf("%d", n.Blobsize) + nodePath := n.InternalPath() - if err = xattr.Set(nodePath, xattrs.ParentidAttr, []byte(n.ParentID)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set parentid attribute") - } - if err = xattr.Set(nodePath, xattrs.NameAttr, []byte(n.Name)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set name attribute") - } - if err = xattr.Set(nodePath, xattrs.BlobIDAttr, []byte(n.BlobID)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set blobid attribute") - } - if err = xattr.Set(nodePath, xattrs.BlobsizeAttr, []byte(fmt.Sprintf("%d", n.Blobsize))); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set blobsize attribute") + attribs[xattrs.OwnerIDAttr] = "" + attribs[xattrs.OwnerIDPAttr] = "" + attribs[xattrs.OwnerTypeAttr] = "" + + if owner != nil { + attribs[xattrs.OwnerIDAttr] = owner.OpaqueId + attribs[xattrs.OwnerIDPAttr] = owner.Idp + attribs[xattrs.OwnerTypeAttr] = utils.UserTypeToString(owner.Type) } - if owner == nil { - if err = xattr.Set(nodePath, xattrs.OwnerIDAttr, []byte("")); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set empty owner id attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerIDPAttr, []byte("")); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set empty owner idp attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerTypeAttr, []byte("")); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set empty owner type attribute") - } - } else { - if err = xattr.Set(nodePath, xattrs.OwnerIDAttr, []byte(owner.OpaqueId)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set owner id attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerIDPAttr, []byte(owner.Idp)); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set owner idp attribute") - } - if err = xattr.Set(nodePath, xattrs.OwnerTypeAttr, []byte(utils.UserTypeToString(owner.Type))); err != nil { - return errors.Wrap(err, "Decomposedfs: could not set owner idp attribute") - } + if err := xattrs.SetMutltiple(nodePath, attribs); err != nil { + return err } return } @@ -183,7 +176,7 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error // lookup parent id in extended attributes var attrBytes []byte - attrBytes, err = xattr.Get(nodePath, xattrs.ParentidAttr) + attrBytes, err = xattrs.Get(nodePath, xattrs.ParentidAttr) switch { case err == nil: n.ParentID = string(attrBytes) @@ -196,17 +189,17 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error } // check if this is a space root - if _, err = xattr.Get(nodePath, xattrs.SpaceNameAttr); err == nil { + if _, err = xattrs.Get(nodePath, xattrs.SpaceNameAttr); err == nil { n.SpaceRoot = n } // lookup name in extended attributes - if attrBytes, err = xattr.Get(nodePath, xattrs.NameAttr); err == nil { + if attrBytes, err = xattrs.Get(nodePath, xattrs.NameAttr); err == nil { n.Name = string(attrBytes) } else { return } // lookup blobID in extended attributes - if attrBytes, err = xattr.Get(nodePath, xattrs.BlobIDAttr); err == nil { + if attrBytes, err = xattrs.Get(nodePath, xattrs.BlobIDAttr); err == nil { n.BlobID = string(attrBytes) } else { return @@ -286,16 +279,14 @@ func (n *Node) Parent() (p *Node, err error) { parentPath := n.lu.InternalPath(n.ParentID) // lookup parent id in extended attributes - var attrBytes []byte - if attrBytes, err = xattr.Get(parentPath, xattrs.ParentidAttr); err == nil { - p.ParentID = string(attrBytes) - } else { + if p.ParentID, err = xattrs.Get(parentPath, xattrs.ParentidAttr); err != nil { + p.ParentID = string("") return } // lookup name in extended attributes - if attrBytes, err = xattr.Get(parentPath, xattrs.NameAttr); err == nil { - p.Name = string(attrBytes) - } else { + if p.Name, err = xattrs.Get(parentPath, xattrs.NameAttr); err != nil { + p.Name = string("") + p.ParentID = string("") return } @@ -324,7 +315,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { var attrBytes []byte var err error // lookup ID in extended attributes - attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDAttr) + attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerIDAttr) switch { case err == nil: owner.OpaqueId = string(attrBytes) @@ -335,7 +326,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { } // lookup IDP in extended attributes - attrBytes, err = xattr.Get(nodePath, xattrs.OwnerIDPAttr) + attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerIDPAttr) switch { case err == nil: owner.Idp = string(attrBytes) @@ -346,7 +337,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { } // lookup type in extended attributes - attrBytes, err = xattr.Get(nodePath, xattrs.OwnerTypeAttr) + attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerTypeAttr) switch { case err == nil: owner.Type = utils.UserTypeMap(string(attrBytes)) @@ -458,7 +449,7 @@ func (n *Node) SetEtag(ctx context.Context, val string) (err error) { return nil } // etag is only valid until the calculated etag changes, is part of propagation - return xattr.Set(nodePath, xattrs.TmpEtagAttr, []byte(val)) + return xattrs.Set(nodePath, xattrs.TmpEtagAttr, val) } // SetFavorite sets the favorite for the current user @@ -482,7 +473,7 @@ func (n *Node) SetFavorite(uid *userpb.UserId, val string) error { nodePath := n.lu.InternalPath(n.ID) // the favorite flag is specific to the user, so we need to incorporate the userid fa := fmt.Sprintf("%s:%s:%s@%s", xattrs.FavPrefix, utils.UserTypeToString(uid.GetType()), uid.GetOpaqueId(), uid.GetIdp()) - return xattr.Set(nodePath, fa, []byte(val)) + return xattrs.Set(nodePath, fa, val) } // AsResourceInfo return the node as CS3 ResourceInfo @@ -502,7 +493,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi var target []byte switch { case fi.IsDir(): - if target, err = xattr.Get(nodePath, xattrs.ReferenceAttr); err == nil { + if target, err = xattrs.Get(nodePath, xattrs.ReferenceAttr); err == nil { nodeType = provider.ResourceType_RESOURCE_TYPE_REFERENCE } else { nodeType = provider.ResourceType_RESOURCE_TYPE_CONTAINER @@ -560,7 +551,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi } // use temporary etag if it is set - if b, err := xattr.Get(nodePath, xattrs.TmpEtagAttr); err == nil { + if b, err := xattrs.Get(nodePath, xattrs.TmpEtagAttr); err == nil { ri.Etag = fmt.Sprintf(`"%x"`, string(b)) // TODO why do we convert string(b)? is the temporary etag stored as string? -> should we use bytes? use hex.EncodeToString? } else if ri.Etag, err = calculateEtag(n.ID, tmTime); err != nil { sublog.Debug().Err(err).Msg("could not calculate etag") @@ -593,7 +584,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi // the favorite flag is specific to the user, so we need to incorporate the userid if uid := u.GetId(); uid != nil { fa := fmt.Sprintf("%s:%s:%s@%s", xattrs.FavPrefix, utils.UserTypeToString(uid.GetType()), uid.GetOpaqueId(), uid.GetIdp()) - if val, err := xattr.Get(nodePath, fa); err == nil { + if val, err := xattrs.Get(nodePath, fa); err == nil { sublog.Debug(). Str("favorite", fa). Msg("found favorite flag") @@ -659,7 +650,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi // only read when key was requested k := attrs[i][len(xattrs.MetadataPrefix):] if _, ok := mdKeysMap[k]; returnAllKeys || ok { - if val, err := xattr.Get(nodePath, attrs[i]); err == nil { + if val, err := xattrs.Get(nodePath, attrs[i]); err == nil { metadata[k] = string(val) } else { sublog.Error().Err(err). @@ -682,7 +673,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi } func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string, ri *provider.ResourceInfo) { - v, err := xattr.Get(nodePath, xattrs.ChecksumPrefix+algo) + v, err := xattrs.Get(nodePath, xattrs.ChecksumPrefix+algo) switch { case err == nil: ri.Checksum = &provider.ResourceChecksum{ @@ -699,7 +690,7 @@ func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string } func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *provider.ResourceInfo) { - v, err := xattr.Get(nodePath, xattrs.ChecksumPrefix+algo) + v, err := xattrs.Get(nodePath, xattrs.ChecksumPrefix+algo) switch { case err == nil: if ri.Opaque == nil { @@ -722,7 +713,7 @@ func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *prov // quota is always stored on the root node func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.ResourceInfo) { - v, err := xattr.Get(nodePath, xattrs.QuotaAttr) + v, err := xattrs.Get(nodePath, xattrs.QuotaAttr) switch { case err == nil: // make sure we have a proper signed int @@ -754,7 +745,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso // HasPropagation checks if the propagation attribute exists and is set to "1" func (n *Node) HasPropagation() (propagation bool) { - if b, err := xattr.Get(n.lu.InternalPath(n.ID), xattrs.PropagationAttr); err == nil { + if b, err := xattrs.Get(n.lu.InternalPath(n.ID), xattrs.PropagationAttr); err == nil { return string(b) == "1" } return false @@ -763,7 +754,7 @@ func (n *Node) HasPropagation() (propagation bool) { // GetTMTime reads the tmtime from the extended attributes func (n *Node) GetTMTime() (tmTime time.Time, err error) { var b []byte - if b, err = xattr.Get(n.lu.InternalPath(n.ID), xattrs.TreeMTimeAttr); err != nil { + if b, err = xattrs.Get(n.lu.InternalPath(n.ID), xattrs.TreeMTimeAttr); err != nil { return } return time.Parse(time.RFC3339Nano, string(b)) @@ -771,13 +762,13 @@ func (n *Node) GetTMTime() (tmTime time.Time, err error) { // SetTMTime writes the tmtime to the extended attributes func (n *Node) SetTMTime(t time.Time) (err error) { - return xattr.Set(n.lu.InternalPath(n.ID), xattrs.TreeMTimeAttr, []byte(t.UTC().Format(time.RFC3339Nano))) + return xattrs.Set(n.lu.InternalPath(n.ID), xattrs.TreeMTimeAttr, t.UTC().Format(time.RFC3339Nano)) } // GetTreeSize reads the treesize from the extended attributes func (n *Node) GetTreeSize() (treesize uint64, err error) { var b []byte - if b, err = xattr.Get(n.InternalPath(), xattrs.TreesizeAttr); err != nil { + if b, err = xattrs.Get(n.InternalPath(), xattrs.TreesizeAttr); err != nil { return } return strconv.ParseUint(string(b), 10, 64) @@ -785,12 +776,12 @@ func (n *Node) GetTreeSize() (treesize uint64, err error) { // SetTreeSize writes the treesize to the extended attributes func (n *Node) SetTreeSize(ts uint64) (err error) { - return xattr.Set(n.InternalPath(), xattrs.TreesizeAttr, []byte(strconv.FormatUint(ts, 10))) + return n.SetMetadata(xattrs.TreesizeAttr, strconv.FormatUint(ts, 10)) } // SetChecksum writes the checksum with the given checksum type to the extended attributes func (n *Node) SetChecksum(csType string, h hash.Hash) (err error) { - return xattr.Set(n.lu.InternalPath(n.ID), xattrs.ChecksumPrefix+csType, h.Sum(nil)) + return n.SetMetadata(xattrs.ChecksumPrefix+csType, string(h.Sum(nil))) } // UnsetTempEtag removes the temporary etag attribute @@ -889,6 +880,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap prov // The function will return a list of opaque strings that can be used to make a ReadGrant call func (n *Node) ListGrantees(ctx context.Context) (grantees []string, err error) { var attrs []string + if attrs, err = xattr.List(n.InternalPath()); err != nil { appctx.GetLogger(ctx).Error().Err(err).Str("node", n.ID).Msg("error listing attributes") return nil, err @@ -904,7 +896,7 @@ func (n *Node) ListGrantees(ctx context.Context) (grantees []string, err error) // ReadGrant reads a CS3 grant func (n *Node) ReadGrant(ctx context.Context, grantee string) (g *provider.Grant, err error) { var b []byte - if b, err = xattr.Get(n.InternalPath(), grantee); err != nil { + if b, err = xattrs.Get(n.InternalPath(), grantee); err != nil { return nil, err } var e *ace.ACE @@ -940,7 +932,7 @@ func (n *Node) ListGrants(ctx context.Context) ([]*provider.Grant, error) { // ReadBlobSizeAttr reads the blobsize from the xattrs func ReadBlobSizeAttr(path string) (int64, error) { - attrBytes, err := xattr.Get(path, xattrs.BlobsizeAttr) + attrBytes, err := xattrs.Get(path, xattrs.BlobsizeAttr) if err != nil { return 0, errors.Wrapf(err, "error reading blobsize xattr") } @@ -1001,7 +993,7 @@ func (n *Node) FindStorageSpaceRoot() error { // IsSpaceRoot checks if the node is a space root func IsSpaceRoot(r *Node) bool { path := r.InternalPath() - if _, err := xattr.Get(path, xattrs.SpaceNameAttr); err == nil { + if _, err := xattrs.Get(path, xattrs.SpaceNameAttr); err == nil { return true } return false @@ -1013,7 +1005,7 @@ var CheckQuota = func(spaceRoot *Node, fileSize uint64) (quotaSufficient bool, e if !enoughDiskSpace(spaceRoot.InternalPath(), fileSize) { return false, errtypes.InsufficientStorage("disk full") } - quotaByte, _ := xattr.Get(spaceRoot.InternalPath(), xattrs.QuotaAttr) + quotaByte, _ := xattrs.Get(spaceRoot.InternalPath(), xattrs.QuotaAttr) var total uint64 if quotaByte == nil { // if quota is not set, it means unlimited diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index d26a358107..87aa1df2e4 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -44,7 +44,6 @@ import ( "github.com/cs3org/reva/pkg/storage/utils/decomposedfs/xattrs" "github.com/cs3org/reva/pkg/utils" "github.com/google/uuid" - "github.com/pkg/xattr" ) const ( @@ -92,9 +91,8 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr } // always enable propagation on the storage space root - nodePath := n.InternalPath() // mark the space root 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") return nil, err } @@ -541,10 +539,9 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, } // TODO apply more filters - - sname := "" - if bytes, err := xattr.Get(n.InternalPath(), xattrs.SpaceNameAttr); err == nil { - sname = string(bytes) + var sname string + if sname, err = n.GetMetadata(xattrs.SpaceNameAttr); err != nil { + // FIXME: Is that a severe problem? } if err := n.FindStorageSpaceRoot(); err != nil { @@ -635,14 +632,14 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, } // quota - v, err := xattr.Get(nodePath, xattrs.QuotaAttr) + v, err := xattrs.Get(nodePath, xattrs.QuotaAttr) if err == nil { // make sure we have a proper signed int // we use the same magic numbers to indicate: // -1 = uncalculated // -2 = unknown // -3 = unlimited - if quota, err := strconv.ParseUint(string(v), 10, 64); err == nil { + if quota, err := strconv.ParseUint(v, 10, 64); err == nil { space.Quota = &provider.Quota{ QuotaMaxBytes: quota, QuotaMaxFiles: math.MaxUint64, // TODO MaxUInt64? = unlimited? why even max files? 0 = unlimited? diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 1fb125ea2b..20689bc7c2 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -122,7 +122,7 @@ func (t *Tree) Setup(owner *userpb.UserId, propagateToRoot bool) error { if propagateToRoot { v = []byte("1") } - if err = xattr.Set(n.InternalPath(), xattrs.PropagationAttr, v); err != nil { + if err = n.SetMetadata(xattrs.PropagationAttr, v); err != nil { return err } @@ -206,8 +206,8 @@ func (t *Tree) linkSpace(spaceType, spaceID, nodeID string) { } func isRootNode(nodePath string) bool { - attrBytes, err := xattr.Get(nodePath, xattrs.ParentidAttr) - return err == nil && string(attrBytes) == "root" + attr, err := xattrs.Get(nodePath, xattrs.ParentidAttr) + return err == nil && string(attr) == "root" } func isSharedNode(nodePath string) bool { if attrs, err := xattr.List(nodePath); err == nil { @@ -310,7 +310,7 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) } // update name attribute - if err := xattr.Set(tgtPath, xattrs.NameAttr, []byte(newNode.Name)); err != nil { + if err := xattrs.Set(tgtPath, xattrs.NameAttr, newNode.Name); err != nil { return errors.Wrap(err, "Decomposedfs: could not set name attribute") } @@ -330,10 +330,10 @@ func (t *Tree) Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) } // update target parentid and name - if err := xattr.Set(tgtPath, xattrs.ParentidAttr, []byte(newNode.ParentID)); err != nil { + if err := xattrs.Set(tgtPath, xattrs.ParentidAttr, newNode.ParentID); err != nil { return errors.Wrap(err, "Decomposedfs: could not set parentid attribute") } - if err := xattr.Set(tgtPath, xattrs.NameAttr, []byte(newNode.Name)); err != nil { + if err := xattrs.Set(tgtPath, xattrs.NameAttr, newNode.Name); err != nil { return errors.Wrap(err, "Decomposedfs: could not set name attribute") } @@ -411,7 +411,7 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) { // set origin location in metadata nodePath := n.InternalPath() - if err := xattr.Set(nodePath, xattrs.TrashOriginAttr, []byte(origin)); err != nil { + if err := n.SetMetadata(xattrs.TrashOriginAttr, origin); err != nil { return err } @@ -523,13 +523,13 @@ func (t *Tree) RestoreRecycleItemFunc(ctx context.Context, spaceid, key, trashPa targetNode.Exists = true // update name attribute - if err := xattr.Set(nodePath, xattrs.NameAttr, []byte(targetNode.Name)); err != nil { + if err := recycleNode.SetMetadata(xattrs.NameAttr, targetNode.Name); err != nil { return errors.Wrap(err, "Decomposedfs: could not set name attribute") } // set ParentidAttr to restorePath's node parent id if trashPath != "" { - if err := xattr.Set(nodePath, xattrs.ParentidAttr, []byte(targetNode.ParentID)); err != nil { + if err := recycleNode.SetMetadata(xattrs.ParentidAttr, targetNode.ParentID); err != nil { return errors.Wrap(err, "Decomposedfs: could not set name attribute") } } @@ -734,13 +734,13 @@ func calculateTreeSize(ctx context.Context, nodePath string) (uint64, error) { size += uint64(blobSize) } else { // read from attr - var b []byte - // xattr.Get will follow the symlink - if b, err = xattr.Get(cPath, xattrs.TreesizeAttr); err != nil { + var b string + // xattrs.Get will follow the symlink + if b, err = xattrs.Get(cPath, xattrs.TreesizeAttr); err != nil { // TODO recursively descend and recalculate treesize continue // continue after an error } - csize, err := strconv.ParseUint(string(b), 10, 64) + csize, err := strconv.ParseUint(b, 10, 64) if err != nil { // TODO recursively descend and recalculate treesize continue // continue after an error @@ -797,48 +797,48 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceid, key, path string) ( return } - var attrBytes []byte + var attrStr string trashNodeID := filepath.Base(link) deletedNodePath = t.lookup.InternalPath(trashNodeID) owner := &userpb.UserId{} // lookup ownerId in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.OwnerIDAttr); err == nil { - owner.OpaqueId = string(attrBytes) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.OwnerIDAttr); err == nil { + owner.OpaqueId = attrStr } else { return } // lookup ownerIdp in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.OwnerIDPAttr); err == nil { - owner.Idp = string(attrBytes) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.OwnerIDPAttr); err == nil { + owner.Idp = attrStr } else { return } // lookup ownerType in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.OwnerTypeAttr); err == nil { - owner.Type = utils.UserTypeMap(string(attrBytes)) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.OwnerTypeAttr); err == nil { + owner.Type = utils.UserTypeMap(string(attrStr)) } else { return } recycleNode = node.New(trashNodeID, "", "", 0, "", owner, t.lookup) // lookup blobID in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.BlobIDAttr); err == nil { - recycleNode.BlobID = string(attrBytes) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.BlobIDAttr); err == nil { + recycleNode.BlobID = attrStr } else { return } // lookup parent id in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.ParentidAttr); err == nil { - recycleNode.ParentID = string(attrBytes) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.ParentidAttr); err == nil { + recycleNode.ParentID = attrStr } else { return } // lookup name in extended attributes - if attrBytes, err = xattr.Get(deletedNodePath, xattrs.NameAttr); err == nil { - recycleNode.Name = string(attrBytes) + if attrStr, err = xattrs.Get(deletedNodePath, xattrs.NameAttr); err == nil { + recycleNode.Name = attrStr } else { return } @@ -871,8 +871,8 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceid, key, path string) ( deletedNodeRootPath = t.lookup.InternalPath(filepath.Base(rootLink)) } // lookup origin path in extended attributes - if attrBytes, err = xattr.Get(deletedNodeRootPath, xattrs.TrashOriginAttr); err == nil { - origin = filepath.Join(string(attrBytes), path) + if attrStr, err = xattrs.Get(deletedNodeRootPath, xattrs.TrashOriginAttr); err == nil { + origin = filepath.Join(string(attrStr), path) } else { log.Error().Err(err).Str("trashItem", trashItem).Str("link", link).Str("deletedNodePath", deletedNodePath).Msg("could not read origin path, restoring to /") } diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 9688e042fc..72da752300 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -22,6 +22,7 @@ import ( "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/pkg/errors" "github.com/pkg/xattr" ) @@ -127,3 +128,55 @@ func CopyMetadata(s, t string, filter func(attributeName string) bool) error { } return nil } + +// The primitive function to set the extended attribute +func Set(filePath string, key string, val string) error { + + if err := xattr.Set(filePath, key, []byte(val)); err != nil { + return errors.Wrap(err, "xattrs: Could not write xtended attribute") + } + return nil +} + +func SetMutltiple(filePath string, attribs map[string]string) error { + + // FIXME: Lock here + for key, val := range attribs { + if err := Set(filePath, key, val); err != nil { + return err + } + } + return nil +} + +// Primitive function to get a value of extended attribs +func Get(filePath, key string) (string, error) { + + v, err := xattr.Get(filePath, key) + if err != nil { + return "", errors.Wrap(err, "xattrs: Can not read value") + } + val := string(v) + return val, nil +} + +// Primitive function to get all extended attributes back +func All(filePath string) (map[string]string, error) { + var attribs = make(map[string]string) + var attrNames []string + var err error + if attrNames, err = xattr.List(filePath); err != nil { + return attribs, errors.Wrap(err, "xattrs: Can not list extended attributes") + } + + for i := range attrNames { + name := attrNames[i] + val, err := xattr.Get(filePath, name) + if err != nil { + return nil, errors.Wrap(err, "Failed to read extended attrib") + } + attribs[name] = string(val) + } + + return attribs, nil +} From 494f69847ecb4eafe1e651b15ba91bec6c1744dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 7 Feb 2022 13:54:21 +0100 Subject: [PATCH 02/16] hound and typos --- pkg/storage/utils/decomposedfs/node/node.go | 5 +++-- pkg/storage/utils/decomposedfs/xattrs/xattrs.go | 10 ++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index c94d326e8b..38eec29b71 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -115,7 +115,7 @@ func (n *Node) ChangeOwner(new *userpb.UserId) (err error) { xattrs.OwnerIDPAttr: new.Idp, xattrs.OwnerTypeAttr: utils.UserTypeToString(new.Type)} - if err := xattrs.SetMutltiple(nodePath, attribs); err != nil { + if err := xattrs.SetMultiple(nodePath, attribs); err != nil { return err } @@ -132,6 +132,7 @@ func (n *Node) SetMetadata(key string, val string) (err error) { return nil } +// GetMetadata reads the metadata for the given key func (n *Node) GetMetadata(key string) (val string, err error) { nodePath := n.InternalPath() if val, err := xattrs.Get(nodePath, key); err != nil { @@ -159,7 +160,7 @@ func (n *Node) WriteMetadata(owner *userpb.UserId) (err error) { attribs[xattrs.OwnerIDPAttr] = owner.Idp attribs[xattrs.OwnerTypeAttr] = utils.UserTypeToString(owner.Type) } - if err := xattrs.SetMutltiple(nodePath, attribs); err != nil { + if err := xattrs.SetMultiple(nodePath, attribs); err != nil { return err } return diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 72da752300..52bee6c4e2 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -129,7 +129,7 @@ func CopyMetadata(s, t string, filter func(attributeName string) bool) error { return nil } -// The primitive function to set the extended attribute +// Set an extended attribute key to the given value func Set(filePath string, key string, val string) error { if err := xattr.Set(filePath, key, []byte(val)); err != nil { @@ -138,7 +138,9 @@ func Set(filePath string, key string, val string) error { return nil } -func SetMutltiple(filePath string, attribs map[string]string) error { +// SetMultiple allows setting multiple key value pairs at once +// TODO the changes are protected with an flock +func SetMultiple(filePath string, attribs map[string]string) error { // FIXME: Lock here for key, val := range attribs { @@ -149,7 +151,7 @@ func SetMutltiple(filePath string, attribs map[string]string) error { return nil } -// Primitive function to get a value of extended attribs +// Get an extended attribute value for the given key func Get(filePath, key string) (string, error) { v, err := xattr.Get(filePath, key) @@ -160,7 +162,7 @@ func Get(filePath, key string) (string, error) { return val, nil } -// Primitive function to get all extended attributes back +// All reads all extended attributes for a node func All(filePath string) (map[string]string, error) { var attribs = make(map[string]string) var attrNames []string From 5b2515255ae44b570612d60da17fb583285d11da Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Mon, 7 Feb 2022 21:26:17 +0100 Subject: [PATCH 03/16] Add changelog. --- changelog/unreleased/consolidate-xattr.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/consolidate-xattr.md diff --git a/changelog/unreleased/consolidate-xattr.md b/changelog/unreleased/consolidate-xattr.md new file mode 100644 index 0000000000..845fe7c135 --- /dev/null +++ b/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 From 5017a9bf0bceed94aada230f48e299b89fcc2a54 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Tue, 8 Feb 2022 18:29:14 +0100 Subject: [PATCH 04/16] Some more fixes to use xattrs functions. --- .../utils/decomposedfs/decomposedfs.go | 2 +- pkg/storage/utils/decomposedfs/node/node.go | 62 +++++++++---------- pkg/storage/utils/decomposedfs/tree/tree.go | 6 +- pkg/storage/utils/decomposedfs/upload.go | 2 +- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 3e6064475d..396dd0cb7f 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -222,7 +222,7 @@ func (fs *Decomposedfs) CreateHome(ctx context.Context) (err error) { // update the owner u := ctxpkg.ContextMustGetUser(ctx) - if err = h.WriteNodeMetadata(u.Id); err != nil { + if err = h.WriteAllNodeMetadata(u.Id); err != nil { return } diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 38eec29b71..117f7cd7e2 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -135,14 +135,14 @@ func (n *Node) SetMetadata(key string, val string) (err error) { // GetMetadata reads the metadata for the given key func (n *Node) GetMetadata(key string) (val string, err error) { nodePath := n.InternalPath() - if val, err := xattrs.Get(nodePath, key); err != nil { - return errors.Wrap(err, "Decomposedfs: could not get extended attribute") + if val, err = xattrs.Get(nodePath, key); err != nil { + return "", errors.Wrap(err, "Decomposedfs: could not get extended attribute") } - return val + return val, nil } -// WriteMetadata writes the Node metadata to disk -func (n *Node) WriteMetadata(owner *userpb.UserId) (err error) { +// WriteAllNodeMetadata writes the Node metadata to disk +func (n *Node) WriteAllNodeMetadata(owner *userpb.UserId) (err error) { attribs := make(map[string]string) attribs[xattrs.ParentidAttr] = n.ParentID @@ -176,11 +176,11 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error nodePath := n.InternalPath() // lookup parent id in extended attributes - var attrBytes []byte - attrBytes, err = xattrs.Get(nodePath, xattrs.ParentidAttr) + var attr string + attr, err = xattrs.Get(nodePath, xattrs.ParentidAttr) switch { case err == nil: - n.ParentID = string(attrBytes) + n.ParentID = attr case isAttrUnset(err): return nil, errtypes.InternalError(err.Error()) case isNotFound(err): @@ -194,14 +194,14 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error n.SpaceRoot = n } // lookup name in extended attributes - if attrBytes, err = xattrs.Get(nodePath, xattrs.NameAttr); err == nil { - n.Name = string(attrBytes) + if attr, err = xattrs.Get(nodePath, xattrs.NameAttr); err == nil { + n.Name = attr } else { return } // lookup blobID in extended attributes - if attrBytes, err = xattrs.Get(nodePath, xattrs.BlobIDAttr); err == nil { - n.BlobID = string(attrBytes) + if attr, err = xattrs.Get(nodePath, xattrs.BlobIDAttr); err == nil { + n.BlobID = string(attr) } else { return } @@ -313,13 +313,13 @@ func (n *Node) Owner() (*userpb.UserId, error) { // TODO what if this is a reference? nodePath := n.InternalPath() // lookup parent id in extended attributes - var attrBytes []byte + var attr string var err error // lookup ID in extended attributes - attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerIDAttr) + attr, err = xattrs.Get(nodePath, xattrs.OwnerIDAttr) switch { case err == nil: - owner.OpaqueId = string(attrBytes) + owner.OpaqueId = string(attr) case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -327,10 +327,10 @@ func (n *Node) Owner() (*userpb.UserId, error) { } // lookup IDP in extended attributes - attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerIDPAttr) + attr, err = xattrs.Get(nodePath, xattrs.OwnerIDPAttr) switch { case err == nil: - owner.Idp = string(attrBytes) + owner.Idp = string(attr) case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -338,10 +338,10 @@ func (n *Node) Owner() (*userpb.UserId, error) { } // lookup type in extended attributes - attrBytes, err = xattrs.Get(nodePath, xattrs.OwnerTypeAttr) + attr, err = xattrs.Get(nodePath, xattrs.OwnerTypeAttr) switch { case err == nil: - owner.Type = utils.UserTypeMap(string(attrBytes)) + owner.Type = utils.UserTypeMap(string(attr)) case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -491,7 +491,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi return } - var target []byte + var target string switch { case fi.IsDir(): if target, err = xattrs.Get(nodePath, xattrs.ReferenceAttr); err == nil { @@ -679,7 +679,7 @@ func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string case err == nil: ri.Checksum = &provider.ResourceChecksum{ Type: storageprovider.PKG2GRPCXS(algo), - Sum: hex.EncodeToString(v), + Sum: hex.EncodeToString([]byte(v)), } case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") @@ -701,7 +701,7 @@ func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *prov } ri.Opaque.Map[algo] = &types.OpaqueEntry{ Decoder: "plain", - Value: []byte(hex.EncodeToString(v)), + Value: []byte(hex.EncodeToString([]byte(v))), } case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") @@ -730,7 +730,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso } ri.Opaque.Map[QuotaKey] = &types.OpaqueEntry{ Decoder: "plain", - Value: v, + Value: []byte(v), } } else { appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("quota", string(v)).Msg("malformed quota") @@ -754,11 +754,11 @@ func (n *Node) HasPropagation() (propagation bool) { // GetTMTime reads the tmtime from the extended attributes func (n *Node) GetTMTime() (tmTime time.Time, err error) { - var b []byte + var b string if b, err = xattrs.Get(n.lu.InternalPath(n.ID), xattrs.TreeMTimeAttr); err != nil { return } - return time.Parse(time.RFC3339Nano, string(b)) + return time.Parse(time.RFC3339Nano, b) } // SetTMTime writes the tmtime to the extended attributes @@ -768,7 +768,7 @@ func (n *Node) SetTMTime(t time.Time) (err error) { // GetTreeSize reads the treesize from the extended attributes func (n *Node) GetTreeSize() (treesize uint64, err error) { - var b []byte + var b string if b, err = xattrs.Get(n.InternalPath(), xattrs.TreesizeAttr); err != nil { return } @@ -896,12 +896,12 @@ func (n *Node) ListGrantees(ctx context.Context) (grantees []string, err error) // ReadGrant reads a CS3 grant func (n *Node) ReadGrant(ctx context.Context, grantee string) (g *provider.Grant, err error) { - var b []byte + var b string if b, err = xattrs.Get(n.InternalPath(), grantee); err != nil { return nil, err } var e *ace.ACE - if e, err = ace.Unmarshal(strings.TrimPrefix(grantee, xattrs.GrantPrefix), b); err != nil { + if e, err = ace.Unmarshal(strings.TrimPrefix(grantee, xattrs.GrantPrefix), []byte(b)); err != nil { return nil, err } return e.Grant(), nil @@ -933,11 +933,11 @@ func (n *Node) ListGrants(ctx context.Context) ([]*provider.Grant, error) { // ReadBlobSizeAttr reads the blobsize from the xattrs func ReadBlobSizeAttr(path string) (int64, error) { - attrBytes, err := xattrs.Get(path, xattrs.BlobsizeAttr) + attr, err := xattrs.Get(path, xattrs.BlobsizeAttr) if err != nil { return 0, errors.Wrapf(err, "error reading blobsize xattr") } - blobSize, err := strconv.ParseInt(string(attrBytes), 10, 64) + blobSize, err := strconv.ParseInt(string(attr), 10, 64) if err != nil { return 0, errors.Wrapf(err, "invalid blobsize xattr format") } @@ -1008,7 +1008,7 @@ var CheckQuota = func(spaceRoot *Node, fileSize uint64) (quotaSufficient bool, e } quotaByte, _ := xattrs.Get(spaceRoot.InternalPath(), xattrs.QuotaAttr) var total uint64 - if quotaByte == nil { + if quotaByte == "" { // if quota is not set, it means unlimited return true, nil } diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 20689bc7c2..02d9be1d33 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -118,9 +118,9 @@ func (t *Tree) Setup(owner *userpb.UserId, propagateToRoot bool) error { } // set propagation flag - v := []byte("0") + v := "0" if propagateToRoot { - v = []byte("1") + v = "1" } if err = n.SetMetadata(xattrs.PropagationAttr, v); err != nil { return err @@ -779,7 +779,7 @@ func (t *Tree) createNode(n *node.Node, owner *userpb.UserId) (err error) { return errors.Wrap(err, "Decomposedfs: error creating node") } - return n.WriteMetadata(owner) + return n.WriteAllNodeMetadata(owner) } // TODO refactor the returned params into Node properties? would make all the path transformations go away... diff --git a/pkg/storage/utils/decomposedfs/upload.go b/pkg/storage/utils/decomposedfs/upload.go index 3a4a4e17b4..895b590d35 100644 --- a/pkg/storage/utils/decomposedfs/upload.go +++ b/pkg/storage/utils/decomposedfs/upload.go @@ -602,7 +602,7 @@ func (upload *fileUpload) FinishUpload(ctx context.Context) (err error) { tryWritingChecksum(&sublog, n, "adler32", adler32h) // who will become the owner? the owner of the parent actually ... not the currently logged in user - err = n.WriteMetadata(&userpb.UserId{ + err = n.WriteAllNodeMetadata(&userpb.UserId{ Idp: upload.info.Storage["OwnerIdp"], OpaqueId: upload.info.Storage["OwnerId"], }) From 3713ed3bfa6ea73af08f85a4b845a806d8c93dec Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 17:50:18 +0100 Subject: [PATCH 05/16] Fix function name in tests. --- pkg/storage/utils/decomposedfs/node/node_test.go | 2 +- pkg/storage/utils/decomposedfs/testhelpers/helpers.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node_test.go b/pkg/storage/utils/decomposedfs/node/node_test.go index 7c59af7fc1..956ccfd6db 100644 --- a/pkg/storage/utils/decomposedfs/node/node_test.go +++ b/pkg/storage/utils/decomposedfs/node/node_test.go @@ -97,7 +97,7 @@ var _ = Describe("Node", func() { Type: userpb.UserType_USER_TYPE_PRIMARY, } - err = n.WriteMetadata(owner) + err = n.WriteAllNodeMetadata(owner) Expect(err).ToNot(HaveOccurred()) n2, err := env.Lookup.NodeFromResource(env.Ctx, ref) Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go index dc2194bb97..102647811a 100644 --- a/pkg/storage/utils/decomposedfs/testhelpers/helpers.go +++ b/pkg/storage/utils/decomposedfs/testhelpers/helpers.go @@ -151,7 +151,7 @@ func (t *TestEnv) CreateTestFile(name, blobID string, blobSize int64, parentID s if err != nil { return nil, err } - err = file.WriteMetadata(t.Owner.Id) + err = file.WriteAllNodeMetadata(t.Owner.Id) if err != nil { return nil, err } From 761c7b36218522682065072eca1896ed9a901001 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 18:10:10 +0100 Subject: [PATCH 06/16] Fix some linting hints. --- pkg/storage/utils/decomposedfs/node/node.go | 6 +++--- pkg/storage/utils/decomposedfs/spaces.go | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 117f7cd7e2..cd230e6eef 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -330,7 +330,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { attr, err = xattrs.Get(nodePath, xattrs.OwnerIDPAttr) switch { case err == nil: - owner.Idp = string(attr) + owner.Idp = attr case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -589,7 +589,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi sublog.Debug(). Str("favorite", fa). Msg("found favorite flag") - favorite = string(val) + favorite = val } } else { sublog.Error().Err(errtypes.UserRequired("userrequired")).Msg("user has no id") @@ -937,7 +937,7 @@ func ReadBlobSizeAttr(path string) (int64, error) { if err != nil { return 0, errors.Wrapf(err, "error reading blobsize xattr") } - blobSize, err := strconv.ParseInt(string(attr), 10, 64) + blobSize, err := strconv.ParseInt(attr, 10, 64) if err != nil { return 0, errors.Wrapf(err, "invalid blobsize xattr format") } diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index 87aa1df2e4..611f0bcdd1 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -542,6 +542,7 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, var sname string if sname, err = n.GetMetadata(xattrs.SpaceNameAttr); err != nil { // FIXME: Is that a severe problem? + appctx.GetLogger(ctx).Debug().Err(err).Msg("space does not have a name attribute") } if err := n.FindStorageSpaceRoot(); err != nil { From 316edd707fa9db25218ab396000a124ef700bbfa Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 18:22:27 +0100 Subject: [PATCH 07/16] Even more linter warning fixes --- pkg/storage/utils/decomposedfs/node/node.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index cd230e6eef..0dbaa42d62 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -319,7 +319,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { attr, err = xattrs.Get(nodePath, xattrs.OwnerIDAttr) switch { case err == nil: - owner.OpaqueId = string(attr) + owner.OpaqueId = attr case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -722,7 +722,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso // -1 = uncalculated // -2 = unknown // -3 = unlimited - if _, err := strconv.ParseInt(string(v), 10, 64); err == nil { + if _, err := strconv.ParseInt(v, 10, 64); err == nil { if ri.Opaque == nil { ri.Opaque = &types.Opaque{ Map: map[string]*types.OpaqueEntry{}, @@ -733,7 +733,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso Value: []byte(v), } } else { - appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("quota", string(v)).Msg("malformed quota") + appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("quota", v).Msg("malformed quota") } case isAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Msg("quota not set") From 14b54f5d8013189da3e3fba29f2c893b57b84d29 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 19:33:45 +0100 Subject: [PATCH 08/16] Even more linting issues --- pkg/storage/utils/decomposedfs/node/node.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 0dbaa42d62..71d8b7d58c 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -652,7 +652,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi k := attrs[i][len(xattrs.MetadataPrefix):] if _, ok := mdKeysMap[k]; returnAllKeys || ok { if val, err := xattrs.Get(nodePath, attrs[i]); err == nil { - metadata[k] = string(val) + metadata[k] = val } else { sublog.Error().Err(err). Str("entry", attrs[i]). @@ -747,7 +747,7 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso // HasPropagation checks if the propagation attribute exists and is set to "1" func (n *Node) HasPropagation() (propagation bool) { if b, err := xattrs.Get(n.lu.InternalPath(n.ID), xattrs.PropagationAttr); err == nil { - return string(b) == "1" + return b == "1" } return false } @@ -772,7 +772,7 @@ func (n *Node) GetTreeSize() (treesize uint64, err error) { if b, err = xattrs.Get(n.InternalPath(), xattrs.TreesizeAttr); err != nil { return } - return strconv.ParseUint(string(b), 10, 64) + return strconv.ParseUint(b, 10, 64) } // SetTreeSize writes the treesize to the extended attributes From 5b332787e94ad5dc6155bef29d0e689929ba06b5 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 19:52:45 +0100 Subject: [PATCH 09/16] And another iteration --- pkg/storage/utils/decomposedfs/node/node.go | 8 ++++---- pkg/storage/utils/decomposedfs/xattrs/xattrs.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 71d8b7d58c..394f63c1e2 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -201,7 +201,7 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error } // lookup blobID in extended attributes if attr, err = xattrs.Get(nodePath, xattrs.BlobIDAttr); err == nil { - n.BlobID = string(attr) + n.BlobID = attr } else { return } @@ -341,7 +341,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { attr, err = xattrs.Get(nodePath, xattrs.OwnerTypeAttr) switch { case err == nil: - owner.Type = utils.UserTypeMap(string(attr)) + owner.Type = utils.UserTypeMap(attr) case isAttrUnset(err), isNotFound(err): fallthrough default: @@ -553,7 +553,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi // use temporary etag if it is set if b, err := xattrs.Get(nodePath, xattrs.TmpEtagAttr); err == nil { - ri.Etag = fmt.Sprintf(`"%x"`, string(b)) // TODO why do we convert string(b)? is the temporary etag stored as string? -> should we use bytes? use hex.EncodeToString? + ri.Etag = fmt.Sprintf(`"%x"`, b) // TODO why do we convert string(b)? is the temporary etag stored as string? -> should we use bytes? use hex.EncodeToString? } else if ri.Etag, err = calculateEtag(n.ID, tmTime); err != nil { sublog.Debug().Err(err).Msg("could not calculate etag") } @@ -1012,7 +1012,7 @@ var CheckQuota = func(spaceRoot *Node, fileSize uint64) (quotaSufficient bool, e // if quota is not set, it means unlimited return true, nil } - total, _ = strconv.ParseUint(string(quotaByte), 10, 64) + total, _ = strconv.ParseUint(quotaByte, 10, 64) // if total is smaller than used, total-used could overflow and be bigger than fileSize if fileSize > total-used || total < used { return false, errtypes.InsufficientStorage("quota exceeded") diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 52bee6c4e2..7e902f9778 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -156,7 +156,7 @@ func Get(filePath, key string) (string, error) { v, err := xattr.Get(filePath, key) if err != nil { - return "", errors.Wrap(err, "xattrs: Can not read value") + return "", errors.Wrap(err, "xattrs: Can not read xattr") } val := string(v) return val, nil From e5c054899e23c2c1f1ffb273404416bc41431511 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Wed, 9 Feb 2022 21:32:19 +0100 Subject: [PATCH 10/16] Linter I hate you --- pkg/storage/utils/decomposedfs/node/node.go | 2 +- pkg/storage/utils/decomposedfs/tree/tree.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 394f63c1e2..710aa4681e 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -525,7 +525,7 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi Type: nodeType, MimeType: mime.Detect(nodeType == provider.ResourceType_RESOURCE_TYPE_CONTAINER, fn), Size: uint64(n.Blobsize), - Target: string(target), + Target: target, PermissionSet: rp, } diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 02d9be1d33..ef94fe30c3 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -207,7 +207,7 @@ func (t *Tree) linkSpace(spaceType, spaceID, nodeID string) { func isRootNode(nodePath string) bool { attr, err := xattrs.Get(nodePath, xattrs.ParentidAttr) - return err == nil && string(attr) == "root" + return err == nil && attr == "root" } func isSharedNode(nodePath string) bool { if attrs, err := xattr.List(nodePath); err == nil { @@ -816,7 +816,7 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceid, key, path string) ( } // lookup ownerType in extended attributes if attrStr, err = xattrs.Get(deletedNodePath, xattrs.OwnerTypeAttr); err == nil { - owner.Type = utils.UserTypeMap(string(attrStr)) + owner.Type = utils.UserTypeMap(attrStr) } else { return } @@ -872,7 +872,7 @@ func (t *Tree) readRecycleItem(ctx context.Context, spaceid, key, path string) ( } // lookup origin path in extended attributes if attrStr, err = xattrs.Get(deletedNodeRootPath, xattrs.TrashOriginAttr); err == nil { - origin = filepath.Join(string(attrStr), path) + origin = filepath.Join(attrStr, path) } else { log.Error().Err(err).Str("trashItem", trashItem).Str("link", link).Str("deletedNodePath", deletedNodePath).Msg("could not read origin path, restoring to /") } From 745b22da8b1d6325343ff3ed06f04f06944c6d8a Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 10 Feb 2022 11:54:35 +0100 Subject: [PATCH 11/16] Use proper Int formatting Co-authored-by: David Christofas --- pkg/storage/utils/decomposedfs/node/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 710aa4681e..6f355d4016 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -148,7 +148,7 @@ func (n *Node) WriteAllNodeMetadata(owner *userpb.UserId) (err error) { attribs[xattrs.ParentidAttr] = n.ParentID attribs[xattrs.NameAttr] = n.Name attribs[xattrs.BlobIDAttr] = n.BlobID - attribs[xattrs.BlobsizeAttr] = fmt.Sprintf("%d", n.Blobsize) + attribs[xattrs.BlobsizeAttr] = strconv.FormatInt(n.Blobsize, 10) nodePath := n.InternalPath() attribs[xattrs.OwnerIDAttr] = "" From 35f36ebb1ad3ff8ca8496fe0ca1f4d633a3cdf50 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 10 Feb 2022 11:54:56 +0100 Subject: [PATCH 12/16] Update pkg/storage/utils/decomposedfs/node/node.go Co-authored-by: David Christofas --- pkg/storage/utils/decomposedfs/node/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 6f355d4016..9c75acb653 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -281,7 +281,7 @@ func (n *Node) Parent() (p *Node, err error) { // lookup parent id in extended attributes if p.ParentID, err = xattrs.Get(parentPath, xattrs.ParentidAttr); err != nil { - p.ParentID = string("") + p.ParentID = "" return } // lookup name in extended attributes From 754bf614188e69b71855b98e514c2ee3d80c8578 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 10 Feb 2022 11:55:05 +0100 Subject: [PATCH 13/16] Update pkg/storage/utils/decomposedfs/node/node.go Co-authored-by: David Christofas --- pkg/storage/utils/decomposedfs/node/node.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 9c75acb653..6368da6e8e 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -286,8 +286,8 @@ func (n *Node) Parent() (p *Node, err error) { } // lookup name in extended attributes if p.Name, err = xattrs.Get(parentPath, xattrs.NameAttr); err != nil { - p.Name = string("") - p.ParentID = string("") + p.Name = "" + p.ParentID = "" return } From 84872a9ec3824054b3f0c502e9f6bb6e2c14a1fa Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 10 Feb 2022 11:56:18 +0100 Subject: [PATCH 14/16] Update pkg/storage/utils/decomposedfs/xattrs/xattrs.go Co-authored-by: David Christofas --- pkg/storage/utils/decomposedfs/xattrs/xattrs.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 7e902f9778..a9b7b3f943 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -164,15 +164,13 @@ func Get(filePath, key string) (string, error) { // All reads all extended attributes for a node func All(filePath string) (map[string]string, error) { - var attribs = make(map[string]string) - var attrNames []string - var err error - if attrNames, err = xattr.List(filePath); err != nil { - return attribs, errors.Wrap(err, "xattrs: Can not list extended attributes") + attrNames, err := xattr.List(filePath) + if err != nil { + return nil, errors.Wrap(err, "xattrs: Can not list extended attributes") } - for i := range attrNames { - name := attrNames[i] + attribs := make(map[string]string, len(attrNames)) + for _, name := range attrNames { val, err := xattr.Get(filePath, name) if err != nil { return nil, errors.Wrap(err, "Failed to read extended attrib") From af69f059baaec0707722e2203cb258852e5c9102 Mon Sep 17 00:00:00 2001 From: Klaas Freitag Date: Thu, 10 Feb 2022 13:18:13 +0100 Subject: [PATCH 15/16] again linting --- pkg/storage/utils/decomposedfs/xattrs/xattrs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index a9b7b3f943..f65900c202 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -169,7 +169,7 @@ func All(filePath string) (map[string]string, error) { return nil, errors.Wrap(err, "xattrs: Can not list extended attributes") } - attribs := make(map[string]string, len(attrNames)) + attribs := make(map[string]string, len(attrNames)) for _, name := range attrNames { val, err := xattr.Get(filePath, name) if err != nil { From 628306869ffa7ff16058931083df9dfeb6ac5240 Mon Sep 17 00:00:00 2001 From: David Christofas Date: Thu, 10 Feb 2022 14:45:19 +0100 Subject: [PATCH 16/16] use correct variable in decomposedfs --- pkg/storage/utils/decomposedfs/decomposedfs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/decomposedfs.go b/pkg/storage/utils/decomposedfs/decomposedfs.go index 6b61f05809..4dd5b313f7 100644 --- a/pkg/storage/utils/decomposedfs/decomposedfs.go +++ b/pkg/storage/utils/decomposedfs/decomposedfs.go @@ -430,11 +430,11 @@ func (fs *Decomposedfs) CreateReference(ctx context.Context, p string, targetURI } childCreated = true - if err := n.SetMetadata(xattrs.ReferenceAttr, 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(), - n.InternalPath(), + childNode.InternalPath(), ) span.SetStatus(codes.Error, err.Error()) return err