Ok after reading code and thinking more (and thanks to Guillaume’s pointer to http://jira.xwiki.org/browse/XWIKI-10377) my conclusion is that:
* There’s no current scalable and performant way to get the list of users from a group, taking into account all use cases (nested groups and users/groups that can be located on different wikis). See my comments at http://jira.xwiki.org/browse/XWIKI-10377 for some ideas/thoughts. But all solutions listed are pretty complex or have cons. * For my need I’ll add a new RightsManager.resolveUsers method that returns an Iterator<DocumentReference>. This won’t be more scalable but it'll allow me to split the creation of MimeMessage one by one without having to wait for all the users to have been extracted from the group. Thanks -Vincent On 9 Feb 2015 at 15:15:14, Guillaume Louis-Marie Delhumeau ([email protected](mailto:[email protected])) wrote: > You forget one important issue with the current API: it does not handle > members across wikis (http://jira.xwiki.org/browse/XWIKI-10377). Your mail > application will probably hit this problem too. > > It would be a shame to write a new API without this. > > 2015-02-09 14:52 GMT+01:00 [email protected] : > > > Hi devs, > > > > I need to code some stuff for the new mail sender API (ability to send > > mails to all users of several groups) and I’ve noticed 5 issues with the > > current group API (XWikiGroupService): > > > > 1) It’s not implemented as components (you need to get the XWiki object > > and call getGroupService() to get an instance) > > 2) The APIs uses String to reference groups (instead of an > > EntityReference/DocumentReference). Note that ideally we should have a Java > > interface to represent a Group and a Group Reference since we could want to > > implement Groups differently than as wiki pages in some future (for > > example, by using JAAS, LDAP, etc) but this is probably for later and for > > now having an EntityReference/DocumentReference is probably good enough > > 3) The API to return all users of a group (getAllMembersNamesForGroup) > > return a Collection and takes as input an offset and a number of > > users to return. This is a very DB-oriented way to scale an API but it’s > > not easy to use in Java when you want to get all users of a group (you need > > to maintain a cursor). What is useful in Java is an API that return an > > Iterator. > > 4) The getAllMembersNamesForGroup() API doesn’t seem to handle subgroups... > > 5) I’ve also noticed that we have **lots** of group-related APIs in > > a RightsManager class (which is javadoc-ed as being internal BTW but used > > elsewhere…), used by the RightsManagerPluginApi. I don’t see why we have > > users or group-related APIs in a an API related to permissions/rights (i.e. > > authentication)… This API suffers from another problem: even though is has > > a resolveUsers(String userOrGroup, XWikiContext context) method to extract > > users from a list of users or group references, it doesn’t scale! Indeed it > > doesn’t accept any offset/count parameters and it returns > > a Collection... > > > > Thus, I have 2 options: > > A) do the conversion between String <-> DocumentReference on my side (in > > the mail sender api) + handle subgroups in the mail sender api + maintain > > cursor in the mail sender code > > B) Start quickly a new component-based Group API which would have just the > > method I need for now (and would be marked unstable). > > > > For B) I‘m thinking about something like: > > > > Option 1: > > ========= > > > > GroupManager > > |_ Iterator getAllUsers() > > > > (in the future we would also have a getMatchingUsers(…)) > > > > By default the impl would retrieve batches of user references from the > > group (say 100 at a time) > > > > However, if you want to get just 1 user for example, this is a bit > > overkill to get 100 DB results. But is that a real use case? So we also > > have option 2: > > > > Option 2: > > ========== > > > > GroupManager > > |_ GroupResultSet getAllUsers() > > > > Where: > > > > GroupResultSet > > |_ Iterator iterator() - default to batch size of 100 > > |_ Iterator iterator(int batchSize) > > |_ Collection get(int nb, int offset) (do we really > > need this api?) > > > > I’d prefer to not use option 2 if possible but there may be use cases I > > don’t see right now that would require 2 and it has the nice advantage of > > being able to add new methods to GroupResultSet without impacting backward > > compatibility. > > > > Note that this discussion between option 1 and 2 is interesting more > > generally speaking since we would need to decide what’s our best practices > > in all our APIs when we require to return large collections of values. So > > far a lot of our old APIs use the (int nb, int offset) solution, which is > > versatile but not easy to use IMO and I don’t find it nice that we mix > > business parameters with technical ones. > > > > Last question would be where to put this new code. I can see 2 choices: > > i) in a new xwiki-platform-group module > > ii) in a new xwiki-platform-user/xwiki-platform-user-api module where we > > would put both APIs for both Users and Groups > > > > option ii) seems better IMO. > > > > WDYT? > > > > Thanks > > -Vincent > > > > > > > > > > > > > > _______________________________________________ > > devs mailing list > > [email protected] > > http://lists.xwiki.org/mailman/listinfo/devs > > > > > > -- > Guillaume Delhumeau ([email protected]) > Research & Development Engineer at XWiki SAS > Committer on the XWiki.org project > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
