From 31ce8a68a782608d296cd23e07bb65031c848f5c Mon Sep 17 00:00:00 2001 From: Paul Faure Date: Fri, 13 Mar 2026 23:34:10 -0400 Subject: [PATCH] fix(proxy): deduplicate CreateHome calls with singleflight and sync.Map The CreateHome middleware fired a gRPC CreateHome request on every authenticated HTTP request. On first login the browser sends ~11 parallel requests, each triggering a redundant call. Use singleflight to collapse concurrent calls for the same user and sync.Map to cache successful results so subsequent requests skip the gRPC call entirely. Fixes: #12068 Co-Authored-By: Claude Opus 4.6 --- .../unreleased/fix-proxy-createhome-dedup.md | 11 +++ services/proxy/pkg/middleware/create_home.go | 80 +++++++++++++------ 2 files changed, 66 insertions(+), 25 deletions(-) create mode 100644 changelog/unreleased/fix-proxy-createhome-dedup.md diff --git a/changelog/unreleased/fix-proxy-createhome-dedup.md b/changelog/unreleased/fix-proxy-createhome-dedup.md new file mode 100644 index 00000000000..8ca8eab37f3 --- /dev/null +++ b/changelog/unreleased/fix-proxy-createhome-dedup.md @@ -0,0 +1,11 @@ +Bugfix: Deduplicate CreateHome calls in proxy middleware + +The CreateHome proxy middleware previously fired a CreateHome gRPC +request on every authenticated HTTP request with no deduplication. +On first login, the browser sends many parallel requests, each +triggering a redundant CreateHome call. This change uses singleflight +to collapse concurrent calls for the same user and caches successful +results in-process so subsequent requests skip the gRPC call entirely. + +https://github.com/owncloud/ocis/pull/XXXX +https://github.com/owncloud/ocis/issues/12068 diff --git a/services/proxy/pkg/middleware/create_home.go b/services/proxy/pkg/middleware/create_home.go index a71f3825354..58b3b8fc49f 100644 --- a/services/proxy/pkg/middleware/create_home.go +++ b/services/proxy/pkg/middleware/create_home.go @@ -3,6 +3,7 @@ package middleware import ( "net/http" "strconv" + "sync" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -14,10 +15,13 @@ import ( "github.com/owncloud/reva/v2/pkg/rgrpc/status" "github.com/owncloud/reva/v2/pkg/rgrpc/todo/pool" "github.com/owncloud/reva/v2/pkg/utils" + "golang.org/x/sync/singleflight" "google.golang.org/grpc/metadata" ) -// CreateHome provides a middleware which sends a CreateHome request to the reva gateway +// CreateHome provides a middleware which sends a CreateHome request to the reva gateway. +// It deduplicates concurrent requests for the same user and caches successful results +// for the lifetime of the process. func CreateHome(optionSetters ...Option) func(next http.Handler) http.Handler { options := newOptions(optionSetters...) logger := options.Logger @@ -37,9 +41,12 @@ type createHome struct { logger log.Logger revaGatewaySelector pool.Selectable[gateway.GatewayAPIClient] roleQuotas map[string]uint64 + + knownHomes sync.Map // map[userID]struct{} — users whose home is confirmed + flight singleflight.Group // collapses concurrent CreateHome calls per user } -func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { +func (m *createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { if !m.shouldServe(req) { m.next.ServeHTTP(w, req) return @@ -48,46 +55,69 @@ func (m createHome) ServeHTTP(w http.ResponseWriter, req *http.Request) { token := req.Header.Get("x-access-token") // we need to pass the token to authenticate the CreateHome request. - //ctx := tokenpkg.ContextSetToken(r.Context(), token) ctx := metadata.AppendToOutgoingContext(req.Context(), revactx.TokenHeader, token) - createHomeReq := &provider.CreateHomeRequest{} u, ok := revactx.ContextGetUser(ctx) - if ok { - roleIDs, err := m.getUserRoles(u) - if err != nil { - m.logger.Error().Err(err).Str("userid", u.Id.OpaqueId).Msg("failed to get roles for user") - errorcode.GeneralException.Render(w, req, http.StatusInternalServerError, "Unauthorized") - return - } - if limit, hasLimit := m.checkRoleQuotaLimit(roleIDs); hasLimit { - createHomeReq.Opaque = utils.AppendPlainToOpaque(nil, "quota", strconv.FormatUint(limit, 10)) - } + if !ok { + m.next.ServeHTTP(w, req) + return } - client, err := m.revaGatewaySelector.Next() + userID := u.Id.OpaqueId + + // Fast path: home already known to exist for this user. + if _, ok := m.knownHomes.Load(userID); ok { + m.next.ServeHTTP(w, req) + return + } + + createHomeReq := &provider.CreateHomeRequest{} + roleIDs, err := m.getUserRoles(u) if err != nil { - m.logger.Err(err).Msg("error selecting next gateway client") - } else { + m.logger.Error().Err(err).Str("userid", userID).Msg("failed to get roles for user") + errorcode.GeneralException.Render(w, req, http.StatusInternalServerError, "Unauthorized") + return + } + if limit, hasLimit := m.checkRoleQuotaLimit(roleIDs); hasLimit { + createHomeReq.Opaque = utils.AppendPlainToOpaque(nil, "quota", strconv.FormatUint(limit, 10)) + } + + // Deduplicate concurrent CreateHome calls for the same user. + _, err, _ = m.flight.Do(userID, func() (interface{}, error) { + client, err := m.revaGatewaySelector.Next() + if err != nil { + m.logger.Err(err).Msg("error selecting next gateway client") + return nil, err + } + createHomeRes, err := client.CreateHome(ctx, createHomeReq) if err != nil { m.logger.Err(err).Msg("error calling CreateHome") - } else if createHomeRes.Status.Code != rpc.Code_CODE_OK { - err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway") - if createHomeRes.Status.Code != rpc.Code_CODE_ALREADY_EXISTS { - m.logger.Err(err).Msg("error when calling Createhome") - } + return nil, err } + + if createHomeRes.Status.Code != rpc.Code_CODE_OK && createHomeRes.Status.Code != rpc.Code_CODE_ALREADY_EXISTS { + err := status.NewErrorFromCode(createHomeRes.Status.Code, "gateway") + m.logger.Err(err).Msg("error when calling CreateHome") + return nil, err + } + + return nil, nil + }) + + // Only cache on success — if CreateHome failed, retry on next request. + if err == nil { + m.knownHomes.Store(userID, struct{}{}) } m.next.ServeHTTP(w, req) } -func (m createHome) shouldServe(req *http.Request) bool { +func (m *createHome) shouldServe(req *http.Request) bool { return req.Header.Get("x-access-token") != "" } -func (m createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { +func (m *createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { var roleIDs []string if err := utils.ReadJSONFromOpaque(user.Opaque, "roles", &roleIDs); err != nil { return nil, err @@ -105,7 +135,7 @@ func (m createHome) getUserRoles(user *userv1beta1.User) ([]string, error) { return dedup, nil } -func (m createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) { +func (m *createHome) checkRoleQuotaLimit(roleIDs []string) (uint64, bool) { if len(roleIDs) == 0 { return 0, false }