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

Store check constraints in the snapshot as Fluent API #15676

Closed
AndriySvyryd opened this issue May 9, 2019 · 6 comments · Fixed by #15688
Closed

Store check constraints in the snapshot as Fluent API #15676

AndriySvyryd opened this issue May 9, 2019 · 6 comments · Fixed by #15688
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@AndriySvyryd
Copy link
Member

No description provided.

@ajcvickers
Copy link
Contributor

@Muppets This is something that was missed in your PR. Would you consider sending a PR to rectify this?

@Muppets
Copy link
Contributor

Muppets commented May 10, 2019

@ajcvickers Sure thing. Are you able to provide some more details on it? Still learning my way around the code base. Any good examples where we do this already?

@AndriySvyryd
Copy link
Member Author

AndriySvyryd commented May 10, 2019

@Muppets You'd have to modify Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator to look for the annotation and generate Fluent API calls on the corresponding EntityTypeBuilder.
It would be easier to find the correct entity type if the check constraint annotation is stored on the entity type itself

Note there's already a disabled test for this - ModelSnapshotSqlServerTest.CheckConstraint_is_stored_in_snapshot_as_fluent_api()

@Muppets
Copy link
Contributor

Muppets commented May 10, 2019

@AndriySvyryd Raised a PR for that change. For this change, I just added a method to get check constraints by table and schema.

Moving a check constraint annotations dictionary to each entity type sounds like a good plan. I can probably rework that since I've had my fingers in it recently. Want me to raise a separate issue for it?

@AndriySvyryd
Copy link
Member Author

AndriySvyryd commented May 10, 2019

@Muppets It's up to you whether to do it in a separate PR or not. But note that the code in #15688 generates duplicate check constraints for entity types mapped to the same table (TPH or table splitting).

@Muppets
Copy link
Contributor

Muppets commented May 12, 2019

@AndriySvyryd Just to check I've got the scenario correct for the duplicate checks. I've added this test which adds a check constraint on a derived type. Right now, this fails as the current code generates duplicate HasCheckConstraint calls. Is this correct?

I'll start the work to refactoring check constraint annotations to be stored on the entity. I'll ping you if I have any questions.

@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label May 15, 2019
@AndriySvyryd AndriySvyryd removed their assignment May 15, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, 3.0.0-preview6 Jun 5, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0-preview6, 3.0.0 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants