JudahGabriel / RavenDB.Identity

RavenDB Identity provider for ASP.NET Core. Let RavenDB manage your users and logins.
https://www.nuget.org/packages/RavenDB.Identity/1.0.0
MIT License
61 stars 29 forks source link

User Id configuration not consistent with RavenDB #31

Closed marcuslindblom closed 4 years ago

marcuslindblom commented 4 years ago

Is it possible to configure the id generation to auto generate ids rather then server generated? I Prefer users/1-A rather then users/0000000000000001074-A.

I use services.Configure<RavenIdentityOptions>(options => options.UserIdType = UserIdType.ServerGenerated); but according to the docs the id should be in the auto generated format rather then server generated.

Screenshot 2020-07-26 at 09 53 50

IMO RavenDB.Identity should be more consistent with how RavenDB works.

If I add users| to my Users class like this

public class User : Raven.Identity.IdentityUser {
    public User() {
        Id = "users|";
    }
}

I expect the Id to be users/1 not users/0000000000000001074-A but that does not work either?

JudahGabriel commented 4 years ago

Thanks for the feedback. I'll see what can be done.

If you configure Raven.Identity to use server-generated IDs, it sets the ID to "Users/", then let's Raven take care of the rest. Thus, "Users/0000000012-A" is coming from Raven.

marcuslindblom commented 4 years ago

I think setting Id = null would generate ids like users/1-A. Id = string.Empty will generate a GUID id. Id = "Users/" will generate Users/0000000012-A and Id = "Users|" will generate users/1. See docs here to make it more consistent with RavenDB. If you decide it's a good idea 💡

marcuslindblom commented 4 years ago

Without knowing the details maybe you could just remove the id handling and leave it to Raven?

If someone want, let's say username style they just do

store.Conventions.RegisterAsyncIdConvention<AppUser>(
    (dbname, user) =>
        Task.FromResult(string.Format("users/{0}", user.UserName)));

And if I want users/1 I just set Id = "users|" in my AppUser class.

JudahGabriel commented 4 years ago

Yeah, the ID generation used to be important part of this library. This library was originally created by someone else, and they used email-based IDs as a way to prevent duplicates.

Unfortunately, that doesn't work in a cluster, so we changed the behavior to use compare/exchange to prevent duplicates. But the ID generation stuck around. 🤷‍♀️

You may be right, perhaps ID generation is best left to the user. I'll push an update in the coming week to allow for that.

JudahGabriel commented 4 years ago

p.s. I've been using Raven since 1.0...and I never knew about the pipe ID thing? How did I miss that? 😮 Thanks for teaching me something new.

marcuslindblom commented 4 years ago

I'm happy to help 😀

cadilhac commented 4 years ago

p.s. I've been using Raven since 1.0...and I never knew about the pipe ID thing? How did I miss that? 😮 Thanks for teaching me something new.

I think it only got introduced in 4.0

JudahGabriel commented 4 years ago

I'll accept that explanation since it makes me appear less ignorant. 😉

JudahGabriel commented 4 years ago

Ok, I've published Raven.Identity v8.0.2 which rids us of the custom ID stuff. 🎉 We now just use whatever Id Raven assigns to the user, or what you've supplied when you create the user. (Including support for "users|".)

// ID will be "AppUsers/1-a"; Raven's default.
var user = new AppUser(...) { Id = null };
userManager.CreateAsync(user, ...);

// ID will be incrementing, e.g. "AppUsers/42"
var user = new AppUser(...) { Id = "AppUsers|" }; // cluster-wide increment
userManager.CreateAsync(user, ...);

// ID will be server-generated, e.g. "AppUsers/0000001-A"
var user = new AppUser(...) { Id = "AppUsers/" };
userManager.CreateAsync(user);

I've also update the migration to change existing IDs. Raven.Identity can work with old user IDs or new, doesn't matter - but you can run the migration if you want consistency in your user IDs.

And if folks really want to use email-based (or username-based) IDs, they can still do so using Raven's built-in Global Identifier Generation Conventions.

cadilhac commented 4 years ago

I see that you have version numbers. This would be great if you publish releases on github.

JudahGabriel commented 4 years ago

Those versions I refer to are Nuget package versions.

marcuslindblom commented 4 years ago

Great news and well done @JudahGabriel 👍🏻