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