[xwiki-devs] [Brainstorming] New Group API?
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
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 vincent@massol.net <vincent@massol.net>:
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 devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Guillaume Delhumeau (gdelhumeau@xwiki.com) Research & Development Engineer at XWiki SAS Committer on the XWiki.org project
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 (gdelhumeau@xwiki.com(mailto:gdelhumeau@xwiki.com)) 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 vincent@massol.net :
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 devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
-- Guillaume Delhumeau (gdelhumeau@xwiki.com) Research & Development Engineer at XWiki SAS Committer on the XWiki.org project _______________________________________________ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
On Mon, Feb 9, 2015 at 3:52 PM, vincent@massol.net <vincent@massol.net> 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 devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
On 10 Feb 2015 at 10:07:22, Marius Dumitru Florea (mariusdumitru.florea@xwiki.com(mailto:mariusdumitru.florea@xwiki.com)) wrote: [snip]
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?)
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).
Indeed, good point. Thanks -Vincent [snip]
participants (3)
-
Guillaume "Louis-Marie" Delhumeau -
Marius Dumitru Florea -
vincent＠massol.net