Skip to content

Commit

Permalink
refactor and changelog
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
@butonic
butonic committed Sep 22, 2020
1 parent 32cc034 commit fffbe6b
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 42 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/ocis-cache-displayname.md
View file
Original file line number Diff line number Diff line change
@@ -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
41 changes: 20 additions & 21 deletions internal/http/services/owncloud/ocs/conversions/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
40 changes: 26 additions & 14 deletions internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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})
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
19 changes: 12 additions & 7 deletions ...cs/handlers/apps/sharing/shares/ttlmap.go → pkg/ttlmap/ttlmap.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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]
Expand All @@ -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 {
Expand Down

0 comments on commit fffbe6b

Please sign in to comment.