SearchWithQ(* string) returns err: 'uritemplate:55:invalid literals'

[mkanderson]

Calling the `SearchWithQ(* string) method so:

	searchString := "foo"
	drv, err := client.DrivesById(drvId).SearchWithQ(&searchString).Get(nil)

returns
'uritemplate:55:invalid literals'

It appears that the single quotes in

msgraph-sdk-go/drive/searchwithq/search_with_q_request_builder.go

Line 29 in e9f3586

m.urlTemplate = "{+baseurl}/drive/microsoft.graph.search(q='{q}')";

are not recognized as valid characters by
https://github.com/yosida95/uritemplate/blob/3a84360807fa88737fe714e7ad55f7469ab8ecc9/parse.go#L45

[baywet]

Hi @mkanderson,
Thanks for trying the Go SDK and for reaching out.
This is something we've already had reported in #97 and I created an issue in the library
Unfortunately I haven't had any answer in 2 weeks, and I'd like to get a go ahead before I spend some time putting together a pull request. Can you please leave a comment in the issue I've created to let the author know this requires attention?
I'll also try to email the person as they seem to be active on GitHub.
Alternatively, we could consider replacing the library by an equivalent, but this library has worked great so far.

[mkanderson]

Ok, thanks. I've commented in the library issue.

[tommed]

Hi guys, just trying to understand the current status of this as we're really keen to use this library if possible...

Is there an alternative to this library given this issue prevents it being used and hasn't moved since December?
We're really keen to integrate with Azure AD and the documentation recommends using this library vs the previous implementation.

Understand the aforementioned issue is in a remote dependency, but couldn't this be easily be replaced if the author isn't responsive?

Is it a case that MS aren't fussed about golang, or is there anyone cause for the lack of time spent on this? Just keen to understand if we need to avoid Azure AD for the near future. Thanks 🙇 🙏

[tommed]

Our experience is currently that v0.18.0 uses reflection to determine the query parameters, but isn't prepending the "$" so throws the error:

Request_BadRequest: Unrecognized query argument specified: 'top,skip'.: error status code received from the API

Apologies if this is a new issue and not related to this ticket, but I think it might be the same issue even if a different cause in the latest code. Any help working around this would be greatly appreciated.

[baywet]

hi @tommed
Can you create a separate issue for top/skip please? it does feel like it's separate. Please include your code snippet.

Understand the aforementioned issue is in a remote dependency, but couldn't this be easily be replaced if the author isn't responsive?

The author replied a while ago now. The issue is that the RFC6570 contradicts itself. We've reached out to the author of the RFC but no conclusive resolution has been reached yet. I'll try nudging the authors

Sorry this is taking so long, it's a very specific issue.

[tommed]

Sure, it happens for all the query parameters, not just those mentioned. search, expand etc.

[tommed]

hi @tommed Can you create a separate issue for top/skip please? it does feel like it's separate. Please include your code snippet.

Understand the aforementioned issue is in a remote dependency, but couldn't this be easily be replaced if the author isn't responsive?

The author replied a while ago now. The issue is that the RFC6570 contradicts itself. We've reached out to the author of the RFC but no conclusive resolution has been reached yet. I'll try nudging the authors

Sorry this is taking so long, it's a very specific issue.

URL templating seems a pretty small piece of functionality to halt the entire sdk up for.. is there not just an alternative if you aren't getting responses. A library of this magnitude shouldn't be held hostage by a smaller dependency - imagine if the dependency has security issues and needs patching 😬

[baywet]

again, at this point we haven't established the library is defective but that the RFC is unclear on this very specific character ' and whether it should be percent encoded or not.
Depending on the answer from the spec authors we can fix it a number of different ways:

• If the RFC authors confirm ' doesn't need to be encoded, we can PR the library this SDK depends on. I already have an agreement about this in an email conversation with the library's author.
• Alternatively if the authors confirm ' doesn't need to be encoded and we don't get prompt support from the library's author, we can replace that dependency as you're pointing out.
• If the authors instruct ' to be encoded, we can encode it in the generation process (template) and then decode it at runtime before sending it to the service (OData expects it decoded).

Right now the impact on the SDK itself it limited to OData functions with string/date parameters, which is probably less than 5% of the overall API surface.

Now, I understand this impacts your ability to move forward, may I suggest the following workaround to unblock you for the time being?

searchString := "foo"
requestBuilder := search.NewSearchWithQRequestBuilder("https://graph.microsoft.com/v1.0/drives/"+driveId+"/microsoft.graph.search(q=%27"+seachString+"%27)", requestAdapter)

drive, err := requestBuilder.Get(&search.SearchWithQRequestBuilderGetOptions{
   Options: abstractions.RequestOption[]{&kiotahttp.ParametersNameDecodingOptions{
       Enable: true,
       ParametersToDecode: []byte{'-', '.', '~', '$', '\''},
   },
})

// kiotahttp is an import of github.com/microsoft/kiota-http-go
// search is an import of microsoftgraph/msgraph-sdk-go/drives/item/searchwithq
// abstractions is an import of github.com/microsoft/kiota-abstractions-go

[baywet]

update on the single quote issue, it took longer than expected because the actual RFC contained a mistake. After clearing this with the authors we're in the process of issuing an errata for the RFC and I've opened a PR on the uri template lib

[baywet]

update, the URI template library PR has been merged. Put together #138 which will close this issue once merged.
Updating the SDK in your application will then be the only thing left to do :)
Thanks for your patience on the matter!