diff --git a/changelog/unreleased/ocis-cache-displayname.md b/changelog/unreleased/ocis-cache-displayname.md new file mode 100644 index 00000000000..bfd9b72f9fe --- /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/handlers/apps/sharing/shares/shares.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go index f64e3dc99da..e2299c853e9 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,7 @@ 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 dn := h.displayNameCache.Get(userid); dn != "" { log.Debug().Str("userid", userid).Msg("cache hit") return dn } @@ -1410,7 +1413,7 @@ 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 } @@ -1627,11 +1630,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 3ee4a0905c9..a559a37ea4f 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 {