nhibernate / NHibernate.AspNetCore.Identity

ASP.NET Core Identity Provider for NHibernate
GNU Lesser General Public License v2.1
64 stars 13 forks source link

Allow customization of IdentityRole/User #5

Closed RianFuro closed 6 years ago

RianFuro commented 6 years ago

UserStore is now generic over TUser and TRole to allow application specific classes of IdentityUser and IdentityRole.

Replaced the XML mapping file for NHibernate with mapping by code classes. This allows users to extend from the mapping classes to provide their own custom mapping for the respective IdentityUser and IdentityRole classes.

Provided an extension method to NHibernate's configuration class to easily add the defined mappings to the user's configuration.

Closes #4


Changes turned out to be rather big. Need to adapt README to reflect new possibilities, but I'd rather only do that after getting green light that everything else is fine :) Couldn't run the unit tests on a whim since they run against a preconfigured postgres db and I didn't really want to set one up just for that... testing by hand turned out alright, could be I missed something though.

RianFuro commented 6 years ago

Just noticed, I changed the naming convention by switching to the mapping by code scheme, will fix that one up. Btw, is there a reason you've deviated from how AspNet Core is naming these tables by default?

beginor commented 6 years ago

wow, what a huge PR! I will review it asap, and if all the tests pass, I will release new version to nuget

beginor commented 6 years ago

As these table names, I use postgresql, which is not case sensitive, if I flow the default table names,it will be aspnetuserroles, aspnetuserlogins, this is not easy to read and use, so I prefer to use _ to split charater in database。

fredericDelaporte commented 6 years ago

if all the tests pass

Having some CI enabled would be better. @beginor, tell me if you need to have by example AppVeyor activated for running on this repository. (You would of course still need to provide an adequate appveyor.yml file. NHibernate core repository also uses TeamCity and Travis. But we rather avoid adding new builds to the former (TC). And for the later (Travis), I have not checked yet how to add a new repository to it.)

beginor commented 6 years ago

I have some experience with CI using gitlab, but I am not familiar with AppVeor. It would be very great if you provide an example of AppVeor, thanks.

beginor commented 6 years ago

@RianFuro Thanks for your pull request! But I will change your mapping by code part to postgres, and I will provide mapping to other database (sql server) later, since we can not provide one mapping for all databases.

RianFuro commented 6 years ago

Hmm... I believe the mapping is actually database agnostic, except for the naming convention. Also while we're on the topic of naming, I believe there's an advantage to stick to Microsofts default naming in this case, since it helps people migrating to this package if they so desire.

I would rather suggest switching to a convention-based approach. FluentNHibernate does that quite nicely and clearly separates the data mapping from naming conventions. However I'm not sure if tying the package to FluentNHibernate is the best idea... Alternatively something like this would resemble that approach.

As for your specific problem with postgres, it's just that postgres is case-sensitive by default and needs quotes around columns and tables to recognize case-sensitive naming. I would suggest using something like this instead on the consuming side if necessary and keep the mapping as agnostic as possible. I actually wonder why NHibernate doesn't just quote everything by default as it's much safer, but I digress.

fredericDelaporte commented 6 years ago

I actually wonder why NHibernate doesn't just quote everything by default as it's much safer

Quoting semantic varies between databases. Some gets case sensitive with quoting (Oracle I believe) while others may not (SQL Server case sensitivity on names is not changed by quoting, it only follows the collation of system tables, ...), ... Better let the user explicitly choosing to quote or not (by using back-tilt ( ` ) in mappings for being database agnostic, NHibernate will translate them to target database quoting).

fredericDelaporte commented 6 years ago

I have some experience with CI using gitlab, but I am not familiar with AppVeor. It would be very great if you provide an example of AppVeor, thanks.

There are some such files in NHibernate other repositories:

I have added this repository to AppVeyor, setup in order to build only branches or PR having an appveyor.yml file. So you can experiment with it in a PR without any consequences. There are more settings that you should be able to access here with your GitHub account (selecting NHibernate as account). But normally you should not need to change anything more there, most settings being settable in appveyor.yml (and ignored in the presence of such a file).