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

Breaking changes in PropertyMapping<T>? #177

Open
the-black-wolf opened this issue Apr 15, 2022 · 3 comments
Open

Breaking changes in PropertyMapping<T>? #177

the-black-wolf opened this issue Apr 15, 2022 · 3 comments

Comments

@the-black-wolf
Copy link

Hi,

upgrading to a new version just broke our code. The first issue is that IsPrimaryKey is now obsolete and will be removed, completely removing the way to dynamically discover a field that is a primary key. The new SetPrimaryKey method resolves setting the key as primary, but there is no way to retrieve which field is the key. This information is now hidden in a private _propertyRegistration we cannot access.

Another problem is that PropertyMapping no longer exposes the property Descriptor. It seems to have been removed in commit 8a9a54e, better said moved to aa registration namespace and hidden from developers.
This was a convenient way to directly access type descriptor directly from Mapping, not worrying about types in generic methods (see example below).

Generally, it has become very difficult to query the dynamically discovered ORM.
Things like this are no longer possible, to retrieve a PK value in a generic repo handler:

private static int? GetKeyValue(T c) => (int?)OrmConfiguration.GetDefaultEntityMapping<T>().GetProperties().First(p => p.IsPrimaryKey).Descriptor.GetValue(c);

There is a workaround, relying on POCO class discovery and fishing for [Key] attribute, but this seemed as more concrete solution as this is what the FstCrud uses to submit data.

@MoonStorm
Copy link
Owner

Yes, version 3.0 has introduced some breaking changes.

In the area you mentioned, I believe the goal was to remove the informational properties in favor of the fluent registration methods. Both in there just made a mess. But yeah, I agree with you, exposing the mapping info shouldn't go away. We'll have something done in the next minor release.

@MoonStorm MoonStorm added this to the 3.1 milestone Apr 15, 2022
@the-black-wolf
Copy link
Author

You can leave the fluent stuff as it was added (its good architecture choice), but just convert IsPrimaryKey to a getter only property, that will allow discovery and prevent properties from messing up the fluent chain. Do that for other stuff also.

@MoonStorm
Copy link
Owner

For now it's back in there but marked as Obsolete but without erroring. We'll find a way to bring them later in a different form.

@MoonStorm MoonStorm removed this from the 3.1 milestone Mar 9, 2023
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

No branches or pull requests

2 participants