From 85fd0158ddc0f36f9dfb87cd4d9527898fbf2101 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 18 Jan 2022 12:01:29 +0100 Subject: [PATCH 1/7] add GetSingleDrive handler --- graph/pkg/service/v0/drives.go | 234 ++++++++++++++++++++++++++++---- graph/pkg/service/v0/service.go | 1 + 2 files changed, 210 insertions(+), 25 deletions(-) diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 0d15adc48df..370da01ae1d 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -7,6 +7,7 @@ import ( "math" "net/http" "net/url" + "path" "path/filepath" "strconv" "strings" @@ -43,42 +44,82 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return } - g.logger.Info().Msg("Calling GetDrives") + g.logger.Info().Interface("query", r.URL.Query()).Msg("Calling GetDrives") ctx := r.Context() - client := g.GetGatewayClient() + filters, err := generateCs3Filters(odataReq) + if err != nil { + g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") + errorcode.NotSupported.Render(w, r, http.StatusNotImplemented, err.Error()) + return + } + res, err := g.ListStorageSpacesWithFilters(ctx, filters) + switch { + case err != nil: + g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return + case res.Status.Code != cs3rpc.Code_CODE_OK: + if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { + // return an empty list + render.Status(r, http.StatusOK) + render.JSON(w, r, &listResponse{}) + return + } + g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) + return + } - permissions := make(map[string]struct{}, 1) - s := sproto.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient) + wdu, err := url.Parse(g.config.Spaces.WebDavBase + g.config.Spaces.WebDavPath) + if err != nil { + g.logger.Error().Err(err).Msg("error parsing url") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return + } + files, err := g.formatDrives(ctx, wdu, res.StorageSpaces) + if err != nil { + g.logger.Error().Err(err).Msg("error encoding response as json") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return + } - _, err = s.GetPermissionByID(ctx, &sproto.GetPermissionByIDRequest{ - PermissionId: settingsSvc.ListAllSpacesPermissionID, - }) + render.Status(r, http.StatusOK) + render.JSON(w, r, &listResponse{Value: files}) +} - // No error means the user has the permission - if err == nil { - permissions[settingsSvc.ListAllSpacesPermissionName] = struct{}{} +// GetSingleDrive does a lookup of a single space by spaceId +func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { + sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") + driveId := chi.URLParam(r, "driveID") + if driveId == "" { + err := fmt.Errorf("no valid space id retrieved") + g.logger.Err(err) + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return } - value, err := json.Marshal(permissions) + // Add the driveId filter to the request query + spaceFilterValue := fmt.Sprintf("id eq %s", driveId) + query := r.URL.Query() + query.Add("$filter", spaceFilterValue) + // Parse the request with odata parser + odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, query) if err != nil { - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) return } + + g.logger.Info().Str("driveID", driveId).Msg("Calling GetSingleDrive") + ctx := r.Context() + filters, err := generateCs3Filters(odataReq) if err != nil { g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") errorcode.NotSupported.Render(w, r, http.StatusNotImplemented, err.Error()) return } - res, err := client.ListStorageSpaces(ctx, &storageprovider.ListStorageSpacesRequest{ - Opaque: &types.Opaque{Map: map[string]*types.OpaqueEntry{ - "permissions": { - Decoder: "json", - Value: value, - }, - }}, - Filters: filters, - }) + res, err := g.ListStorageSpacesWithFilters(ctx, filters) switch { case err != nil: g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") @@ -86,9 +127,8 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { return case res.Status.Code != cs3rpc.Code_CODE_OK: if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { - // return an empty list - render.Status(r, http.StatusOK) - render.JSON(w, r, &listResponse{}) + g.logger.Error().Str("driveId", driveId).Msg("no space found") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("no space found with id %s", driveId)) return } g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") @@ -102,7 +142,81 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } - files, err := g.formatDrives(ctx, wdu, res.StorageSpaces) + spaces, err := g.formatDrives(ctx, wdu, res.StorageSpaces) + if err != nil { + g.logger.Error().Err(err).Msg("error encoding response") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return + } + switch num := len(spaces); { + case num == 0: + g.logger.Error().Str("driveId", driveId).Msg("no space found") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("no space found with id %s", driveId)) + return + case num == 1: + render.Status(r, http.StatusOK) + render.JSON(w, r, spaces[0]) + default: + g.logger.Error().Int("number", num).Msg("an unexpected number of spaces was found") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "an unexpected number of spaces was found") + return + } +} + +// GetRootDriveChildren implements the Service interface. +func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { + g.logger.Info().Msg("Calling GetRootDriveChildren") + ctx := r.Context() + + client, err := g.GetClient() + if err != nil { + g.logger.Error().Err(err).Msg("could not get client") + errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) + return + } + + res, err := client.GetHome(ctx, &storageprovider.GetHomeRequest{}) + switch { + case err != nil: + g.logger.Error().Err(err).Msg("error sending get home grpc request") + errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) + return + case res.Status.Code != cs3rpc.Code_CODE_OK: + if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.Status.Message) + return + } + g.logger.Error().Err(err).Msg("error sending get home grpc request") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) + return + } + + lRes, err := client.ListContainer(ctx, &storageprovider.ListContainerRequest{ + Ref: &storageprovider.Reference{ + Path: res.Path, + }, + }) + switch { + case err != nil: + g.logger.Error().Err(err).Msg("error sending list container grpc request") + errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) + return + case res.Status.Code != cs3rpc.Code_CODE_OK: + if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.Status.Message) + return + } + if res.Status.Code == cs3rpc.Code_CODE_PERMISSION_DENIED { + // TODO check if we should return 404 to not disclose existing items + errorcode.AccessDenied.Render(w, r, http.StatusForbidden, res.Status.Message) + return + } + g.logger.Error().Err(err).Msg("error sending list container grpc request") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) + return + } + + files, err := formatDriveItems(lRes.Infos) if err != nil { g.logger.Error().Err(err).Msg("error encoding response as json") errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) @@ -331,6 +445,76 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, mds []*storag } // TODO this overwrites the quota that might already have been mapped in cs3StorageSpaceToDrive above ... move this into the cs3StorageSpaceToDrive method? res.Quota, err = g.getDriveQuota(ctx, space) +// ListStorageSpacesWithFilters List Storage Spaces using filters +func (g Graph) ListStorageSpacesWithFilters(ctx context.Context, filters []*storageprovider.ListStorageSpacesRequest_Filter) (*storageprovider.ListStorageSpacesResponse, error) { + client, err := g.GetClient() + if err != nil { + g.logger.Err(err).Msg("error getting grpc client") + return nil, err + } + + permissions := make(map[string]struct{}, 1) + s := sproto.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient) + + _, err = s.GetPermissionByID(ctx, &sproto.GetPermissionByIDRequest{ + PermissionId: settingsSvc.ListAllSpacesPermissionID, + }) + + // No error means the user has the permission + if err == nil { + permissions[settingsSvc.ListAllSpacesPermissionName] = struct{}{} + } + value, err := json.Marshal(permissions) + if err != nil { + return nil, err + } + + res, err := client.ListStorageSpaces(ctx, &storageprovider.ListStorageSpacesRequest{ + Opaque: &types.Opaque{Map: map[string]*types.OpaqueEntry{ + "permissions": { + Decoder: "json", + Value: value, + }, + }}, + Filters: filters, + }) + return res, nil +} + +func cs3TimestampToTime(t *types.Timestamp) time.Time { + return time.Unix(int64(t.Seconds), int64(t.Nanos)) +} + +func cs3ResourceToDriveItem(res *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) { + size := new(int64) + *size = int64(res.Size) // uint64 -> int :boom: + name := path.Base(res.Path) + + driveItem := &libregraph.DriveItem{ + Id: &res.Id.OpaqueId, + Name: &name, + ETag: &res.Etag, + Size: size, + } + if res.Mtime != nil { + lastModified := cs3TimestampToTime(res.Mtime) + driveItem.LastModifiedDateTime = &lastModified + } + if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE { + driveItem.File = &libregraph.OpenGraphFile{ // FIXME We cannot use libregraph.File here because the openapi codegenerator autodetects 'File' as a go type ... + MimeType: &res.MimeType, + } + } + if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER { + driveItem.Folder = &libregraph.Folder{} + } + return driveItem, nil +} + +func formatDriveItems(mds []*storageprovider.ResourceInfo) ([]*libregraph.DriveItem, error) { + responses := make([]*libregraph.DriveItem, 0, len(mds)) + for i := range mds { + res, err := cs3ResourceToDriveItem(mds[i]) if err != nil { return nil, err } diff --git a/graph/pkg/service/v0/service.go b/graph/pkg/service/v0/service.go index 5d62e3cdb74..529fec816b3 100644 --- a/graph/pkg/service/v0/service.go +++ b/graph/pkg/service/v0/service.go @@ -117,6 +117,7 @@ func NewService(opts ...Option) Service { r.Post("/", svc.CreateDrive) r.Route("/{driveID}", func(r chi.Router) { r.Patch("/", svc.UpdateDrive) + r.Get("/", svc.GetSingleDrive) }) }) }) From e10b524d7124ce1dab14a1774465bbe4e8729a75 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 18 Jan 2022 16:42:30 +0100 Subject: [PATCH 2/7] add Tests and Test Steps --- .../features/apiSpaces/listSpaces.feature | 32 ++++++++++ .../features/bootstrap/SpacesContext.php | 60 +++++++++++++++++-- 2 files changed, 88 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/features/apiSpaces/listSpaces.feature b/tests/acceptance/features/apiSpaces/listSpaces.feature index 175c562cc77..ac3afc65e90 100644 --- a/tests/acceptance/features/apiSpaces/listSpaces.feature +++ b/tests/acceptance/features/apiSpaces/listSpaces.feature @@ -79,3 +79,35 @@ Feature: List and create spaces | name | Project Venus | | quota@@@total | 2000 | | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + + Scenario: A user can list his personal space via multiple endpoints + When user "Alice" lists all available spaces via the GraphApi with query "$filter=driveType eq 'personal'" + Then the json responded should contain a space "Alice Hansen" with these key and value pairs: + | key | value | + | driveType | personal | + | name | Alice Hansen | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + When user "Alice" looks up the single space "Alice Hansen" via the GraphApi by using its id + Then the json responded should contain a space "Alice Hansen" with these key and value pairs: + | key | value | + | driveType | personal | + | name | Alice Hansen | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + + Scenario: A user can list his created spaces via multiple endpoints + Given the administrator has given "Alice" the role "Admin" using the settings api + When user "Alice" creates a space "Project Venus" of type "project" with quota "2000" using the GraphApi + Then the HTTP status code should be "201" + And the json responded should contain a space "Project Venus" with these key and value pairs: + | key | value | + | driveType | project | + | name | Project Venus | + | quota@@@total | 2000 | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | + When user "Alice" looks up the single space "Project Venus" via the GraphApi by using its id + Then the json responded should contain a space "Project Venus" with these key and value pairs: + | key | value | + | driveType | project | + | name | Project Venus | + | quota@@@total | 2000 | + | root@@@webDavUrl | %base_url%/dav/spaces/%space_id% | diff --git a/tests/acceptance/features/bootstrap/SpacesContext.php b/tests/acceptance/features/bootstrap/SpacesContext.php index c5d5a220ae5..b0e03b2ef8c 100644 --- a/tests/acceptance/features/bootstrap/SpacesContext.php +++ b/tests/acceptance/features/bootstrap/SpacesContext.php @@ -232,7 +232,7 @@ public function setUpScenario(BeforeScenarioScope $scope): void { } /** - * Send Graph List Spaces Request + * Send Graph List My Spaces Request * * @param string $user * @param string $password @@ -245,7 +245,7 @@ public function setUpScenario(BeforeScenarioScope $scope): void { * * @throws GuzzleException */ - public function listSpacesRequest( + public function listMySpacesRequest( string $user, string $password, string $urlArguments = '', @@ -258,6 +258,34 @@ public function listSpacesRequest( return HttpRequestHelper::get($fullUrl, $xRequestId, $user, $password, $headers, $body); } + /** + * Send Graph List Single Space Request + * + * @param string $user + * @param string $password + * @param string $urlArguments + * @param string $xRequestId + * @param array $body + * @param array $headers + * + * @return ResponseInterface + * + * @throws GuzzleException + */ + public function listSingleSpaceRequest( + string $user, + string $password, + string $spaceId, + string $urlArguments = '', + string $xRequestId = '', + array $body = [], + array $headers = [] + ): ResponseInterface { + $fullUrl = $this->baseUrl . "/graph/v1.0/drives/" . $spaceId . "/" . $urlArguments; + + return HttpRequestHelper::get($fullUrl, $xRequestId, $user, $password, $headers, $body); + } + /** * Send Graph Create Space Request * @@ -342,7 +370,7 @@ public function sendPutRequestToUrl( */ public function theUserListsAllHisAvailableSpacesUsingTheGraphApi(string $user): void { $this->featureContext->setResponse( - $this->listSpacesRequest( + $this->listMySpacesRequest( $user, $this->featureContext->getPasswordForUser($user) ) @@ -362,7 +390,7 @@ public function theUserListsAllHisAvailableSpacesUsingTheGraphApi(string $user): */ public function theUserListsAllHisAvailableSpacesUsingTheGraphApiWithFilter(string $user, string $query): void { $this->featureContext->setResponse( - $this->listSpacesRequest( + $this->listMySpacesRequest( $user, $this->featureContext->getPasswordForUser($user), "?". $query @@ -370,6 +398,30 @@ public function theUserListsAllHisAvailableSpacesUsingTheGraphApiWithFilter(stri ); } + /** + * @When /^user "([^"]*)" looks up the single space "([^"]*)" via the GraphApi by using its id$/ + * + * @param string $user + * @param string $query + * + * @return void + * + * @throws GuzzleException + */ + public function theUserLooksUpTheSingleSpaceUsingTheGraphApiByUsingItsId(string $user, string $spaceName): void { + $space = $this->getSpaceByName($user, $spaceName); + Assert::assertIsArray($space); + Assert::assertNotEmpty($spaceId = $space["id"]); + Assert::assertNotEmpty($spaceWebDavUrl = $space["root"]["webDavUrl"]); + $this->featureContext->setResponse( + $this->listSingleSpaceRequest( + $user, + $this->featureContext->getPasswordForUser($user), + $spaceId + ) + ); + } + /** * @When /^user "([^"]*)" creates a space "([^"]*)" of type "([^"]*)" with the default quota using the GraphApi$/ * From e0a4280de4c9c1ff196d3b615942169996582ffb Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Tue, 18 Jan 2022 16:45:14 +0100 Subject: [PATCH 3/7] add changelog --- changelog/unreleased/single-space-enpoint.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/unreleased/single-space-enpoint.md diff --git a/changelog/unreleased/single-space-enpoint.md b/changelog/unreleased/single-space-enpoint.md new file mode 100644 index 00000000000..c77e6a461ac --- /dev/null +++ b/changelog/unreleased/single-space-enpoint.md @@ -0,0 +1,5 @@ +Enhancement: Add endpoint to retrieve a single space + +We added the endpoint ``/drives/{driveID}`` to get a single space by id from the server. + +https://github.com/owncloud/ocis/pull/2978 From 5b9f31e8704c0489163d613d14aaa098dcfe11a0 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 19 Jan 2022 11:57:42 +0100 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Alex Unger <6905948+refs@users.noreply.github.com> --- graph/pkg/service/v0/drives.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 370da01ae1d..79204b17c35 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -127,8 +127,8 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { return case res.Status.Code != cs3rpc.Code_CODE_OK: if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { - g.logger.Error().Str("driveId", driveId).Msg("no space found") - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("no space found with id %s", driveId)) + g.logger.Error().Str("driveId", driveId).Msg(fmt.Sprintf("space with id `%s` not found", driveId)) + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("space with id `%s` not found", driveId)) return } g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") From 6b8a9f9127172eba6e830f57e529760a11fbe1cd Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 19 Jan 2022 12:07:02 +0100 Subject: [PATCH 5/7] create filter directly, improve logging --- graph/pkg/service/v0/drives.go | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 79204b17c35..4693a0d37c4 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -90,7 +90,6 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { // GetSingleDrive does a lookup of a single space by spaceId func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { - sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/") driveId := chi.URLParam(r, "driveID") if driveId == "" { err := fmt.Errorf("no valid space id retrieved") @@ -98,27 +97,20 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } - // Add the driveId filter to the request query - spaceFilterValue := fmt.Sprintf("id eq %s", driveId) - query := r.URL.Query() - query.Add("$filter", spaceFilterValue) - // Parse the request with odata parser - odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, query) - if err != nil { - g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") - errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error()) - return - } g.logger.Info().Str("driveID", driveId).Msg("Calling GetSingleDrive") ctx := r.Context() - filters, err := generateCs3Filters(odataReq) - if err != nil { - g.logger.Err(err).Interface("query", r.URL.Query()).Msg("query error") - errorcode.NotSupported.Render(w, r, http.StatusNotImplemented, err.Error()) - return + var filters []*storageprovider.ListStorageSpacesRequest_Filter + filterDriveId := &storageprovider.ListStorageSpacesRequest_Filter{ + Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_ID, + Term: &storageprovider.ListStorageSpacesRequest_Filter_Id{ + Id: &storageprovider.StorageSpaceId{ + OpaqueId: driveId, + }, + }, } + filters = append(filters, filterDriveId) res, err := g.ListStorageSpacesWithFilters(ctx, filters) switch { case err != nil: @@ -127,6 +119,8 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { return case res.Status.Code != cs3rpc.Code_CODE_OK: if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { + // the client is doing a lookup for a specific space, therefore we need to return + // not found to the caller g.logger.Error().Str("driveId", driveId).Msg(fmt.Sprintf("space with id `%s` not found", driveId)) errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("space with id `%s` not found", driveId)) return @@ -151,14 +145,14 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { switch num := len(spaces); { case num == 0: g.logger.Error().Str("driveId", driveId).Msg("no space found") - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("no space found with id %s", driveId)) + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("space with id `%s` not found", driveId)) return case num == 1: render.Status(r, http.StatusOK) render.JSON(w, r, spaces[0]) default: - g.logger.Error().Int("number", num).Msg("an unexpected number of spaces was found") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "an unexpected number of spaces was found") + g.logger.Error().Int("number", num).Msg("expected to find a single space but found more") + errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, "expected to find a single space but found more") return } } From 4d4dca512b762b89ba15577763192169793ce582 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 19 Jan 2022 12:30:23 +0100 Subject: [PATCH 6/7] merge master --- graph/pkg/service/v0/drives.go | 155 ++++++--------------------------- graph/pkg/service/v0/graph.go | 6 ++ 2 files changed, 32 insertions(+), 129 deletions(-) diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 4693a0d37c4..94b1f692780 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -7,7 +7,6 @@ import ( "math" "net/http" "net/url" - "path" "path/filepath" "strconv" "strings" @@ -56,7 +55,7 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { res, err := g.ListStorageSpacesWithFilters(ctx, filters) switch { case err != nil: - g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + g.logger.Error().Err(err).Msg(ListStorageSpacesTransportErr) errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return case res.Status.Code != cs3rpc.Code_CODE_OK: @@ -66,7 +65,7 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, &listResponse{}) return } - g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + g.logger.Error().Err(err).Msg(ListStorageSpacesReturnsErr) errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) return } @@ -90,42 +89,42 @@ func (g Graph) GetDrives(w http.ResponseWriter, r *http.Request) { // GetSingleDrive does a lookup of a single space by spaceId func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { - driveId := chi.URLParam(r, "driveID") - if driveId == "" { + driveID := chi.URLParam(r, "driveID") + if driveID == "" { err := fmt.Errorf("no valid space id retrieved") g.logger.Err(err) errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) return } - g.logger.Info().Str("driveID", driveId).Msg("Calling GetSingleDrive") + g.logger.Info().Str("driveID", driveID).Msg("Calling GetSingleDrive") ctx := r.Context() var filters []*storageprovider.ListStorageSpacesRequest_Filter - filterDriveId := &storageprovider.ListStorageSpacesRequest_Filter{ + filterDriveID := &storageprovider.ListStorageSpacesRequest_Filter{ Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_ID, Term: &storageprovider.ListStorageSpacesRequest_Filter_Id{ Id: &storageprovider.StorageSpaceId{ - OpaqueId: driveId, + OpaqueId: driveID, }, }, } - filters = append(filters, filterDriveId) + filters = append(filters, filterDriveID) res, err := g.ListStorageSpacesWithFilters(ctx, filters) switch { case err != nil: - g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + g.logger.Error().Err(err).Msg(ListStorageSpacesTransportErr) errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) return case res.Status.Code != cs3rpc.Code_CODE_OK: if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { // the client is doing a lookup for a specific space, therefore we need to return // not found to the caller - g.logger.Error().Str("driveId", driveId).Msg(fmt.Sprintf("space with id `%s` not found", driveId)) - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("space with id `%s` not found", driveId)) + g.logger.Error().Str("driveID", driveID).Msg(fmt.Sprintf(NoSpaceFoundMessage, driveID)) + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf(NoSpaceFoundMessage, driveID)) return } - g.logger.Error().Err(err).Msg("error sending list storage spaces grpc request") + g.logger.Error().Err(err).Msg(ListStorageSpacesReturnsErr) errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) return } @@ -144,8 +143,8 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { } switch num := len(spaces); { case num == 0: - g.logger.Error().Str("driveId", driveId).Msg("no space found") - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf("space with id `%s` not found", driveId)) + g.logger.Error().Str("driveID", driveID).Msg("no space found") + errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, fmt.Sprintf(NoSpaceFoundMessage, driveID)) return case num == 1: render.Status(r, http.StatusOK) @@ -157,70 +156,6 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { } } -// GetRootDriveChildren implements the Service interface. -func (g Graph) GetRootDriveChildren(w http.ResponseWriter, r *http.Request) { - g.logger.Info().Msg("Calling GetRootDriveChildren") - ctx := r.Context() - - client, err := g.GetClient() - if err != nil { - g.logger.Error().Err(err).Msg("could not get client") - errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) - return - } - - res, err := client.GetHome(ctx, &storageprovider.GetHomeRequest{}) - switch { - case err != nil: - g.logger.Error().Err(err).Msg("error sending get home grpc request") - errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) - return - case res.Status.Code != cs3rpc.Code_CODE_OK: - if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.Status.Message) - return - } - g.logger.Error().Err(err).Msg("error sending get home grpc request") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) - return - } - - lRes, err := client.ListContainer(ctx, &storageprovider.ListContainerRequest{ - Ref: &storageprovider.Reference{ - Path: res.Path, - }, - }) - switch { - case err != nil: - g.logger.Error().Err(err).Msg("error sending list container grpc request") - errorcode.ServiceNotAvailable.Render(w, r, http.StatusInternalServerError, err.Error()) - return - case res.Status.Code != cs3rpc.Code_CODE_OK: - if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { - errorcode.ItemNotFound.Render(w, r, http.StatusNotFound, res.Status.Message) - return - } - if res.Status.Code == cs3rpc.Code_CODE_PERMISSION_DENIED { - // TODO check if we should return 404 to not disclose existing items - errorcode.AccessDenied.Render(w, r, http.StatusForbidden, res.Status.Message) - return - } - g.logger.Error().Err(err).Msg("error sending list container grpc request") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, res.Status.Message) - return - } - - files, err := formatDriveItems(lRes.Infos) - if err != nil { - g.logger.Error().Err(err).Msg("error encoding response as json") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) - return - } - - render.Status(r, http.StatusOK) - render.JSON(w, r, &listResponse{Value: files}) -} - // CreateDrive creates a storage drive (space). func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) { us, ok := ctxpkg.ContextGetUser(r.Context()) @@ -439,18 +374,23 @@ func (g Graph) formatDrives(ctx context.Context, baseURL *url.URL, mds []*storag } // TODO this overwrites the quota that might already have been mapped in cs3StorageSpaceToDrive above ... move this into the cs3StorageSpaceToDrive method? res.Quota, err = g.getDriveQuota(ctx, space) + if err != nil { + return nil, err + } + responses = append(responses, res) + } + + return responses, nil +} + // ListStorageSpacesWithFilters List Storage Spaces using filters func (g Graph) ListStorageSpacesWithFilters(ctx context.Context, filters []*storageprovider.ListStorageSpacesRequest_Filter) (*storageprovider.ListStorageSpacesResponse, error) { - client, err := g.GetClient() - if err != nil { - g.logger.Err(err).Msg("error getting grpc client") - return nil, err - } + client := g.GetGatewayClient() permissions := make(map[string]struct{}, 1) s := sproto.NewPermissionService("com.owncloud.api.settings", grpc.DefaultClient) - _, err = s.GetPermissionByID(ctx, &sproto.GetPermissionByIDRequest{ + _, err := s.GetPermissionByID(ctx, &sproto.GetPermissionByIDRequest{ PermissionId: settingsSvc.ListAllSpacesPermissionID, }) @@ -472,50 +412,7 @@ func (g Graph) ListStorageSpacesWithFilters(ctx context.Context, filters []*stor }}, Filters: filters, }) - return res, nil -} - -func cs3TimestampToTime(t *types.Timestamp) time.Time { - return time.Unix(int64(t.Seconds), int64(t.Nanos)) -} - -func cs3ResourceToDriveItem(res *storageprovider.ResourceInfo) (*libregraph.DriveItem, error) { - size := new(int64) - *size = int64(res.Size) // uint64 -> int :boom: - name := path.Base(res.Path) - - driveItem := &libregraph.DriveItem{ - Id: &res.Id.OpaqueId, - Name: &name, - ETag: &res.Etag, - Size: size, - } - if res.Mtime != nil { - lastModified := cs3TimestampToTime(res.Mtime) - driveItem.LastModifiedDateTime = &lastModified - } - if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_FILE { - driveItem.File = &libregraph.OpenGraphFile{ // FIXME We cannot use libregraph.File here because the openapi codegenerator autodetects 'File' as a go type ... - MimeType: &res.MimeType, - } - } - if res.Type == storageprovider.ResourceType_RESOURCE_TYPE_CONTAINER { - driveItem.Folder = &libregraph.Folder{} - } - return driveItem, nil -} - -func formatDriveItems(mds []*storageprovider.ResourceInfo) ([]*libregraph.DriveItem, error) { - responses := make([]*libregraph.DriveItem, 0, len(mds)) - for i := range mds { - res, err := cs3ResourceToDriveItem(mds[i]) - if err != nil { - return nil, err - } - responses = append(responses, res) - } - - return responses, nil + return res, err } func cs3StorageSpaceToDrive(baseURL *url.URL, space *storageprovider.StorageSpace) (*libregraph.Drive, error) { diff --git a/graph/pkg/service/v0/graph.go b/graph/pkg/service/v0/graph.go index 3523dd8fb5e..8117859194e 100644 --- a/graph/pkg/service/v0/graph.go +++ b/graph/pkg/service/v0/graph.go @@ -85,3 +85,9 @@ func (g Graph) GetHTTPClient() HTTPClient { type listResponse struct { Value interface{} `json:"value,omitempty"` } + +const ( + NoSpaceFoundMessage = "space with id `%s` not found" + ListStorageSpacesTransportErr = "transport error sending list storage spaces grpc request" + ListStorageSpacesReturnsErr = "list storage spaces grpc request returns an errorcode in the response" +) From 3375207a0c8660d850cfb32477bfd0a93ac6dfa5 Mon Sep 17 00:00:00 2001 From: Michael Barz Date: Wed, 19 Jan 2022 13:39:17 +0100 Subject: [PATCH 7/7] make code more efficient --- graph/pkg/service/v0/drives.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/graph/pkg/service/v0/drives.go b/graph/pkg/service/v0/drives.go index 94b1f692780..2757c8cd982 100644 --- a/graph/pkg/service/v0/drives.go +++ b/graph/pkg/service/v0/drives.go @@ -100,16 +100,16 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) { g.logger.Info().Str("driveID", driveID).Msg("Calling GetSingleDrive") ctx := r.Context() - var filters []*storageprovider.ListStorageSpacesRequest_Filter - filterDriveID := &storageprovider.ListStorageSpacesRequest_Filter{ - Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_ID, - Term: &storageprovider.ListStorageSpacesRequest_Filter_Id{ - Id: &storageprovider.StorageSpaceId{ - OpaqueId: driveID, + filters := []*storageprovider.ListStorageSpacesRequest_Filter{ + { + Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_ID, + Term: &storageprovider.ListStorageSpacesRequest_Filter_Id{ + Id: &storageprovider.StorageSpaceId{ + OpaqueId: driveID, + }, }, }, } - filters = append(filters, filterDriveID) res, err := g.ListStorageSpacesWithFilters(ctx, filters) switch { case err != nil: