test reading from space.yaml

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

graph/Makefile

@@ -26,6 +26,7 @@ include ../.make/generate.mk
 .PHONY: ci-go-generate
 ci-go-generate: $(MOCKERY) # CI runs ci-node-generate automatically before this target
 	$(MOCKERY) --dir pkg/service/v0 --case underscore --name GatewayClient
+	$(MOCKERY) --dir pkg/service/v0 --case underscore --name HTTPClient
 
 .PHONY: ci-node-generate
 ci-node-generate:

graph/mocks/http_client.go

@@ -0,0 +1,37 @@
+// Code generated by mockery v2.9.4. DO NOT EDIT.
+
+package mocks
+
+import (
+	http "net/http"
+
+	mock "github.com/stretchr/testify/mock"
+)
+
+// HTTPClient is an autogenerated mock type for the HTTPClient type
+type HTTPClient struct {
+	mock.Mock
+}
+
+// Do provides a mock function with given fields: req
+func (_m *HTTPClient) Do(req *http.Request) (*http.Response, error) {
+	ret := _m.Called(req)
+
+	var r0 *http.Response
+	if rf, ok := ret.Get(0).(func(*http.Request) *http.Response); ok {
+		r0 = rf(req)
+	} else {
+		if ret.Get(0) != nil {
+			r0 = ret.Get(0).(*http.Response)
+		}
+	}
+
+	var r1 error
+	if rf, ok := ret.Get(1).(func(*http.Request) error); ok {
+		r1 = rf(req)
+	} else {
+		r1 = ret.Error(1)
+	}
+
+	return r0, r1
+}

graph/pkg/service/v0/driveitems.go

@@ -5,12 +5,12 @@ import (
 	"fmt"
 	"net/http"
 	"path"
-	"path/filepath"
 	"time"
 
 	cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
+	"github.com/cs3org/reva/pkg/utils"
 	"github.com/go-chi/render"
 	libregraph "github.com/owncloud/libre-graph-api-go"
 	"github.com/owncloud/ocis/graph/pkg/service/v0/errorcode"
@@ -21,7 +21,7 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) {
 	g.logger.Info().Msg("Calling GetRootDriveChildren")
 	ctx := r.Context()
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	res, err := client.GetHome(ctx, &storageprovider.GetHomeRequest{})
 	switch {
@@ -77,12 +77,12 @@ func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) {
 
 func (g Graph) getDriveItem(ctx context.Context, root *storageprovider.ResourceId, relativePath string) (*libregraph.DriveItem, error) {
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	ref := &storageprovider.Reference{
 		ResourceId: root,
 		// the path is always relative to the root of the drive, not the location of the .config/ocis/space.yaml file
-		Path: filepath.Join("./", relativePath),
+		Path: utils.MakeRelativePath(relativePath),
 	}
 	res, err := client.Stat(ctx, &storageprovider.StatRequest{Ref: ref})
 	if err != nil {
@@ -119,15 +119,19 @@ func cs3ResourceToDriveItem(res *storageprovider.ResourceInfo) (*libregraph.Driv
 
 	driveItem := &libregraph.DriveItem{
 		Id:   &res.Id.OpaqueId,
-		Name: &name,
-		ETag: &res.Etag,
 		Size: size,
 	}
+	if name != "" {
+		driveItem.Name = &name
+	}
+	if res.Etag != "" {
+		driveItem.ETag = &res.Etag
+	}
 	if res.Mtime != nil {
 		lastModified := cs3TimestampToTime(res.Mtime)
 		driveItem.LastModifiedDateTime = &lastModified
 	}
-	if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE {
+	if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE && res.MimeType != "" {
 		driveItem.File = &libregraph.OpenGraphFile{ // FIXME We cannot use libregraph.File here because the openapi codegenerator autodetects 'File' as a go type ...
 			MimeType: &res.MimeType,
 		}

graph/pkg/service/v0/drives.go

@@ -2,7 +2,6 @@ package svc
 
 import (
 	"context"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"math"
@@ -47,7 +46,7 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) {
 	g.logger.Info().Msg("Calling GetDrives")
 	ctx := r.Context()
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	permissions := make(map[string]struct{}, 1)
 	s := sproto.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient)
@@ -133,7 +132,7 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 	drive := libregraph.Drive{}
 	if err := json.NewDecoder(r.Body).Decode(&drive); err != nil {
 		errorcode.GeneralException.Render(w, r, http.StatusBadRequest, "invalid schema definition")
@@ -224,7 +223,7 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	updateSpaceRequest := &storageprovider.UpdateStorageSpaceRequest{
 		// Prepare the object to apply the diff from. The properties on StorageSpace will overwrite
@@ -397,7 +396,7 @@ func cs3StorageSpaceToDrive(baseURL *url.URL, space *storageprovider.StorageSpac
 }
 
 func (g Graph) getDriveQuota(ctx context.Context, space *storageprovider.StorageSpace) (*libregraph.Quota, error) {
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	req := &gateway.GetQuotaRequest{
 		Ref: &storageprovider.Reference{
@@ -472,7 +471,7 @@ func (g Graph) getExtendedSpaceProperties(ctx context.Context, space *storagepro
 		}
 	}
 
-	client := g.GetClient()
+	client := g.GetGatewayClient()
 
 	dlReq := &storageprovider.InitiateFileDownloadRequest{
 		Ref: &storageprovider.Reference{
@@ -531,10 +530,7 @@ func (g Graph) getExtendedSpaceProperties(ctx context.Context, space *storagepro
 	}
 	httpReq.Header.Set(headers.TokenTransportHeader, tk)
 
-	http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{
-		InsecureSkipVerify: g.config.Spaces.Insecure, //nolint:gosec
-	}
-	httpClient := &http.Client{}
+	httpClient := g.GetHTTPClient()
 
 	resp, err := httpClient.Do(httpReq) // nolint:bodyclose
 	if err != nil {
@@ -561,7 +557,19 @@ func (g Graph) getExtendedSpaceProperties(ctx context.Context, space *storagepro
 
 	spaceProperties := ExtendedSpaceProperties{}
 	if err := yaml.NewDecoder(resp.Body).Decode(&spaceProperties); err != nil {
-		return nil, err
+		g.logger.Debug().Err(err).Msg("invalid space yaml, ignoring")
+
+		// cache an empty instance
+		// TODO insert an 'invalid yaml' item? how can we return an error to the user?
+		spacePropertiesEntry := spacePropertiesEntry{
+			spaceProperties: ExtendedSpaceProperties{},
+			rootMtime:       space.Mtime,
+		}
+		if err := g.spacePropertiesCache.SetWithTTL(spaceRootStatKey(space.Root), spacePropertiesEntry, time.Second*time.Duration(g.config.Spaces.ExtendedSpacePropertiesCacheTTL)); err != nil {
+			g.logger.Error().Err(err).Msg("could not cache extended space properties")
+		}
+
+		return &spacePropertiesEntry.spaceProperties, nil
 	}
 
 	// cache properties

graph/pkg/service/v0/graph.go

@@ -16,7 +16,7 @@ import (
 
 //go:generate make generate
 
-// GatewayClient is the subset of the gateway.GatewayAPIClient that's being uses to interact with the gateway
+// GatewayClient is the subset of the gateway.GatewayAPIClient that is being used to interact with the gateway
 type GatewayClient interface {
 	//gateway.GatewayAPIClient
 
@@ -48,6 +48,11 @@ type GatewayClient interface {
 	GetQuota(ctx context.Context, in *gateway.GetQuotaRequest, opts ...grpc.CallOption) (*provider.GetQuotaResponse, error)
 }
 
+// HTTPClient is the subset of the http.Client that is being used to interact with the download gateway
+type HTTPClient interface {
+	Do(req *http.Request) (*http.Response, error)
+}
+
 // GetGatewayServiceClientFunc is a callback used to pass in a mock during testing
 type GetGatewayServiceClientFunc func() (GatewayClient, error)
 
@@ -57,7 +62,8 @@ type Graph struct {
 	mux                  *chi.Mux
 	logger               *log.Logger
 	identityBackend      identity.Backend
-	client               GatewayClient
+	gatewayClient        GatewayClient
+	httpClient           HTTPClient
 	spacePropertiesCache *ttlcache.Cache
 }
 
@@ -67,8 +73,13 @@ func (g Graph) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 }
 
 // GetClient returns a gateway client to talk to reva
-func (g Graph) GetClient() GatewayClient {
-	return g.client
+func (g Graph) GetGatewayClient() GatewayClient {
+	return g.gatewayClient
+}
+
+// GetClient returns a gateway client to talk to reva
+func (g Graph) GetHTTPClient() HTTPClient {
+	return g.httpClient
 }
 
 type listResponse struct {

graph/pkg/service/v0/graph_test.go

@@ -2,9 +2,13 @@ package svc_test
 
 import (
 	"context"
+	"fmt"
+	"io"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 
+	gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/pkg/rgrpc/status"
 	. "github.com/onsi/ginkgo"
@@ -13,21 +17,25 @@ import (
 	"github.com/owncloud/ocis/graph/pkg/config"
 	service "github.com/owncloud/ocis/graph/pkg/service/v0"
 	"github.com/stretchr/testify/mock"
+	"google.golang.org/grpc"
 )
 
 var _ = Describe("Graph", func() {
 	var (
-		svc    service.Service
-		client *mocks.GatewayClient
-		ctx    context.Context
+		svc           service.Service
+		gatewayClient *mocks.GatewayClient
+		httpClient    *mocks.HTTPClient
+		ctx           context.Context
 	)
 
 	JustBeforeEach(func() {
 		ctx = context.Background()
-		client = &mocks.GatewayClient{}
+		gatewayClient = &mocks.GatewayClient{}
+		httpClient = &mocks.HTTPClient{}
 		svc = service.NewService(
 			service.Config(config.DefaultConfig()),
-			service.GatewayServiceClient(client),
+			service.WithGatewayClient(gatewayClient),
+			service.WithHTTPClient(httpClient),
 		)
 	})
 
@@ -36,9 +44,10 @@ var _ = Describe("Graph", func() {
 			Expect(svc).ToNot(BeNil())
 		})
 	})
+
 	Describe("drive", func() {
 		It("can list an empty list of spaces", func() {
-			client.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{
+			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{
 				Status:        status.NewOK(ctx),
 				StorageSpaces: []*provider.StorageSpace{},
 			}, nil)
@@ -48,5 +57,177 @@ var _ = Describe("Graph", func() {
 			svc.GetDrives(rr, r)
 			Expect(rr.Code).To(Equal(http.StatusOK))
 		})
+
+		It("can list a space without owner", func() {
+			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{
+				Status: status.NewOK(ctx),
+				StorageSpaces: []*provider.StorageSpace{
+					{
+						Id:        &provider.StorageSpaceId{OpaqueId: "aspaceid"},
+						SpaceType: "aspacetype",
+						Root: &provider.ResourceId{
+							StorageId: "aspaceid",
+							OpaqueId:  "anopaqueid",
+						},
+						Name: "aspacename",
+					},
+				},
+			}, nil)
+			gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{
+				Status: status.NewNotFound(ctx, "not found"),
+			}, nil)
+			gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{
+				Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"),
+			}, nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives", nil)
+			rr := httptest.NewRecorder()
+			svc.GetDrives(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusOK))
+
+			body, _ := io.ReadAll(rr.Body)
+			Expect(body).To(MatchJSON(`
+			{
+				"value":[
+					{
+						"driveType":"aspacetype",
+						"id":"aspaceid!anopaqueid",
+						"name":"aspacename",
+						"root":{
+							"id":"aspaceid!anopaqueid",
+							"webDavUrl":"https://localhost:9200/dav/spaces/aspaceid!anopaqueid"
+						}
+					}
+				]
+			}		
+			`))
+		})
+
+		It("can list a space with extended properties from a space.yaml", func() {
+			gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{
+				Status: status.NewOK(ctx),
+				StorageSpaces: []*provider.StorageSpace{
+					{
+						Id:        &provider.StorageSpaceId{OpaqueId: "aspaceid"},
+						SpaceType: "aspacetype",
+						Root: &provider.ResourceId{
+							StorageId: "aspaceid",
+							OpaqueId:  "anopaqueid",
+						},
+						Name: "aspacename",
+					},
+				},
+			}, nil)
+			gatewayClient.On("InitiateFileDownload", mock.Anything, mock.Anything).Return(&gateway.InitiateFileDownloadResponse{
+				Status: status.NewOK(ctx),
+				Protocols: []*gateway.FileDownloadProtocol{
+					{
+						Protocol:         "spaces",
+						DownloadEndpoint: "ignored",
+					},
+				},
+			}, nil)
+			// mock space.yaml
+			httpClient.On("Do", mock.Anything, mock.Anything).Return(&http.Response{
+				StatusCode: http.StatusOK,
+				Body: io.NopCloser(strings.NewReader(`---
+version: "1.0"
+description: read from yaml
+special:
+  readme: readme2.md
+  image: .img/space.png
+`)),
+			}, nil)
+			gatewayClient.On("GetQuota", mock.Anything, mock.Anything).Return(&provider.GetQuotaResponse{
+				Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"),
+			}, nil)
+			gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(
+				func(_ context.Context, req *provider.StatRequest, _ ...grpc.CallOption) *provider.StatResponse {
+					switch req.Ref.GetPath() {
+					case "./readme2.md":
+						return &provider.StatResponse{
+							Status: status.NewOK(ctx),
+							Info: &provider.ResourceInfo{
+								Type: provider.ResourceType_RESOURCE_TYPE_FILE,
+								Path: "readme2.md",
+								Id: &provider.ResourceId{
+									StorageId: "aspaceid",
+									OpaqueId:  "readmeid",
+								},
+								PermissionSet: &provider.ResourcePermissions{
+									Stat: true,
+								},
+								Size: 10,
+							},
+						}
+					case "./.img/space.png":
+						return &provider.StatResponse{
+							Status: status.NewOK(ctx),
+							Info: &provider.ResourceInfo{
+								Type: provider.ResourceType_RESOURCE_TYPE_FILE,
+								Path: "space.png",
+								Id: &provider.ResourceId{
+									StorageId: "aspaceid",
+									OpaqueId:  "imageid",
+								},
+								PermissionSet: &provider.ResourcePermissions{
+									Stat: true,
+								},
+								Size: 20,
+							},
+						}
+					default:
+						return &provider.StatResponse{
+							Status: status.NewNotFound(ctx, "not found"),
+						}
+					}
+				},
+				nil)
+
+			r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives", nil)
+			rr := httptest.NewRecorder()
+			svc.GetDrives(rr, r)
+
+			Expect(rr.Code).To(Equal(http.StatusOK))
+
+			body, _ := io.ReadAll(rr.Body)
+			Expect(body).To(MatchJSON(`
+			{
+				"value":[
+					{
+						"driveType":"aspacetype",
+						"id":"aspaceid!anopaqueid",
+						"name":"aspacename",
+						"description":"read from yaml",
+						"root":{
+							"id":"aspaceid!anopaqueid",
+							"webDavUrl":"https://localhost:9200/dav/spaces/aspaceid!anopaqueid"
+						},
+						"special": [
+						  {
+							"id": "readmeid",
+							"name": "readme2.md",
+							"size": 10,
+							"specialFolder": {
+							  "name": "readme"
+							},
+							"webDavUrl": "https://localhost:9200/dav/spaces/aspaceid/readme2.md"
+						  },
+						  {
+							"id": "imageid",
+							"name": "space.png",
+							"size": 20,
+							"specialFolder": {
+							  "name": "image"
+							},
+							"webDavUrl": "https://localhost:9200/dav/spaces/aspaceid/.img/space.png"
+						  }
+						]
+					}
+				]
+			}		
+			`))
+		})
 	})
 })

graph/pkg/service/v0/option.go

@@ -12,10 +12,11 @@ type Option func(o *Options)
 
 // Options defines the available options for this package.
 type Options struct {
-	Logger               log.Logger
-	Config               *config.Config
-	Middleware           []func(http.Handler) http.Handler
-	GatewayServiceClient GatewayClient
+	Logger        log.Logger
+	Config        *config.Config
+	Middleware    []func(http.Handler) http.Handler
+	GatewayClient GatewayClient
+	HTTPClient    HTTPClient
 }
 
 // newOptions initializes the available default options.
@@ -50,9 +51,16 @@ func Middleware(val ...func(http.Handler) http.Handler) Option {
 	}
 }
 
-// GatewayServiceClient provides a function to set the middleware option.
-func GatewayServiceClient(val GatewayClient) Option {
+// WithGatewayClient provides a function to set the gateway client option.
+func WithGatewayClient(val GatewayClient) Option {
 	return func(o *Options) {
-		o.GatewayServiceClient = val
+		o.GatewayClient = val
+	}
+}
+
+// WithHTTPClient provides a function to set the http client option.
+func WithHTTPClient(val HTTPClient) Option {
+	return func(o *Options) {
+		o.HTTPClient = val
 	}
 }

graph/pkg/service/v0/service.go

@@ -1,6 +1,7 @@
 package svc
 
 import (
+	"crypto/tls"
 	"net/http"
 
 	"github.com/ReneKroon/ttlcache/v2"
@@ -64,15 +65,23 @@ func NewService(opts ...Option) Service {
 		identityBackend:      backend,
 		spacePropertiesCache: ttlcache.NewCache(),
 	}
-	if options.GatewayServiceClient == nil {
+	if options.GatewayClient == nil {
 		var err error
-		svc.client, err = pool.GetGatewayServiceClient(options.Config.Reva.Address)
+		svc.gatewayClient, err = pool.GetGatewayServiceClient(options.Config.Reva.Address)
 		if err != nil {
 			options.Logger.Error().Err(err).Msg("Could not get gateway client")
 			return nil
 		}
 	} else {
-		svc.client = options.GatewayServiceClient
+		svc.gatewayClient = options.GatewayClient
+	}
+	if options.HTTPClient == nil {
+		http.DefaultTransport.(*http.Transport).TLSClientConfig = &tls.Config{
+			InsecureSkipVerify: options.Config.Spaces.Insecure, //nolint:gosec
+		}
+		svc.httpClient = &http.Client{}
+	} else {
+		svc.httpClient = options.HTTPClient
 	}
 
 	m.Route(options.Config.HTTP.Root, func(r chi.Router) {