Skip to content

Fix reference handling in $batch - #482

Merged
xuzhg merged 4 commits into
mainfrom
FixBatchReferences
Feb 15, 2022
Merged

Fix reference handling in $batch#482
xuzhg merged 4 commits into
mainfrom
FixBatchReferences

Conversation

@mikepizzo

@mikepizzo mikepizzo commented Feb 11, 2022

Copy link
Copy Markdown
Contributor

Fixes #360
Fixes #481
Fixes #365

-Handle relative references in $batch by making URL absolute before copying.
-Allow requests within a batch to reference other requests within the batch whether or not they are in the same changeset.
-Added a using for ODataMessageReader in UnbufferedODataBatchHandler.

Work remaining:
[x] Add Tests
[x] Fix Reference Handling in UnbufferedODataBatchHandler.

…opying.

-Allow requests within a batch to reference other requests within the batch whether or not they are in the same changeset.
-Added a using for ODataMessageReader in UnbufferedODataBatchHandler.
@mikepizzo
mikepizzo marked this pull request as ready for review February 11, 2022 17:28
Comment thread src/Microsoft.AspNetCore.OData/Microsoft.AspNetCore.OData.xml
Fix accidental removal of IsDateOnly and IsTimeOnly signatures from Microsoft.AspNetCore.OData.xml
@mikepizzo
mikepizzo requested review from gathogojr and xuzhg February 11, 2022 18:30
List<ODataBatchRequestItem> requests = new List<ODataBatchRequestItem>();
ODataBatchReader batchReader = await reader.CreateODataBatchReaderAsync().ConfigureAwait(false);
Guid batchId = Guid.NewGuid();
Dictionary<string, string> contentToLocationMapping = new Dictionary<string, string>();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Dictionary<string, string> contentToLocationMapping = new Dictionary<string, string>();
Dictionary<string, string> contentIdToLocationMapping = new Dictionary<string, string>();

@gathogojr gathogojr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a minor comment otherwise LGTM :shipit:

@xuzhg xuzhg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:shipit:

List<ODataBatchRequestItem> requests = new List<ODataBatchRequestItem>();
ODataBatchReader batchReader = await reader.CreateODataBatchReaderAsync().ConfigureAwait(false);
Guid batchId = Guid.NewGuid();
Dictionary<string, string> contentToLocationMapping = new Dictionary<string, string>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why don't we create the dictionary in the "class" itself?

requests.Add(new ChangeSetRequestItem(changeSetContexts));

ChangeSetRequestItem requestItem = new ChangeSetRequestItem(changeSetContexts);
requestItem.ContentIdToLocationMapping = contentToLocationMapping;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ContentIdToLocationMapping

Then, we don't need to assign a dic outside?
Then, we can remove the "set" for a dictionary property?

@xuzhg
xuzhg merged commit 27ee30c into main Feb 15, 2022
@gathogojr
gathogojr deleted the FixBatchReferences branch February 16, 2022 05:04
kakone pushed a commit to kakone/AspNetCoreOData that referenced this pull request Apr 3, 2022
* -Handle relative references in $batch by making URL absolute before copying.
-Allow requests within a batch to reference other requests within the batch whether or not they are in the same changeset.
-Added a using for ODataMessageReader in UnbufferedODataBatchHandler.

* E2E tests to verify changes

* Tests for UnbufferedODataBatchHandler

* Update Microsoft.AspNetCore.OData.xml

Fix accidental removal of IsDateOnly and IsTimeOnly signatures from Microsoft.AspNetCore.OData.xml

Co-authored-by: John Gathogo <john.gathogo@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid Uri for $ref request in a batch request with single changeset Batch reference new entity error

3 participants