Repository: owncloud/ocis
Pull request #5369 (2023-01-10 11:42:54)

Collect global Envvars
Compose a list of envvars living in more than 1 service


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ff55427aa0a..aa6e0bec168 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,7 +15,6 @@ The following sections list the changes for unreleased.
 * Enhancement - Bump reva version: [#5243](https://github.com/owncloud/ocis/pull/5243)
 * Enhancement - Drive group permissions: [#5312](https://github.com/owncloud/ocis/pull/5312)
 * Enhancement - Make the group members addition limit configurable: [#5357](https://github.com/owncloud/ocis/pull/5357)
-* Enhancement - Graph Drives IdentitySet displayName: [#5347](https://github.com/owncloud/ocis/pull/5347)
 * Enhancement - Extended search: [#5221](https://github.com/owncloud/ocis/pull/5221)
 * Enhancement - Resource tags: [#5227](https://github.com/owncloud/ocis/pull/5227)
 
diff --git a/changelog/unreleased/collect-global-envvars.md b/changelog/unreleased/collect-global-envvars.md
new file mode 100644
index 00000000000..7dd08f6889a
--- /dev/null
+++ b/changelog/unreleased/collect-global-envvars.md
@@ -0,0 +1,5 @@
+Enhancement: Collect global envvars
+
+Compose a list of all envvars living in more than 1 service
+
+https://github.com/owncloud/ocis/pull/5367
diff --git a/changelog/unreleased/enhancement-identity-set-display-name.md b/changelog/unreleased/enhancement-identity-set-display-name.md
deleted file mode 100644
index 6c9c13ddfe7..00000000000
--- a/changelog/unreleased/enhancement-identity-set-display-name.md
+++ /dev/null
@@ -1,7 +0,0 @@
-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
diff --git a/docs/helpers/adoc-generator.go.tmpl b/docs/helpers/adoc-generator.go.tmpl
index 0ed23f86e53..977363ad152 100644
--- a/docs/helpers/adoc-generator.go.tmpl
+++ b/docs/helpers/adoc-generator.go.tmpl
@@ -9,11 +9,14 @@ import (
 	"regexp"
 	"strings"
 	"text/template"
+	"sort"
 
     {{- range $key, $value := .}}
     pkg{{$key}} "{{$value}}"
-    {{- end}})
+    {{- end}}
+)
 
+// ConfigField is the representation of one configuration field
 type ConfigField struct {
 	EnvVars         []string
 	DefaultValue    string
@@ -23,6 +26,7 @@ type ConfigField struct {
 	DeprecationLink string
 }
 
+// DeprecationField holds information about deprecation
 type DeprecationField struct {
 	DeprecationVersion     string
 	DeprecationInfo        string
@@ -30,6 +34,15 @@ type DeprecationField struct {
 	RemovalVersion         string
 }
 
+// EnvVar holds information about one envvar
+type EnvVar struct {
+    Name 	 string
+    DefaultValue string
+    Type 	 string
+    Description  string
+    Services 	 []string
+}
+
 type templateData struct {
 	ExtensionName   string
 	Fields          []ConfigField
@@ -38,44 +51,66 @@ type templateData struct {
 }
 
 func main() {
-fmt.Println("Generating adoc documentation for environment variables:")
-content, err := os.ReadFile("../../docs/templates/ADOC.tmpl")
-if err != nil {
-    log.Fatal(err)
-}
-replacer := strings.NewReplacer(
-		"github.com/owncloud/ocis/v2/services/", "",
-		"/pkg/config/defaults", "",
-	)
-var fields []ConfigField
-var deprecations []DeprecationField
-var targetFile *os.File
-tpl := template.Must(template.New("").Parse(string(content)))
-
-m := map[string]interface{}{
-{{- range $key, $value := .}}
-	"{{$value}}": *pkg{{$key}}.FullDefaultConfig(),
-{{- end }}
-}
+	fmt.Println("Generating adoc documentation for environment variables:")
+	content, err := os.ReadFile("../../docs/templates/ADOC.tmpl")
+	if err != nil {
+	    log.Fatal(err)
+	}
+	replacer := strings.NewReplacer(
+			"github.com/owncloud/ocis/v2/services/", "",
+			"/pkg/config/defaults", "",
+		)
+	var fields []ConfigField
+	var deprecations []DeprecationField
+	var targetFile *os.File
+	tpl := template.Must(template.New("").Parse(string(content)))
 
-    targetFolder := "../../docs/services/_includes/adoc/"
-    for pkg, conf := range m {
-	fields, deprecations = GetAnnotatedVariables(conf)
-	var hasDeprecations bool
-	if len(deprecations) > 0 {
-		hasDeprecations = true
+	m := map[string]interface{}{
+	{{- range $key, $value := .}}
+		"{{$value}}": *pkg{{$key}}.FullDefaultConfig(),
+	{{- end }}
 	}
 
-	if len(fields) > 0 || len(deprecations) > 0 {
-	fmt.Printf("... %s\n", pkg)
-		targetFile, err = os.Create(filepath.Join(targetFolder, replacer.Replace(pkg) + "_configvars.adoc"))
+	targetFolder := "../../docs/services/_includes/adoc/"
+	all := make(map[string]EnvVar)
+	for pkg, conf := range m {
+	    service := replacer.Replace(pkg)
+	    fields, deprecations = GetAnnotatedVariables(conf)
+	    var hasDeprecations bool
+	    if len(deprecations) > 0 {
+		    hasDeprecations = true
+	    }
+
+	    for _, f := range fields {
+		for _, e := range f.EnvVars {
+		    if env, ok := all[e]; ok {
+			env.Services = append(env.Services, service)
+			sort.Slice(env.Services, func(i, j int) bool {
+			    return env.Services[i] < env.Services[j]
+			})
+			all[e] = env
+		    } else {
+			all[e] = EnvVar{
+			    Name:         e,
+			    Description:  f.Description,
+			    Type: 	  f.Type,
+			    DefaultValue: f.DefaultValue,
+			    Services:     []string{service},
+			}
+		    }
+		}
+	    }
+
+	    if len(fields) > 0 || len(deprecations) > 0 {
+		fmt.Printf("... %s\n", pkg)
+		targetFile, err = os.Create(filepath.Join(targetFolder, service + "_configvars.adoc"))
 		if err != nil {
 			log.Fatalf("Failed to create target file: %s", err)
 		}
 		defer targetFile.Close()
 
 		td := templateData{
-			ExtensionName: replacer.Replace(pkg),
+			ExtensionName: service,
 			Fields:        fields,
 			Deprecations: deprecations,
 			HasDeprecations: hasDeprecations ,
@@ -83,8 +118,43 @@ m := map[string]interface{}{
 		if err := tpl.Execute(targetFile, td); err != nil {
 			log.Fatalf("Failed to execute template: %s", err)
 		}
+	    }
 	}
-    }
+
+	// render global env vars
+	tmplValues := make([]map[string]interface{}, 0)
+	for _, env := range all {
+	    if len(env.Services) > 1 {
+		tmplValues = append(tmplValues, map[string]interface{}{
+			"Name":         env.Name,
+			"Services":     env.Services,
+			"Description":  env.Description,
+			"DefaultValue": env.DefaultValue,
+			"Type":         env.Type,
+		})	    
+	    }
+	}
+
+	// sort
+	sort.Slice(tmplValues, func(i, j int) bool {
+		return tmplValues[i]["Name"].(string) < tmplValues[j]["Name"].(string)
+	})
+
+	glc, err := os.ReadFile("../../docs/templates/ADOC_global.tmpl")
+	if err != nil {
+	    log.Fatal(err)
+	}
+
+	gltpl := template.Must(template.New("").Parse(string(glc)))
+	glfile, err := os.Create(filepath.Join(targetFolder, "global_configvars.adoc"))
+	if err != nil {
+		log.Fatalf("Failed to create target file: %s", err)
+	}
+
+	if err := gltpl.Execute(glfile, tmplValues); err != nil {
+	    log.Printf("Failed to execute template: %s", err)
+	}
+
 	fmt.Println("done")
 }
 
diff --git a/docs/templates/ADOC_global.tmpl b/docs/templates/ADOC_global.tmpl
new file mode 100644
index 00000000000..cbc43b850cd
--- /dev/null
+++ b/docs/templates/ADOC_global.tmpl
@@ -0,0 +1,33 @@
+// collected through docs/helpers/adoc-generator.go.tmpl
+
+[.landscape]
+[caption=]
+.Environment variables with global scope available in multiple services
+[width="100%",cols="30%,25%,~,~,~",options="header"]
+|===
+| Name
+| Services
+| Type
+| Default Value
+| Description
+
+{{ range . }}
+
+a| `{{ .Name }}`
+
+a| [subs=attributes+]
+{{- range .Services}}
+* xref:{s-path}/{{ . }}.adoc[{{ . }}] +
+{{- end }}
+
+a| [subs=-attributes]
+++{{ .Type }} ++
+
+a| [subs=-attributes]
+++{{ .DefaultValue }} ++
+
+a| [subs=-attributes]
+{{ .Description }}
+
+{{- end }}
+|===
diff --git a/go.mod b/go.mod
index 429b843e4b7..3778861b49d 100644
--- a/go.mod
+++ b/go.mod
@@ -42,7 +42,6 @@ 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
diff --git a/go.sum b/go.sum
index 4a009167dcb..c19a0e297a3 100644
--- a/go.sum
+++ b/go.sum
@@ -823,8 +823,6 @@ 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=
diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go
index b3618c72e96..5fa9843a319 100644
--- a/services/graph/pkg/config/config.go
+++ b/services/graph/pkg/config/config.go
@@ -37,8 +37,6 @@ 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 {
diff --git a/services/graph/pkg/config/defaults/defaultconfig.go b/services/graph/pkg/config/defaults/defaultconfig.go
index 1d06dad3fdb..5f4d15cdb67 100644
--- a/services/graph/pkg/config/defaults/defaultconfig.go
+++ b/services/graph/pkg/config/defaults/defaultconfig.go
@@ -3,7 +3,6 @@ package defaults
 import (
 	"path"
 	"strings"
-	"time"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/config/defaults"
 	"github.com/owncloud/ocis/v2/ocis-pkg/shared"
@@ -39,10 +38,6 @@ 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",
@@ -141,10 +136,4 @@ 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)
 }
diff --git a/services/graph/pkg/server/http/server.go b/services/graph/pkg/server/http/server.go
index 02a7f177a44..89070cd11ec 100644
--- a/services/graph/pkg/server/http/server.go
+++ b/services/graph/pkg/server/http/server.go
@@ -122,8 +122,7 @@ func Server(opts ...Option) (http.Service, error) {
 		// no gatewayclient needed
 	}
 
-	var handle svc.Service
-	handle, err = svc.NewService(
+	handle := svc.NewService(
 		svc.Logger(options.Logger),
 		svc.Config(options.Config),
 		svc.Middleware(middlewares...),
@@ -134,7 +133,7 @@ func Server(opts ...Option) (http.Service, error) {
 		svc.WithSearchService(searchsvc.NewSearchProviderService("com.owncloud.api.search", grpc.DefaultClient())),
 	)
 
-	if err != nil {
+	if handle == nil {
 		return http.Service{}, errors.New("could not initialize graph service")
 	}
 
diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go
index c106d2ed781..15379c68d12 100644
--- a/services/graph/pkg/service/v0/drives.go
+++ b/services/graph/pkg/service/v0/drives.go
@@ -23,7 +23,6 @@ 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"
@@ -576,44 +575,25 @@ 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 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()
-					}
-
-					identitySet = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp, DisplayName: group.GetDisplayName()}}
+				var identity libregraph.IdentitySet
+
+				if _, ok := groupsMap[id]; !ok {
+					identity = libregraph.IdentitySet{User: &libregraph.Identity{Id: &tmp}}
 				} else {
-					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()}}
+					identity = libregraph.IdentitySet{Group: &libregraph.Identity{Id: &tmp}}
 				}
 
 				// we need to map the permissions to the roles
 				switch {
 				// having RemoveGrant qualifies you as a manager
 				case perm.RemoveGrant:
-					managerIdentities = append(managerIdentities, identitySet)
+					managerIdentities = append(managerIdentities, identity)
 				// InitiateFileUpload means you are an editor
 				case perm.InitiateFileUpload:
-					editorIdentities = append(editorIdentities, identitySet)
+					editorIdentities = append(editorIdentities, identity)
 				// Stat permission at least makes you a viewer
 				case perm.Stat:
-					viewerIdentities = append(viewerIdentities, identitySet)
+					viewerIdentities = append(viewerIdentities, identity)
 				}
 			}
 
diff --git a/services/graph/pkg/service/v0/graph.go b/services/graph/pkg/service/v0/graph.go
index 6978317db7b..db97c56f8c2 100644
--- a/services/graph/pkg/service/v0/graph.go
+++ b/services/graph/pkg/service/v0/graph.go
@@ -12,8 +12,7 @@ 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/v3"
-	libregraph "github.com/owncloud/libre-graph-api-go"
+	"github.com/jellydator/ttlcache/v2"
 	"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"
@@ -101,9 +100,7 @@ type Graph struct {
 	gatewayClient            GatewayClient
 	roleService              RoleService
 	permissionsService       Permissions
-	spacePropertiesCache     *ttlcache.Cache[string, interface{}]
-	usersCache               *ttlcache.Cache[string, libregraph.User]
-	groupsCache              *ttlcache.Cache[string, libregraph.Group]
+	spacePropertiesCache     *ttlcache.Cache
 	eventsPublisher          events.Publisher
 	searchService            searchsvc.SearchProviderService
 }
diff --git a/services/graph/pkg/service/v0/instrument.go b/services/graph/pkg/service/v0/instrument.go
index eafb980fef9..c1567cd4048 100644
--- 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) instrument {
+func NewInstrument(next Service, metrics *metrics.Metrics) Service {
 	return instrument{
 		next:    next,
 		metrics: metrics,
diff --git a/services/graph/pkg/service/v0/logging.go b/services/graph/pkg/service/v0/logging.go
index 1e3369b6ce1..b5c832b7969 100644
--- 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) logging {
+func NewLogging(next Service, logger log.Logger) Service {
 	return logging{
 		next:   next,
 		logger: logger,
diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go
index 98ce2d1a27a..a4858d0f22a 100644
--- a/services/graph/pkg/service/v0/service.go
+++ b/services/graph/pkg/service/v0/service.go
@@ -3,16 +3,14 @@ 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/v3"
-	libregraph "github.com/owncloud/libre-graph-api-go"
+	"github.com/jellydator/ttlcache/v2"
+
 	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"
@@ -78,40 +76,17 @@ type Service interface {
 }
 
 // NewService returns a service implementation for Service.
-func NewService(opts ...Option) (Graph, error) {
+func NewService(opts ...Option) Service {
 	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:     spacePropertiesCache,
-		usersCache:               usersCache,
-		groupsCache:              groupsCache,
+		spacePropertiesCache:     ttlcache.NewCache(),
 		eventsPublisher:          options.EventsPublisher,
 		gatewayClient:            options.GatewayClient,
 		searchService:            options.SearchService,
@@ -154,11 +129,11 @@ func NewService(opts ...Option) (Graph, error) {
 				pemData, err := os.ReadFile(options.Config.Identity.LDAP.CACert)
 				if err != nil {
 					options.Logger.Error().Err(err).Msgf("Error initializing LDAP Backend")
-					return svc, err
+					return nil
 				}
 				if !certs.AppendCertsFromPEM(pemData) {
 					options.Logger.Error().Msgf("Error initializing LDAP Backend. Adding CA cert failed")
-					return svc, err
+					return nil
 				}
 				tlsConf.RootCAs = certs
 			}
@@ -174,7 +149,7 @@ func NewService(opts ...Option) (Graph, error) {
 			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 svc, err
+				return nil
 			}
 			svc.identityBackend = lb
 			if options.IdentityEducationBackend == nil {
@@ -186,9 +161,8 @@ func NewService(opts ...Option) (Graph, error) {
 				}
 			}
 		default:
-			err := fmt.Errorf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend)
-			options.Logger.Err(err)
-			return svc, err
+			options.Logger.Error().Msgf("Unknown Identity Backend: '%s'", options.Config.Identity.Backend)
+			return nil
 		}
 	} else {
 		svc.identityBackend = options.IdentityBackend
@@ -310,7 +284,7 @@ func NewService(opts ...Option) (Graph, error) {
 		})
 	})
 
-	return svc, nil
+	return svc
 }
 
 // parseHeaderPurge parses the 'Purge' header.
diff --git a/services/graph/pkg/service/v0/tracing.go b/services/graph/pkg/service/v0/tracing.go
index 160289f7416..99daa0cb2ca 100644
--- 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) tracing {
+func NewTracing(next Service) Service {
 	return tracing{
 		next: next,
 	}


Test patch:
diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go
index 2feb50ae54b..c29952ee0c9 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/educationschools_test.go b/services/graph/pkg/service/v0/educationschools_test.go
index 262f736863f..b94878cc394 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/educationuser_test.go b/services/graph/pkg/service/v0/educationuser_test.go
index adc45de6e19..98ee06809bd 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go
index 9a5cc34c6e7..f5cf009092f 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go
index 10c341299eb..d5ada43d196 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/password_test.go b/services/graph/pkg/service/v0/password_test.go
index 0184b5e173c..51b272804a4 100644
--- 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),
diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go
index 070f547d816..ddc98a3ab52 100644
--- 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),
diff --git a/tests/acceptance/features/apiSpacesShares/shareSpaces.feature b/tests/acceptance/features/apiSpacesShares/shareSpaces.feature
index 75b03955c7f..035871cf486 100644
--- a/tests/acceptance/features/apiSpacesShares/shareSpaces.feature
+++ b/tests/acceptance/features/apiSpacesShares/shareSpaces.feature
@@ -60,7 +60,7 @@ Feature: Share spaces
       | name      | share space |
     When user "Alice" unshares a space "share space" to user "Brian"
     Then the HTTP status code should be "200"
-    And the user "Brian" should not have a space called "share space"
+    Then the user "Brian" should not have a space called "share space"
 
 
   Scenario Outline: Owner of a space cannot see the space after removing his access to the space
@@ -188,44 +188,3 @@ Feature: Share spaces
       | manager |
       | editor  |
       | viewer  |
-
-
-  Scenario Outline: The user has no access to the space if access for the group has been removed
-    Given group "group2" has been created
-    And the administrator has added a user "Brian" to the group "group2" using GraphApi
-    And user "Alice" has shared a space "share space" to group "group2" with role "<role>"
-    When user "Alice" unshares a space "share space" to group "group2"
-    Then the HTTP status code should be "200"
-    And the user "Brian" should not have a space called "share space"
-    Examples:
-      | role    |
-      | manager |
-      | editor  |
-      | viewer  |
-
-
-  Scenario: The user has no access to the space if he has been removed from the group
-    Given group "group2" has been created
-    And the administrator has added a user "Brian" to the group "group2" using GraphApi
-    And the administrator has added a user "Bob" to the group "group2" using GraphApi
-    And user "Alice" has shared a space "share space" to group "group2" with role "editor"
-    When the administrator removes the following users from the following groups using the Graph API
-      | username | groupname |
-      | Brian    | group2    |
-    Then the HTTP status code of responses on all endpoints should be "204"
-    And the user "Brian" should not have a space called "share space"
-    But the user "Bob" should have a space called "share space" with these key and value pairs:
-      | key       | value       |
-      | driveType | project     |
-      | name      | share space |
-
-
-  Scenario: Users don't have access to the space if the group has been deleted
-    Given group "group2" has been created
-    And the administrator has added a user "Brian" to the group "group2" using GraphApi
-    And the administrator has added a user "Bob" to the group "group2" using GraphApi
-    And user "Alice" has shared a space "share space" to group "group2" with role "editor"
-    When the administrator deletes group "group2" using the Graph API
-    Then the HTTP status code should be "204"
-    And the user "Brian" should not have a space called "share space"
-    And the user "Bob" should not have a space called "share space"
\ No newline at end of file
diff --git a/tests/acceptance/features/bootstrap/GraphContext.php b/tests/acceptance/features/bootstrap/GraphContext.php
index 4d48836667d..92b857452fd 100644
--- a/tests/acceptance/features/bootstrap/GraphContext.php
+++ b/tests/acceptance/features/bootstrap/GraphContext.php
@@ -1021,7 +1021,7 @@ class GraphContext implements Context {
 	 *
 	 * @return void
 	 */
-	public function userDeletesGroupUsingTheGraphApi(string $group, ?string $user = null): void {
+	public function userDeletesGroupUsingTheGraphApi(string $group, ?string $user): void {
 		$groupId = $this->featureContext->getAttributeOfCreatedGroup($group, "id");
 		$response = $this->userDeletesGroupWithGroupId($groupId, $user);
 		$this->featureContext->setResponse($response);
diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php
index 7d1e0d7e7d5..c4d77784ee7 100644
--- a/tests/acceptance/features/bootstrap/SpacesContext.php
+++ b/tests/acceptance/features/bootstrap/SpacesContext.php
@@ -2040,11 +2040,11 @@ class SpacesContext implements Context {
 	}
 
 	/**
-	 * @Given /^user "([^"]*)" has shared a space "([^"]*)" to (?:user|group) "([^"]*)" with role "([^"]*)"$/
+	 * @Given /^user "([^"]*)" has shared a space "([^"]*)" to user "([^"]*)" with role "([^"]*)"$/
 	 *
 	 * @param  string $user
 	 * @param  string $spaceName
-	 * @param  string $recipient
+	 * @param  string $userRecipient
 	 * @param  string $role
 	 *
 	 * @return void
@@ -2053,10 +2053,10 @@ class SpacesContext implements Context {
 	public function userHasSharedSpace(
 		string $user,
 		string $spaceName,
-		string $recipient,
+		string $userRecipient,
 		string $role
 	): void {
-		$this->sendShareSpaceRequest($user, $spaceName, $recipient, $role);
+		$this->sendShareSpaceRequest($user, $spaceName, $userRecipient, $role);
 		$expectedHTTPStatus = "200";
 		$this->featureContext->theHTTPStatusCodeShouldBe(
 			$expectedHTTPStatus,
@@ -2067,11 +2067,11 @@ class SpacesContext implements Context {
 	}
 
 	/**
-	 * @When /^user "([^"]*)" unshares a space "([^"]*)" to (?:user|group) "([^"]*)"$/
+	 * @When /^user "([^"]*)" unshares a space "([^"]*)" to user "([^"]*)"$/
 	 *
 	 * @param  string $user
 	 * @param  string $spaceName
-	 * @param  string $recipient
+	 * @param  string $userRecipient
 	 *
 	 * @return void
 	 * @throws GuzzleException
@@ -2079,10 +2079,10 @@ class SpacesContext implements Context {
 	public function sendUnshareSpaceRequest(
 		string $user,
 		string $spaceName,
-		string $recipient
+		string $userRecipient
 	): void {
 		$space = $this->getSpaceByName($user, $spaceName);
-		$fullUrl = $this->baseUrl . $this->ocsApiUrl . "/" . $space['id'] . "?shareWith=" . $recipient;
+		$fullUrl = $this->baseUrl . $this->ocsApiUrl . "/" . $space['id'] . "?shareWith=" . $userRecipient;
 
 		$this->featureContext->setResponse(
 			HttpRequestHelper::delete(