enhancement: refine the profile photo service and introduce httpDataProviders which allows reusing the endpoints

This commit is contained in:
Florian Schade
2025-05-20 16:09:12 +02:00
parent eccc900918
commit 250400639a
6 changed files with 451 additions and 116 deletions
@@ -7,7 +7,6 @@ import (
"net/http"
"github.com/go-chi/render"
revactx "github.com/opencloud-eu/reva/v2/pkg/ctx"
"github.com/opencloud-eu/reva/v2/pkg/storage/utils/metadata"
"github.com/opencloud-eu/opencloud/pkg/log"
@@ -21,7 +20,7 @@ type (
GetPhoto(ctx context.Context, id string) ([]byte, error)
// UpdatePhoto retrieves the requested photo
UpdatePhoto(ctx context.Context, id string, rc io.Reader) error
UpdatePhoto(ctx context.Context, id string, r io.Reader) error
// DeletePhoto deletes the requested photo
DeletePhoto(ctx context.Context, id string) error
@@ -34,9 +33,6 @@ var (
// ErrNoBytes is returned when no bytes are found
ErrNoBytes = errors.New("no bytes")
// ErrNoUser is returned when no user is found
ErrNoUser = errors.New("no user found")
)
// UsersUserProfilePhotoService is the implementation of the UsersUserProfilePhotoProvider interface
@@ -71,8 +67,8 @@ func (s UsersUserProfilePhotoService) DeletePhoto(ctx context.Context, id string
}
// UpdatePhoto updates the requested photo
func (s UsersUserProfilePhotoService) UpdatePhoto(ctx context.Context, id string, rc io.Reader) error {
photo, err := io.ReadAll(rc)
func (s UsersUserProfilePhotoService) UpdatePhoto(ctx context.Context, id string, r io.Reader) error {
photo, err := io.ReadAll(r)
if err != nil {
return err
}
@@ -98,66 +94,61 @@ func NewUsersUserProfilePhotoApi(usersUserProfilePhotoService UsersUserProfilePh
}, nil
}
// GetProfilePhoto provides the requested photo
func (api UsersUserProfilePhotoApi) GetProfilePhoto(w http.ResponseWriter, r *http.Request) {
id, ok := api.getUserID(w, r)
if !ok {
return
}
// GetProfilePhoto creates a handler which renders the corresponding photo
func (api UsersUserProfilePhotoApi) GetProfilePhoto(h HTTPDataHandler[string]) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
v, ok := h(w, r)
if !ok {
return
}
photo, err := api.usersUserProfilePhotoService.GetPhoto(r.Context(), id)
if err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusNotFound, "failed to get photo")
return
}
photo, err := api.usersUserProfilePhotoService.GetPhoto(r.Context(), v)
if err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusNotFound, "failed to get photo")
return
}
render.Status(r, http.StatusOK)
_, _ = w.Write(photo)
render.Status(r, http.StatusOK)
_, _ = w.Write(photo)
}
}
// UpsertProfilePhoto updates or inserts (initial create) the requested photo
func (api UsersUserProfilePhotoApi) UpsertProfilePhoto(w http.ResponseWriter, r *http.Request) {
id, ok := api.getUserID(w, r)
if !ok {
return
}
// UpsertProfilePhoto creates a handler which updates or creates the corresponding photo
func (api UsersUserProfilePhotoApi) UpsertProfilePhoto(h HTTPDataHandler[string]) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
v, ok := h(w, r)
if !ok {
return
}
if err := api.usersUserProfilePhotoService.UpdatePhoto(r.Context(), id, r.Body); err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusNotFound, "failed to update photo")
return
}
defer func() {
_ = r.Body.Close()
}()
if err := api.usersUserProfilePhotoService.UpdatePhoto(r.Context(), v, r.Body); err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "failed to update photo")
return
}
defer func() {
_ = r.Body.Close()
}()
render.Status(r, http.StatusOK)
render.Status(r, http.StatusOK)
}
}
// DeleteProfilePhoto deletes the requested photo
func (api UsersUserProfilePhotoApi) DeleteProfilePhoto(w http.ResponseWriter, r *http.Request) {
id, ok := api.getUserID(w, r)
if !ok {
return
}
// DeleteProfilePhoto creates a handler which deletes the corresponding photo
func (api UsersUserProfilePhotoApi) DeleteProfilePhoto(h HTTPDataHandler[string]) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
v, ok := h(w, r)
if !ok {
return
}
if err := api.usersUserProfilePhotoService.DeletePhoto(r.Context(), id); err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusNotFound, "failed to delete photo")
return
}
if err := api.usersUserProfilePhotoService.DeletePhoto(r.Context(), v); err != nil {
api.logger.Debug().Err(err)
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "failed to delete photo")
return
}
render.Status(r, http.StatusOK)
}
func (api UsersUserProfilePhotoApi) getUserID(w http.ResponseWriter, r *http.Request) (string, bool) {
u, ok := revactx.ContextGetUser(r.Context())
if !ok {
api.logger.Debug().Msg(ErrNoUser.Error())
errorcode.GeneralException.Render(w, r, http.StatusMethodNotAllowed, ErrNoUser.Error())
return "", false
render.Status(r, http.StatusOK)
}
return u.GetId().GetOpaqueId(), true
}
@@ -1,13 +1,118 @@
package svc_test
import (
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/opencloud-eu/opencloud/pkg/log"
"github.com/opencloud-eu/opencloud/services/graph/mocks"
svc "github.com/opencloud-eu/opencloud/services/graph/pkg/service/v0"
)
func TestNewUsersUserProfilePhotoApi(t *testing.T) {
panic("add UsersUserProfilePhotoApi tests")
}
func TestUsersUserProfilePhotoApi(t *testing.T) {
var (
usersUserProfilePhotoProvider = mocks.NewUsersUserProfilePhotoProvider(t)
dummyDataProvider = func(w http.ResponseWriter, r *http.Request) (string, bool) {
return "123", true
}
)
func TestNewUsersUserProfilePhotoService(t *testing.T) {
panic("add UsersUserProfilePhotoService tests")
api, err := svc.NewUsersUserProfilePhotoApi(usersUserProfilePhotoProvider, log.NopLogger())
assert.NoError(t, err)
t.Run("GetProfilePhoto", func(t *testing.T) {
r := httptest.NewRequest(http.MethodGet, "/", nil)
ep := api.GetProfilePhoto(dummyDataProvider)
t.Run("fails if photo provider errors", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) {
return nil, errors.New("any")
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusNotFound, w.Code)
})
t.Run("successfully returns the requested photo", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().GetPhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) ([]byte, error) {
return []byte("photo"), nil
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
assert.Equal(t, "photo", w.Body.String())
})
})
t.Run("DeleteProfilePhoto", func(t *testing.T) {
r := httptest.NewRequest(http.MethodDelete, "/", nil)
ep := api.DeleteProfilePhoto(dummyDataProvider)
t.Run("fails if photo provider errors", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error {
return errors.New("any")
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusInternalServerError, w.Code)
})
t.Run("successfully deletes the requested photo", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().DeletePhoto(mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string) error {
return nil
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
})
})
t.Run("UpsertProfilePhoto", func(t *testing.T) {
r := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("body"))
ep := api.UpsertProfilePhoto(dummyDataProvider)
t.Run("fails if photo provider errors", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error {
return errors.New("any")
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusInternalServerError, w.Code)
})
t.Run("successfully upserts the photo", func(t *testing.T) {
w := httptest.NewRecorder()
usersUserProfilePhotoProvider.EXPECT().UpdatePhoto(mock.Anything, mock.Anything, mock.Anything).RunAndReturn(func(ctx context.Context, s string, r io.Reader) error {
return nil
}).Once()
ep.ServeHTTP(w, r)
assert.Equal(t, http.StatusOK, w.Code)
})
})
}
+42
View File
@@ -0,0 +1,42 @@
package svc
import (
"errors"
"fmt"
"net/http"
"net/url"
"github.com/go-chi/chi/v5"
revactx "github.com/opencloud-eu/reva/v2/pkg/ctx"
"github.com/opencloud-eu/opencloud/services/graph/pkg/errorcode"
)
// HTTPDataHandler returns data from the request, it should exit early and return false in the case of any error
type HTTPDataHandler[T any] func(w http.ResponseWriter, r *http.Request) (T, bool)
var (
// ErrNoUser is returned when no user is found
ErrNoUser = errors.New("no user found")
)
// GetUserIDFromCTX extracts the user from the request
func GetUserIDFromCTX(w http.ResponseWriter, r *http.Request) (string, bool) {
u, ok := revactx.ContextGetUser(r.Context())
if !ok {
errorcode.GeneralException.Render(w, r, http.StatusMethodNotAllowed, ErrNoUser.Error())
}
return u.GetId().GetOpaqueId(), ok
}
func GetSlugValue(key string) HTTPDataHandler[string] {
return func(w http.ResponseWriter, r *http.Request) (string, bool) {
v, err := url.PathUnescape(chi.URLParam(r, key))
if err != nil {
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, fmt.Sprintf(`failed to get slug: "%s"`, key))
}
return v, err == nil
}
}
+58 -53
View File
@@ -144,14 +144,57 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
identity.IdentityCacheWithGroupsTTL(time.Duration(options.Config.Spaces.GroupsCacheTTL)),
)
storage, err := metadata.NewCS3Storage(
options.Config.Metadata.GatewayAddress,
options.Config.Metadata.StorageAddress,
options.Config.Metadata.SystemUserID,
options.Config.Metadata.SystemUserIDP,
options.Config.Metadata.SystemUserAPIKey,
)
if err != nil {
return Graph{}, err
}
baseGraphService := BaseGraphService{
logger: &options.Logger,
identityCache: identityCache,
gatewaySelector: options.GatewaySelector,
config: options.Config,
availableRoles: unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(options.Config.UnifiedRoles.AvailableRoles...)),
}
drivesDriveItemService, err := NewDrivesDriveItemService(options.Logger, options.GatewaySelector)
if err != nil {
return Graph{}, err
}
drivesDriveItemApi, err := NewDrivesDriveItemApi(drivesDriveItemService, baseGraphService, options.Logger)
if err != nil {
return Graph{}, err
}
driveItemPermissionsService, err := NewDriveItemPermissionsService(options.Logger, options.GatewaySelector, identityCache, options.Config)
if err != nil {
return Graph{}, err
}
driveItemPermissionsApi, err := NewDriveItemPermissionsApi(driveItemPermissionsService, options.Logger, options.Config)
if err != nil {
return Graph{}, err
}
usersUserProfilePhotoService, err := NewUsersUserProfilePhotoService(storage)
if err != nil {
return Graph{}, err
}
usersUserProfilePhotoApi, err := NewUsersUserProfilePhotoApi(usersUserProfilePhotoService, options.Logger)
if err != nil {
return Graph{}, err
}
svc := Graph{
BaseGraphService: BaseGraphService{
logger: &options.Logger,
identityCache: identityCache,
gatewaySelector: options.GatewaySelector,
config: options.Config,
availableRoles: unifiedrole.GetRoles(unifiedrole.RoleFilterIDs(options.Config.UnifiedRoles.AvailableRoles...)),
},
BaseGraphService: baseGraphService,
mux: m,
specialDriveItemsCache: spacePropertiesCache,
eventsPublisher: options.EventsPublisher,
@@ -206,47 +249,6 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
requireAdmin = options.RequireAdminMiddleware
}
drivesDriveItemService, err := NewDrivesDriveItemService(options.Logger, options.GatewaySelector)
if err != nil {
return svc, err
}
drivesDriveItemApi, err := NewDrivesDriveItemApi(drivesDriveItemService, svc.BaseGraphService, options.Logger)
if err != nil {
return svc, err
}
driveItemPermissionsService, err := NewDriveItemPermissionsService(options.Logger, options.GatewaySelector, identityCache, options.Config)
if err != nil {
return svc, err
}
driveItemPermissionsApi, err := NewDriveItemPermissionsApi(driveItemPermissionsService, options.Logger, options.Config)
if err != nil {
return svc, err
}
storage, err := metadata.NewCS3Storage(
options.Config.Metadata.GatewayAddress,
options.Config.Metadata.StorageAddress,
options.Config.Metadata.SystemUserID,
options.Config.Metadata.SystemUserIDP,
options.Config.Metadata.SystemUserAPIKey,
)
if err != nil {
return svc, err
}
usersUserProfilePhotoService, err := NewUsersUserProfilePhotoService(storage)
if err != nil {
return svc, err
}
usersUserProfilePhotoApi, err := NewUsersUserProfilePhotoApi(usersUserProfilePhotoService, options.Logger)
if err != nil {
return svc, err
}
m.Route(options.Config.HTTP.Root, func(r chi.Router) {
r.Use(middleware.StripSlashes)
@@ -315,11 +317,11 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
})
r.Get("/drives", svc.GetDrives(APIVersion_1))
r.Post("/changePassword", svc.ChangeOwnPassword)
r.Route("/photo", func(r chi.Router) {
r.Get("/", usersUserProfilePhotoApi.GetProfilePhoto)
r.Put("/", usersUserProfilePhotoApi.UpsertProfilePhoto)
r.Patch("/", usersUserProfilePhotoApi.UpsertProfilePhoto)
r.Delete("/", usersUserProfilePhotoApi.DeleteProfilePhoto)
r.Route("/photo/$value", func(r chi.Router) {
r.Get("/", usersUserProfilePhotoApi.GetProfilePhoto(GetUserIDFromCTX))
r.Put("/", usersUserProfilePhotoApi.UpsertProfilePhoto(GetUserIDFromCTX))
r.Patch("/", usersUserProfilePhotoApi.UpsertProfilePhoto(GetUserIDFromCTX))
r.Delete("/", usersUserProfilePhotoApi.DeleteProfilePhoto(GetUserIDFromCTX))
})
})
r.Route("/users", func(r chi.Router) {
@@ -329,6 +331,9 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
r.Get("/", svc.GetUser)
r.Get("/drive", svc.GetUserDrive)
r.Post("/exportPersonalData", svc.ExportPersonalData)
r.Route("/photo/$value", func(r chi.Router) {
r.Get("/", usersUserProfilePhotoApi.GetProfilePhoto(GetSlugValue("userID")))
})
r.With(requireAdmin).Delete("/", svc.DeleteUser)
r.With(requireAdmin).Patch("/", svc.PatchUser)
if svc.roleService != nil {