Preferences driver refactor and cbox sql implementation (cs3org#2696)

changelog/unreleased/preferences-refactor.md

@@ -0,0 +1,7 @@
+Enhancement: Preferences driver refactor and cbox sql implementation
+
+This PR uses the updated CS3APIs which accepts a namespace in addition to a
+single string key to recognize a user preference. It also refactors the GRPC
+service to support multiple drivers and adds the cbox SQL implementation.
+
+https://github.com/cs3org/reva/pull/2696

cmd/reva/preferences.go

@@ -40,6 +40,7 @@ var preferencesCommand = func() *command {
 
 		subcommand := cmd.Args()[0]
 		key := cmd.Args()[1]
+		ns := cmd.Args()[2]
 
 		client, err := getClient()
 		if err != nil {
@@ -50,12 +51,15 @@ var preferencesCommand = func() *command {
 
 		switch subcommand {
 		case "set":
-			if cmd.NArg() < 3 {
+			if cmd.NArg() < 4 {
 				return errors.New("Invalid arguments: " + cmd.Usage())
 			}
-			value := cmd.Args()[2]
+			value := cmd.Args()[3]
 			req := &preferences.SetKeyRequest{
-				Key: key,
+				Key: &preferences.PreferenceKey{
+					Namespace: ns,
+					Key:       key,
+				},
 				Val: value,
 			}
 
@@ -70,7 +74,10 @@ var preferencesCommand = func() *command {
 
 		case "get":
 			req := &preferences.GetKeyRequest{
-				Key: key,
+				Key: &preferences.PreferenceKey{
+					Namespace: ns,
+					Key:       key,
+				},
 			}
 
 			res, err := client.GetKey(ctx, req)

cmd/revad/runtime/loader.go

@@ -40,6 +40,7 @@ import (
 	_ "github.com/cs3org/reva/pkg/ocm/provider/authorizer/loader"
 	_ "github.com/cs3org/reva/pkg/ocm/share/manager/loader"
 	_ "github.com/cs3org/reva/pkg/permission/manager/loader"
+	_ "github.com/cs3org/reva/pkg/preferences/loader"
 	_ "github.com/cs3org/reva/pkg/publicshare/manager/loader"
 	_ "github.com/cs3org/reva/pkg/rhttp/datatx/manager/loader"
 	_ "github.com/cs3org/reva/pkg/share/cache/loader"

go.mod

@@ -19,7 +19,7 @@ require (
 	github.com/cheggaaa/pb v1.0.29
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
-	github.com/cs3org/go-cs3apis v0.0.0-20220214084335-d975ab5d6e65
+	github.com/cs3org/go-cs3apis v0.0.0-20220330081745-2ad58f5932b9
 	github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8
 	github.com/dgraph-io/ristretto v0.1.0
 	github.com/eventials/go-tus v0.0.0-20200718001131-45c7ec8f5d59
@@ -93,4 +93,4 @@ replace (
 	github.com/eventials/go-tus => github.com/andrewmostello/go-tus v0.0.0-20200314041820-904a9904af9a
 	github.com/oleiade/reflections => github.com/oleiade/reflections v1.0.1
 	google.golang.org/grpc => google.golang.org/grpc v1.26.0 // temporary downgrade
-)
+)

go.sum

@@ -210,10 +210,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
 github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
 github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
-github.com/cs3org/go-cs3apis v0.0.0-20211214102128-4e8745ab1654 h1:ha5tiuuFyDrwKUrVEc3TrRDFgTKVQ9NGDRmEP0PRPno=
-github.com/cs3org/go-cs3apis v0.0.0-20211214102128-4e8745ab1654/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
-github.com/cs3org/go-cs3apis v0.0.0-20220214084335-d975ab5d6e65 h1:cee0dhBsF8KofV2TM52T41eOo1QLSgtgEZsjYmC5dhU=
-github.com/cs3org/go-cs3apis v0.0.0-20220214084335-d975ab5d6e65/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
+github.com/cs3org/go-cs3apis v0.0.0-20220330081745-2ad58f5932b9 h1:SuPu5Mc2mpz+J059XML+cMd0i5FZR4t/kROS3SaIsnU=
+github.com/cs3org/go-cs3apis v0.0.0-20220330081745-2ad58f5932b9/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=

internal/grpc/services/authregistry/authregistry.go

@@ -116,17 +116,17 @@ func (s *service) ListAuthProviders(ctx context.Context, req *registrypb.ListAut
 	return res, nil
 }
 
-func (s *service) GetAuthProvider(ctx context.Context, req *registrypb.GetAuthProviderRequest) (*registrypb.GetAuthProviderResponse, error) {
+func (s *service) GetAuthProviders(ctx context.Context, req *registrypb.GetAuthProvidersRequest) (*registrypb.GetAuthProvidersResponse, error) {
 	pinfo, err := s.reg.GetProvider(ctx, req.Type)
 	if err != nil {
-		return &registrypb.GetAuthProviderResponse{
+		return &registrypb.GetAuthProvidersResponse{
 			Status: status.NewInternal(ctx, err, "error getting auth provider for type: "+req.Type),
 		}, nil
 	}
 
-	res := &registrypb.GetAuthProviderResponse{
-		Status:   status.NewOK(ctx),
-		Provider: pinfo,
+	res := &registrypb.GetAuthProvidersResponse{
+		Status:    status.NewOK(ctx),
+		Providers: []*registrypb.ProviderInfo{pinfo},
 	}
 	return res, nil
 }

internal/grpc/services/gateway/authprovider.go

@@ -213,7 +213,7 @@ func (s *svc) findAuthProvider(ctx context.Context, authType string) (authpb.Pro
 		return nil, err
 	}
 
-	res, err := c.GetAuthProvider(ctx, &registry.GetAuthProviderRequest{
+	res, err := c.GetAuthProviders(ctx, &registry.GetAuthProvidersRequest{
 		Type: authType,
 	})
 
@@ -222,9 +222,9 @@ func (s *svc) findAuthProvider(ctx context.Context, authType string) (authpb.Pro
 		return nil, err
 	}
 
-	if res.Status.Code == rpc.Code_CODE_OK && res.Provider != nil {
+	if res.Status.Code == rpc.Code_CODE_OK && res.Providers != nil && len(res.Providers) > 0 {
 		// TODO(labkode): check for capabilities here
-		c, err := pool.GetAuthProviderServiceClient(res.Provider.Address)
+		c, err := pool.GetAuthProviderServiceClient(res.Providers[0].Address)
 		if err != nil {
 			err = errors.Wrap(err, "gateway: error getting an auth provider client")
 			return nil, err

internal/grpc/services/preferences/preferences.go

@@ -20,38 +20,73 @@ package preferences
 
 import (
 	"context"
-	"sync"
 
 	"google.golang.org/grpc"
 
-	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
-	preferences "github.com/cs3org/go-cs3apis/cs3/preferences/v1beta1"
-	ctxpkg "github.com/cs3org/reva/pkg/ctx"
+	preferencespb "github.com/cs3org/go-cs3apis/cs3/preferences/v1beta1"
+	"github.com/cs3org/reva/pkg/errtypes"
+	"github.com/cs3org/reva/pkg/preferences"
+	"github.com/cs3org/reva/pkg/preferences/registry"
 	"github.com/cs3org/reva/pkg/rgrpc"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
+	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 )
 
-type contextUserRequiredErr string
-
-func (err contextUserRequiredErr) Error() string { return string(err) }
-
 func init() {
 	rgrpc.Register("preferences", New)
 }
 
-// m maps user to map of user preferences.
-// m = map[userToken]map[key]value
-var m = make(map[string]map[string]string)
+type config struct {
+	Driver  string                            `mapstructure:"driver"`
+	Drivers map[string]map[string]interface{} `mapstructure:"drivers"`
+}
+
+func (c *config) init() {
+	if c.Driver == "" {
+		c.Driver = "memory"
+	}
+}
 
-var mutex = &sync.Mutex{}
+type service struct {
+	conf *config
+	pm   preferences.Manager
+}
 
-type service struct{}
+func getPreferencesManager(c *config) (preferences.Manager, error) {
+	if f, ok := registry.NewFuncs[c.Driver]; ok {
+		return f(c.Drivers[c.Driver])
+	}
+	return nil, errtypes.NotFound("driver not found: " + c.Driver)
+}
+
+func parseConfig(m map[string]interface{}) (*config, error) {
+	c := &config{}
+	if err := mapstructure.Decode(m, c); err != nil {
+		err = errors.Wrap(err, "error decoding conf")
+		return nil, err
+	}
+	return c, nil
+}
 
 // New returns a new PreferencesServiceServer
 func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
-	service := &service{}
-	return service, nil
+	c, err := parseConfig(m)
+	if err != nil {
+		return nil, err
+	}
+
+	c.init()
+
+	pm, err := getPreferencesManager(c)
+	if err != nil {
+		return nil, err
+	}
+
+	return &service{
+		conf: c,
+		pm:   pm,
+	}, nil
 }
 
 func (s *service) Close() error {
@@ -63,72 +98,36 @@ func (s *service) UnprotectedEndpoints() []string {
 }
 
 func (s *service) Register(ss *grpc.Server) {
-	preferences.RegisterPreferencesAPIServer(ss, s)
+	preferencespb.RegisterPreferencesAPIServer(ss, s)
 }
 
-func getUser(ctx context.Context) (*userpb.User, error) {
-	u, ok := ctxpkg.ContextGetUser(ctx)
-	if !ok {
-		err := errors.Wrap(contextUserRequiredErr("userrequired"), "preferences: error getting user from ctx")
-		return nil, err
-	}
-	return u, nil
-}
-
-func (s *service) SetKey(ctx context.Context, req *preferences.SetKeyRequest) (*preferences.SetKeyResponse, error) {
-	key := req.Key
-	value := req.Val
-
-	u, err := getUser(ctx)
+func (s *service) SetKey(ctx context.Context, req *preferencespb.SetKeyRequest) (*preferencespb.SetKeyResponse, error) {
+	err := s.pm.SetKey(ctx, req.Key.Key, req.Key.Namespace, req.Val)
 	if err != nil {
-		err = errors.Wrap(err, "preferences: failed to call getUser")
-		return &preferences.SetKeyResponse{
-			Status: status.NewUnauthenticated(ctx, err, "user not found or invalid"),
-		}, err
+		return &preferencespb.SetKeyResponse{
+			Status: status.NewInternal(ctx, err, "error setting key"),
+		}, nil
 	}
 
-	name := u.Username
-
-	mutex.Lock()
-	defer mutex.Unlock()
-	if len(m[name]) == 0 {
-		m[name] = map[string]string{key: value}
-	} else {
-		usersettings := m[name]
-		usersettings[key] = value
-	}
-
-	return &preferences.SetKeyResponse{
+	return &preferencespb.SetKeyResponse{
 		Status: status.NewOK(ctx),
 	}, nil
 }
 
-func (s *service) GetKey(ctx context.Context, req *preferences.GetKeyRequest) (*preferences.GetKeyResponse, error) {
-	key := req.Key
-	u, err := getUser(ctx)
+func (s *service) GetKey(ctx context.Context, req *preferencespb.GetKeyRequest) (*preferencespb.GetKeyResponse, error) {
+	val, err := s.pm.GetKey(ctx, req.Key.Key, req.Key.Namespace)
 	if err != nil {
-		err = errors.Wrap(err, "preferences: failed to call getUser")
-		return &preferences.GetKeyResponse{
-			Status: status.NewUnauthenticated(ctx, err, "user not found or invalid"),
-		}, err
-	}
-
-	name := u.Username
-
-	mutex.Lock()
-	defer mutex.Unlock()
-	if len(m[name]) != 0 {
-		if value, ok := m[name][key]; ok {
-			return &preferences.GetKeyResponse{
-				Status: status.NewOK(ctx),
-				Val:    value,
-			}, nil
+		st := status.NewInternal(ctx, err, "error retrieving key")
+		if _, ok := err.(errtypes.IsNotFound); ok {
+			st = status.NewNotFound(ctx, "key not found")
 		}
+		return &preferencespb.GetKeyResponse{
+			Status: st,
+		}, nil
 	}
 
-	res := &preferences.GetKeyResponse{
-		Status: status.NewNotFound(ctx, "key not found"),
-		Val:    "",
-	}
-	return res, nil
+	return &preferencespb.GetKeyResponse{
+		Status: status.NewOK(ctx),
+		Val:    val,
+	}, nil
 }

internal/http/services/appprovider/appprovider.go

@@ -33,10 +33,10 @@ import (
 	"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
 	"github.com/cs3org/reva/pkg/rhttp"
 	"github.com/cs3org/reva/pkg/rhttp/global"
-	"github.com/cs3org/reva/pkg/rhttp/router"
 	"github.com/cs3org/reva/pkg/sharedconf"
 	"github.com/cs3org/reva/pkg/utils"
 	"github.com/cs3org/reva/pkg/utils/resourceid"
+	"github.com/go-chi/chi/v5"
 	ua "github.com/mileusna/useragent"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
@@ -47,7 +47,7 @@ func init() {
 	global.Register("appprovider", New)
 }
 
-// Config holds the config options that need to be passed down to all ocdav handlers
+// Config holds the config options for the HTTP appprovider service
 type Config struct {
 	Prefix     string `mapstructure:"prefix"`
 	GatewaySvc string `mapstructure:"gatewaysvc"`
@@ -62,7 +62,8 @@ func (c *Config) init() {
 }
 
 type svc struct {
-	conf *Config
+	conf   *Config
+	router *chi.Mux
 }
 
 // New returns a new ocmd object
@@ -74,12 +75,26 @@ func New(m map[string]interface{}, log *zerolog.Logger) (global.Service, error)
 	}
 	conf.init()
 
+	r := chi.NewRouter()
 	s := &svc{
-		conf: conf,
+		conf:   conf,
+		router: r,
 	}
+
+	if err := s.routerInit(); err != nil {
+		return nil, err
+	}
+
 	return s, nil
 }
 
+func (s *svc) routerInit() error {
+	s.router.Get("/list", s.handleList)
+	s.router.Post("/new", s.handleNew)
+	s.router.Post("/open", s.handleOpen)
+	return nil
+}
+
 // Close performs cleanup.
 func (s *svc) Close() error {
 	return nil
@@ -95,29 +110,7 @@ func (s *svc) Unprotected() []string {
 
 func (s *svc) Handler() http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		var head string
-		head, r.URL.Path = router.ShiftPath(r.URL.Path)
-
-		switch r.Method {
-		case "POST":
-			switch head {
-			case "new":
-				s.handleNew(w, r)
-			case "open":
-				s.handleOpen(w, r)
-			default:
-				writeError(w, r, appErrorUnimplemented, "unsupported POST endpoint", nil)
-			}
-		case "GET":
-			switch head {
-			case "list":
-				s.handleList(w, r)
-			default:
-				writeError(w, r, appErrorUnimplemented, "unsupported GET endpoint", nil)
-			}
-		default:
-			writeError(w, r, appErrorUnimplemented, "unsupported method", nil)
-		}
+		s.router.ServeHTTP(w, r)
 	})
 }