[full-ci] add identitySet displayName property to the group and user sets (#5347)

* add identitySet displayName property to the group and user sets
2026-01-04 11:19:39 -06:00 · 2023-01-10 15:42:13 +01:00
parent 9bdd1e9ce9
commit bb1540fc45
19 changed files with 104 additions and 31 deletions
--- a/changelog/unreleased/enhancement-identity-set-display-name.md
+++ b/changelog/unreleased/enhancement-identity-set-display-name.md
@@ -0,0 +1,7 @@
+Enhancement: Graph Drives IdentitySet displayName
+
+We've added the IdentitySet displayName property to the group and user sets for the graph drives endpoint.
+The values for groups and users get cached.
+
+https://github.com/owncloud/ocis/pull/5347
+https://github.com/owncloud/web/pull/8178
--- a/go.mod
+++ b/go.mod
@@ -42,6 +42,7 @@ require (
 	github.com/gorilla/mux v1.8.0
 	github.com/grpc-ecosystem/grpc-gateway/v2 v2.13.0
 	github.com/jellydator/ttlcache/v2 v2.11.1
+	github.com/jellydator/ttlcache/v3 v3.0.1
 	github.com/justinas/alice v1.2.0
 	github.com/libregraph/idm v0.3.1-0.20220808071235-17bb032176de
 	github.com/libregraph/lico v0.54.1-0.20220325072321-31efc3995d63
--- a/go.sum
+++ b/go.sum
@@ -823,6 +823,8 @@ github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 h1:BQSFePA1RWJOl
 github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99/go.mod h1:1lJo3i6rXxKeerYnT8Nvf0QmHCRC1n8sfWVwXF2Frvo=
 github.com/jellydator/ttlcache/v2 v2.11.1 h1:AZGME43Eh2Vv3giG6GeqeLeFXxwxn1/qHItqWZl6U64=
 github.com/jellydator/ttlcache/v2 v2.11.1/go.mod h1:RtE5Snf0/57e+2cLWFYWCCsLas2Hy3c5Z4n14XmSvTI=
+github.com/jellydator/ttlcache/v3 v3.0.1 h1:cHgCSMS7TdQcoprXnWUptJZzyFsqs18Lt8VVhRuZYVU=
+github.com/jellydator/ttlcache/v3 v3.0.1/go.mod h1:WwTaEmcXQ3MTjOm4bsZoDFiCu/hMvNWLO1w67RXz6h4=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
 github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
 github.com/jhump/protoreflect v1.6.0 h1:h5jfMVslIg6l29nsMs0D8Wj17RDVdNYti0vDN/PZZoE=
--- a/services/graph/pkg/config/config.go
+++ b/services/graph/pkg/config/config.go
@@ -37,6 +37,8 @@ type Spaces struct {
 	WebDavPath                      string `yaml:"webdav_path" env:"GRAPH_SPACES_WEBDAV_PATH" desc:"The WebDAV subpath for spaces."`
 	DefaultQuota                    string `yaml:"default_quota" env:"GRAPH_SPACES_DEFAULT_QUOTA" desc:"The default quota in bytes."`
 	ExtendedSpacePropertiesCacheTTL int    `yaml:"extended_space_properties_cache_ttl" env:"GRAPH_SPACES_EXTENDED_SPACE_PROPERTIES_CACHE_TTL" desc:"Max TTL in seconds for the spaces property cache."`
+	UsersCacheTTL                   int    `yaml:"users_cache_ttl" env:"GRAPH_SPACES_USERS_CACHE_TTL" desc:"Max TTL in seconds for the spaces users cache."`
+	GroupsCacheTTL                  int    `yaml:"groups_cache_ttl" env:"GRAPH_SPACES_GROUPS_CACHE_TTL" desc:"Max TTL in seconds for the spaces groups cache."`
 }

 type LDAP struct {
--- a/services/graph/pkg/config/defaults/defaultconfig.go
+++ b/services/graph/pkg/config/defaults/defaultconfig.go
@@ -3,6 +3,7 @@ package defaults
 import (
 	"path"
 	"strings"
+	"time"

 	"github.com/owncloud/ocis/v2/ocis-pkg/config/defaults"
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
@@ -38,6 +39,10 @@ func DefaultConfig() *config.Config {
 			WebDavBase:   "https://localhost:9200",
 			WebDavPath:   "/dav/spaces/",
 			DefaultQuota: "1000000000",
+			// 30 minutes
+			GroupsCacheTTL: 1800,
+			// 30 minutes
+			UsersCacheTTL: 1800,
 		},
 		Identity: config.Identity{
 			Backend: "ldap",
@@ -136,4 +141,10 @@ func Sanitize(cfg *config.Config) {
 	if cfg.HTTP.Root != "/" {
 		cfg.HTTP.Root = strings.TrimSuffix(cfg.HTTP.Root, "/")
 	}
+
+	// convert ttl to millisecond
+	// the config is in seconds, therefore we need multiply it.
+	cfg.Spaces.ExtendedSpacePropertiesCacheTTL = cfg.Spaces.ExtendedSpacePropertiesCacheTTL * int(time.Second)
+	cfg.Spaces.GroupsCacheTTL = cfg.Spaces.GroupsCacheTTL * int(time.Second)
+	cfg.Spaces.UsersCacheTTL = cfg.Spaces.UsersCacheTTL * int(time.Second)
 }
--- a/services/graph/pkg/server/http/server.go
+++ b/services/graph/pkg/server/http/server.go
@@ -122,7 +122,8 @@ func Server(opts ...Option) (http.Service, error) {
 		// no gatewayclient needed
 	}

-	handle := svc.NewService(
+	var handle svc.Service
+	handle, err = svc.NewService(
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(middlewares...),
@@ -133,7 +134,7 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.WithSearchService(searchsvc.NewSearchProviderService("com.owncloud.api.search", grpc.DefaultClient())),
 	)

-	if handle == nil {
+	if err != nil {
 		return http.Service{}, errors.New("could not initialize graph service")
 	}

--- a/services/graph/pkg/service/v0/driveitems_test.go
+++ b/services/graph/pkg/service/v0/driveitems_test.go
@@ -64,7 +64,7 @@ var _ = Describe("Driveitems", func() {
 		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}

 		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.EventsPublisher(&eventsPublisher),
--- a/services/graph/pkg/service/v0/drives.go
+++ b/services/graph/pkg/service/v0/drives.go
@@ -23,6 +23,7 @@ import (
 	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/render"
+	"github.com/jellydator/ttlcache/v3"
 	libregraph "github.com/owncloud/libre-graph-api-go"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/grpc"
 	v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0"
@@ -575,25 +576,44 @@ func (g Graph) cs3StorageSpaceToDrive(ctx context.Context, baseURL *url.URL, spa
 				// libregraph.Identity and if we pass the pointer from the loop every identity
 				// will have the same id.
 				tmp := id
-				var identity libregraph.IdentitySet
+				var identitySet libregraph.IdentitySet
+				if _, ok := groupsMap[id]; ok {
+					var group libregraph.Group
+					if item := g.groupsCache.Get(id); item == nil {
+						if requestedGroup, err := g.identityBackend.GetGroup(ctx, id, url.Values{}); err == nil {
+							group = *requestedGroup
+							g.groupsCache.Set(id, group, ttlcache.DefaultTTL)
+						}
+					} else {
+						group = item.Value()
+					}

-				if _, ok := groupsMap[id]; !ok {
-					identity = libregraph.IdentitySet{User: &libregraph.Identity{Id: &tmp}}
+					identitySet = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp, DisplayName: group.GetDisplayName()}}
 				} else {
-					identity = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp}}
+					var user libregraph.User
+					if item := g.usersCache.Get(id); item == nil {
+						if requestedUser, err := g.identityBackend.GetUser(ctx, id, url.Values{}); err == nil {
+							user = *requestedUser
+							g.usersCache.Set(id, user, ttlcache.DefaultTTL)
+						}
+					} else {
+						user = item.Value()
+					}
+
+					identitySet = libregraph.IdentitySet{User: &libregraph.Identity{Id: &tmp, DisplayName: user.GetDisplayName()}}
 				}

 				// we need to map the permissions to the roles
 				switch {
 				// having RemoveGrant qualifies you as a manager
 				case perm.RemoveGrant:
-					managerIdentities = append(managerIdentities, identity)
+					managerIdentities = append(managerIdentities, identitySet)
 				// InitiateFileUpload means you are an editor
 				case perm.InitiateFileUpload:
-					editorIdentities = append(editorIdentities, identity)
+					editorIdentities = append(editorIdentities, identitySet)
 				// Stat permission at least makes you a viewer
 				case perm.Stat:
-					viewerIdentities = append(viewerIdentities, identity)
+					viewerIdentities = append(viewerIdentities, identitySet)
 				}
 			}

--- a/services/graph/pkg/service/v0/educationschools_test.go
+++ b/services/graph/pkg/service/v0/educationschools_test.go
@@ -65,7 +65,7 @@ var _ = Describe("Schools", func() {
 		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}

 		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.WithIdentityEducationBackend(identityEducationBackend),
--- a/services/graph/pkg/service/v0/educationuser_test.go
+++ b/services/graph/pkg/service/v0/educationuser_test.go
@@ -69,7 +69,7 @@ var _ = Describe("EducationUsers", func() {
 		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}

 		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.EventsPublisher(&eventsPublisher),
--- a/services/graph/pkg/service/v0/graph.go
+++ b/services/graph/pkg/service/v0/graph.go
@@ -12,7 +12,8 @@ import (
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/events"
 	"github.com/go-chi/chi/v5"
-	"github.com/jellydator/ttlcache/v2"
+	"github.com/jellydator/ttlcache/v3"
+	libregraph "github.com/owncloud/libre-graph-api-go"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	searchsvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/search/v0"
 	settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
@@ -100,7 +101,9 @@ type Graph struct {
 	gatewayClient            GatewayClient
 	roleService              RoleService
 	permissionsService       Permissions
-	spacePropertiesCache     *ttlcache.Cache
+	spacePropertiesCache     *ttlcache.Cache[string, interface{}]
+	usersCache               *ttlcache.Cache[string, libregraph.User]
+	groupsCache              *ttlcache.Cache[string, libregraph.Group]
 	eventsPublisher          events.Publisher
 	searchService            searchsvc.SearchProviderService
 }
--- a/services/graph/pkg/service/v0/graph_test.go
+++ b/services/graph/pkg/service/v0/graph_test.go
@@ -67,7 +67,7 @@ var _ = Describe("Graph", func() {
 		gatewayClient = &mocks.GatewayClient{}
 		eventsPublisher = mocks.Publisher{}
 		permissionService = mocks.Permissions{}
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.EventsPublisher(&eventsPublisher),
--- a/services/graph/pkg/service/v0/groups_test.go
+++ b/services/graph/pkg/service/v0/groups_test.go
@@ -70,7 +70,7 @@ var _ = Describe("Groups", func() {
 		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}

 		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.EventsPublisher(&eventsPublisher),
--- a/services/graph/pkg/service/v0/instrument.go
+++ b/services/graph/pkg/service/v0/instrument.go
@@ -7,7 +7,7 @@ import (
 )

 // NewInstrument returns a service that instruments metrics.
-func NewInstrument(next Service, metrics *metrics.Metrics) Service {
+func NewInstrument(next Service, metrics *metrics.Metrics) instrument {
 	return instrument{
 		next:    next,
 		metrics: metrics,
--- a/services/graph/pkg/service/v0/logging.go
+++ b/services/graph/pkg/service/v0/logging.go
@@ -7,7 +7,7 @@ import (
 )

 // NewLogging returns a service that logs messages.
-func NewLogging(next Service, logger log.Logger) Service {
+func NewLogging(next Service, logger log.Logger) logging {
 	return logging{
 		next:   next,
 		logger: logger,
--- a/services/graph/pkg/service/v0/password_test.go
+++ b/services/graph/pkg/service/v0/password_test.go
@@ -70,7 +70,7 @@ var _ = Describe("Users changing their own password", func() {
 		Expect(err).To(BeNil())

 		eventsPublisher = mocks.Publisher{}
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.WithIdentityBackend(identityBackend),
--- a/services/graph/pkg/service/v0/service.go
+++ b/services/graph/pkg/service/v0/service.go
@@ -3,14 +3,16 @@ package svc
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"fmt"
 	"net/http"
 	"os"
 	"strconv"
+	"time"

 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
-	"github.com/jellydator/ttlcache/v2"
-
+	"github.com/jellydator/ttlcache/v3"
+	libregraph "github.com/owncloud/libre-graph-api-go"
 	ocisldap "github.com/owncloud/ocis/v2/ocis-pkg/ldap"
 	"github.com/owncloud/ocis/v2/ocis-pkg/roles"
 	"github.com/owncloud/ocis/v2/ocis-pkg/service/grpc"
@@ -76,17 +78,40 @@ type Service interface {
 }

 // NewService returns a service implementation for Service.
-func NewService(opts ...Option) Service {
+func NewService(opts ...Option) (Graph, error) {
 	options := newOptions(opts...)

 	m := chi.NewMux()
 	m.Use(options.Middleware...)

+	spacePropertiesCache := ttlcache.New[string, interface{}](
+		ttlcache.WithTTL[string, interface{}](
+			time.Duration(options.Config.Spaces.ExtendedSpacePropertiesCacheTTL),
+		),
+	)
+	go spacePropertiesCache.Start()
+
+	usersCache := ttlcache.New[string, libregraph.User](
+		ttlcache.WithTTL[string, libregraph.User](
+			time.Duration(options.Config.Spaces.UsersCacheTTL),
+		),
+	)
+	go usersCache.Start()
+
+	groupsCache := ttlcache.New[string, libregraph.Group](
+		ttlcache.WithTTL[string, libregraph.Group](
+			time.Duration(options.Config.Spaces.GroupsCacheTTL),
+		),
+	)
+	go groupsCache.Start()
+
 	svc := Graph{
 		config:                   options.Config,
 		mux:                      m,
 		logger:                   &options.Logger,
-		spacePropertiesCache:     ttlcache.NewCache(),
+		spacePropertiesCache:     spacePropertiesCache,
+		usersCache:               usersCache,
+		groupsCache:              groupsCache,
 		eventsPublisher:          options.EventsPublisher,
 		gatewayClient:            options.GatewayClient,
 		searchService:            options.SearchService,
@@ -129,11 +154,11 @@ func NewService(opts ...Option) Service {
 				pemData, err := os.ReadFile(options.Config.Identity.LDAP.CACert)
 				if err != nil {
 					options.Logger.Error().Err(err).Msgf("Error initializing LDAP Backend")
-					return nil
+					return svc, err
 				}
 				if !certs.AppendCertsFromPEM(pemData) {
 					options.Logger.Error().Msgf("Error initializing LDAP Backend. Adding CA cert failed")
-					return nil
+					return svc, err
 				}
 				tlsConf.RootCAs = certs
 			}
@@ -149,7 +174,7 @@ func NewService(opts ...Option) Service {
 			lb, err := identity.NewLDAPBackend(conn, options.Config.Identity.LDAP, &options.Logger)
 			if err != nil {
 				options.Logger.Error().Msgf("Error initializing LDAP Backend: '%s'", err)
-				return nil
+				return svc, err
 			}
 			svc.identityBackend = lb
 			if options.IdentityEducationBackend == nil {
@@ -161,8 +186,9 @@ func NewService(opts ...Option) Service {
 				}
 			}
 		default:
-			options.Logger.Error().Msgf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend)
-			return nil
+			err := fmt.Errorf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend)
+			options.Logger.Err(err)
+			return svc, err
 		}
 	} else {
 		svc.identityBackend = options.IdentityBackend
@@ -284,7 +310,7 @@ func NewService(opts ...Option) Service {
 		})
 	})

-	return svc
+	return svc, nil
 }

 // parseHeaderPurge parses the 'Purge' header.
--- a/services/graph/pkg/service/v0/tracing.go
+++ b/services/graph/pkg/service/v0/tracing.go
@@ -5,7 +5,7 @@ import (
 )

 // NewTracing returns a service that instruments traces.
-func NewTracing(next Service) Service {
+func NewTracing(next Service) tracing {
 	return tracing{
 		next: next,
 	}
--- a/services/graph/pkg/service/v0/users_test.go
+++ b/services/graph/pkg/service/v0/users_test.go
@@ -69,7 +69,7 @@ var _ = Describe("Users", func() {
 		cfg.GRPCClientTLS = &shared.GRPCClientTLS{}

 		_ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...)
-		svc = service.NewService(
+		svc, _ = service.NewService(
 			service.Config(cfg),
 			service.WithGatewayClient(gatewayClient),
 			service.EventsPublisher(&eventsPublisher),