mirror of
https://github.com/owncloud/ocis
synced 2026-04-25 17:25:21 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
11
changelog/unreleased/fix-proxy-createhome-dedup.md
Normal file
11
changelog/unreleased/fix-proxy-createhome-dedup.md
Normal file
@@ -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
|
||||
@@ -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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user