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

[3.1.1] Set ordinal key values for embedded owned collection entities #19100

Merged
merged 2 commits into from
Dec 10, 2019

Conversation

AndriySvyryd
Copy link
Member

@AndriySvyryd AndriySvyryd commented Nov 28, 2019

Fixes #18948

Description

Cosmos provider allows to save collections of owned entities as embedded documents in a JSON array property in the owner document without storing the key values explicitly. They are synthesized from the owner key and the ordinal in the collection. However we didn't synthesize the values when saving the entities, only when querying.

The fix is to synthesize the key values when saving the entities. And also consistently allow readonly properties to be set the current store values during SaveChanges to allow propagation of the new values.

Customer Impact

The bug leads to the newly saved entities having a different identity and causes duplication in the owned collections if queried after saving a new entity using the same context.

How found

Customer-reported

Test coverage

We did not have tests querying back the entities that were just saved using the same context.

Regression?

No

Risk

Low, only affects the Cosmos provider

@AndriySvyryd
Copy link
Member Author

AndriySvyryd commented Dec 2, 2019

Moving the key modification detection logic turned out to require more changes than I initially though, so filed #19135 to consider it later.

Consistently allow readonly properties to be set the current store values during SaveChanges to allow propagation of the new values.

Fixes #18948
@ajcvickers ajcvickers modified the milestones: 3.1.1, 3.1.x Dec 5, 2019
Copy link
Member

@smitpatel smitpatel left a comment

Choose a reason for hiding this comment

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

:shipit:

@smitpatel
Copy link
Member

Will this go in 3.1.1 or 3.1.2?

@ajcvickers
Copy link
Member

@smitpatel If Tactics approves, then it will go in 3.1.2. (3.1.1 is closed down now.)

@AndriySvyryd AndriySvyryd changed the title [3.1.1] Set ordinal key values for embedded owned collection entities [3.1.2] Set ordinal key values for embedded owned collection entities Dec 6, 2019
@smitpatel
Copy link
Member

Closed 3.1.1 milestone.

@ajcvickers ajcvickers changed the title [3.1.2] Set ordinal key values for embedded owned collection entities [3.1.1] Set ordinal key values for embedded owned collection entities Dec 10, 2019
@ajcvickers
Copy link
Member

@AndriySvyryd This is approved for 3.1.1 servicing. Please merge ASAP.

@AndriySvyryd AndriySvyryd merged commit f6200f9 into release/3.1 Dec 10, 2019
@AndriySvyryd AndriySvyryd deleted the Issue18948 branch December 10, 2019 19:09
@ajcvickers ajcvickers removed this from the 3.1.x milestone Dec 10, 2019
@jamshedd
Copy link
Member

Approved for 3.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants