Microsoft Graph Api batch update with 6.4.0

[dejan-kosak]

Hi, I am facing issue when migrating to Microsoft Graph API 6 (6.4.0 currently).

As proposed by @baywet in https://github.com/microsoftgraph/msgraph-sdk-java/issues/1845 I am opening new ticket.

I see issues with batch, when I am testing the code with mocked expected response (Same tests that were working with v5).

Client configuration is:

 @Bean
    public GraphServiceClient getGraphClient() {
        final ClientSecretCredential clientSecretCredential = new ClientSecretCredentialBuilder()
            .clientId(clientId)
            .clientSecret(clientSecret)
            .tenantId(tenantId)
            .build();

        GraphServiceClient graphClient = new GraphServiceClient(clientSecretCredential, scopes.split(","));

        // Required for overriding host for testing. In case of test, host is not blank and contain wiremock url.
        if (StringUtils.isNotBlank(host)) {
            graphClient.getRequestAdapter().setBaseUrl(host);

            // override that was done in v5
            // graphClient.setServiceRoot(host);
        }
        return graphClient;
    }

Simplified code

BatchRequestContentCollection batchRequestContent = new BatchRequestContentCollection(graphServiceClient);

// memberIds size is around 10
for (String memberId : memberIds) {
  RequestInformation ri = graphServiceClient
    .users()
    .byUserId(memberId)
    .toGetRequestInformation(requestConfiguration -> requestConfiguration.queryParameters.expand = new String[]{"manager"});

  // overriding baseurl to be able to run test
  // without this I get "AADSTS7000215: Invalid client secret provided."
  ri.pathParameters.put("baseurl", graphServiceClient.getRequestAdapter().getBaseUrl());

  batchRequestContent.addBatchRequestStep(ri);
}

// this line is causing code to block execution until timeout
BatchResponseContentCollection responseContent = graphServiceClient.getBatchRequestBuilder().post(batchRequestContent, null);

1. As you can see from the code, my first question is why I have to override "baseurl" in such ugly way? If I don't, request will not be sent to my mocked endpoint. Instead it will fetch the regular one, returning "AADSTS7000215: Invalid client secret provided.". Note: other operations does not require that. i.e. GroupCollectionResponse groupsCollection = graphServiceClient.groups().get(requestConfiguration -> ...); works fine.

2. When I override "baseurl", I am able to get to last line of my code, where batch is executed, but processing is for some reason blocked until my test timeout's after 120s.

Am I doing something wrong here? Thanks!

[Ndiritu]

(Triage notes) Hacky baseUrl override happening because we create & expose a new request adapter instance rather than expose the base request adapter via getRequestAdapter(). Is this by design that we only want the baseURL to be set during GraphServiceClient initialization and not anytime after? @baywet @andrueastman

[baywet]

@dejan-kosak Thanks for creating this new issue. Can you please share the baseURL value BEFORE you override it? And what you override it to? Also it'd be interesting to trace which component is adding the latency if you can step through.

@Ndiritu this is just keeping a copy of the reference, but everything should be pointing to the same instance under the hood, unless you can point me to a place where a request adapter is newed up? As you can see the reference is passed here and then along the chain of request builders as the consumer uses the fluent API.

[dejan-kosak]

@baywet "baseurl" value, before override is "https://graph.microsoft.com/v1.0", after it is my mock address "http://localhost:3013".

I can trace it down to https://github.com/microsoftgraph/msgraph-sdk-java-core/blob/94093eaa875c0f4088e0be6457358b51638d379f/src/main/java/com/microsoft/graph/core/content/BatchRequestContent.java#L177. Return statement at line 178 is not reached.

[baywet]

Thanks for the additional information. It's extremely strange it hangs there since:

1. those are native java APIs
2. there's no inversion of control (the stream data is not being pulled, it's already in memory) There's probably something to look at in terms of the different stream behaviours and how they might run into lock contention or a similar problem when they are missing some initialization settings @ndiritu

@dejan-kosak can you please provide more context about the runtime (which jvm, which version, etc...) please?

[baywet]

yep... buffer is probably filling up

[dejan-kosak]

I am using

IMPLEMENTOR="Amazon.com Inc."
IMPLEMENTOR_VERSION="Corretto-21.0.0.35.1"
JAVA_RUNTIME_VERSION="21+35-LTS"
JAVA_VERSION="21"
OS_ARCH="aarch64"
OS_NAME="Darwin"

[baywet]

Thanks, out of curiosity: were you using v5 before? if so were you using batching? and if so, were you seeing back-pressure behaviour in that implementation?

[dejan-kosak]

We are using batching with v5 at the moment, yes. No issues seen there.

[Ndiritu]

More debugging insights https://github.com/microsoftgraph/msgraph-sdk-java-core/issues/1537

[baywet]

Thanks for the additional information. In the previous implementation, the batch task implementation was writing straight to the network stream. Which probably means it wasn't experiencing back-pressure (or maybe only when network was slow/unstable). https://github.com/microsoftgraph/msgraph-sdk-java-core/blob/019f895ccd4ee2d59c6013e823679b802aaf9503/src/main/java/com/microsoft/graph/http/CoreHttpProvider.java#L334

I'm going to transfer this issue to the core repo because this is where the code is located.

@Ndiritu when you have a couple of minutes, can you explore tweaking the buffer size in the code linked here ?

[dejan-kosak]

Any news on the topic? Thanks.

[yaojiqunaer]

PipedInputStream in = new PipedInputStream(outputStream.size() + 1);

Is there a way to fix it? I don’t want to overwrite it with another class. @Ndiritu