Skip to content

feat(graph): add the v1beta1 endpoints for a single drive #3941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pbleser-oc wants to merge 1 commit into
base: workpagages
Choose a base branch
from
+371 −32
Open

feat(graph): add the v1beta1 endpoints for a single drive #3941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions services/graph/pkg/service/v0/drives.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,19 @@ func (g Graph) getDrives(r *http.Request, unrestricted bool, apiVersion APIVersi
return spaces, nil
}

// GetSingleDrive does a lookup of a single space by spaceId
// GetSingleDrive returns the space with the given ID, in the v1.0 format.
func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) {
g.getSingleDrive(w, r, APIVersion_1)
}

// GetSingleDriveV1Beta1 returns the space with the given ID, in the v1beta1
// format: its expanded root permissions name unified roles.
func (g Graph) GetSingleDriveV1Beta1(w http.ResponseWriter, r *http.Request) {
g.getSingleDrive(w, r, APIVersion_1_Beta_1)
}

// getSingleDrive returns the space with the given ID, in the format of the given API version.
func (g Graph) getSingleDrive(w http.ResponseWriter, r *http.Request, apiVersion APIVersion) {
ctx := r.Context()
log := g.logger.SubloggerWithRequestID(ctx).With().Interface("query", r.URL.Query()).Logger()
log.Debug().Msg("calling get drive")
Expand Down Expand Up @@ -326,7 +337,7 @@ func (g Graph) GetSingleDrive(w http.ResponseWriter, r *http.Request) {
return
}

spaces, err := g.formatDrives(ctx, webDavBaseURL, res.StorageSpaces, APIVersion_1, expandPermissions, getFieldMask(odataReq))
spaces, err := g.formatDrives(ctx, webDavBaseURL, res.StorageSpaces, apiVersion, expandPermissions, getFieldMask(odataReq))
if err != nil {
log.Debug().Err(err).Msg("could not get drive: error parsing grpc response")
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
Expand Down Expand Up @@ -361,8 +372,18 @@ func (g Graph) canCreateSpace(ctx context.Context, ownPersonalHome bool) bool {
return true
}

// CreateDrive creates a storage drive (space).
// CreateDrive creates a space and returns it in the v1.0 format.
func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) {
g.createDrive(w, r, APIVersion_1)
}

// CreateDriveV1Beta1 creates a space and returns it in the v1beta1 format.
func (g Graph) CreateDriveV1Beta1(w http.ResponseWriter, r *http.Request) {
g.createDrive(w, r, APIVersion_1_Beta_1)
}

// createDrive creates a space and returns it in the format of the given API version.
func (g Graph) createDrive(w http.ResponseWriter, r *http.Request, apiVersion APIVersion) {
ctx := r.Context()
log := g.logger.SubloggerWithRequestID(ctx).With().Interface("query", r.URL.Query()).Logger()
log.Debug().Msg("calling create drive")
Expand Down Expand Up @@ -515,7 +536,7 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) {
}
}

spaces, err := g.formatDrives(ctx, webDavBaseURL, []*storageprovider.StorageSpace{space}, APIVersion_1, false, nil)
spaces, err := g.formatDrives(ctx, webDavBaseURL, []*storageprovider.StorageSpace{space}, apiVersion, false, nil)
if err != nil {
log.Debug().Err(err).Msg("could not get drive: error parsing grpc response")
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
Expand All @@ -531,8 +552,19 @@ func (g Graph) CreateDrive(w http.ResponseWriter, r *http.Request) {
render.JSON(w, r, spaces[0])
}

// UpdateDrive updates the properties of a storage drive (space).
// UpdateDrive updates the properties of a space and returns it in the v1.0 format.
func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) {
g.updateDrive(w, r, APIVersion_1)
}

// UpdateDriveV1Beta1 updates the properties of a space and returns it in the
// v1beta1 format.
func (g Graph) UpdateDriveV1Beta1(w http.ResponseWriter, r *http.Request) {
g.updateDrive(w, r, APIVersion_1_Beta_1)
}

// updateDrive updates the properties of a space and returns it in the format of the given API version.
func (g Graph) updateDrive(w http.ResponseWriter, r *http.Request, apiVersion APIVersion) {
log := g.logger.SubloggerWithRequestID(r.Context()).With().Interface("query", r.URL.Query()).Logger()
log.Debug().Msg("calling update drive")

Expand Down Expand Up @@ -687,7 +719,7 @@ func (g Graph) UpdateDrive(w http.ResponseWriter, r *http.Request) {
}
}

spaces, err := g.formatDrives(r.Context(), webDavBaseURL, []*storageprovider.StorageSpace{resp.StorageSpace}, APIVersion_1, false, nil)
spaces, err := g.formatDrives(r.Context(), webDavBaseURL, []*storageprovider.StorageSpace{resp.StorageSpace}, apiVersion, false, nil)
if err != nil {
log.Debug().Err(err).Msg("could not update drive: error parsing grpc response")
errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
Expand Down
70 changes: 62 additions & 8 deletions services/graph/pkg/service/v0/graph_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,7 @@ var _ = Describe("Graph", func() {
Expect(libreError.Error.Message).To(Equal("invalid spacename: spacenames must not contain [/ \\ . : ? * \" > < |]"))
Expect(libreError.Error.Code).To(Equal(errorcode.InvalidRequest.String()))
})
It("can create a project space", func() {
DescribeTable("can create a project space", func(path string, createDrive func(service.Service, http.ResponseWriter, *http.Request)) {
gatewayClient.On("CreateStorageSpace", mock.Anything, mock.Anything).Return(&provider.CreateStorageSpaceResponse{
Status: status.NewOK(ctx),
StorageSpace: &provider.StorageSpace{
Expand Down Expand Up @@ -878,9 +878,9 @@ var _ = Describe("Graph", func() {
}, nil)

jsonBody := []byte(`{"name": "Test Space", "driveType": "project", "description": "This space is for testing", "DriveAlias": "project/testspace", "@libre.graph.contentType": "application/vnd.opencloud.test"}`)
r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/drives", bytes.NewBuffer(jsonBody)).WithContext(ctx)
r := httptest.NewRequest(http.MethodPost, path, bytes.NewBuffer(jsonBody)).WithContext(ctx)
rr := httptest.NewRecorder()
svc.CreateDrive(rr, r)
createDrive(svc, rr, r)
Expect(rr.Code).To(Equal(http.StatusCreated))

body, _ := io.ReadAll(rr.Body)
Expand All @@ -892,7 +892,10 @@ var _ = Describe("Graph", func() {
Expect(*response.DriveAlias).To(Equal("project/testspace"))
Expect(*response.Description).To(Equal("This space is for testing"))
Expect(*response.LibreGraphContentType).To(Equal("application/vnd.opencloud.test"))
})
},
Entry("with v1.0", "/graph/v1.0/drives", service.Service.CreateDrive),
Entry("with v1beta1", "/graph/v1beta1/drives", service.Service.CreateDriveV1Beta1),
)
Context("drive id handling", func() {
const spaceID = "0b9a7b84-3e71-4a4e-9c34-0e6a4c3f5f21"

Expand Down Expand Up @@ -1235,6 +1238,54 @@ var _ = Describe("Graph", func() {
Expect(*drive.GetQuota().Total).To(Equal(int64(500)))
})

DescribeTable("returns the expanded root permissions in the format of the API version",
func(path string, getSingleDrive func(service.Service, http.ResponseWriter, *http.Request), check func(permission gjson.Result)) {
gatewayClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return(&provider.ListStorageSpacesResponse{
Status: status.NewOK(ctx),
StorageSpaces: []*provider.StorageSpace{
{
Opaque: utils.AppendJSONToOpaque(nil, "grants", map[string]provider.ResourcePermissions{
"1": *conversions.NewManagerRole().CS3ResourcePermissions(),
}),
Id: &provider.StorageSpaceId{OpaqueId: "spaceid"},
SpaceType: "project",
Root: &provider.ResourceId{
StorageId: "pro-1",
SpaceId: "spaceid",
OpaqueId: "spaceid",
},
Name: "aspacename",
},
},
}, nil)
gatewayClient.On("GetUser", mock.Anything, mock.Anything).Return(&userprovider.GetUserResponse{
Status: status.NewUnimplemented(ctx, fmt.Errorf("not supported"), "not supported"),
}, nil)

r := httptest.NewRequest(http.MethodGet, path+"?$expand=root($expand=permissions)", nil)
rctx := chi.NewRouteContext()
rctx.URLParams.Add("driveID", "spaceid")
r = r.WithContext(context.WithValue(ctx, chi.RouteCtxKey, rctx))
getSingleDrive(svc, rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))

permissions := gjson.Get(rr.Body.String(), "root.permissions")
Expect(permissions.Get("#").Int()).To(Equal(int64(1)))
Expect(permissions.Get("0.grantedToV2.user.id").Str).To(Equal("1"))
check(permissions.Get("0"))
},
Entry("with v1.0: the old role name", "/graph/v1.0/drives/spaceid", service.Service.GetSingleDrive,
func(permission gjson.Result) {
Expect(permission.Get("roles").Array()).To(ConsistOf(HaveField("Str", "manager")))
Expect(permission.Get("grantedToIdentities").Exists()).To(BeTrue())
}),
Entry("with v1beta1: the ID of the unified role", "/graph/v1beta1/drives/spaceid", service.Service.GetSingleDriveV1Beta1,
func(permission gjson.Result) {
Expect(permission.Get("roles").Array()).To(ConsistOf(HaveField("Str", unifiedrole.UnifiedRoleManagerID)))
Expect(permission.Get("grantedToIdentities").Exists()).To(BeFalse())
}),
)

It("gets the special drive items", func() {
gatewayClient.On("GetPath", mock.Anything, mock.Anything).Return(&provider.GetPathResponse{
Status: status.NewOK(ctx),
Expand Down Expand Up @@ -1343,7 +1394,7 @@ var _ = Describe("Graph", func() {
Expect(rr.Code).To(Equal(http.StatusBadRequest))
})

It("sets the description, alias, name and content type", func() {
DescribeTable("sets the description, alias, name and content type", func(path string, updateDrive func(service.Service, http.ResponseWriter, *http.Request)) {
drive := libregraph.NewDrive("thename")
drive.SetDriveAlias("thealias")
drive.SetDescription("thedescription")
Expand All @@ -1362,11 +1413,11 @@ var _ = Describe("Graph", func() {
Status: status.NewNotFound(ctx, "no special files found"),
}, nil)

r := httptest.NewRequest(http.MethodPatch, "/graph/v1.0/drives/{driveID}/", bytes.NewBuffer(driveJson))
r := httptest.NewRequest(http.MethodPatch, path, bytes.NewBuffer(driveJson))
rctx := chi.NewRouteContext()
rctx.URLParams.Add("driveID", "spaceid")
r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, nil), chi.RouteCtxKey, rctx))
svc.UpdateDrive(rr, r)
updateDrive(svc, rr, r)
Expect(rr.Code).To(Equal(http.StatusOK))

gatewayClient.AssertCalled(GinkgoT(), "UpdateStorageSpace", mock.Anything, mock.MatchedBy(func(req *provider.UpdateStorageSpaceRequest) bool {
Expand All @@ -1375,7 +1426,10 @@ var _ = Describe("Graph", func() {
utils.ReadPlainFromOpaque(req.StorageSpace.Opaque, "spaceAlias") == drive.GetDriveAlias() &&
utils.ReadPlainFromOpaque(req.StorageSpace.Opaque, "contentType") == drive.GetLibreGraphContentType()
}))
})
},
Entry("with v1.0", "/graph/v1.0/drives/spaceid", service.Service.UpdateDrive),
Entry("with v1beta1", "/graph/v1beta1/drives/spaceid", service.Service.UpdateDriveV1Beta1),
)

DescribeTable("rejects an invalid name without updating the space",
func(name string) {
Expand Down
7 changes: 7 additions & 0 deletions services/graph/pkg/service/v0/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,13 @@ type Service interface { //nolint:interfacebloat
GetDrivesV1(w http.ResponseWriter, r *http.Request)
GetDrivesV1Beta1(w http.ResponseWriter, r *http.Request)
GetSingleDrive(w http.ResponseWriter, r *http.Request)
GetSingleDriveV1Beta1(w http.ResponseWriter, r *http.Request)
GetAllDrivesV1(w http.ResponseWriter, r *http.Request)
GetAllDrivesV1Beta1(w http.ResponseWriter, r *http.Request)
CreateDrive(w http.ResponseWriter, r *http.Request)
CreateDriveV1Beta1(w http.ResponseWriter, r *http.Request)
UpdateDrive(w http.ResponseWriter, r *http.Request)
UpdateDriveV1Beta1(w http.ResponseWriter, r *http.Request)
DeleteDrive(w http.ResponseWriter, r *http.Request)

GetSharedByMe(w http.ResponseWriter, r *http.Request)
Expand Down Expand Up @@ -250,13 +253,17 @@ func NewService(opts ...Option) (Graph, error) { //nolint:maintidx
})
r.Route("/drives", func(r chi.Router) {
r.Get("/", svc.GetAllDrives(APIVersion_1_Beta_1))
r.Post("/", svc.CreateDriveV1Beta1)
r.Route("/{driveID}", func(r chi.Router) {
// Rewrites MS Graph colon-syntax lookups (root:/path,
// items/{id}:/path) to /items/{resolvedID}... before chi
// matches the leaf route. Must sit here, on the
// /drives/{driveID} sub-router, so it can rewrite the
// remaining RoutePath. See graphm.ResolveGraphPath.
r.Use(graphm.ResolveGraphPath(options.GatewaySelector, options.Logger))
r.Get("/", svc.GetSingleDriveV1Beta1)
r.Patch("/", svc.UpdateDriveV1Beta1)
r.Delete("/", svc.DeleteDrive)
r.Route("/root", func(r chi.Router) {
r.Post("/children", drivesDriveItemApi.CreateDriveItem)
r.Post("/invite", driveItemPermissionsApi.SpaceRootInvite)
Expand Down
49 changes: 39 additions & 10 deletions tests/acceptance/TestHelpers/GraphHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
namespace TestHelpers;

use GuzzleHttp\Exception\GuzzleException;
use InvalidArgumentException;
use Psr\Http\Message\RequestInterface;
use Psr\Http\Message\ResponseInterface;

Expand Down Expand Up @@ -207,6 +208,24 @@ public static function getBetaFullUrl(string $baseUrl, string $path): string {
return $baseUrl . '/graph/v1beta1/' . $path;
}

/**
* the URL of a path of the Graph API, in the given version of the API
*
* @param string $baseUrl
* @param string $apiVersion "v1.0" or "v1beta1"
* @param string $path
*
* @return string
* @throws InvalidArgumentException
*/
public static function getFullUrlForApiVersion(string $baseUrl, string $apiVersion, string $path): string {
return match ($apiVersion) {
'v1.0' => self::getFullUrl($baseUrl, $path),
'v1beta1' => self::getBetaFullUrl($baseUrl, $path),
default => throw new InvalidArgumentException("unknown Graph API version '$apiVersion'"),
};
}

/**
* @param string $baseUrl
* @param string $xRequestId
Expand Down Expand Up @@ -953,6 +972,7 @@ public static function preparePatchUserPayload(
* @param string $body
* @param string $xRequestId
* @param array $headers
* @param string $apiVersion "v1.0" or "v1beta1"
*
* @return ResponseInterface
* @throws GuzzleException
Expand All @@ -963,9 +983,10 @@ public static function createSpace(
string $password,
string $body,
string $xRequestId = '',
array $headers = []
array $headers = [],
string $apiVersion = 'v1.0'
): ResponseInterface {
$url = self::getFullUrl($baseUrl, 'drives');
$url = self::getFullUrlForApiVersion($baseUrl, $apiVersion, 'drives');

return HttpRequestHelper::post($url, $xRequestId, $user, $password, $headers, $body);
}
Expand All @@ -980,6 +1001,7 @@ public static function createSpace(
* @param string $spaceId
* @param string $xRequestId
* @param array $headers
* @param string $apiVersion "v1.0" or "v1beta1"
*
* @return ResponseInterface
* @throws GuzzleException
Expand All @@ -991,9 +1013,10 @@ public static function updateSpace(
$body,
string $spaceId,
string $xRequestId = '',
array $headers = []
array $headers = [],
string $apiVersion = 'v1.0'
): ResponseInterface {
$url = self::getFullUrl($baseUrl, 'drives/' . $spaceId);
$url = self::getFullUrlForApiVersion($baseUrl, $apiVersion, 'drives/' . $spaceId);

return HttpRequestHelper::sendRequest($url, $xRequestId, 'PATCH', $user, $password, $headers, $body);
}
Expand Down Expand Up @@ -1067,6 +1090,7 @@ public static function getAllSpaces(
* @param string $xRequestId
* @param array $body
* @param array $headers
* @param string $apiVersion "v1.0" or "v1beta1"
*
* @return ResponseInterface
*
Expand All @@ -1080,9 +1104,10 @@ public static function getSingleSpace(
string $urlArguments = '',
string $xRequestId = '',
array $body = [],
array $headers = []
array $headers = [],
string $apiVersion = 'v1.0'
): ResponseInterface {
$url = self::getFullUrl($baseUrl, 'drives/' . $spaceId . "/" . $urlArguments);
$url = self::getFullUrlForApiVersion($baseUrl, $apiVersion, 'drives/' . $spaceId . "/" . $urlArguments);

return HttpRequestHelper::get($url, $xRequestId, $user, $password, $headers, $body);
}
Expand All @@ -1095,6 +1120,7 @@ public static function getSingleSpace(
* @param string $password
* @param string $spaceId
* @param string $xRequestId
* @param string $apiVersion "v1.0" or "v1beta1"
*
* @return ResponseInterface
* @throws GuzzleException
Expand All @@ -1104,9 +1130,10 @@ public static function disableSpace(
string $user,
string $password,
string $spaceId,
string $xRequestId = ''
string $xRequestId = '',
string $apiVersion = 'v1.0'
): ResponseInterface {
$url = self::getFullUrl($baseUrl, 'drives/' . $spaceId);
$url = self::getFullUrlForApiVersion($baseUrl, $apiVersion, 'drives/' . $spaceId);

return HttpRequestHelper::delete(
$url,
Expand All @@ -1124,6 +1151,7 @@ public static function disableSpace(
* @param string $password
* @param string $spaceId
* @param string $xRequestId
* @param string $apiVersion "v1.0" or "v1beta1"
*
* @return ResponseInterface
* @throws GuzzleException
Expand All @@ -1133,9 +1161,10 @@ public static function deleteSpace(
string $user,
string $password,
string $spaceId,
string $xRequestId = ''
string $xRequestId = '',
string $apiVersion = 'v1.0'
): ResponseInterface {
$url = self::getFullUrl($baseUrl, 'drives/' . $spaceId);
$url = self::getFullUrlForApiVersion($baseUrl, $apiVersion, 'drives/' . $spaceId);
$header = ["Purge" => "T"];

return HttpRequestHelper::delete(
Expand Down
Loading