Replace concept of User in new User API, and more

I’m personally mixed on the general proposal. I’m ok with removing an unnecessary class, but not sure about the names. Basically I’m wondering if we shouldn’t call User the new UserConfiguration class.
I understand that you call it UserConfiguration because it’s a ConfigurationSource, but I’m wondering if it wouldn’t be easier to understand it as a User object for semantic reason.
The first example I have is that UserManager#create returns a UserConfiguration which I find already a bit confusing.

I’m a bit afraid that we’d need the concept of User for future APIs and to be stuck again because we need to break again some APIs because we don’t have it. Now I don’t have real examples of usage right now.

That’s exactly what I proposed on the #xwiki chat last week. It means having a UserResolver return different User objects depending on whether you’re resolving for a given CS or another (and that feels weird). Otherwise it makes the API really not good and forcing to resolve a User to then resolve a configuration, and the User object has no data at all which shows it’s useless and make it overly complex to use for no advantage.

If you check all the places I’ve modified already to use the new User API you’ll see that we never need the User object. So we just retrieve it to get the config data and then we discard it. I haven’t yet found a single use case (even for the user in the xwiki context, we just need the user reference) where we’d need a User object.

The only place where having a User object makes sense is the createUser() API but that represents 1 use case out of thousands and you don’t want to penalize 99% of the use cases for just this one. Moreoever even with createUser() you’re not going to do anything with the User object… since there’s nothing to do with it.

An object is good when it needs to hold state. Gathering all the user configs and storing them in memory in a User object is just stupid, because in all cases you just need to get one or a few config options. It’s much better to do what we do already, ie have a config cache. (In addition it would duplicate the cache)

If you don’t need to hold state then what you need is a Manager/Service. This is what UserManager is about.

I went through the same mental process as you both did… :slight_smile: And it came as a shock to me that se don’t need the User object. What we need now is someone to find a counter example use case where we need it.

Actually there is stuff in the User class which don’t make any sense in the UserConfiguration: user personal info like the name and the mail.

I read again the proposal, and actually I haven’t understood that we wouldn’t have a resolver for UserReference, so this looks overly complicated to me:

I wouldn’t know which one to use to access some basic user informations.

They make sense to me, that’s configuration. You can change your mail, you can change your name. The do not represent the user. The user is represented by its reference.

no no we do have a UserReferenceResolver ofc.

I think you keep mixing UserResolver with UserReferenceResolver :wink:

No it’s definitely not configuration and I can assure you no user will understand why the name of a user have to be gather from UserConfiguration. Anything for which inherit does not make sense have nothing to so in UserConfiguration for me.

1 Like

I didn’t talk about UserReferenceResolver but a resolver for UserReference, so a resolver that would take a reference as input: so I was talking about the UserResolver.

A resolver for UserReference is called a UserReferenceResolver :slight_smile:

vocabulary issue, I was talking about UserResolver

So UserResolver doesn’t exist because there’s no more User. So what we have is a UserConfiguration resolver instead. Either statically in UserManager (which makes all the CRUD operations in one place) or dynamically with a real UserConfigurationResolver as we had with UserResolver. I’m fine with both. For the Script Service API it’s much nicer without the resolve() call but we could still do that even with a resolver I think. But that doesn’t change the User vs UserConfiguration problem.

For me it is. And I don’t see the point of making it more complex than it should be, i.e. making the API a lot more complex because we wouldn’t the ability to override the name or email if not set. We also need this for Guest and SuperAdmin. Also if the user has no name we still need a name for it ('Unnamed" for example or “Unknown User”). Same for mail you can have the use case of defining a global catch all email for your wiki for some use cases. It’s not because you have this ability that you’re going to use it. Right now you can already do this, by creating a first_name xproperty in XWikiPreferences for example, yet it’s not a problem.

So I see it quite interesting because:

  • It unifies the API (same API for all config data). You don’t need think about whether a given property can be overridden or not
  • You can find use cases where you’d want to have the override.
  • It’s when you call that APIs that you decide if you want to get the overrides or not, so depending on your use cases you can decide. But you have the power to have it or not.

I have tried hard to keep the User object over the whole weekend and last week (I spent at least 10 hours just thinking about it and trying various APIs). And I couldn’t find a solution that made sense. There’s just no point of having a User object with state in memory when all you want to get are config data for it. It’s just not efficient and doubles the caches.

What I’m sure of, is that we need an easy API to get user data without getting a User object since that’s the main use case and the User object gets in the way. Whether we also need some User object for some specific case, I don’t know.

So I’m going to reformulate this proposal with the User/UserResolver but with UserConfiguration and UserConfigurationResolver so that at least we have an API to get user data since that’s the main need for now.

The only consequence it would mean for the future, is that a future User object would not need to hold config data since there’ll be the UserConfigurationResolver for them.


I’m really not a big fan of using UserConfiguration instead of User, but I guess we can change it later if we need to since it’ll be Unstable, and if you’re stuck because of this, better to move on. so I’m -0 for it

Not a big fan either but as long as there is no more debatable entry points like UserManager and create() that return a UserConfiguration it should not be a big blocker for the future so fine with me.

Instead of UserConfiguration I prefer to use UserProperties for 3 reasons:

  • UserConfiguration is already used and it follows the current best practice naming pattern of being the configuration for the User module.
  • The API (inherited from CS) is getProperty() so we’re retrieving user properties. It’s consistent.
  • It would solve @tmortagne’s point above about some properties not being configuration. For example “first name” is a user property for sure. Whether it’s inheritable or not is another matter but from an API POV it’s not a problem to ask for it through UserProperties.