Skip to content
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.

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

There is no way to list items in the drive #67

Closed
augbed opened this issue Jan 19, 2022 · 23 comments
Closed

There is no way to list items in the drive #67

augbed opened this issue Jan 19, 2022 · 23 comments
Assignees
Labels
bug Something isn't working fixed

Comments

@augbed
Copy link

augbed commented Jan 19, 2022

It seems like there's currently no way of actually listing drive items.
Requests like: GET /me/drive/root/children or GET /drives/{drive-id}/items/{item-id}/children are not possible using this sdk it seems.

I would expect to be able to do something like
client.Me().Drive().Root().Children().Get(nil)

Am I missing something or is this just not possible? Also it seems that it's possible to construct invalid requests as in doing GET /drive/items and it just fails silently.

@ghost ghost added the Needs Triage 🔍 label Jan 19, 2022
@baywet baywet self-assigned this Jan 19, 2022
@baywet baywet added blocked resolving this issue is blocked by an upstream dependency bug Something isn't working and removed Needs Triage 🔍 labels Jan 19, 2022
@baywet
Copy link
Member

baywet commented Jan 19, 2022

Hi @augbed
Thanks for trying out the SDK and for reaching out.
This gap is due to the fact that the open api description doesn't contain a path item for /me/drive/root, so our generator is not generating code for those APIs at the moment.

We're tracking this and that issue in the conversion library that generates the OpenAPI description. When they are addressed and the newer version of the library is released, the additional fluent API paths should show up.
This might take a few additional weeks to happen though, in the meantime, you should be able to get around the issue by using a similar request builder forcing the request url to https://graph.microsoft.com/v1.0/me/drive/root which will give you access to a similar experience.

Lastly, we're aware the SDK fails silently at the moment whenever an error is returned, this is something we're tracking in our list of known issues to resolve before GA

I hope this helps, let us know if you have further questions. I'll leave the issue open until the path items show up.

@mkanderson
Copy link

mkanderson commented Mar 1, 2022

Hi, could you please elaborate on how to do this?

you should be able to get around the issue by using a similar request builder forcing the request url to https://graph.microsoft.com/v1.0/me/drive/root

My naive attempt does not work:

import (
	"fmt"

	azidentity "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
	a "github.com/microsoft/kiota/authentication/go/azure"
	sdk "github.com/microsoftgraph/msgraph-sdk-go"
	drive "github.com/microsoftgraph/msgraph-sdk-go/shares/item/driveitem"
	"github.com/spf13/viper"
)

func Dump() {

	cred, err := azidentity.NewClientSecretCredential(
		viper.GetString("auth.tenant_id"),
		viper.GetString("auth.client_id"),
		viper.GetString("auth.client_secret"),
		&azidentity.ClientSecretCredentialOptions{},
	)

	if err != nil {
		fmt.Printf("Error creating credentials: %v\n", err)
	}

	auth, err := a.NewAzureIdentityAuthenticationProvider(cred)
	if err != nil {
		fmt.Printf("Error authenticating: %v\n", err)
		return
	}

	adapter, err := sdk.NewGraphRequestAdapter(auth)
	if err != nil {
		fmt.Printf("Error creating adapter: %v\n", err)
		return
	}

	targetUrl := "https://graph.microsoft.com/v1.0/me/drive/root"

	client := drive.NewDriveItemRequestBuilder(targetUrl, adapter)
	if err != nil {
		fmt.Printf("Error creating client: %v\n", err)
		return
	}

	result, err := client.Get(nil)
	if err != nil {
		fmt.Printf("Error retrieving resource: %v\n", err)
	}
	fmt.Printf("result = %+v\n", result)

}

Results in:

Error retrieving resource: Get "/shares//driveItem": unsupported protocol scheme ""
result = <nil>

Thanks!

@baywet
Copy link
Member

baywet commented Mar 1, 2022

Your code is correct, there seems to be a bug in the generation process however.

m.pathParameters = pathParameters;

should be

-m.pathParameters = pathParameters;
+m.pathParameters = urlTplParams;

The code generating this is here https://github.com/microsoft/kiota/blob/4a4750ca8e03a57e094197e36937630fee3a103c/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs#L254

and should be

-writer.WriteLine($"m.{property.Name.ToFirstCharacterLowerCase()} = {parameter.Name};");
+writer.WriteLine($"m.{property.Name.ToFirstCharacterLowerCase()} = {variableName};");

Submitting a PR shortly.

@baywet
Copy link
Member

baywet commented Mar 1, 2022

I just put together this PR, but after digging into it further it doesn't look like that's the root of the issue, looking further into this.

@baywet
Copy link
Member

baywet commented Mar 1, 2022

After further investigations it looks like the request builder constructor, and the CreateGetRequestInformation method are doing the right thing (able to get the raw URL when calling get URI).

However when the request adapter calls GetUri, it's not getting the right value.
https://github.com/microsoft/kiota/blob/4a4750ca8e03a57e094197e36937630fee3a103c/http/go/nethttp/nethttp_request_adapter.go#L115

It seems like dereferencing the request information in the Get method might be what's causing the issue, but further investigations are needed. (since the SetUri method is defined on a pointer receiver, not on the struct)

res, err := m.requestAdapter.SendAsync(*requestInfo, func () i04eb5309aeaafadd28374d79c8471df9b267510b4dc2e3144c378c50f6fd7b55.Parsable { return i4a838ef194e4c99e9f2c63ba10dab9cb120a89367c1d4ab0daa63bb424e20d87.NewDriveItem() }, nil)

@mkanderson
Copy link

It seems that GetUri is getting called twice. The first time everything looks good:

(dlv) 
*github.com/microsoft/kiota/abstractions/go.RequestInformation {
	Method: GET (0),
	uri: *net/url.URL {
		Scheme: "https",
		Opaque: "",
		User: *net/url.Userinfo nil,
		Host: "graph.microsoft.com",
		Path: "/v1.0/me/drive/root",
		RawPath: "",
		ForceQuery: false,
		RawQuery: "",
		Fragment: "",
		RawFragment: "",},
	Headers: map[string]string [],
	QueryParameters: map[string]string [],
	Content: []uint8 len: 0, cap: 0, nil,
	PathParameters: map[string]string [],
	UrlTemplate: "{+baseurl}/shares/{sharedDriveItem_id}/driveItem{?select,expand}",
	options: map[string]github.com/microsoft/kiota/abstractions/go.RequestOption [],}

..then it runs a second time, apparently after acquiring a token and losing the RequestInformation data along the way somewhere.

(dlv) print request
*github.com/microsoft/kiota/abstractions/go.RequestInformation {
	Method: GET (0),
	uri: *net/url.URL nil,
	Headers: map[string]string [
		"Authorization": "Bearer eyJ0eXAiOiJKV1QiLCJub25jZSI6Ilk5WUk1ay0tbTZHdFBuVmFDNXotT...+1535 more", 
	],
	QueryParameters: map[string]string [],
	Content: []uint8 len: 0, cap: 0, nil,
	PathParameters: map[string]string [],
	UrlTemplate: "{+baseurl}/shares/{sharedDriveItem_id}/driveItem{?select,expand}",
	options: map[string]github.com/microsoft/kiota/abstractions/go.RequestOption [],}

@baywet
Copy link
Member

baywet commented Mar 2, 2022

Thanks for that precious additional information.
Yes, the authentication provider interface accepts a value, not a reference, which is just bad design for something that needs to mutate the object.

Here is what happens at a high level:

  1. the executor method (Get) passes a value copy of the request information. This value contains references to maps (PathParameters & co)
  2. the request adapter passes yet another value copy to the authentication provider
  3. the authentication provider gets the uri, which clears the paths parameters (SetUri) and sets the uri reference on the copy. It also sets the authorization header
  4. because references to map between the copies are the same, the header is kept, and the path parameters have been cleared, the the uri value that was set is lost
  5. request adapter tries to get the uri again, the uri value on its copy is nil, and the path parameters are empty (the copies references to the path parameters map are all the same), it goes sideways from there

I'll update my pull request to account for all of that.

@mkanderson
Copy link

Ah, tricky. Thanks for the analysis.

@baywet
Copy link
Member

baywet commented Mar 2, 2022

You're welcome! I just update the initial PR to fix that.
Once it gets merged we'll need to update the core sdk and trigger a generation here.

@baywet
Copy link
Member

baywet commented Mar 3, 2022

Update: kiota PR was merged, the reference update PR in core as well, now all that's required is a generation here. It will happen by next Tuesday. Thanks for your patience.

@baywet
Copy link
Member

baywet commented Mar 10, 2022

update, the ability to use request builders with raw urls has been fixed, to update:

go get -u github.com/microsoftgraph/msgraph-sdk-go@v0.13.0

Some of the additional path segments are also present now thanks to leveraging the newer metadata
https://github.com/microsoftgraph/msgraph-sdk-go/tree/main/drive/root/children

@mkanderson
Copy link

mkanderson commented Mar 15, 2022

Great, thanks!
Now I'm getting the following:

Error retrieving resource: content type application/json does not have a factory registered to be parsed

I guess I need to pass some code to process the response. Is there an example of what that would look like somewhere?

@baywet
Copy link
Member

baywet commented Mar 15, 2022

this is strange as those factories are registered by default with the client.

ida96af0f171bb75f894a4013a6b3146a4397c58f11adb81a2b7cbea9314783a9.RegisterDefaultSerializer(func() i04eb5309aeaafadd28374d79c8471df9b267510b4dc2e3144c378c50f6fd7b55.SerializationWriterFactory { return id1ae20a9e00c372d14381acc2299aa946a25894316974139983388e4b11bb84b.NewJsonSerializationWriterFactory() })

would you mind sharing your client initialization code please?

@mkanderson
Copy link

It is the code posted above.
#67 (comment)

@mkanderson
Copy link

mkanderson commented Mar 15, 2022

Here's the code as a single main package:

package main

import (
	"fmt"

	azidentity "github.com/Azure/azure-sdk-for-go/sdk/azidentity"
	a "github.com/microsoft/kiota/authentication/go/azure"
	sdk "github.com/microsoftgraph/msgraph-sdk-go"
	drive "github.com/microsoftgraph/msgraph-sdk-go/shares/item/driveitem"
)

func main() {

	tenant_id := "***"
	client_id := "***"
	client_secret := "***"

	cred, err := azidentity.NewClientSecretCredential(
		tenant_id,
		client_id,
		client_secret,
		&azidentity.ClientSecretCredentialOptions{},
	)

	if err != nil {
		fmt.Printf("Error creating credentials: %v\n", err)
	}

	auth, err := a.NewAzureIdentityAuthenticationProvider(cred)
	if err != nil {
		fmt.Printf("Error authenticating: %v\n", err)
		return
	}

	adapter, err := sdk.NewGraphRequestAdapter(auth)
	if err != nil {
		fmt.Printf("Error creating adapter: %v\n", err)
		return
	}

	targetUrl := "https://graph.microsoft.com/v1.0/me/drive/root"

	client := drive.NewDriveItemRequestBuilder(targetUrl, adapter)
	if err != nil {
		fmt.Printf("Error creating client: %v\n", err)
		return
	}

	result, err := client.Get(nil)
	if err != nil {
		fmt.Printf("Error retrieving resource: %v\n", err)
	}
	fmt.Printf("result = %+v\n", result)
}

My go.mod looks like:

module mant/go-msgraph-scratch

go 1.17

require (
	github.com/Azure/azure-sdk-for-go/sdk/azidentity v0.13.2
	github.com/microsoft/kiota/authentication/go/azure v0.0.0-20220315131435-4518a14f989a
	github.com/microsoftgraph/msgraph-sdk-go v0.13.0
)

require (
	github.com/Azure/azure-sdk-for-go/sdk/azcore v0.21.0 // indirect
	github.com/Azure/azure-sdk-for-go/sdk/internal v0.9.1 // indirect
	github.com/AzureAD/microsoft-authentication-library-for-go v0.4.0 // indirect
	github.com/cjlapao/common-go v0.0.18 // indirect
	github.com/golang-jwt/jwt v3.2.1+incompatible // indirect
	github.com/google/uuid v1.3.0 // indirect
	github.com/kylelemons/godebug v1.1.0 // indirect
	github.com/microsoft/kiota/abstractions/go v0.0.0-20220308162731-fb6ab0cd5ea2 // indirect
	github.com/microsoft/kiota/http/go/nethttp v0.0.0-20220308162731-fb6ab0cd5ea2 // indirect
	github.com/microsoft/kiota/serialization/go/json v0.0.0-20220308162731-fb6ab0cd5ea2 // indirect
	github.com/microsoftgraph/msgraph-sdk-go-core v0.0.13 // indirect
	github.com/pkg/browser v0.0.0-20210115035449-ce105d075bb4 // indirect
	github.com/yosida95/uritemplate/v3 v3.0.1 // indirect
	golang.org/x/crypto v0.0.0-20201216223049-8b5274cf687f // indirect
	golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect
	golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e // indirect
	golang.org/x/text v0.3.7 // indirect
)

@baywet
Copy link
Member

baywet commented Mar 15, 2022

aaah thanks for sharing this, can you try replacing this?

+      client := msgraphsdk.NewGraphServiceClient(adapter)
+      drive, err := client.Me().Drive().Get(nil)
+      if err != nil {
+	        fmt.Printf("Error getting the drive: %v\n", err)
+	        return
+      }
-       targetUrl := "https://graph.microsoft.com/v1.0/me/drive/root"

-	client := drive.NewDriveItemRequestBuilder(targetUrl, adapter)
-	if err != nil {
-		fmt.Printf("Error creating client: %v\n", err)
-		return
-	}

-      result, err := client.Get(nil)
+     result, err := client.DrivesById(*drive.GetId()).Root().Get(nil)
	if err != nil {
		fmt.Printf("Error retrieving resource: %v\n", err)
	}
	fmt.Printf("result = %+v\n", result)

@mkanderson
Copy link

This gets:
Error getting the drive: error status code received from the API

@baywet
Copy link
Member

baywet commented Mar 15, 2022

which is expected since your app is using app only and the /me endpoints require a delegated context. see the extended answer I provided on another issue

@mkanderson
Copy link

Ah, right:

(dlv) print err
error(*github.com/microsoftgraph/msgraph-sdk-go/models/microsoft/graph/odataerrors.ODataError) *{
	ApiError: github.com/microsoft/kiota/abstractions/go.ApiError {Message: ""},
	additionalData: map[string]interface {} [],
	error: github.com/microsoftgraph/msgraph-sdk-go/models/microsoft/graph/odataerrors.MainErrorable(*github.com/microsoftgraph/msgraph-sdk-go/models/microsoft/graph/odataerrors.MainError) *{
		additionalData: map[string]interface {} [...],
		code: *"BadRequest",
		details: []github.com/microsoftgraph/msgraph-sdk-go/models/microsoft/graph/odataerrors.ErrorDetailsable len: 0, cap: 0, nil,
		innererror: github.com/microsoftgraph/msgraph-sdk-go/models/microsoft/graph/odataerrors.InnerErrorable nil,
		message: *"/me request is only valid with delegated authentication flow.",
		target: *string nil,},}

@mkanderson
Copy link

I'm still interested in getting a working query with a raw url though.
My rather poor choice of the /me endpoint was arbitrary.

@baywet
Copy link
Member

baywet commented Mar 15, 2022

I'm curious to why would you rather do that than using the chained style API?

The code you posted earlier should work, you just need to new up the client before executing the request to register the default providers. If you don't intend on using the service client and Go complains about an unused variable, you can alternatively register the default serializers yourself, see this answer

@mkanderson
Copy link

I'm curious to why would you rather do that than using the chained style API?

I don't have any pressing need at this point, just curious.

The code you posted earlier should work, you just need to new up the client before executing the request to register the default providers. If you don't intend on using the service client and Go complains about an unused variable, you can alternatively register the default serializers yourself, see this answer

Ok. Thanks, I'll try it out.

@baywet
Copy link
Member

baywet commented May 4, 2022

Quick update on this, I thought I had provided one but apparently I forgot.

For the drives (and things that live under a drive) there's a canonical path and there are additional paths to access the same information.
Due to limitations with OData, the current API design and other things, we've chosen to only emit the canonical path in the SDK.

Effectively /drives/{drive-id}/items/{item-id}/children is available since the March 8th generation.
For /me/drive(s), /users/{id}/drive(s), /group/{id}/drive(s), /sites/{id}/drive(s) and other non-canonical paths, only the drive(s) part of the path will be present, but nothing underneath it. This is to avoid over-expansion of the SDKs and the SDKs becoming enormous.

If you want to access data under those, and your application context doesn't have all the ids (drive id + drive item id...), the best thing is to:

  1. query the non-canonical path to get the drive id. client.Me().Drive().Get(nil)
  2. revert to the canonical path with the drive id to explore data underneath. client.DrivesById("idFromPreviousQuery").Root().Items().Get(nil)

If you want to perform a single request because your application context has all the ids it needs, you can either use the chained methods on the canonical path, or use a request builder with a raw URL (see my previous answers).

I hope this clarifies the situation.
I'm going to go ahead and close this issue since all the deficiencies have been addressed at this point. Feel free to comment out if I forgot anything.

@baywet baywet closed this as completed May 4, 2022
@baywet baywet added fixed and removed blocked resolving this issue is blocked by an upstream dependency labels May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed
Projects
None yet
Development

No branches or pull requests

3 participants