From 58fd2d3305425cfac71b914d1e9239c03403fee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 8 Jan 2021 17:33:40 +0000 Subject: [PATCH 1/5] allow setting favorites, mtime and a temporary etag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/fs/ocis/metadata.go | 142 +++++++++++++++++++++--- pkg/storage/fs/ocis/node.go | 188 ++++++++++++++++++++++++++------ pkg/storage/fs/ocis/ocis.go | 2 +- 3 files changed, 282 insertions(+), 50 deletions(-) diff --git a/pkg/storage/fs/ocis/metadata.go b/pkg/storage/fs/ocis/metadata.go index ff167a8b41..47cdbc885c 100644 --- a/pkg/storage/fs/ocis/metadata.go +++ b/pkg/storage/fs/ocis/metadata.go @@ -20,20 +20,37 @@ package ocis import ( "context" + "fmt" "path/filepath" + "strconv" + "strings" + "time" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/errtypes" + "github.com/cs3org/reva/pkg/user" "github.com/pkg/errors" "github.com/pkg/xattr" ) +func parseMTime(v string) (t time.Time, err error) { + p := strings.SplitN(v, ".", 2) + var sec, nsec int64 + if sec, err = strconv.ParseInt(p[0], 10, 64); err == nil { + if len(p) > 1 { + nsec, err = strconv.ParseInt(p[1], 10, 64) + } + } + return time.Unix(sec, nsec), err +} + func (fs *ocisfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Reference, md *provider.ArbitraryMetadata) (err error) { n, err := fs.lu.NodeFromResource(ctx, ref) if err != nil { return errors.Wrap(err, "ocisfs: error resolving ref") } + sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger() if !n.Exists { err = errtypes.NotFound(filepath.Join(n.ParentID, n.Name)) @@ -52,14 +69,67 @@ func (fs *ocisfs) SetArbitraryMetadata(ctx context.Context, ref *provider.Refere } nodePath := n.lu.toInternalPath(n.ID) + + errs := []error{} + // TODO should we really continue updating when an error occurs? + if md.Metadata != nil { + if val, ok := md.Metadata["mtime"]; ok { + delete(md.Metadata, "mtime") + err := n.SetMtime(ctx, val) + if err != nil { + errs = append(errs, errors.Wrap(err, "could not set mtime")) + } + } + // TODO(jfd) special handling for atime? + // TODO(jfd) allow setting birth time (btime)? + // TODO(jfd) any other metadata that is interesting? fileid? + // TODO unset when file is updated + // TODO unset when folder is updated or add timestamp to etag? + if val, ok := md.Metadata["etag"]; ok { + delete(md.Metadata, "etag") + err := n.SetEtag(ctx, val) + if err != nil { + errs = append(errs, errors.Wrap(err, "could not set etag")) + } + } + if val, ok := md.Metadata[_favoriteKey]; ok { + delete(md.Metadata, _favoriteKey) + if u, ok := user.ContextGetUser(ctx); ok { + if uid := u.GetId(); uid != nil { + if err := n.SetFavorite(uid, val); err != nil { + sublog.Error().Err(err). + Interface("user", u). + Msg("could not set favorite flag") + errs = append(errs, errors.Wrap(err, "could not set favorite flag")) + } + } else { + sublog.Error().Interface("user", u).Msg("user has no id") + errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "user has no id")) + } + } else { + sublog.Error().Interface("user", u).Msg("error getting user from ctx") + errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")) + } + } + } for k, v := range md.Metadata { - // TODO set etag as temporary etag tmpEtagAttr attrName := metadataPrefix + k if err = xattr.Set(nodePath, attrName, []byte(v)); err != nil { - return errors.Wrap(err, "ocisfs: could not set metadata attribute "+attrName+" to "+k) + errs = append(errs, errors.Wrap(err, "ocisfs: could not set metadata attribute "+attrName+" to "+k)) } } - return + + switch len(errs) { + case 0: + return fs.tp.Propagate(ctx, n) + case 1: + // TODO Propagate if anything changed + return errs[0] + default: + // TODO Propagate if anything changed + // TODO how to return multiple errors? + return errors.New("multiple errors occurred, see log for details") + } } func (fs *ocisfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Reference, keys []string) (err error) { @@ -67,6 +137,7 @@ func (fs *ocisfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refe if err != nil { return errors.Wrap(err, "ocisfs: error resolving ref") } + sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger() if !n.Exists { err = errtypes.NotFound(filepath.Join(n.ParentID, n.Name)) @@ -74,7 +145,7 @@ func (fs *ocisfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refe } ok, err := fs.p.HasPermission(ctx, n, func(rp *provider.ResourcePermissions) bool { - // TODO use SetArbitraryMetadata grant to CS3 api, tracked in https://github.com/cs3org/cs3apis/issues/91 + // TODO use SetArbitraryMetadata grant to CS3 api, tracked in https://github.com/cs3org/cs3apis/issues/91 return rp.InitiateFileUpload }) switch { @@ -85,22 +156,57 @@ func (fs *ocisfs) UnsetArbitraryMetadata(ctx context.Context, ref *provider.Refe } nodePath := n.lu.toInternalPath(n.ID) - for i := range keys { - attrName := metadataPrefix + keys[i] - if err = xattr.Remove(nodePath, attrName); err != nil { - // a non-existing attribute will return an error, which we can ignore - // (using string compare because the error type is syscall.Errno and not wrapped/recognizable) - if e, ok := err.(*xattr.Error); !ok || !(e.Err.Error() == "no data available" || - // darwin - e.Err.Error() == "attribute not found") { - appctx.GetLogger(ctx).Error().Err(err). - Interface("node", n). - Str("key", keys[i]). - Msg("could not unset metadata") + errs := []error{} + for _, k := range keys { + switch k { + case _favoriteKey: + if u, ok := user.ContextGetUser(ctx); ok { + // 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", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) + if err := xattr.Remove(nodePath, fa); err != nil { + sublog.Error().Err(err). + Interface("user", u). + Str("key", fa). + Msg("could not unset favorite flag") + errs = append(errs, errors.Wrap(err, "could not unset favorite flag")) + } + } else { + sublog.Error(). + Interface("user", u). + Msg("user has no id") + errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "user has no id")) + } } else { - err = nil + sublog.Error(). + Interface("user", u). + Msg("error getting user from ctx") + errs = append(errs, errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")) + } + default: + if err = xattr.Remove(nodePath, metadataPrefix+k); err != nil { + // a non-existing attribute will return an error, which we can ignore + // (using string compare because the error type is syscall.Errno and not wrapped/recognizable) + if e, ok := err.(*xattr.Error); !ok || !(e.Err.Error() == "no data available" || + // darwin + e.Err.Error() == "attribute not found") { + sublog.Error().Err(err). + Str("key", k). + Msg("could not unset metadata") + errs = append(errs, errors.Wrap(err, "could not unset metadata")) + } } } } - return + switch len(errs) { + case 0: + return fs.tp.Propagate(ctx, n) + case 1: + // TODO Propagate if anything changed + return errs[0] + default: + // TODO Propagate if anything changed + // TODO how to return multiple errors? + return errors.New("multiple errors occurred, see log for details") + } } diff --git a/pkg/storage/fs/ocis/node.go b/pkg/storage/fs/ocis/node.go index 851597f12e..259780d6ae 100644 --- a/pkg/storage/fs/ocis/node.go +++ b/pkg/storage/fs/ocis/node.go @@ -34,7 +34,6 @@ import ( "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/errtypes" "github.com/cs3org/reva/pkg/mime" - "github.com/cs3org/reva/pkg/sdk/common" "github.com/cs3org/reva/pkg/storage/utils/ace" "github.com/cs3org/reva/pkg/user" "github.com/pkg/errors" @@ -45,6 +44,8 @@ import ( const ( _shareTypesKey = "http://owncloud.org/ns/share-types" _userShareType = "0" + + _favoriteKey = "http://owncloud.org/ns/favorite" ) // Node represents a node in the tree and provides methods to get a Parent or Child instance @@ -328,6 +329,97 @@ func (n *Node) PermissionSet(ctx context.Context) *provider.ResourcePermissions return noPermissions } +// calculateEtag returns a hash of fileid + tmtime (or mtime) +func calculateEtag(nodeID string, tmTime time.Time) (string, error) { + h := md5.New() + if _, err := io.WriteString(h, nodeID); err != nil { + return "", err + } + if tb, err := tmTime.UTC().MarshalBinary(); err == nil { + if _, err := h.Write(tb); err != nil { + return "", err + } + } else { + return "", err + } + return fmt.Sprintf(`"%x"`, h.Sum(nil)), nil +} + +// SetMtime sets the mtime and atime of a node +func (n *Node) SetMtime(ctx context.Context, mtime string) error { + sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger() + if mt, err := parseMTime(mtime); err == nil { + nodePath := n.lu.toInternalPath(n.ID) + // updating mtime also updates atime + if err := os.Chtimes(nodePath, mt, mt); err != nil { + sublog.Error().Err(err). + Time("mtime", mt). + Msg("could not set mtime") + return errors.Wrap(err, "could not set mtime") + } + } else { + sublog.Error().Err(err). + Str("mtime", mtime). + Msg("could not parse mtime") + return errors.Wrap(err, "could not parse mtime") + } + return nil +} + +// SetEtag sets the temporary etag of a node if it differs from the current etag +func (n *Node) SetEtag(ctx context.Context, val string) (err error) { + sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger() + nodePath := n.lu.toInternalPath(n.ID) + var tmTime time.Time + if tmTime, err = n.GetTMTime(); err != nil { + // no tmtime, use mtime + var fi os.FileInfo + if fi, err = os.Lstat(nodePath); err != nil { + return + } + tmTime = fi.ModTime() + } + var etag string + if etag, err = calculateEtag(n.ID, tmTime); err != nil { + return + } + + // sanitize etag + val = fmt.Sprintf("\"%s\"", strings.Trim(val, "\"")) + if etag == val { + sublog.Debug(). + Str("etag", val). + Msg("ignoring request to update identical etag") + return nil + } + // etag is only valid until the calculated etag changes, is part of propagation + return xattr.Set(nodePath, tmpEtagAttr, []byte(val)) +} + +// SetFavorite sets the favorite for the current user +// TODO we should not mess with the user here ... the favorites is now a user specific property for a file +// that cannot be mapped to extended attributes without leaking who has marked a file as a favorite +// it is a specific case of a tag, which is user individual as well +// TODO there are different types of tags +// 1. public that are managed by everyone +// 2. private tags that are only visible to the user +// 3. system tags that are only visible to the system +// 4. group tags that are only visible to a group ... +// urgh ... well this can be solved using different namespaces +// 1. public = p: +// 2. private = u:: for user specific +// 3. system = s: for system +// 4. group = g:: +// 5. app? = a:: for apps? +// obviously this only is secure when the u/s/g/a namespaces are not accessible by users in the filesystem +// public tags can be mapped to extended attributes +func (n *Node) SetFavorite(uid *userpb.UserId, val string) error { + nodePath := n.lu.toInternalPath(n.ID) + // the favorite flag is specific to the user, so we need to incorporate the userid + fa := fmt.Sprintf("%s%s@%s", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) + return xattr.Set(nodePath, fa, []byte(val)) +} + // AsResourceInfo return the node as CS3 ResourceInfo func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissions, mdKeys []string) (ri *provider.ResourceInfo, err error) { sublog := appctx.GetLogger(ctx).With().Interface("node", n).Logger() @@ -379,31 +471,21 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi sublog.Debug().Err(err).Msg("could not determine owner") } - // etag currently is a hash of fileid + tmtime (or mtime) // TODO make etag of files use fileid and checksum - // TODO implment adding temporery etag in an attribute to restore backups - h := md5.New() - if _, err := io.WriteString(h, n.ID); err != nil { - return nil, err - } + var tmTime time.Time if tmTime, err = n.GetTMTime(); err != nil { // no tmtime, use mtime tmTime = fi.ModTime() } - if tb, err := tmTime.UTC().MarshalBinary(); err == nil { - if _, err := h.Write(tb); err != nil { - return nil, err - } - } else { - return nil, err - } // use temporary etag if it is set if b, err := xattr.Get(nodePath, tmpEtagAttr); err == nil { ri.Etag = fmt.Sprintf(`"%x"`, string(b)) } else { - ri.Etag = fmt.Sprintf(`"%x"`, h.Sum(nil)) + if ri.Etag, err = calculateEtag(n.ID, tmTime); err != nil { + sublog.Debug().Err(err).Msg("could not calculate etag") + } } // mtime uses tmtime if present @@ -414,30 +496,74 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi Nanos: uint32(un % 1000000000), } - // TODO only read the requested metadata attributes - if attrs, err := xattr.List(nodePath); err == nil { - ri.ArbitraryMetadata = &provider.ArbitraryMetadata{ - Metadata: map[string]string{}, - } - for i := range attrs { - if strings.HasPrefix(attrs[i], metadataPrefix) { - k := strings.TrimPrefix(attrs[i], metadataPrefix) - if v, err := xattr.Get(nodePath, attrs[i]); err == nil { - ri.ArbitraryMetadata.Metadata[k] = string(v) - } else { - sublog.Error().Err(err).Str("attr", attrs[i]).Msg("could not get attribute value") + mdKeysMap := make(map[string]struct{}) + for _, k := range mdKeys { + mdKeysMap[k] = struct{}{} + } + + var returnAllKeys bool + if _, ok := mdKeysMap["*"]; len(mdKeys) == 0 || ok { + returnAllKeys = true + } + + metadata := map[string]string{} + + // read favorite flag for the current user + if _, ok := mdKeysMap[_favoriteKey]; returnAllKeys || ok { + favorite := "" + if u, ok := user.ContextGetUser(ctx); ok { + // 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", favPrefix, uid.GetOpaqueId(), uid.GetIdp()) + if val, err := xattr.Get(nodePath, fa); err == nil { + sublog.Debug(). + Str("favorite", fa). + Msg("found favorite flag") + favorite = string(val) } + } else { + sublog.Error().Err(errtypes.UserRequired("userrequired")).Msg("user has no id") } + } else { + sublog.Error().Err(errtypes.UserRequired("userrequired")).Msg("error getting user from ctx") } - } else { - sublog.Error().Err(err).Msg("could not list attributes") + metadata[_favoriteKey] = favorite } - if common.FindString(mdKeys, _shareTypesKey) != -1 { + // share indicator + if _, ok := mdKeysMap[_shareTypesKey]; returnAllKeys || ok { if n.hasUserShares(ctx) { - ri.ArbitraryMetadata.Metadata[_shareTypesKey] = _userShareType + metadata[_shareTypesKey] = _userShareType + } + } + + // only read the requested metadata attributes + list, err := xattr.List(nodePath) + if err != nil { + sublog.Error().Err(err).Msg("error getting list of extended attributes") + } else { + for _, entry := range list { + // filter out non-custom properties + if !strings.HasPrefix(entry, metadataPrefix) { + continue + } + // only read when key was requested + k := entry[len(metadataPrefix):] + if _, ok := mdKeysMap[k]; returnAllKeys || ok { + if val, err := xattr.Get(nodePath, entry); err == nil { + metadata[k] = string(val) + } else { + sublog.Error().Err(err). + Str("entry", entry). + Msg("error retrieving xattr metadata") + } + } + } } + ri.ArbitraryMetadata = &provider.ArbitraryMetadata{ + Metadata: metadata, + } sublog.Debug(). Interface("ri", ri). diff --git a/pkg/storage/fs/ocis/ocis.go b/pkg/storage/fs/ocis/ocis.go index 1c3e211511..3e59a70bf3 100644 --- a/pkg/storage/fs/ocis/ocis.go +++ b/pkg/storage/fs/ocis/ocis.go @@ -63,7 +63,7 @@ const ( grantPrefix string = ocisPrefix + "grant." metadataPrefix string = ocisPrefix + "md." // TODO implement favorites metadata flag - //favPrefix string = ocisPrefix + "fav." // favorite flag, per user + favPrefix string = ocisPrefix + "fav." // favorite flag, per user // a temporary etag for a folder that is removed when the mtime propagation happens tmpEtagAttr string = ocisPrefix + "tmp.etag" From f0fe7fdb92ea002ae238d7a251f9e3a8b0d9efcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 11 Jan 2021 11:12:32 +0000 Subject: [PATCH 2/5] request unknown properties as arbitrary metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../http/services/owncloud/ocdav/propfind.go | 70 ++++++++++++++----- .../http/services/owncloud/ocdav/proppatch.go | 6 +- .../http/services/owncloud/ocdav/trashbin.go | 4 +- .../owncloud/ocs/data/capabilities.go | 1 + .../cloud/capabilities/capabilities.go | 1 + pkg/storage/fs/ocis/ocis.go | 5 +- 6 files changed, 64 insertions(+), 23 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index b0c17af211..17a7f3d922 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -45,6 +45,15 @@ import ( "github.com/pkg/errors" ) +const ( + _nsDav = "DAV:" + _nsOwncloud = "http://owncloud.org/ns" + _nsOCS = "http://open-collaboration-services.org/ns" + + _propOcFavorite = "http://owncloud.org/ns/favorite" + _propOcShareTypes = "http://owncloud.org/ns/share-types" +) + // ns is the namespace that is prefixed to the path in the cs3 namespace func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) { ctx := r.Context() @@ -98,14 +107,23 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) return } + metadataKeys := []string{} + if pf.Allprop != nil { + metadataKeys = append(metadataKeys, "*") + } else { + for i := range pf.Prop { + if requiresExplicitFetching(&pf.Prop[i]) { + metadataKeys = append(metadataKeys, metadataKeyOf(&pf.Prop[i])) + } + } + } + info := res.Info infos := []*provider.ResourceInfo{info} if info.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER && depth == "1" { req := &provider.ListContainerRequest{ - Ref: ref, - ArbitraryMetadataKeys: []string{ - "http://owncloud.org/ns/share-types", - }, + Ref: ref, + ArbitraryMetadataKeys: metadataKeys, } res, err := client.ListContainer(ctx, req) if err != nil { @@ -130,10 +148,8 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) Spec: &provider.Reference_Path{Path: path}, } req := &provider.ListContainerRequest{ - Ref: ref, - ArbitraryMetadataKeys: []string{ - "http://owncloud.org/ns/share-types", - }, + Ref: ref, + ArbitraryMetadataKeys: metadataKeys, } res, err := client.ListContainer(ctx, req) if err != nil { @@ -188,6 +204,23 @@ func (s *svc) handlePropfind(w http.ResponseWriter, r *http.Request, ns string) } } +func requiresExplicitFetching(n *xml.Name) bool { + switch n.Space { + case _nsDav: + return false + case _nsOwncloud: + switch n.Local { + case "favorite", "share-types": + return true + default: + return false + } + case _nsOCS: + return false + } + return true +} + // from https://github.com/golang/net/blob/e514e69ffb8bc3c76a71ae40de0118d794855992/webdav/xml.go#L178-L205 func readPropfind(r io.Reader) (pf propfindXML, status int, err error) { c := countingReader{r: r} @@ -253,6 +286,7 @@ func (s *svc) newPropNS(namespace string, local string, val string) *propertyXML } } +// TODO properly use the space func (s *svc) newProp(key, val string) *propertyXML { return &propertyXML{ XMLName: xml.Name{Space: "", Local: key}, @@ -368,8 +402,8 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:favorite", "0")) } else if amd := k.GetMetadata(); amd == nil { response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:favorite", "0")) - } else if v, ok := amd["http://owncloud.org/ns/favorite"]; ok && v != "" { - response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:favorite", "1")) + } else if v, ok := amd[_propOcFavorite]; ok && v != "" { + response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:favorite", v)) } else { response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:favorite", "0")) } @@ -387,7 +421,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide size := fmt.Sprintf("%d", md.Size) for i := range pf.Prop { switch pf.Prop[i].Space { - case "http://owncloud.org/ns": + case _nsOwncloud: switch pf.Prop[i].Local { // TODO(jfd): maybe phoenix and the other clients can just use this id as an opaque string? // I tested the desktop client and phoenix to annotate which properties are requestted, see below cases @@ -491,7 +525,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:favorite", "0")) } else if amd := k.GetMetadata(); amd == nil { propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:favorite", "0")) - } else if v, ok := amd["http://owncloud.org/ns/favorite"]; ok && v != "" { + } else if v, ok := amd[_propOcFavorite]; ok && v != "" { propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:favorite", "1")) } else { propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:favorite", "0")) @@ -511,7 +545,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide case "share-types": // desktop k := md.GetArbitraryMetadata() amd := k.GetMetadata() - if amdv, ok := amd[fmt.Sprintf("%s/%s", pf.Prop[i].Space, pf.Prop[i].Local)]; ok { + if amdv, ok := amd[metadataKeyOf(&pf.Prop[i])]; ok { st := fmt.Sprintf("%s", amdv) propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:share-types", st)) } else { @@ -546,7 +580,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide default: propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:"+pf.Prop[i].Local, "")) } - case "DAV:": + case _nsDav: switch pf.Prop[i].Local { case "getetag": // both if md.Etag != "" { @@ -586,7 +620,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide default: propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("d:"+pf.Prop[i].Local, "")) } - case "http://open-collaboration-services.org/ns": + case _nsOCS: switch pf.Prop[i].Local { // ocs:share-permissions indicate clients the maximum permissions that can be granted: // 1 = read @@ -614,7 +648,7 @@ func (s *svc) mdToPropResponse(ctx context.Context, pf *propfindXML, md *provide propstatNotFound.Prop = append(propstatNotFound.Prop, s.newPropNS(pf.Prop[i].Space, pf.Prop[i].Local, "")) } else if amd := k.GetMetadata(); amd == nil { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newPropNS(pf.Prop[i].Space, pf.Prop[i].Local, "")) - } else if v, ok := amd[fmt.Sprintf("%s/%s", pf.Prop[i].Space, pf.Prop[i].Local)]; ok && v != "" { + } else if v, ok := amd[metadataKeyOf(&pf.Prop[i])]; ok && v != "" { propstatOK.Prop = append(propstatOK.Prop, s.newPropNS(pf.Prop[i].Space, pf.Prop[i].Local, v)) } else { propstatNotFound.Prop = append(propstatNotFound.Prop, s.newPropNS(pf.Prop[i].Space, pf.Prop[i].Local, "")) @@ -654,6 +688,10 @@ func (c *countingReader) Read(p []byte) (int, error) { return n, err } +func metadataKeyOf(n *xml.Name) string { + return fmt.Sprintf("%s/%s", n.Space, n.Local) +} + // http://www.webdav.org/specs/rfc4918.html#ELEMENT_prop (for propfind) type propfindProps []xml.Name diff --git a/internal/http/services/owncloud/ocdav/proppatch.go b/internal/http/services/owncloud/ocdav/proppatch.go index 218bcd5c0f..b0be5ab325 100644 --- a/internal/http/services/owncloud/ocdav/proppatch.go +++ b/internal/http/services/owncloud/ocdav/proppatch.go @@ -218,7 +218,7 @@ func (s *svc) formatProppatchResponse(ctx context.Context, acceptedProps []xml.N func (s *svc) isBooleanProperty(prop string) bool { // TODO add other properties we know to be boolean? - return prop == "http://owncloud.org/ns/favorite" + return prop == _propOcFavorite } func (s *svc) as0or1(val string) string { @@ -307,9 +307,9 @@ func readProppatch(r io.Reader) (patches []Proppatch, status int, err error) { for _, op := range pu.SetRemove { remove := false switch op.XMLName { - case xml.Name{Space: "DAV:", Local: "set"}: + case xml.Name{Space: _nsDav, Local: "set"}: // No-op. - case xml.Name{Space: "DAV:", Local: "remove"}: + case xml.Name{Space: _nsDav, Local: "remove"}: for _, p := range op.Prop { if len(p.InnerXML) > 0 { return nil, http.StatusBadRequest, errInvalidProppatch diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index e931bef312..4fa4e3dc1d 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/internal/http/services/owncloud/ocdav/trashbin.go @@ -295,7 +295,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, pf *pr size := fmt.Sprintf("%d", item.Size) for i := range pf.Prop { switch pf.Prop[i].Space { - case "http://owncloud.org/ns": + case _nsOwncloud: switch pf.Prop[i].Local { case "oc:size": if item.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { @@ -314,7 +314,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, pf *pr default: propstatNotFound.Prop = append(propstatNotFound.Prop, s.newProp("oc:"+pf.Prop[i].Local, "")) } - case "DAV:": + case _nsDav: switch pf.Prop[i].Local { case "getcontentlength": if item.Type == provider.ResourceType_RESOURCE_TYPE_CONTAINER { diff --git a/internal/http/services/owncloud/ocs/data/capabilities.go b/internal/http/services/owncloud/ocs/data/capabilities.go index b96bf99b2c..40fd128cd8 100644 --- a/internal/http/services/owncloud/ocs/data/capabilities.go +++ b/internal/http/services/owncloud/ocs/data/capabilities.go @@ -99,6 +99,7 @@ type CapabilitiesFiles struct { BigFileChunking ocsBool `json:"bigfilechunking" xml:"bigfilechunking"` Undelete ocsBool `json:"undelete" xml:"undelete"` Versioning ocsBool `json:"versioning" xml:"versioning"` + Favorites ocsBool `json:"favorites" xml:"favorites"` BlacklistedFiles []string `json:"blacklisted_files" xml:"blacklisted_files>element" mapstructure:"blacklisted_files"` TusSupport *CapabilitiesFilesTusSupport `json:"tus_support" xml:"tus_support" mapstructure:"tus_support"` } diff --git a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go index a71a311d15..77932654e0 100644 --- a/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go +++ b/internal/http/services/owncloud/ocs/handlers/cloud/capabilities/capabilities.go @@ -102,6 +102,7 @@ func (h *Handler) Init(c *config.Config) { } // h.c.Capabilities.Files.Undelete is boolean // h.c.Capabilities.Files.Versioning is boolean + // h.c.Capabilities.Files.Favorites is boolean // dav diff --git a/pkg/storage/fs/ocis/ocis.go b/pkg/storage/fs/ocis/ocis.go index 3e59a70bf3..e1f8e7fd79 100644 --- a/pkg/storage/fs/ocis/ocis.go +++ b/pkg/storage/fs/ocis/ocis.go @@ -62,8 +62,9 @@ const ( // grantPrefix is the prefix for sharing related extended attributes grantPrefix string = ocisPrefix + "grant." metadataPrefix string = ocisPrefix + "md." - // TODO implement favorites metadata flag - favPrefix string = ocisPrefix + "fav." // favorite flag, per user + + // favorite flag, per user + favPrefix string = ocisPrefix + "fav." // a temporary etag for a folder that is removed when the mtime propagation happens tmpEtagAttr string = ocisPrefix + "tmp.etag" From 9181ed0b4bdf698d559e9649ce8fe1c86f38c9d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 11 Jan 2021 13:48:49 +0000 Subject: [PATCH 3/5] update expected failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- tests/acceptance/expected-failures-on-OCIS-storage.txt | 3 +-- tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/expected-failures-on-OCIS-storage.txt b/tests/acceptance/expected-failures-on-OCIS-storage.txt index 78a23b2fa5..c130ad94b7 100644 --- a/tests/acceptance/expected-failures-on-OCIS-storage.txt +++ b/tests/acceptance/expected-failures-on-OCIS-storage.txt @@ -115,6 +115,7 @@ apiAuthWebDav/webDavPUTAuth.feature:38 apiCapabilities/capabilitiesWithNormalUser.feature:11 # # https://github.com/owncloud/ocis-reva/issues/39 REPORT request not implemented +# https://github.com/cs3org/reva/issues/1394 ocis needs an api to list all files of a user he marked as favorite or that are tagged with a certain tag # And other missing implementation of favorites # apiFavorites/favorites.feature:91 @@ -1087,8 +1088,6 @@ apiWebdavProperties2/getFileProperties.feature:71 # # https://github.com/owncloud/ocis/issues/567 cannot get share-types webdav property # -apiWebdavProperties2/getFileProperties.feature:135 -apiWebdavProperties2/getFileProperties.feature:136 apiWebdavProperties2/getFileProperties.feature:174 apiWebdavProperties2/getFileProperties.feature:175 # diff --git a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt index 9173ef7b2c..49fd19672c 100644 --- a/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt +++ b/tests/acceptance/expected-failures-on-OWNCLOUD-storage.txt @@ -115,6 +115,7 @@ apiAuthWebDav/webDavPUTAuth.feature:38 apiCapabilities/capabilitiesWithNormalUser.feature:11 # # https://github.com/owncloud/ocis-reva/issues/39 REPORT request not implemented +# https://github.com/cs3org/reva/issues/1394 ocis needs an api to list all files of a user he marked as favorite or that are tagged with a certain tag # And other missing implementation of favorites # apiFavorites/favorites.feature:91 From 4f2a1288fe00288485e6c0f68e5f0fd6f20583df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 12 Jan 2021 07:42:42 +0000 Subject: [PATCH 4/5] fix lint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- internal/http/services/owncloud/ocdav/propfind.go | 3 +-- pkg/storage/fs/ocis/node.go | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/internal/http/services/owncloud/ocdav/propfind.go b/internal/http/services/owncloud/ocdav/propfind.go index 17a7f3d922..7c4a689e42 100644 --- a/internal/http/services/owncloud/ocdav/propfind.go +++ b/internal/http/services/owncloud/ocdav/propfind.go @@ -50,8 +50,7 @@ const ( _nsOwncloud = "http://owncloud.org/ns" _nsOCS = "http://open-collaboration-services.org/ns" - _propOcFavorite = "http://owncloud.org/ns/favorite" - _propOcShareTypes = "http://owncloud.org/ns/share-types" + _propOcFavorite = "http://owncloud.org/ns/favorite" ) // ns is the namespace that is prefixed to the path in the cs3 namespace diff --git a/pkg/storage/fs/ocis/node.go b/pkg/storage/fs/ocis/node.go index 259780d6ae..c4ba183c15 100644 --- a/pkg/storage/fs/ocis/node.go +++ b/pkg/storage/fs/ocis/node.go @@ -482,10 +482,8 @@ func (n *Node) AsResourceInfo(ctx context.Context, rp *provider.ResourcePermissi // use temporary etag if it is set if b, err := xattr.Get(nodePath, tmpEtagAttr); err == nil { ri.Etag = fmt.Sprintf(`"%x"`, string(b)) - } else { - if ri.Etag, err = calculateEtag(n.ID, tmTime); err != nil { - sublog.Debug().Err(err).Msg("could not calculate etag") - } + } else if ri.Etag, err = calculateEtag(n.ID, tmTime); err != nil { + sublog.Debug().Err(err).Msg("could not calculate etag") } // mtime uses tmtime if present From f4b5c93dd124d80a66b7785e9c32f9beb2453f66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 14 Jan 2021 11:34:20 +0000 Subject: [PATCH 5/5] add changelog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/ocis-favorites-etags-mtime-metadata.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/ocis-favorites-etags-mtime-metadata.md diff --git a/changelog/unreleased/ocis-favorites-etags-mtime-metadata.md b/changelog/unreleased/ocis-favorites-etags-mtime-metadata.md new file mode 100644 index 0000000000..004175a62c --- /dev/null +++ b/changelog/unreleased/ocis-favorites-etags-mtime-metadata.md @@ -0,0 +1,7 @@ +Enhancement: allow setting favorites, mtime and a temporary etag + +We now let the ocis driver persist favorites, set temporary etags and the mtime as arbitrary metadata. + +https://github.com/cs3org/reva/pull/1393 +https://github.com/owncloud/ocis/issues/567 +https://github.com/cs3org/reva/issues/1394