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

Bulk: Adds SessionToken and ActivityId to item responses #2739

Merged
merged 3 commits into from
Sep 16, 2021

Conversation

ealsur
Copy link
Member

@ealsur ealsur commented Sep 16, 2021

Bulk operations, when un-packed from the Bulk response, were not wiring the Session Token and ActivityId into the respective resulting operations.

All the operations included in a Bulk response contain the same Session Token and ActivityId as they were all committed in the same transaction.

Closes #2738

@@ -56,6 +56,7 @@ public async Task CreateItemStream_WithBulk()
ResponseMessage result = await task;
Assert.AreEqual(HttpStatusCode.Created, result.StatusCode);
Assert.IsTrue(result.Headers.RequestCharge > 0);
Assert.IsNotNull(result.Headers.Session);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a test that does CRUD operations with and without bulk. Then compare the headers and make sure that both have the same headers filled?

Copy link
Member Author

Choose a reason for hiding this comment

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

But the headers would have different values?

Copy link
Contributor

@j82w j82w Sep 16, 2021

Choose a reason for hiding this comment

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

Just validate that it has the same keys in the headers. To avoid any missing headers in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

They will never have the exact same headers, we are not copying the collection of headers, but individual values that make sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should still have a test to validate and prevent any new headers from getting lost. If we have known headers that we are excluding then we can just skip those headers.

j82w
j82w previously approved these changes Sep 16, 2021
@ealsur ealsur changed the title Bulk: Adds SessionToken to item responses Bulk: Adds SessionToken and ActivityId to item responses Sep 16, 2021
@ealsur ealsur merged commit d215f13 into master Sep 16, 2021
@ealsur ealsur deleted the users/ealsur/bulksession branch September 16, 2021 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bulk: Not including session token in response
2 participants