fixes #40: persist members and admins in group updates

When creating or updating a group, the provided members and admins should not be ignored.

Note that this change will result in admins/members being removed, when an update to an existing group does not properly include the existing members and admins. Prior to this commit, no modification would have been made to the admins and members.

Author: guusdk

changelog.html

@@ -52,6 +52,7 @@ <h1>
     <li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/99'>#99</a>] - Fix hard-coded link to localhost for OpenAPI yaml link in docs.</li>
     <li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/96'>#96</a>] - Don't require auth for readiness/liveness endpoints</li>
     <li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/74'>#74</a>] - Return HTTP status code 404 instead of 500 when passing incorrect MUC service/room name</li>
+    <li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/40'>#40</a>] - When creating or updating a group, use the members and admins that are provided in the input data.</li>
 </ul>
 
 <p><b>1.8.0</b> April 6, 2022</p>

src/java/org/jivesoftware/openfire/plugin/rest/controller/GroupController.java

@@ -16,12 +16,11 @@
 
 package org.jivesoftware.openfire.plugin.rest.controller;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
+import java.util.*;
 
 import javax.ws.rs.core.Response;
 
+import org.jivesoftware.openfire.XMPPServer;
 import org.jivesoftware.openfire.group.Group;
 import org.jivesoftware.openfire.group.GroupAlreadyExistsException;
 import org.jivesoftware.openfire.group.GroupManager;
@@ -30,6 +29,7 @@
 import org.jivesoftware.openfire.plugin.rest.exceptions.ExceptionType;
 import org.jivesoftware.openfire.plugin.rest.exceptions.ServiceException;
 import org.jivesoftware.openfire.plugin.rest.utils.MUCRoomUtils;
+import org.xmpp.packet.JID;
 
 /**
  * The Class GroupController.
@@ -56,7 +56,7 @@ public static GroupController getInstance() {
      */
     public List<GroupEntity> getGroups() throws ServiceException {
         Collection<Group> groups = GroupManager.getInstance().getGroups();
-        List<GroupEntity> groupEntities = new ArrayList<GroupEntity>();
+        List<GroupEntity> groupEntities = new ArrayList<>();
         for (Group group : groups) {
             GroupEntity groupEntity = new GroupEntity(group.getName(), group.getDescription());
             groupEntities.add(groupEntity);
@@ -86,7 +86,7 @@ public GroupEntity getGroup(String groupName) throws ServiceException {
         GroupEntity groupEntity = new GroupEntity(group.getName(), group.getDescription());
         groupEntity.setAdmins(MUCRoomUtils.convertJIDsToStringList(group.getAdmins()));
         groupEntity.setMembers(MUCRoomUtils.convertJIDsToStringList(group.getMembers()));
-        groupEntity.setShared(group.getProperties().get("sharedRoster.showInRoster")=="onlyGroup");
+        groupEntity.setShared("onlyGroup".equals(group.getProperties().get("sharedRoster.showInRoster")));
 
         return groupEntity;
     }
@@ -104,6 +104,26 @@ public Group createGroup(GroupEntity groupEntity) throws ServiceException {
         Group group;
         if (groupEntity != null && !groupEntity.getName().isEmpty()) {
             try {
+                final Collection<JID> newMembers = new HashSet<>();
+                final Collection<JID> newAdmins = new HashSet<>();
+
+                // Input validation.
+                for (final String newMember : groupEntity.getMembers()) {
+                    try {
+                        newMembers.add(newMember.contains("@") ? new JID(newMember) : XMPPServer.getInstance().createJID(newMember, null));
+                    } catch (IllegalArgumentException e) {
+                        throw new ServiceException("Cannot parse a member value as a JID: " + newMember, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
+                    }
+                }
+                for (final String newAdmin : groupEntity.getAdmins()) {
+                    try {
+                        newAdmins.add(newAdmin.contains("@") ? new JID(newAdmin) : XMPPServer.getInstance().createJID(newAdmin, null));
+                    } catch (IllegalArgumentException e) {
+                        throw new ServiceException("Cannot parse a admin value as a JID: " + newAdmin, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
+                    }
+                }
+
+                // Start creating the group.
                 group = GroupManager.getInstance().createGroup(groupEntity.getName());
                 group.setDescription(groupEntity.getDescription());
 
@@ -112,6 +132,17 @@ public Group createGroup(GroupEntity groupEntity) throws ServiceException {
 
                 group.getProperties().put("sharedRoster.displayName", groupEntity.getName());
                 group.getProperties().put("sharedRoster.groupList", "");
+
+                final Collection<JID> members = group.getMembers();
+                for (final JID newMember : newMembers) {
+                    // Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
+                    members.add(newMember);
+                }
+                final Collection<JID> admins = group.getAdmins();
+                for (final JID newAdmin : newAdmins) {
+                    // Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
+                    admins.add(newAdmin);
+                }
             } catch (GroupAlreadyExistsException e) {
                 throw new ServiceException("Could not create a group", groupEntity.getName(),
                         ExceptionType.GROUP_ALREADY_EXISTS, Response.Status.CONFLICT, e);
@@ -136,10 +167,65 @@ public Group updateGroup(String groupName, GroupEntity groupEntity) throws Servi
         if (groupEntity != null && !groupEntity.getName().isEmpty()) {
             if (groupName.equals(groupEntity.getName())) {
                 try {
+                    final Collection<JID> newMembers = new HashSet<>();
+                    final Collection<JID> newAdmins = new HashSet<>();
+
+                    // Input validation.
                     group = GroupManager.getInstance().getGroup(groupName);
+                    for (final String newMember : groupEntity.getMembers()) {
+                        try {
+                            newMembers.add(newMember.contains("@") ? new JID(newMember) : XMPPServer.getInstance().createJID(newMember, null));
+                        } catch (IllegalArgumentException e) {
+                            throw new ServiceException("Cannot parse a member value as a JID: " + newMember, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
+                        }
+                    }
+                    for (final String newAdmin : groupEntity.getAdmins()) {
+                        try {
+                            newAdmins.add(newAdmin.contains("@") ? new JID(newAdmin) : XMPPServer.getInstance().createJID(newAdmin, null));
+                        } catch (IllegalArgumentException e) {
+                            throw new ServiceException("Cannot parse a admin value as a JID: " + newAdmin, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
+                        }
+                    }
+
                     group.setDescription(groupEntity.getDescription());
                     final String showInRoster = groupEntity.getShared() ? "onlyGroup" : "nobody";
                     group.getProperties().put("sharedRoster.showInRoster", showInRoster);
+
+                    // Correct the member-list that already is in the group to match the desired state.
+                    final Collection<JID> members = group.getMembers();
+                    final Iterator<JID> membersIterator = members.iterator();
+                    while (membersIterator.hasNext()) {
+                        final JID oldMember = membersIterator.next();
+                        if (newMembers.contains(oldMember)) {
+                            // Already exists. No need to add it again.
+                            newMembers.remove(oldMember);
+                        } else {
+                            // No longer exists. Remove from group.
+                            membersIterator.remove();
+                        }
+                    }
+                    for (final JID newMember : newMembers) {
+                        // Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
+                        members.add(newMember);
+                    }
+
+                    // Correct the admin-list that already is in the group to match the desired state.
+                    final Collection<JID> admins = group.getAdmins();
+                    final Iterator<JID> adminsIterator = admins.iterator();
+                    while (adminsIterator.hasNext()) {
+                        final JID oldAdmin = adminsIterator.next();
+                        if (newAdmins.contains(oldAdmin)) {
+                            // Already exists. No need to add it again.
+                            newAdmins.remove(oldAdmin);
+                        } else {
+                            // No longer exists. Remove from group.
+                            adminsIterator.remove();
+                        }
+                    }
+                    for (final JID newAdmin : newAdmins) {
+                        // Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
+                        admins.add(newAdmin);
+                    }
                 } catch (GroupNotFoundException e) {
                     throw new ServiceException("Could not find group", groupName, ExceptionType.GROUP_NOT_FOUND,
                             Response.Status.NOT_FOUND, e);

src/java/org/jivesoftware/openfire/plugin/rest/service/GroupService.java

@@ -61,7 +61,7 @@ public GroupEntities getGroups() throws ServiceException
         description = "Create a new user group.",
         responses = {
             @ApiResponse(responseCode = "201", description = "Group created."),
-            @ApiResponse(responseCode = "400", description = "Group or group name missing."),
+            @ApiResponse(responseCode = "400", description = "Group or group name missing, or invalid syntax for a property."),
             @ApiResponse(responseCode = "409", description = "Group already exists.")
         })
     @Consumes({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
@@ -94,7 +94,7 @@ public GroupEntity getGroup(@Parameter(description = "The name of the group that
         description = "Updates / overwrites an existing user group. Note that the name of the group cannot be changed.",
         responses = {
             @ApiResponse(responseCode = "200", description = "Group updated."),
-            @ApiResponse(responseCode = "400", description = "Group or group name missing, or name does not match existing group."),
+            @ApiResponse(responseCode = "400", description = "Group or group name missing, or name does not match existing group, or invalid syntax for a property."),
             @ApiResponse(responseCode = "404", description = "Group with this name not found."),
         })
     @Consumes({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })

src/java/org/jivesoftware/openfire/plugin/rest/service/UserGroupService.java

@@ -59,9 +59,10 @@ public UserGroupsEntity getUserGroups(
 
     @POST
     @Operation( summary = "Add user to groups",
-        description = "Add a particular user to a collection of groups.",
+        description = "Add a particular user to a collection of groups. When a group that is provided does not exist, it will be automatically created if possible.",
         responses = {
             @ApiResponse(responseCode = "201", description = "The user was added to all groups."),
+            @ApiResponse(responseCode = "400", description = "When the username cannot be parsed into a JID.")
         })
     @Consumes({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
     public Response addUserToGroups(
@@ -76,9 +77,10 @@ public Response addUserToGroups(
     @POST
     @Path("/{groupName}")
     @Operation( summary = "Add user to group",
-        description = "Add a particular user to a particular group.",
+        description = "Add a particular user to a particular group. When the group that does not exist, it will be automatically created if possible.",
         responses = {
             @ApiResponse(responseCode = "201", description = "The user was added to the groups."),
+            @ApiResponse(responseCode = "400", description = "When the username cannot be parsed into a JID.")
         })
     public Response addUserToGroup(
             @Parameter(description = "The username of the user that is to be added to a group.", required = true) @PathParam("username") String username,