From 32cc034816f2faca242b67b5c6d8c58981b2bcf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 11 Sep 2020 23:01:57 +0200 Subject: [PATCH 1/2] cache displaynames in ocs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../services/owncloud/ocs/conversions/main.go | 26 ++ .../handlers/apps/sharing/shares/shares.go | 226 +++++------------- .../handlers/apps/sharing/shares/ttlmap.go | 77 ++++++ .../owncloud/ocs/response/response.go | 9 - 4 files changed, 168 insertions(+), 170 deletions(-) create mode 100644 internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go diff --git a/internal/http/services/owncloud/ocs/conversions/main.go b/internal/http/services/owncloud/ocs/conversions/main.go index 1b1d3bf689..5d9734e957 100644 --- a/internal/http/services/owncloud/ocs/conversions/main.go +++ b/internal/http/services/owncloud/ocs/conversions/main.go @@ -20,6 +20,7 @@ package conversions import ( + "context" "fmt" "net/http" "path" @@ -272,6 +273,29 @@ func AsCS3Permissions(p int, rp *provider.ResourcePermissions) *provider.Resourc return rp } +// UserShare2ShareData converts a cs3api user share into shareData data model +// TODO(jfd) merge userShare2ShareData with publicShare2ShareData +func UserShare2ShareData(ctx context.Context, share *collaboration.Share) (*ShareData, error) { + sd := &ShareData{ + Permissions: UserSharePermissions2OCSPermissions(share.GetPermissions()), + ShareType: ShareTypeUser, + UIDOwner: LocalUserIDToString(share.GetCreator()), + UIDFileOwner: LocalUserIDToString(share.GetOwner()), + ShareWith: LocalUserIDToString(share.GetGrantee().GetId()), + } + + if share.Id != nil && share.Id.OpaqueId != "" { + sd.ID = share.Id.OpaqueId + } + if share.Ctime != nil { + sd.STime = share.Ctime.Seconds // TODO CS3 api birth time = btime + } + // actually clients should be able to GET and cache the user info themselves ... + // TODO only return the userid, let the clientso look up the displayname + // TODO check grantee type for user vs group + return sd, nil +} + // PublicShare2ShareData converts a cs3api public share into shareData data model func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL string) *ShareData { var expiration string @@ -318,6 +342,8 @@ func LocalUserIDToString(userID *userpb.UserId) string { } // UserIDToString transforms a cs3api user id into an ocs data model +// TODO This should be used instead of LocalUserIDToString bit it requires interpreting an @ on the client side +// TODO An alternative would be to send the idp / iss as an additional attribute. might be less intrusive func UserIDToString(userID *userpb.UserId) string { if userID == nil || userID.OpaqueId == "" { return "" diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index a530ab292b..f64e3dc99d 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -30,6 +30,7 @@ import ( "strings" "time" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" invitepb "github.com/cs3org/go-cs3apis/cs3/ocm/invite/v1beta1" ocmprovider "github.com/cs3org/go-cs3apis/cs3/ocm/provider/v1beta1" @@ -54,12 +55,14 @@ import ( type Handler struct { gatewayAddr string publicURL string + ttlcache *TTLMap } // Init initializes this and any contained handlers func (h *Handler) Init(c *config.Config) error { h.gatewayAddr = c.GatewaySvc h.publicURL = c.Config.Host + h.ttlcache = New(1000, 60) return nil } @@ -298,7 +301,7 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "grpc create share request failed", err) return } - s, err := h.userShare2ShareData(ctx, createShareResponse.Share) + s, err := conversions.UserShare2ShareData(ctx, createShareResponse.Share) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) return @@ -308,6 +311,7 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error adding fileinfo to share", err) return } + h.addDisplaynames(ctx, c, s) response.WriteOCSSuccess(w, r, s) } @@ -739,7 +743,7 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin if err == nil && uRes.GetShare() != nil { resourceID = uRes.Share.ResourceId - share, err = h.userShare2ShareData(ctx, uRes.Share) + share, err = conversions.UserShare2ShareData(ctx, uRes.Share) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) return @@ -781,6 +785,7 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } + h.addDisplaynames(ctx, client, share) response.WriteOCSSuccess(w, r, []*conversions.ShareData{share}) } @@ -866,7 +871,7 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, shareID st return } - share, err := h.userShare2ShareData(ctx, gRes.Share) + share, err := conversions.UserShare2ShareData(ctx, gRes.Share) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) return @@ -903,6 +908,7 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, shareID st return } + h.addDisplaynames(ctx, uClient, share) response.WriteOCSSuccess(w, r, share) } @@ -1048,7 +1054,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { return } - data, err := h.userShare2ShareData(r.Context(), rs.Share) + data, err := conversions.UserShare2ShareData(r.Context(), rs.Share) if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, err.Error(), err) return @@ -1071,6 +1077,7 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { return } + h.addDisplaynames(r.Context(), gwc, data) shares = append(shares, data) } @@ -1272,7 +1279,7 @@ func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListS // build OCS response payload for _, s := range lsUserSharesResponse.Shares { - share, err := h.userShare2ShareData(ctx, s) + share, err := conversions.UserShare2ShareData(ctx, s) if err != nil { return nil, err } @@ -1305,6 +1312,7 @@ func (h *Handler) listUserShares(r *http.Request, filters []*collaboration.ListS if err != nil { return nil, err } + h.addDisplaynames(ctx, c, share) log.Debug().Interface("share", s).Interface("info", rInfo).Interface("shareData", share).Msg("mapped") ocsDataPayload = append(ocsDataPayload, share) @@ -1347,174 +1355,70 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf // item type s.ItemType = conversions.ResourceType(info.GetType()).String() - c, err := pool.GetGatewayServiceClient(h.gatewayAddr) - if err != nil { - return err - } - - // owner, err := c.GetUser(ctx, &userpb.GetUserRequest{ - // UserId: info.Owner, - // }) - // if err != nil { - // return err - // } - - // if owner.Status.Code == rpc.Code_CODE_OK { - // // TODO the user from GetUser might not have an ID set, so we are using the one we have - // s.DisplaynameFileOwner = owner.GetUser().DisplayName - // } else { - // err := errors.New("could not look up share owner") - // log.Err(err). - // Str("user_idp", info.Owner.GetIdp()). - // Str("user_opaque_id", info.Owner.GetOpaqueId()). - // Str("code", owner.Status.Code.String()). - // Msg(owner.Status.Message) - // return err - // } - // file owner might not yet be set. Use file info if s.UIDFileOwner == "" { - // TODO we don't know if info.Owner is always set. - s.UIDFileOwner = response.UserIDToString(info.Owner) - } - if s.DisplaynameFileOwner == "" && info.Owner != nil { - owner, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: info.Owner, - }) - if err != nil { - return err - } - - if owner.Status.Code == rpc.Code_CODE_OK { - // TODO the user from GetUser might not have an ID set, so we are using the one we have - s.DisplaynameFileOwner = owner.GetUser().DisplayName - } else { - err := errors.New("could not look up share owner") - log.Err(err). - Str("user_idp", info.Owner.GetIdp()). - Str("user_opaque_id", info.Owner.GetOpaqueId()). - Str("code", owner.Status.Code.String()). - Msg(owner.Status.Message) - return err - } + s.UIDFileOwner = info.GetOwner().GetOpaqueId() } // share owner might not yet be set. Use file info if s.UIDOwner == "" { - // TODO we don't know if info.Owner is always set. - s.UIDOwner = response.UserIDToString(info.Owner) - } - if s.DisplaynameOwner == "" && info.Owner != nil { - owner, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: info.Owner, - }) - - if err != nil { - return err - } - - if owner.Status.Code == rpc.Code_CODE_OK { - // TODO the user from GetUser might not have an ID set, so we are using the one we have - s.DisplaynameOwner = owner.User.DisplayName - } else { - err := errors.New("could not look up file owner") - log.Err(err). - Str("user_idp", info.Owner.GetIdp()). - Str("user_opaque_id", info.Owner.GetOpaqueId()). - Str("code", owner.Status.Code.String()). - Msg(owner.Status.Message) - return err - } + s.UIDOwner = info.GetOwner().GetOpaqueId() } } return nil } -// TODO(jfd) merge userShare2ShareData with publicShare2ShareData -func (h *Handler) userShare2ShareData(ctx context.Context, share *collaboration.Share) (*conversions.ShareData, error) { - sd := &conversions.ShareData{ - Permissions: conversions.UserSharePermissions2OCSPermissions(share.GetPermissions()), - ShareType: conversions.ShareTypeUser, - } - - c, err := pool.GetGatewayServiceClient(h.gatewayAddr) - if err != nil { - return nil, err - } - +func (h *Handler) getDisplayname(ctx context.Context, c gateway.GatewayAPIClient, userid string) string { log := appctx.GetLogger(ctx) + if dn := h.ttlcache.Get(userid); dn != "" { + log.Debug().Str("userid", userid).Msg("cache hit") + return dn + } + log.Debug().Str("userid", userid).Msg("cache miss") + res, err := c.GetUser(ctx, &userpb.GetUserRequest{ + UserId: &userpb.UserId{ + OpaqueId: userid, + }, + }) + if err != nil { + log.Err(err). + Str("userid", userid). + Msg("could not look up user") + return "" + } + if res.GetStatus().GetCode() != rpc.Code_CODE_OK { + log.Err(err). + Str("opaque_id", userid). + Int32("code", int32(res.GetStatus().GetCode())). + Str("message", res.GetStatus().GetMessage()). + Msg("get user call failed") + return "" + } + if res.User == nil { + log.Debug(). + Str("opaque_id", userid). + Int32("code", int32(res.GetStatus().GetCode())). + Str("message", res.GetStatus().GetMessage()). + Msg("user not found") + return "" + } + if res.User.DisplayName == "" { + log.Debug(). + Str("opaque_id", userid). + Int32("code", int32(res.GetStatus().GetCode())). + Str("message", res.GetStatus().GetMessage()). + Msg("Displayname empty") + return "" + } + + h.ttlcache.Put(userid, res.User.DisplayName) + log.Debug().Str("userid", userid).Msg("cache update") + return res.User.DisplayName +} - if share.Creator != nil { - creator, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: share.Creator, - }) - if err != nil { - return nil, err - } - - if creator.Status.Code == rpc.Code_CODE_OK { - // TODO the user from GetUser might not have an ID set, so we are using the one we have - sd.UIDOwner = response.UserIDToString(share.Creator) - sd.DisplaynameOwner = creator.GetUser().DisplayName - } else { - log.Err(errors.Wrap(err, "could not look up creator")). - Str("user_idp", share.Creator.GetIdp()). - Str("user_opaque_id", share.Creator.GetOpaqueId()). - Str("code", creator.Status.Code.String()). - Msg(creator.Status.Message) - return nil, err - } - } - if share.Owner != nil { - owner, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: share.Owner, - }) - if err != nil { - return nil, err - } - - if owner.Status.Code == rpc.Code_CODE_OK { - // TODO the user from GetUser might not have an ID set, so we are using the one we have - sd.UIDFileOwner = response.UserIDToString(share.Owner) - sd.DisplaynameFileOwner = owner.GetUser().DisplayName - } else { - log.Err(errors.Wrap(err, "could not look up owner")). - Str("user_idp", share.Owner.GetIdp()). - Str("user_opaque_id", share.Owner.GetOpaqueId()). - Str("code", owner.Status.Code.String()). - Msg(owner.Status.Message) - return nil, err - } - } - if share.Grantee.Id != nil { - grantee, err := c.GetUser(ctx, &userpb.GetUserRequest{ - UserId: share.Grantee.GetId(), - }) - if err != nil { - return nil, err - } - - if grantee.Status.Code == rpc.Code_CODE_OK { - // TODO the user from GetUser might not have an ID set, so we are using the one we have - sd.ShareWith = response.UserIDToString(share.Grantee.Id) - sd.ShareWithDisplayname = grantee.GetUser().DisplayName - } else { - log.Err(errors.Wrap(err, "could not look up grantee")). - Str("user_idp", share.Grantee.GetId().GetIdp()). - Str("user_opaque_id", share.Grantee.GetId().GetOpaqueId()). - Str("code", grantee.Status.Code.String()). - Msg(grantee.Status.Message) - return nil, err - } - } - if share.Id != nil && share.Id.OpaqueId != "" { - sd.ID = share.Id.OpaqueId - } - if share.Ctime != nil { - sd.STime = share.Ctime.Seconds // TODO CS3 api birth time = btime - } - // actually clients should be able to GET and cache the user info themselves ... - // TODO check grantee type for user vs group - return sd, nil +func (h *Handler) addDisplaynames(ctx context.Context, c gateway.GatewayAPIClient, s *conversions.ShareData) { + s.DisplaynameOwner = h.getDisplayname(ctx, c, s.UIDOwner) + s.DisplaynameFileOwner = h.getDisplayname(ctx, c, s.UIDFileOwner) + s.ShareWithDisplayname = h.getDisplayname(ctx, c, s.ShareWith) } func (h *Handler) isPublicShare(r *http.Request, oid string) bool { diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go new file mode 100644 index 0000000000..3ee4a0905c --- /dev/null +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go @@ -0,0 +1,77 @@ +// Copyright 2018-2020 CERN +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// In applying this license, CERN does not waive the privileges and immunities +// granted to it by virtue of its status as an Intergovernmental Organization +// or submit itself to any jurisdiction. + +package shares + +import ( + "sync" + "time" +) + +// below code copied from https://stackoverflow.com/a/25487392 +type item struct { + value string + lastAccess int64 +} + +type TTLMap struct { + m map[string]*item + l sync.Mutex +} + +func New(ln int, maxTTL int) (m *TTLMap) { + m = &TTLMap{m: make(map[string]*item, ln)} + go func() { + for now := range time.Tick(time.Second) { + m.l.Lock() + for k, v := range m.m { + if now.Unix()-v.lastAccess > int64(maxTTL) { + delete(m.m, k) + } + } + m.l.Unlock() + } + }() + return +} + +func (m *TTLMap) Len() int { + return len(m.m) +} + +func (m *TTLMap) Put(k, v string) { + m.l.Lock() + it, ok := m.m[k] + if !ok { + it = &item{value: v} + m.m[k] = it + } + it.lastAccess = time.Now().Unix() + m.l.Unlock() +} + +func (m *TTLMap) Get(k string) (v string) { + m.l.Lock() + if it, ok := m.m[k]; ok { + v = it.value + it.lastAccess = time.Now().Unix() + } + m.l.Unlock() + return + +} diff --git a/internal/http/services/owncloud/ocs/response/response.go b/internal/http/services/owncloud/ocs/response/response.go index 2ca93e099c..4dedb1a30d 100644 --- a/internal/http/services/owncloud/ocs/response/response.go +++ b/internal/http/services/owncloud/ocs/response/response.go @@ -26,7 +26,6 @@ import ( "net/http" "reflect" - user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" "github.com/cs3org/reva/pkg/appctx" ) @@ -183,14 +182,6 @@ func WriteOCSResponse(w http.ResponseWriter, r *http.Request, res Response, err } } -// UserIDToString returns a userid string with an optional idp separated by @: "[@]" -func UserIDToString(userID *user.UserId) string { - if userID == nil || userID.OpaqueId == "" { - return "" - } - return userID.OpaqueId -} - func encodeXML(res Response) ([]byte, error) { marshalled, err := xml.Marshal(res.OCS) if err != nil { From fffbe6b96a70899cc465a6ff0b8ef5358717d683 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 21 Sep 2020 14:00:07 +0200 Subject: [PATCH 2/2] refactor and 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-cache-displayname.md | 5 +++ .../services/owncloud/ocs/conversions/main.go | 41 +++++++++---------- .../handlers/apps/sharing/shares/shares.go | 40 +++++++++++------- .../sharing/shares => pkg/ttlmap}/ttlmap.go | 19 +++++---- 4 files changed, 63 insertions(+), 42 deletions(-) create mode 100644 changelog/unreleased/ocis-cache-displayname.md rename {internal/http/services/owncloud/ocs/handlers/apps/sharing/shares => pkg/ttlmap}/ttlmap.go (79%) diff --git a/changelog/unreleased/ocis-cache-displayname.md b/changelog/unreleased/ocis-cache-displayname.md new file mode 100644 index 0000000000..bfd9b72f9f --- /dev/null +++ b/changelog/unreleased/ocis-cache-displayname.md @@ -0,0 +1,5 @@ +Bugfix: Cache display names in ocs service + +The ocs list shares endpoint may need to fetch the displayname for multiple different users. We are now caching the lookup fo 60 seconds to save redundant RPCs to the users service. + +https://github.com/cs3org/reva/pull/1161 diff --git a/internal/http/services/owncloud/ocs/conversions/main.go b/internal/http/services/owncloud/ocs/conversions/main.go index 5d9734e957..4f74d924cf 100644 --- a/internal/http/services/owncloud/ocs/conversions/main.go +++ b/internal/http/services/owncloud/ocs/conversions/main.go @@ -305,30 +305,29 @@ func PublicShare2ShareData(share *link.PublicShare, r *http.Request, publicURL s expiration = "" } - shareWith := "" - if share.PasswordProtected { - shareWith = "***redacted***" + sd := &ShareData{ + // share.permissions are mapped below + // Displaynames are added later + ID: share.Id.OpaqueId, + ShareType: ShareTypePublicLink, + STime: share.Ctime.Seconds, // TODO CS3 api birth time = btime + Token: share.Token, + Expiration: expiration, + MimeType: share.Mtime.String(), + Name: share.DisplayName, + MailSend: 0, + URL: publicURL + path.Join("/", "#/s/"+share.Token), + Permissions: publicSharePermissions2OCSPermissions(share.GetPermissions()), + UIDOwner: LocalUserIDToString(share.Creator), + UIDFileOwner: LocalUserIDToString(share.Owner), } - return &ShareData{ - // share.permissions ar mapped below - // DisplaynameOwner: creator.DisplayName, - // DisplaynameFileOwner: share.GetCreator().String(), - ID: share.Id.OpaqueId, - ShareType: ShareTypePublicLink, - ShareWith: shareWith, - ShareWithDisplayname: shareWith, - STime: share.Ctime.Seconds, // TODO CS3 api birth time = btime - Token: share.Token, - Expiration: expiration, - MimeType: share.Mtime.String(), - Name: share.DisplayName, - MailSend: 0, - URL: publicURL + path.Join("/", "#/s/"+share.Token), - Permissions: publicSharePermissions2OCSPermissions(share.GetPermissions()), - UIDOwner: LocalUserIDToString(share.Creator), - UIDFileOwner: LocalUserIDToString(share.Owner), + if share.PasswordProtected { + sd.ShareWith = "***redacted***" + sd.ShareWithDisplayname = "***redacted***" } + + return sd // actually clients should be able to GET and cache the user info themselves ... // TODO check grantee type for user vs group } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index f64e3dc99d..fe170753a5 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go @@ -48,21 +48,22 @@ import ( "github.com/cs3org/reva/pkg/appctx" "github.com/cs3org/reva/pkg/rgrpc/todo/pool" "github.com/cs3org/reva/pkg/rhttp/router" + "github.com/cs3org/reva/pkg/ttlmap" "github.com/pkg/errors" ) // Handler implements the shares part of the ownCloud sharing API type Handler struct { - gatewayAddr string - publicURL string - ttlcache *TTLMap + gatewayAddr string + publicURL string + displayNameCache *ttlmap.TTLMap } // Init initializes this and any contained handlers func (h *Handler) Init(c *config.Config) error { h.gatewayAddr = c.GatewaySvc h.publicURL = c.Config.Host - h.ttlcache = New(1000, 60) + h.displayNameCache = ttlmap.New(1000, 60) return nil } @@ -312,6 +313,7 @@ func (h *Handler) createUserShare(w http.ResponseWriter, r *http.Request) { return } h.addDisplaynames(ctx, c, s) + response.WriteOCSSuccess(w, r, s) } @@ -427,11 +429,11 @@ func (h *Handler) createPublicLinkShare(w http.ResponseWriter, r *http.Request) s := conversions.PublicShare2ShareData(createRes.Share, r, h.publicURL) err = h.addFileInfo(ctx, s, statRes.Info) - if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err) return } + h.addDisplaynames(ctx, c, s) response.WriteOCSSuccess(w, r, s) } @@ -784,8 +786,8 @@ func (h *Handler) getShare(w http.ResponseWriter, r *http.Request, shareID strin log.Error().Err(err).Str("status", statResponse.Status.Code.String()).Msg("error mapping share data") response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error mapping share data", err) } - h.addDisplaynames(ctx, client, share) + response.WriteOCSSuccess(w, r, []*conversions.ShareData{share}) } @@ -907,8 +909,8 @@ func (h *Handler) updateShare(w http.ResponseWriter, r *http.Request, shareID st response.WriteOCSError(w, r, response.MetaServerError.StatusCode, err.Error(), err) return } - h.addDisplaynames(ctx, uClient, share) + response.WriteOCSSuccess(w, r, share) } @@ -1076,8 +1078,8 @@ func (h *Handler) listSharesWithMe(w http.ResponseWriter, r *http.Request) { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, err.Error(), err) return } - h.addDisplaynames(r.Context(), gwc, data) + shares = append(shares, data) } @@ -1180,6 +1182,7 @@ func (h *Handler) listPublicShares(r *http.Request, filters []*link.ListPublicSh if h.addFileInfo(ctx, sData, statResponse.Info) != nil { return nil, err } + h.addDisplaynames(ctx, c, sData) log.Debug().Interface("share", share).Interface("info", statResponse.Info).Interface("shareData", share).Msg("mapped") @@ -1369,7 +1372,10 @@ func (h *Handler) addFileInfo(ctx context.Context, s *conversions.ShareData, inf func (h *Handler) getDisplayname(ctx context.Context, c gateway.GatewayAPIClient, userid string) string { log := appctx.GetLogger(ctx) - if dn := h.ttlcache.Get(userid); dn != "" { + if userid == "" { + return "" + } + if dn := h.displayNameCache.Get(userid); dn != "" { log.Debug().Str("userid", userid).Msg("cache hit") return dn } @@ -1410,15 +1416,21 @@ func (h *Handler) getDisplayname(ctx context.Context, c gateway.GatewayAPIClient return "" } - h.ttlcache.Put(userid, res.User.DisplayName) + h.displayNameCache.Put(userid, res.User.DisplayName) log.Debug().Str("userid", userid).Msg("cache update") return res.User.DisplayName } func (h *Handler) addDisplaynames(ctx context.Context, c gateway.GatewayAPIClient, s *conversions.ShareData) { - s.DisplaynameOwner = h.getDisplayname(ctx, c, s.UIDOwner) - s.DisplaynameFileOwner = h.getDisplayname(ctx, c, s.UIDFileOwner) - s.ShareWithDisplayname = h.getDisplayname(ctx, c, s.ShareWith) + if s.DisplaynameOwner == "" { + s.DisplaynameOwner = h.getDisplayname(ctx, c, s.UIDOwner) + } + if s.DisplaynameFileOwner == "" { + s.DisplaynameFileOwner = h.getDisplayname(ctx, c, s.UIDFileOwner) + } + if s.ShareWithDisplayname == "" { + s.ShareWithDisplayname = h.getDisplayname(ctx, c, s.ShareWith) + } } func (h *Handler) isPublicShare(r *http.Request, oid string) bool { @@ -1627,11 +1639,11 @@ func (h *Handler) updatePublicShare(w http.ResponseWriter, r *http.Request, shar s := conversions.PublicShare2ShareData(publicShare, r, h.publicURL) err = h.addFileInfo(r.Context(), s, statRes.Info) - if err != nil { response.WriteOCSError(w, r, response.MetaServerError.StatusCode, "error enhancing response with share data", err) return } + h.addDisplaynames(r.Context(), gwC, s) response.WriteOCSSuccess(w, r, s) } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go b/pkg/ttlmap/ttlmap.go similarity index 79% rename from internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go rename to pkg/ttlmap/ttlmap.go index 3ee4a0905c..a559a37ea4 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/ttlmap.go +++ b/pkg/ttlmap/ttlmap.go @@ -16,24 +16,26 @@ // granted to it by virtue of its status as an Intergovernmental Organization // or submit itself to any jurisdiction. -package shares +package ttlmap import ( "sync" "time" ) -// below code copied from https://stackoverflow.com/a/25487392 -type item struct { - value string - lastAccess int64 -} - +// TTLMap is a simple kv cache, based on https://stackoverflow.com/a/25487392 +// The ttl of an item will be reset whenever it is read or written. type TTLMap struct { m map[string]*item l sync.Mutex } +type item struct { + value string + lastAccess int64 +} + +// New creates a new ttl cache, preallocating space for ln items and the given maxttl func New(ln int, maxTTL int) (m *TTLMap) { m = &TTLMap{m: make(map[string]*item, ln)} go func() { @@ -50,10 +52,12 @@ func New(ln int, maxTTL int) (m *TTLMap) { return } +// Len returns the current number of items in the cache func (m *TTLMap) Len() int { return len(m.m) } +// Put sets or overwrites an item, resetting the ttl func (m *TTLMap) Put(k, v string) { m.l.Lock() it, ok := m.m[k] @@ -65,6 +69,7 @@ func (m *TTLMap) Put(k, v string) { m.l.Unlock() } +// Get retrieves an item from the cache, resetting the ttl func (m *TTLMap) Get(k string) (v string) { m.l.Lock() if it, ok := m.m[k]; ok {