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… 
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 
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
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.
Ok?
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:
-
UserConfigurationis 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.
Proposal:
// Get all user properties, with inheritance (internally using the "all" CS)
@Inject
@Named("all")
private UserProperties userProperties;
...
userProperties.resolve(CurrentUserReference.INSTANCE).getEditor();
// Get direct user properties (ie from XWikiUsers for the "document" implementation), without inheritance (internally using the "user" CS)
@Inject
@Named("user")
private UserProperties userProperties;
// Get user properties from the current space, with inheritance (internally using the "user" CS)
@Inject
@Named("space")
private UserProperties userProperties;
Note: it’s important to be able to get the UserPreferences instance at each level for the write use case (to be able to set properties, see my first post in this thread about the write API).
Scripting API:
// Get all user properties, with inheritance (internally using the "all" CS)
$services.user.allProperties.editor
// Get direct user properties (ie from XWikiUsers for the "document" implementation), without inheritance (internally using the "user" CS)
$services.user.directProperties.editor
// General API
$services.user.getProperties("<UserProperties hint>").editor
Documentation updated:
Hi,
(admittedly scanned rather superficially the discussion, but:)
The current proposal looks very twisted to me. The whole point about the new user API (from my POV) was to have Users as first class citizens in the XWiki API and stop handling them as consequences (objects in pages with properties, etc.) rather than design.
What was in my mind initially and what makes sense to me, after quickly reading the above is something like this:
- User class (first class citizen in the API). Probably better to be an interface, but example:
public class User {
private UserProperties properties;
private UserReference reference;
public getReference();
public get/setProperty(...) { properties.get/set(...) }
or
public getProperties() { return properties } <-- uses getter/setter on UserProperties instead of on User which also allows for typed getters
}
Note: UserProperties seems to have some known typed getters, but would probably also need an untyped getter/setter, for extensibility (AFAICS, it already does not have typed getters for all the default properties we have in the XWikiUsers xwiki class).
- UserManager: performs CRUD (create/get/save/delete) on User objects (also going into the UserProperties and saving to DB any changes that might have been done in the live UserProperties object).
interface UserManager {
User create(UserReference reference);
User get(UserReference reference);
void delete(UserReference reference);
void save(User user);
/* Optional, convenience/optimization methods: */
boolean exists(UserReference reference);
User getCurrent();
Object getProperty(UserReference reference, String propertyName);
void setProperty(UserReference, String propertyName, Object value);
Various implementations of UserManager would produce various implementations of User. Not sure we need a UserManagerProvider as well, in order to openly and easily accommodate for alternative and configurable UserManager implementations which would replace the default (XWikiDocument based) one or if we should just rely on the component overriding mechanism (which is a bit more intrusive).
UserConfiguration sounds really bad to me because the main things about a user’s data are his profile information (name, description, website, phone number, etc.). UserProperties sounds a bit better, indeed, and can also accept things that might be regarded as “configuration”, like what editor to use, etc.
Yes, technically, a user is mostly just a reference and a bunch of properties around it, but we could have said the same thing about Document or any other API citizen. You need a class to pull it all together and to also help the API callers better wrap their minds around them, on familiar terms. I’m sure we could be able to optimize any caching issues in the background. Again, I don’t see much of a difference between User and Document (XWikiDocument).
It has already been said that creating users is complicated/unnatural, but I’d also add the even getting a couple of users and putting them in a collection would result in getting a couple of UserProperties and losing them in the collection, since the UserProperties object does not seem to link back to the UserReference that it corresponds to. Sure, we could probably fix this with a new getter/setter, but it feels so awkward and it would have been much more appropriate to handle Users instead.
The entire resolver logic and the handling of superadmin/guest got me running around places while looking at the default implementation (https://github.com/xwiki/xwiki-platform/tree/master/xwiki-platform-core/xwiki-platform-user/xwiki-platform-user-default). Overall, I had the deep impression that it’s overly complicated.
I’m also -0. I’m not available to work on it, so I could not impose for more, as I would have otherwise leaned more towards -1.
I’m sure I’m missing things, as I did not sit on the topic enough (with respect to Vincent’s work so far). Just wanted to share my POV, as the current proposal looks unnecessarily complex to me and not natural to use. (…or maybe it’s just not what I had expected, don’t know). Hoping you might get some new ideas out of it.
Thanks for your work, Vincent. I’ve been waiting for a long time for a users API.
Thanks Edy. I’ll try to reply to the rest but there’s so much to say, I’m not sure I’ll reach the end and I’m quite tired at this time of the day I really appreciate that you took the time to look at it.
I think most of the issue stem from the fact that I didn’t state enough the requirements so I’ve started putting them at: https://design.xwiki.org/xwiki/bin/view/Proposal/UserAPI
Please be more specific and show me an example of using the API that is twisted. That will allow us to discuss on firm grounds.
The whole point of the User API is to be able to do anything related to users with this API (instead of being scattered in various places of the platform + be independent of the user store). The fact that there’s a User object or not is an implementation detail of the API. If we can do everything without a User object, I don’t see the problem. In other words, there’s nothing in the requirements that say “we must have a User object”.
This is missing Requirements 1, 6 and possibly 2 depending on how you implement it. For 1 and 6 it’s self explanatory. for requirement 2 it’s more complex to understand but this is what I’ve tried to explain at length on the #xwiki chat, in this thread and to Simon in a conf call Basically it boils down to being able to retrieve a single property without having to load all the properties (including fallbacks).
We have that already, check the UserProperties class which extends ConfigurationSource.
This is one possibility and it’s interesting. But it’s a lot less flexible than having discrete UserPropertiesResolvers since it forces to have as many UserManager component implementations as there are needs for the “create”, the “get”, the “delete”, etc. If you check what’s been implemented already you’ll see that just for the “get” there are plenty of needs.
Now this still breaks Requirement 1:
$services.user.get('...').properties.editor
vs
$services.user.properties.editor
It means resolving a User (and possibly loading it in memory) before being able to access its properties. Exactly like XWikiDocument (which forced us to have a big cache because otherwise just getting a single xproperty value is too costly. To this day, we still don’t scale BTW with the number of xobjects because of this).
Also note that your API doesn’t support Requirement 6.
Yes, and the Document API is pretty bad IMO.
Small note that when you need a cache it’s usually an acknowledgment that your API is not performant enough and you need a cache to compensate.
I agree here. See the new model API (in a branch in platform) that shows that we identified that we need to be able to retrieve any Entity using an EntityReference (and not the whole Document as one). There was a cache of Entity (vs currently a cache of Document), and that’s the equivalent of a cache of User properties (vs a cache of User objects).
Not sure what you mean. For me the big missing thing today is that there’s no write API for Users, i.e. the way to create or modify user properties is by going through XWikiDocument, XObject, and XProperties and thus violates Requirement 7.
Ok that’s interesting. What is the use case exactly (for storing a collection of Users)? I can think of one example, for example doing a batch read of emails for, say, all users in a Group, because you want to send some email to them. The way to do this is by iterating or storing all UserReference objects and then when you need the properties to use a XWikiPropertiesResolver. Do you see a use case that would require storing User objects?
The API I have implemented is complicated but I believe it’s because it’s powerful and takes into account a lot of use cases. It’s very similar to the EntityReference API with the resolvers and serializers. I’ve received the comment that it’s a complex API. It is, but it has also served us well, with our ability to implement new components for them as we found new needs, without breaking the API.
Now it seems you’ve looked at the implementation which is not something you really need to do. What’s more important to me is that you look at the usage of the API and to tell me if you find it hard to use. A good API has to be easy to use (Requirement 1). Could you tell me what you find hard in the API? If you check the places where I’ve used it so far you’ll find that it’s been used like this:
Java code:
@Inject
private UserPropertiesResolver propertiesResolver;
...
propertiesResolver.resolve(CurrentUserReference.INSTANCE).getEditor();
Script:
#if ($services.user.allProperties.type == 'ADVANCED'...
That will represent 99% of the usages of the API, with:
@Inject
@Named("all")
private UserPropertiesResolver propertiesResolver;
...
propertiesResolver.resolve(CurrentUserReference.INSTANCE).getEditor();
Thank you. I’ve spent a lot of time working on this day and night for quite a while now, turning everything in my head and refactoring as I found more use cases that I didn’t plan and that required changing the API to accommodate for them. I’m sure there’ll be more.
In order to progress, I’ve decided (and Simon was ok too, and Thomas kind of ok) to implement the API to get User properties. It’s still open as to whether we’ll need a User objet in the future. TBH I don’t know but I haven’t a single use case that requires it for now. If you remember I had a User object before and I can tell you I didn’t break all my work for pleasure. I was really sad and tried to fight to keep that User object, but I couldn’t make it work and fulfill the Requirements, especially Requirement 6 which was the big blocker.
Thank you. I appreciate the feedback. I hope my answers have shed a bit more light. I can accept that I didn’t make the best API but it’s the best I’ve found (so far) to accommodate Requirements 1 through 8 from https://design.xwiki.org/xwiki/bin/view/Proposal/UserAPI. Note that I haven’t yet implemented Requirements 4 and 8 and it’s possible that when I start implementing them, they’ll tell me/us something more. I’m a big believer in listening to your API when you’re designing them, and refactoring as you listen to it and it tells you what’s wrong. So, as everyone else, I started with the concept of User and constructed everything around, to find I was in a dead end from an API POV with Requirement 6, which pushed me in the direction of doing a flexible properties-oriented API. Maybe I’ve been too close to the implementation to see straight, I don’t know.
All I know is that ATM this is the best I could find to meet the requirements. If you can find some alternative that includes requirement 1, 2, 6 then it would be nice.
Thanks