specifying .select causes a 400 : Bad Request

[Krisoblucki-okta]

Expected behavior

Specifying .select(FIELDS_TO_RETURN) should return a specific set of fields on each request to post/get/patch

Actual behavior

400 Bad Request the $select field appears twice in the request url

Steps to reproduce the behavior

IUserCollectionRequest request = graphServiceClient.users().buildRequest().select(FIELDS_TO_RETURN);
return request.post(user);

[Krisoblucki-okta]

The problem is that getRequestUrl is called multiple times and in somewhat unfortunate places. The fix is to:

1. add a getBaseRequestUrl method in IHttpRequest first
2. rename the requestUrl variable in BaseRequest to baseRequestUrl and also in the constructor and use that in the few places it is used now in conjunction with a local variable with the same name :|.
3. implement getBaseRequestUrl in BaseRequest and return the plain url without any functions or options
4. call getBaseRequestUrl instead of getRequestUrl in the 150 some odd Base*Request.java generated classes

I am more than happy to do this and submit a PR, what is the process of changing the base for the generated files?

[Krisoblucki-okta]

The changes for 1-3 above #197

[NakulSabharwal]

Hi, thanks for providing steps using which I am able to repro your issue. Will review your PR and look into its solution that why select statement is being appended twice.

[Krisoblucki-okta]

@NakulSabharwal I also have a change to the MSGraph-SDK-Code-Generator but it's not clear to me what the steps are to satisfy the dependencies. The changes in this PR would have to go in first, because the generated code would rely on these changes.

For example BaseEntityCollectionRequest.java.tt would look like this
`

public void post(final <#=TypeName(c)#> new<#=TypeName(c)#>, final ICallback<<#=TypeName(c)#>> callback) {
    final String requestUrl = getBaseRequest().getBaseRequestUrl().toString();
    new <#=TypeRequestBuilder(c)#>(requestUrl, getBaseRequest().getClient(), /* Options */ null)
        .buildRequest(getBaseRequest().getOptions())
        .post(new<#=TypeName(c)#>, callback);
}

public <#=TypeName(c)#> post(final <#=TypeName(c)#> new<#=TypeName(c)#>) throws ClientException {
    final String requestUrl = getBaseRequest().getBaseRequestUrl().toString();
    return new <#=TypeRequestBuilder(c)#>(requestUrl, getBaseRequest().getClient(), /* Options */ null)
        .buildRequest(getBaseRequest().getOptions())
        .post(new<#=TypeName(c)#>);
}

`

[NakulSabharwal]

Since this statement final String requestUrl = getBaseRequest().getBaseUrl(); appends once and then ...buildRequest(getBaseRequest().getOptions()) .post(newUser); appends again so I got your solution but instead we can add a function only in BaseRequest.java -

public String getBaseRequestUrl() {
	return this.requestUrl;
} 

Then in MSGraph-SDK-Code-Generator we can have
BaseEntityCollectionRequest -

public void post(final <#=TypeName(c)#> new<#=TypeName(c)#>, finaI Callback<<#=TypeName(c)#>> callback) {
    final String requestUrl = getBaseRequest().getBaseRequestUrl();
    new <#=TypeRequestBuilder(c)#>(requestUrl, getBaseRequest().getClient(), /* Options */ null)
        .buildRequest(getBaseRequest().getOptions())
        .post(new<#=TypeName(c)#>, callback);
}

public <#=TypeName(c)#> post(final <#=TypeName(c)#> new<#=TypeName(c)#>) throws ClientException {
    final String requestUrl = getBaseRequest().getBaseRequestUrl();
    return new <#=TypeRequestBuilder(c)#>(requestUrl, getBaseRequest().getClient(), /* Options */ null)
        .buildRequest(getBaseRequest().getOptions())
        .post(new<#=TypeName(c)#>);
}

This way first the sdk change will go in and then generator change will go in.
Also, the please rebase the PR which you have provided.

[Krisoblucki-okta]

@NakulSabharwal apologies for this taking for ever, it fell off my radar. I rebased the PR, is there anything else that you need from me on this?

[baywet]

thanks for your patience on the matter. I just authored microsoftgraph/MSGraph-SDK-Code-Generator#281 which fixes the issue by passing only the header options and you can see the resulting code changes in #445 .
This issue will be closed when the PR gets merged.