diff --git a/graph/Makefile b/graph/Makefile index 720a6756f..88ecd177d 100644 --- a/graph/Makefile +++ b/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: diff --git a/graph/mocks/http_client.go b/graph/mocks/http_client.go new file mode 100644 index 000000000..d8e18327c --- /dev/null +++ b/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 +} diff --git a/graph/pkg/service/v0/driveitems.go b/graph/pkg/service/v0/driveitems.go index 69e371633..6341446b0 100644 --- a/graph/pkg/service/v0/driveitems.go +++ b/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, } diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 7ffa66c82..e233a9acd 100644 --- a/graph/pkg/service/v0/drives.go +++ b/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 diff --git a/graph/pkg/service/v0/graph.go b/graph/pkg/service/v0/graph.go index 7127c8278..3523dd8fb 100644 --- a/graph/pkg/service/v0/graph.go +++ b/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 { diff --git a/graph/pkg/service/v0/graph_test.go b/graph/pkg/service/v0/graph_test.go index 6bcf59159..ed90d6509 100644 --- a/graph/pkg/service/v0/graph_test.go +++ b/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" + } + ] + } + ] + } + `)) + }) }) }) diff --git a/graph/pkg/service/v0/option.go b/graph/pkg/service/v0/option.go index 49664dde6..a60edd85a 100644 --- a/graph/pkg/service/v0/option.go +++ b/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 } } diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 990a764e9..5d62e3cdb 100644 --- a/graph/pkg/service/v0/service.go +++ b/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) {