Skip to content

Conversation

@ajcvickers
Copy link
Contributor

Fixes #31617

@ajcvickers ajcvickers requested a review from a team September 10, 2023 16:54
@ajcvickers
Copy link
Contributor Author

@roji Well, that was fun. Turs out there was a significant bug where the element type from the type mapping never made it to the IProperty. With that fixed, it looks like query can use IsPrimitiveCollection now.

@ajcvickers ajcvickers requested a review from roji September 11, 2023 18:02
Base automatically changed from 230910_ItsAHorse to release/8.0 September 11, 2023 18:38
/// See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> for more information and examples.
/// </para>
/// </remarks>
public class ElementMappingConvention : IModelFinalizingConvention
Copy link
Member

Choose a reason for hiding this comment

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

Merge this into type mapping convention

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AndriySvyryd What do you mean by "type mapping convention?"

Copy link
Member

Choose a reason for hiding this comment

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

Oh, looks like we've removed it already. This can also be calculated in Property, but this can be done post-8.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think this needs to get revisited after 8. Related #31417.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #31702.

VerifyError(
CoreStrings.IdentifyingRelationshipCycle("A -> B"),
modelBuilder);
VerifyError(CoreStrings.RelationshipCycle("B", "AId", "ValueConverter"), modelBuilder);
Copy link
Member

Choose a reason for hiding this comment

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

Why did the exception change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because type mapping now attempts to determine keys before getting to the validation, and this detects the same cycle and throws.

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.

Mapping string as primitive collection throws

4 participants