From bc9a05aef47a6b4bbc730c37fb1b8e1e279a1d19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 1 Dec 2022 08:23:35 +0100 Subject: [PATCH 1/3] Add GetRootDriveChildren to the service interface --- services/graph/pkg/service/v0/instrument.go | 5 +++++ services/graph/pkg/service/v0/logging.go | 5 +++++ services/graph/pkg/service/v0/service.go | 1 + services/graph/pkg/service/v0/tracing.go | 5 +++++ 4 files changed, 16 insertions(+) diff --git a/services/graph/pkg/service/v0/instrument.go b/services/graph/pkg/service/v0/instrument.go index 7b3faaf720..351d90aa65 100644 --- a/services/graph/pkg/service/v0/instrument.go +++ b/services/graph/pkg/service/v0/instrument.go @@ -128,3 +128,8 @@ func (i instrument) GetAllDrives(w http.ResponseWriter, r *http.Request) { func (i instrument) CreateDrive(w http.ResponseWriter, r *http.Request) { i.next.CreateDrive(w, r) } + +// GetRootDriveChildren implements the Service interface. +func (i instrument) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { + i.next.GetRootDriveChildren(w, r) +} diff --git a/services/graph/pkg/service/v0/logging.go b/services/graph/pkg/service/v0/logging.go index 645ccd0ef0..0beaf93b1b 100644 --- a/services/graph/pkg/service/v0/logging.go +++ b/services/graph/pkg/service/v0/logging.go @@ -128,3 +128,8 @@ func (l logging) GetAllDrives(w http.ResponseWriter, r *http.Request) { func (l logging) CreateDrive(w http.ResponseWriter, r *http.Request) { l.next.CreateDrive(w, r) } + +// GetRootDriveChildren implements the Service interface. +func (l logging) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { + l.next.GetRootDriveChildren(w, r) +} diff --git a/services/graph/pkg/service/v0/service.go b/services/graph/pkg/service/v0/service.go index 3fc75935c6..ac8f37b27f 100644 --- a/services/graph/pkg/service/v0/service.go +++ b/services/graph/pkg/service/v0/service.go @@ -50,6 +50,7 @@ type Service interface { GetDrives(w http.ResponseWriter, r *http.Request) GetSingleDrive(w http.ResponseWriter, r *http.Request) GetAllDrives(w http.ResponseWriter, r *http.Request) + GetRootDriveChildren(w http.ResponseWriter, r *http.Request) CreateDrive(w http.ResponseWriter, r *http.Request) UpdateDrive(w http.ResponseWriter, r *http.Request) DeleteDrive(w http.ResponseWriter, r *http.Request) diff --git a/services/graph/pkg/service/v0/tracing.go b/services/graph/pkg/service/v0/tracing.go index 404236a90c..8e38329794 100644 --- a/services/graph/pkg/service/v0/tracing.go +++ b/services/graph/pkg/service/v0/tracing.go @@ -124,3 +124,8 @@ func (t tracing) GetAllDrives(w http.ResponseWriter, r *http.Request) { func (t tracing) CreateDrive(w http.ResponseWriter, r *http.Request) { t.next.CreateDrive(w, r) } + +// GetRootDriveChildren implements the Service interface. +func (t tracing) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { + t.next.GetRootDriveChildren(w, r) +} From e87eff95e64dbdd24b11f97a87adb0ccd6753079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 1 Dec 2022 08:24:25 +0100 Subject: [PATCH 2/3] Make method which is only used internally private --- services/graph/pkg/service/v0/driveitems.go | 4 ++-- services/graph/pkg/service/v0/drives.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index b1c5c825de..58b87cf38b 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -215,8 +215,8 @@ func (g Graph) getPathForResource(ctx context.Context, id storageprovider.Resour return res.Path, err } -// GetExtendedSpaceProperties reads properties from the opaque and transforms them into driveItems -func (g Graph) GetExtendedSpaceProperties(ctx context.Context, baseURL *url.URL, space *storageprovider.StorageSpace) []libregraph.DriveItem { +// getExtendedSpaceProperties reads properties from the opaque and transforms them into driveItems +func (g Graph) getExtendedSpaceProperties(ctx context.Context, baseURL *url.URL, space *storageprovider.StorageSpace) []libregraph.DriveItem { var spaceItems []libregraph.DriveItem if space.Opaque == nil { return nil diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index f222bf7f69..7e06c66d38 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -469,7 +469,7 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, storageSpaces // can't access disabled space if utils.ReadPlainFromOpaque(storageSpace.Opaque, "trashed") != "trashed" { - res.Special = g.GetExtendedSpaceProperties(ctx, baseURL, storageSpace) + res.Special = g.getExtendedSpaceProperties(ctx, baseURL, storageSpace) res.Quota, err = g.getDriveQuota(ctx, storageSpace) if err != nil { return nil, err From d1958a0802d3c557552af5b61c59a9c696186059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Duffeck?= Date: Thu, 1 Dec 2022 08:24:46 +0100 Subject: [PATCH 3/3] Increase test coverage --- .../graph/pkg/service/v0/driveitems_test.go | 130 ++++++++++++++++++ services/graph/pkg/service/v0/graph_test.go | 63 +++++++++ services/graph/pkg/service/v0/groups_test.go | 1 + 3 files changed, 194 insertions(+) create mode 100644 services/graph/pkg/service/v0/driveitems_test.go diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go new file mode 100644 index 0000000000..5198477b74 --- /dev/null +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -0,0 +1,130 @@ +package svc_test + +import ( + "context" + "encoding/json" + "io/ioutil" + "net/http" + "net/http/httptest" + "time" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/v2/pkg/rgrpc/status" + "github.com/cs3org/reva/v2/pkg/utils" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" + + libregraph "github.com/owncloud/libre-graph-api-go" + ogrpc "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" + "github.com/owncloud/ocis/v2/ocis-pkg/shared" + "github.com/owncloud/ocis/v2/services/graph/mocks" + "github.com/owncloud/ocis/v2/services/graph/pkg/config" + "github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults" + identitymocks "github.com/owncloud/ocis/v2/services/graph/pkg/identity/mocks" + service "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0" +) + +type itemsList struct { + Value []*libregraph.DriveItem +} + +var _ = Describe("Driveitems", func() { + var ( + svc service.Service + ctx context.Context + cfg *config.Config + gatewayClient *mocks.GatewayClient + eventsPublisher mocks.Publisher + identityBackend *identitymocks.Backend + + rr *httptest.ResponseRecorder + + newGroup *libregraph.Group + ) + + BeforeEach(func() { + eventsPublisher.On("Publish", mock.Anything, mock.Anything, mock.Anything).Return(nil) + + rr = httptest.NewRecorder() + + identityBackend = &identitymocks.Backend{} + gatewayClient = &mocks.GatewayClient{} + newGroup = libregraph.NewGroup() + newGroup.SetMembersodataBind([]string{"/users/user1"}) + newGroup.SetId("group1") + + rr = httptest.NewRecorder() + ctx = context.Background() + + cfg = defaults.FullDefaultConfig() + cfg.Identity.LDAP.CACert = "" // skip the startup checks, we don't use LDAP at all in this tests + cfg.TokenManager.JWTSecret = "loremipsum" + cfg.Commons = &shared.Commons{} + cfg.GRPCClientTLS = &shared.GRPCClientTLS{} + + _ = ogrpc.Configure(ogrpc.GetClientOptions(cfg.GRPCClientTLS)...) + svc = service.NewService( + service.Config(cfg), + service.WithGatewayClient(gatewayClient), + service.EventsPublisher(&eventsPublisher), + service.WithIdentityBackend(identityBackend), + ) + }) + + Describe("GetRootDriveChildren", func() { + It("handles failing GetHome", func() { + gatewayClient.On("GetHome", mock.Anything, mock.Anything).Return(&provider.GetHomeResponse{ + Status: status.NewNotFound(ctx, "not found"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drive/root/children", nil) + svc.GetRootDriveChildren(rr, r) + Expect(rr.Code).To(Equal(http.StatusNotFound)) + }) + + It("handles failing GetHome", func() { + gatewayClient.On("GetHome", mock.Anything, mock.Anything).Return(&provider.GetHomeResponse{ + Status: status.NewInternal(ctx, "not found"), + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drive/root/children", nil) + svc.GetRootDriveChildren(rr, r) + Expect(rr.Code).To(Equal(http.StatusInternalServerError)) + }) + + It("succeeds", func() { + mtime := time.Now() + gatewayClient.On("GetHome", mock.Anything, mock.Anything).Return(&provider.GetHomeResponse{ + Status: status.NewOK(ctx), + Path: "/", + }, nil) + gatewayClient.On("ListContainer", mock.Anything, mock.Anything).Return(&provider.ListContainerResponse{ + Status: status.NewOK(ctx), + Infos: []*provider.ResourceInfo{ + { + Type: provider.ResourceType_RESOURCE_TYPE_FILE, + Id: &provider.ResourceId{StorageId: "storageid", SpaceId: "spaceid", OpaqueId: "opaqueid"}, + Etag: "etag", + Mtime: utils.TimeToTS(mtime), + }, + }, + }, nil) + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drive/root/children", nil) + svc.GetRootDriveChildren(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + data, err := ioutil.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + res := itemsList{} + + err = json.Unmarshal(data, &res) + Expect(err).ToNot(HaveOccurred()) + + Expect(len(res.Value)).To(Equal(1)) + Expect(res.Value[0].GetLastModifiedDateTime().Equal(mtime)).To(BeTrue()) + Expect(res.Value[0].GetETag()).To(Equal("etag")) + Expect(res.Value[0].GetId()).To(Equal("storageid$spaceid!opaqueid")) + }) + }) +}) diff --git a/services/graph/pkg/service/v0/graph_test.go b/services/graph/pkg/service/v0/graph_test.go index 84b72c2ddb..a1987607d0 100644 --- a/services/graph/pkg/service/v0/graph_test.go +++ b/services/graph/pkg/service/v0/graph_test.go @@ -872,6 +872,69 @@ var _ = Describe("Graph", func() { Expect(err).ToNot(HaveOccurred()) Expect(*drive.GetQuota().Total).To(Equal(int64(500))) }) + + It("gets the special drive items", func() { + gatewayClient.On("GetPath", mock.Anything, mock.Anything).Return(&provider.GetPathResponse{ + Status: status.NewOK(ctx), + Path: "thepath", + }, nil) + gatewayClient.On("Stat", mock.Anything, mock.Anything).Return(&provider.StatResponse{ + Status: status.NewOK(ctx), + Info: &provider.ResourceInfo{ + Id: &provider.ResourceId{ + StorageId: "pro-1", + SpaceId: "spaceID", + OpaqueId: "specialID", + }, + }, + }, nil) + gatewayClient.On("ListStorageSpaces", + mock.Anything, + mock.MatchedBy( + func(req *provider.ListStorageSpacesRequest) bool { + return len(req.Filters) == 1 && req.Filters[0].Term.(*provider.ListStorageSpacesRequest_Filter_Id).Id.OpaqueId == "spaceid" + })). + Return(&provider.ListStorageSpacesResponse{ + Status: status.NewOK(ctx), + StorageSpaces: []*provider.StorageSpace{ + { + Opaque: &typesv1beta1.Opaque{ + Map: map[string]*typesv1beta1.OpaqueEntry{ + service.ReadmeSpecialFolderName: { + Decoder: "plain", + Value: []byte("readme"), + }, + }, + }, + Id: &provider.StorageSpaceId{OpaqueId: "spaceid"}, + SpaceType: "aspacetype", + Root: &provider.ResourceId{ + StorageId: "pro-1", + SpaceId: "sameID", + OpaqueId: "sameID", + }, + Name: "aspacename", + }, + }, + }, nil) + + r := httptest.NewRequest(http.MethodGet, "/graph/v1.0/me/drives/{driveID}/", nil) + rctx := chi.NewRouteContext() + rctx.URLParams.Add("driveID", "spaceid") + r = r.WithContext(context.WithValue(ctxpkg.ContextSetUser(ctx, nil), chi.RouteCtxKey, rctx)) + svc.GetSingleDrive(rr, r) + Expect(rr.Code).To(Equal(http.StatusOK)) + + data, err := ioutil.ReadAll(rr.Body) + Expect(err).ToNot(HaveOccurred()) + + drive := libregraph.Drive{} + err = json.Unmarshal(data, &drive) + Expect(err).ToNot(HaveOccurred()) + Expect(*drive.GetQuota().Total).To(Equal(int64(500))) + Expect(len(drive.GetSpecial())).To(Equal(1)) + Expect(drive.GetSpecial()[0].GetId()).To(Equal("pro-1$spaceID!specialID")) + }) }) Describe("Update a drive", func() { diff --git a/services/graph/pkg/service/v0/groups_test.go b/services/graph/pkg/service/v0/groups_test.go index 84d5b5a833..6c91074afb 100644 --- a/services/graph/pkg/service/v0/groups_test.go +++ b/services/graph/pkg/service/v0/groups_test.go @@ -54,6 +54,7 @@ var _ = Describe("Groups", func() { eventsPublisher.On("Publish", mock.Anything, mock.Anything, mock.Anything).Return(nil) identityBackend = &identitymocks.Backend{} + gatewayClient = &mocks.GatewayClient{} newGroup = libregraph.NewGroup() newGroup.SetMembersodataBind([]string{"/users/user1"}) newGroup.SetId("group1")