Skip to content

Batch Request Enhancements #56

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

Jump to bottom
Merged
andrueastman merged 1 commit into from
Oct 28, 2019
Merged

Batch Request Enhancements #56

andrueastman merged 1 commit into from
Oct 28, 2019

Conversation

@andrueastman
Copy link
Contributor

@andrueastman andrueastman commented Oct 24, 2019

This PR closes #51

Changes proposed in this pull request

  1. Return ids on creating a batchRequestStep without providing one i.e. making a call like
string userRequestId = batchRequestContent.AddBatchRequestStep(userRequest);
  1. Allow de-serializing of responseContent to known type on getting response from ID
User user = await returnedResponse.GetResponseByIdAsync<User>(userRequestId);

This therefore means a user can perform the following scenario and its variants

// Get request objects from graph
IUserRequest userRequest = graphClient.Me.Request();
IUserEventsCollectionRequest eventsRequest = graphClient.Me.Events.Request();

BatchRequestContent batchRequestContent = new BatchRequestContent();

// Add the batchRequest steps and get back the ids for the steps
var userRequestId = batchRequestContent.AddBatchRequestStep(userRequest);
var eventsRequestId = batchRequestContent.AddBatchRequestStep(eventsRequest);

// send out request
BatchResponseContent returnedResponse = await graphClient.Batch.Request().PostAsync(batchRequestContent);

// deserialize response based on known return type
User user = await returnedResponse.GetResponseByIdAsync<User>(userRequestId);
UserEventsCollectionResponse events = await returnedResponse.GetResponseByIdAsync<UserEventsCollectionResponse>(eventsRequestId);

Batch Request Enhancements
47171da 
- Return ids on creating step without providing one
- Allow deseriaizing of requests to known type on geting response from ID
- Added tests
@andrueastman andrueastman self-assigned this Oct 24, 2019
darrelmiller
darrelmiller reviewed Oct 27, 2019
View reviewed changes
src/Microsoft.Graph.Core/Requests/Content/BatchResponseContent.cs
{
var responseHandler = new ResponseHandler(new Serializer());

if (!httpResponseMessage.IsSuccessStatusCode)
Copy link
Contributor

@darrelmiller darrelmiller Oct 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to discuss why this isn't being done inside the response handler. The code you have written is good, but we shouldn't need to duplicate this code here in BatchResponseContent. We can fix this later without a breaking change.

@andrueastman andrueastman merged commit 3c24714 into dev Oct 28, 2019
@andrueastman andrueastman deleted the andrueastman/bath_enhancements branch October 28, 2019 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MIchaelMainer MIchaelMainer Awaiting requested review from MIchaelMainer

@peombwa peombwa Awaiting requested review from peombwa

@irvinesunday irvinesunday Awaiting requested review from irvinesunday

1 more reviewer

@darrelmiller darrelmiller darrelmiller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@andrueastman andrueastman

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Batch pattern in 1.19.0-preview.1 suggested changes to help with referencing responses

3 participants

@andrueastman @darrelmiller