Skip to content

[Internal] DTS: Refactors JSON property names into shared constant and changes etag constant#5818

Merged
Meghana-Palaparthi merged 1 commit intomasterfrom
users/Meghana-Palaparthi/DTS_Etag_rename
Apr 29, 2026
Merged

[Internal] DTS: Refactors JSON property names into shared constant and changes etag constant#5818
Meghana-Palaparthi merged 1 commit intomasterfrom
users/Meghana-Palaparthi/DTS_Etag_rename

Conversation

@Meghana-Palaparthi
Copy link
Copy Markdown
Contributor

Pull Request Template

Description

Extract all JSON field name string literals from DistributedTransactionSerializer into internal const string constants at the top of the class

Replace the literals in the serializer body with the new constants.

Update all test files that previously hardcoded these same strings in GetProperty/TryGetProperty calls to reference DistributedTransactionSerializer.<Constant>
instead — ensuring any future rename of a property automatically propagates to tests without a separate search-and-replace:

  • DistributedTransactionOperationResult.cs
  • DistributedTransactionSerializerTests.cs
  • DistributedTransactionCommitterTests.cs
  • DistributedTransactionTests.cs (emulator)

No behavioral changes. All 58 unit tests pass.

Additionally, updates the constant etag to ifMatch in the request body to align with the wire contract update.

Type of change

Please delete options that are not relevant.

  • [] Bug fix (non-breaking change which fixes an issue)
  • [] New feature (non-breaking change which adds functionality)
  • [✓] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [] This change requires a documentation update

Closing issues

To automatically close an issue: closes #IssueNumber

…onstant

Extract all JSON field name string literals from DistributedTransactionSerializer into `internal const string` constants at the top of the class

Replace the literals in the serializer body with the new constants.

Update all test files that previously hardcoded these same strings in `GetProperty`/`TryGetProperty` calls to reference `DistributedTransactionSerializer.<Constant>`
 instead — ensuring any future rename of a property automatically propagates to tests without a separate search-and-replace:
 - `DistributedTransactionOperationResult.cs`
 - `DistributedTransactionSerializerTests.cs`
 - `DistributedTransactionCommitterTests.cs`
 - `DistributedTransactionTests.cs` (emulator)

 No behavioral changes. All 58 unit tests pass.

Additionally, updates the constant `etag` to `ifMatch` in the request body to align with the wire contract update.
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
Successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines:
Successfully started running 1 pipeline(s).

@Meghana-Palaparthi Meghana-Palaparthi enabled auto-merge (squash) April 29, 2026 16:34
Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

I think the reason the issue I mentioned is not caught in the tests is because the tests are just checking with the constants that were introduced in this file.
How about updating the tests to actually check for our newly introduced constants with the values being returend from the actual response from the wire?
If we cannot do it yet, maybe checking the constants agains the values we were checking them before?

@kushagraThapar
Copy link
Copy Markdown
Member

I think the reason the issue I mentioned is not caught in the tests is because the tests are just checking with the constants that were introduced in this file. How about updating the tests to actually check for our newly introduced constants with the values being returend from the actual response from the wire? If we cannot do it yet, maybe checking the constants agains the values we were checking them before?

Synced up on this one, we are in process of adding new tests to our service and then we should be able to do end to end integration tests on these constants.
@Meghana-Palaparthi can you please add a todo on these tests to convert them to integration tests once your service tests are merged in and ready to be used, thanks!

Copy link
Copy Markdown
Member

@kushagraThapar kushagraThapar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @Meghana-Palaparthi

@Meghana-Palaparthi Meghana-Palaparthi merged commit 6cb33e3 into master Apr 29, 2026
33 checks passed
@Meghana-Palaparthi Meghana-Palaparthi deleted the users/Meghana-Palaparthi/DTS_Etag_rename branch April 29, 2026 18:08
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.

3 participants