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<String> 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<DocumentReference>. 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<DocumentReference>... 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<DocumentReference> 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<DocumentReference> iterator() - default to batch size of 100 |_ Iterator<DocumentReference> iterator(int batchSize) |_ Collection<DocumentReference> 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
