On Mon, Feb 9, 2015 at 3:52 PM, [email protected] <[email protected]> wrote: > 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?) Definitely. For an API that may return a very large list we need to be able to paginate the display (e.g. group members live table). Thanks, Marius > > 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 _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
