MoonStorm / FastCrud

fast .NET ORM for strongly typed people
MIT License
506 stars 128 forks source link

Breaking changes in PropertyMapping<T>? #177

Open the-black-wolf opened 2 years ago

the-black-wolf commented 2 years ago

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 8a9a54e5330885d56d14f7abecae457411019df3, 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 commented 2 years ago

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.

the-black-wolf commented 2 years ago

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 commented 1 year ago

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.