Add option to make Content-ID header optional #2713

quanterion · 2023-08-01T12:31:27Z

Issues

*This pull request adds ability to read OData 2.0 batch requests that do not require Content-ID header

Description

Add options to disable throwing exceptions if no Content-ID header is present in chgangeset

Checklist (Uncheck if it is not completed)

Test cases added
Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

xuzhg · 2023-08-02T18:22:04Z

src/Microsoft.OData.Core/MultipartMixed/ODataMultipartMixedBatchReader.cs

+ }
+ else
+ {
+ this.currentContentId = Guid.NewGuid().ToString();


Is it better to change the type of 'CurrentContentId' to System.Guid?

currentContentId is an arbitrary value of user-provided header. I assign it to guid just for specific case

xuzhg · 2023-08-02T18:23:24Z

src/Microsoft.OData.Core/ODataMessageReaderSettings.cs

@@ -235,6 +236,12 @@ public ODataMessageQuotas MessageQuotas
 /// </summary>
 internal bool ThrowOnUndeclaredPropertyForNonOpenType { get; private set; }

+ /// <summary>
+ /// Require Content-Id header in changesets
+ /// If turned off allows to read OData 2.0 requests without Content-Id header present.


What do you mean about "OData 2.0" ? It's a OData spec version number?

@xuzhg @mikepizzo Do we really still support pre 4.0 versions on ODL 7.x?

To be precise this small addition not to support 4.0 metadata, just for reading requests

xuzhg · 2023-08-02T18:24:43Z

test/FunctionalTests/Microsoft.OData.Core.Tests/ODataMultipartMixedBatchReaderTests.cs

+ [Fact]
+ public void ReadMultipartMixedBatchRequestWthoutContentId()
+ {
+ var payload = Regex.Replace(batchRequestPayload, "Content-ID: .", "");


we can't use the String.Replace to replace it?

Because I want to remove all Content-ID headers, not just one (in regex symbol . means any symbol)

src/Microsoft.OData.Core/MultipartMixed/ODataMultipartMixedBatchReader.cs

src/Microsoft.OData.Core/ODataMessageReaderSettings.cs

src/Microsoft.OData.Core/ValidationKinds.cs

test/FunctionalTests/Microsoft.OData.Core.Tests/ODataMultipartMixedBatchReaderTests.cs

quanterion · 2023-09-07T09:17:01Z

@habbes Address comments, please have a look

quanterion · 2023-09-26T07:58:03Z

@habbes Gentle ping

habbes · 2024-01-24T13:16:58Z

@quanterion looks good to me. Kindly rebase your branch with master.

habbes

Looks good to me, since the default for the new ThrowIfMissingContentIdChangeset option preserves the existing behaviour of throwing an exception. Kindly resolve the conflicts with the master branch.

habbes · 2024-01-24T13:20:45Z

I've fixed the conflicts with the master branch.

pull-request-quantifier-deprecated · 2024-01-24T14:03:11Z

This PR has 109 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Medium
Size       : +97 -12
Percentile : 41.8%

Total files changed: 9

Change summary by file extension:
.cs : +86 -5
.txt : +11 -7

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detected.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

add option to make Content-ID header optional

5bbd4f4

pull-request-quantifier-deprecated bot added the Extra Small label Aug 1, 2023

ElizabethOkerio requested review from gathogojr, KenitoInc, habbes, xuzhg and ElizabethOkerio August 2, 2023 10:04

xuzhg reviewed Aug 2, 2023

View reviewed changes

xuzhg previously approved these changes Aug 14, 2023

View reviewed changes

habbes requested changes Sep 5, 2023

View reviewed changes

habbes reviewed Sep 5, 2023

View reviewed changes

test/FunctionalTests/Microsoft.OData.Core.Tests/ODataMultipartMixedBatchReaderTests.cs Show resolved Hide resolved

habbes reviewed Sep 5, 2023

View reviewed changes

test/FunctionalTests/Microsoft.OData.Core.Tests/ODataMultipartMixedBatchReaderTests.cs Outdated Show resolved Hide resolved

address comments

b307092

quanterion dismissed xuzhg’s stale review via b307092 September 6, 2023 08:43

pull-request-quantifier-deprecated bot added Small and removed Extra Small labels Sep 6, 2023

fix tests

b36aa74

habbes reviewed Jan 24, 2024

View reviewed changes

Merge branch 'master' into rkolesnikov/optional-conent-id-header

44fb308

habbes approved these changes Jan 24, 2024

View reviewed changes

pull-request-quantifier-deprecated bot added Medium and removed Small labels Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to make Content-ID header optional #2713

Add option to make Content-ID header optional #2713

quanterion commented Aug 1, 2023

xuzhg Aug 2, 2023

quanterion Aug 2, 2023

xuzhg Aug 2, 2023

quanterion Aug 2, 2023

KenitoInc Aug 8, 2023

quanterion Aug 9, 2023

xuzhg Aug 2, 2023

quanterion Aug 2, 2023 •

edited

Loading

quanterion commented Sep 7, 2023

quanterion commented Sep 26, 2023

habbes commented Jan 24, 2024

habbes left a comment •

edited

Loading

habbes commented Jan 24, 2024

pull-request-quantifier-deprecated bot commented Jan 24, 2024

What can I do to optimize my changes

How to interpret the change counts in git diff output

Add option to make Content-ID header optional #2713

Are you sure you want to change the base?

Add option to make Content-ID header optional #2713

Conversation

quanterion commented Aug 1, 2023

Issues

Description

Checklist (Uncheck if it is not completed)

Additional work necessary

xuzhg Aug 2, 2023

Choose a reason for hiding this comment

quanterion Aug 2, 2023

Choose a reason for hiding this comment

xuzhg Aug 2, 2023

Choose a reason for hiding this comment

quanterion Aug 2, 2023

Choose a reason for hiding this comment

KenitoInc Aug 8, 2023

Choose a reason for hiding this comment

quanterion Aug 9, 2023

Choose a reason for hiding this comment

xuzhg Aug 2, 2023

Choose a reason for hiding this comment

quanterion Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

quanterion commented Sep 7, 2023

quanterion commented Sep 26, 2023

habbes commented Jan 24, 2024

habbes left a comment • edited Loading

Choose a reason for hiding this comment

habbes commented Jan 24, 2024

pull-request-quantifier-deprecated bot commented Jan 24, 2024

What can I do to optimize my changes

How to interpret the change counts in git diff output

quanterion Aug 2, 2023 •

edited

Loading

habbes left a comment •

edited

Loading