Skip to content

[release/8.0] Respect the difference between null/empty for owned collections mapped to JSON#31831

Merged
ajcvickers merged 1 commit intorelease/8.0from
fix29348_after_cleanup
Oct 3, 2023
Merged

[release/8.0] Respect the difference between null/empty for owned collections mapped to JSON#31831
ajcvickers merged 1 commit intorelease/8.0from
fix29348_after_cleanup

Conversation

@maumar
Copy link
Copy Markdown
Contributor

@maumar maumar commented Sep 22, 2023

Fixes #29348
Fixes #31731

Description

A empty collection of related entities is different from a null collection in both the EF model and JSON. This change ensures we preserve the difference.

Customer impact

JSON collections that should be non-null and empty as instead null leading to app crashes.

How found

Customer reported and internal testing.

Regression

Yes.

Testing

Added test coverage.

Risk

Low.

@maumar
Copy link
Copy Markdown
Contributor Author

maumar commented Sep 22, 2023

@ajcvickers fyi

@ajcvickers ajcvickers force-pushed the fix29348_after_cleanup branch from d3115c7 to 14d4469 Compare October 2, 2023 19:12
@ajcvickers ajcvickers changed the title fix to #29348 - JSON Columns produce NULL value for empty collections [release/8.0] Respect the difference between null/empty for owned collections mapped to JSON Oct 2, 2023
@ajcvickers ajcvickers marked this pull request as ready for review October 2, 2023 19:13
@ajcvickers
Copy link
Copy Markdown
Contributor

@maumar built on your work and I think all cases are correct now.

@ajcvickers ajcvickers requested a review from a team October 2, 2023 19:14
@ajcvickers ajcvickers added this to the 8.0.0 milestone Oct 2, 2023
…lections mapped to JSON

Fixes #29348
Fixes #31731

Collaboration between @maumar and @ajcvickers

### Description

A empty collection of related entities is different from a null collection in both the EF model and JSON. This change ensures we preserve the difference.

### Customer impact

JSON collections that should be non-null and empty as instead null leading to app crashes.

### How found

Customer reported and internal testing.

### Regression

Yes.

### Testing

Added test coverage.

### Risk

Low.
@maumar maumar force-pushed the fix29348_after_cleanup branch from 14d4469 to 77bd25b Compare October 3, 2023 03:00
@maumar
Copy link
Copy Markdown
Contributor Author

maumar commented Oct 3, 2023

@ajcvickers changes look good, just added some cleanup in the tests, Github won't let me sign off on the pr, so you will have to push the button :)

@ajcvickers ajcvickers merged commit 52d56bb into release/8.0 Oct 3, 2023
@ajcvickers ajcvickers deleted the fix29348_after_cleanup branch October 3, 2023 09:21
}
await context.SaveChangesAsync();

var saved = context.Database.SqlQueryRaw<string>("select OwnedCollectionRoot from JsonEntitiesBasic where Id = 2").ToList();
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.

@maumar this needs to normalize quotes, e.g. fails on PG

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.

JSON: empty json collection is not correctly materialized, but rather returned as null JSON Columns produce NULL value for empty collections

3 participants