Merge pull request DSpace#1265 from mwoodiupui/DS-3024

[DS-3024] "Administrator" and "Anonymous" groups can be renamed, which would cause them to no longer function in 6.x

dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java

@@ -318,7 +318,8 @@ public Group createWorkflowGroup(Context context, Collection collection, int ste
             Group g = groupService.create(context);
             context.restoreAuthSystemState();
 
-            g.setName(context, "COLLECTION_" + collection.getID() + "_WORKFLOW_STEP_" + step);
+            groupService.setName(context, g,
+                    "COLLECTION_" + collection.getID() + "_WORKFLOW_STEP_" + step);
             groupService.update(context, g);
             setWorkflowGroup(collection, step, g);
 
@@ -395,7 +396,8 @@ public Group createSubmitters(Context context, Collection collection) throws SQL
             submitters = groupService.create(context);
             context.restoreAuthSystemState();
 
-            submitters.setName(context, "COLLECTION_" + collection.getID() + "_SUBMIT");
+            groupService.setName(context, submitters,
+                    "COLLECTION_" + collection.getID() + "_SUBMIT");
             groupService.update(context, submitters);
         }
 
@@ -435,7 +437,7 @@ public Group createAdministrators(Context context, Collection collection) throws
             admins = groupService.create(context);
             context.restoreAuthSystemState();
 
-            admins.setName(context, "COLLECTION_" + collection.getID() + "_ADMIN");
+            groupService.setName(context, admins, "COLLECTION_" + collection.getID() + "_ADMIN");
             groupService.update(context, admins);
         }
 

dspace-api/src/main/java/org/dspace/content/CommunityServiceImpl.java

@@ -270,7 +270,7 @@ public Group createAdministrators(Context context, Community community) throws S
             admins = groupService.create(context);
             context.restoreAuthSystemState();
 
-            admins.setName(context, "COMMUNITY_" + community.getID() + "_ADMIN");
+            groupService.setName(context, admins, "COMMUNITY_" + community.getID() + "_ADMIN");
             groupService.update(context, admins);
         }
 

dspace-api/src/main/java/org/dspace/content/packager/RoleIngester.java

@@ -356,7 +356,7 @@ else if(parent.getType()==Constants.COMMUNITY)
                 }
 
                 // Always set the name:  parent.createBlop() is guessing
-                groupObj.setName(context, name);
+                groupService.setName(context, groupObj, name);
 
                 log.info("Created Group {}.", groupObj.getName());
             }

dspace-api/src/main/java/org/dspace/eperson/Group.java

@@ -24,9 +24,8 @@
 
 /**
  * Class representing a group of e-people.
- * 
+ *
  * @author David Stuve
- * @version $Revision$
  */
 @Entity
 @Table(name = "epersongroup" )
@@ -45,6 +44,10 @@ public class Group extends DSpaceObject implements DSpaceObjectLegacySupport
     @Column(name="eperson_group_id", insertable = false, updatable = false)
     private Integer legacyId;
 
+    /** This Group may not be deleted or renamed. */
+    @Column
+    private Boolean permanent = false;
+
     /** lists of epeople and groups in the group */
     @ManyToMany(fetch = FetchType.LAZY)
     @JoinTable(
@@ -147,14 +150,14 @@ public List<Group> getMemberGroups()
     {
         return groups;
     }
-    
+
     /**
      * Return <code>true</code> if <code>other</code> is the same Group as
      * this object, <code>false</code> otherwise
-     * 
+     *
      * @param obj
      *            object to compare to
-     * 
+     *
      * @return <code>true</code> if object passed in represents the same group
      *         as this object
      */
@@ -171,11 +174,7 @@ public boolean equals(Object obj)
              return false;
          }
          final Group other = (Group) obj;
-         if (!this.getID().equals(other.getID()))
-         {
-             return false;
-         }
-         return true;
+         return this.getID().equals(other.getID());
      }
 
      @Override
@@ -201,9 +200,11 @@ public String getName()
         return getGroupService().getName(this);
     }
 
-    public void setName(Context context, String name) throws SQLException
+    /** Change the name of this Group. */
+    void setName(Context context, String name) throws SQLException
     {
-        getGroupService().setMetadataSingleValue(context, this, MetadataSchema.DC_SCHEMA, "title", null, null, name);
+        getGroupService().setMetadataSingleValue(context, this,
+                MetadataSchema.DC_SCHEMA, "title", null, null, name);
     }
 
     public boolean isGroupsChanged() {
@@ -230,4 +231,27 @@ private GroupService getGroupService() {
         }
         return groupService;
     }
+
+    /**
+     * May this Group be renamed or deleted?  (The content of any group may be
+     * changed.)
+     *
+     * @return true if this Group may not be renamed or deleted.
+     */
+    public Boolean isPermanent()
+    {
+        return permanent;
+    }
+
+    /**
+     * May this Group be renamed or deleted?  (The content of any group may be
+     * changed.)
+     *
+     * @param permanence true if this group may not be renamed or deleted.
+     */
+    void setPermanent(boolean permanence)
+    {
+        permanent = permanence;
+        setModified();
+    }
 }

dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java

@@ -8,7 +8,6 @@
 package org.dspace.eperson;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.log4j.Logger;
 import org.dspace.authorize.AuthorizeConfiguration;
 import org.dspace.authorize.AuthorizeException;
 import org.dspace.authorize.service.AuthorizeService;
@@ -29,10 +28,12 @@
 import org.dspace.event.Event;
 import org.springframework.beans.factory.annotation.Autowired;
 
-import java.nio.ByteBuffer;
 import java.sql.SQLException;
 import java.util.*;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * Service implementation for the Group object.
  * This class is responsible for all business logic calls for the Group object and is autowired by spring.
@@ -42,7 +43,7 @@
  */
 public class GroupServiceImpl extends DSpaceObjectServiceImpl<Group> implements GroupService
 {
-    private static final Logger log = Logger.getLogger(GroupServiceImpl.class);
+    private static final Logger log = LoggerFactory.getLogger(GroupServiceImpl.class);
 
     @Autowired(required = true)
     protected GroupDAO groupDAO;
@@ -93,7 +94,14 @@ public Group create(Context context) throws SQLException, AuthorizeException {
 
     @Override
     public void setName(Context context, Group group, String name) throws SQLException {
-        setMetadataSingleValue(context, group, MetadataSchema.DC_SCHEMA, "title", null, null, name);
+        if (group.isPermanent())
+        {
+            log.error("Attempt to rename permanent Group {} to {}.",
+                    group.getName(), name);
+            throw new SQLException("Attempt to rename a permanent Group");
+        }
+        else
+            group.setName(context, name);
     }
 
     @Override
@@ -165,7 +173,7 @@ public boolean isMember(Context context, Group group) throws SQLException {
 
     @Override
     public List<Group> allMemberGroups(Context context, EPerson ePerson) throws SQLException {
-        Set<Group> groups = new HashSet<Group>();
+        Set<Group> groups = new HashSet<>();
 
         if (ePerson != null)
         {
@@ -210,7 +218,7 @@ public List<EPerson> allMembers(Context c, Group g) throws SQLException
 
         // Get all groups which are a member of this group
         List<Group2GroupCache> group2GroupCaches = group2GroupCacheDAO.findByParent(c, g);
-        Set<Group> groups = new HashSet<Group>();
+        Set<Group> groups = new HashSet<>();
         for (Group2GroupCache group2GroupCache : group2GroupCaches) {
             groups.add(group2GroupCache.getChild());
         }
@@ -296,7 +304,14 @@ public int searchResultCount(Context context, String query) throws SQLException
 
     @Override
     public void delete(Context context, Group group) throws SQLException {
-        context.addEvent(new Event(Event.DELETE, Constants.GROUP, group.getID(), group.getName(), getIdentifiers(context, group)));
+        if (group.isPermanent())
+        {
+            log.error("Attempt to delete permanent Group $", group.getName());
+            throw new SQLException("Attempt to delete a permanent Group");
+        }
+
+        context.addEvent(new Event(Event.DELETE, Constants.GROUP, group.getID(),
+                group.getName(), getIdentifiers(context, group)));
 
         //Remove the supervised group from any workspace items linked to us.
         group.getSupervisedItems().clear();
@@ -339,7 +354,7 @@ public int getSupportsTypeConstant() {
     public boolean isEmpty(Group group)
     {
         // the only fast check available is on epeople...
-        boolean hasMembers = (group.getMembers().size() != 0);
+        boolean hasMembers = (!group.getMembers().isEmpty());
 
         if (hasMembers)
         {
@@ -367,6 +382,7 @@ public void initDefaultGroupNames(Context context) throws SQLException, Authoriz
         {
             anonymousGroup = groupService.create(context);
             anonymousGroup.setName(context, Group.ANONYMOUS);
+            anonymousGroup.setPermanent(true);
             groupService.update(context, anonymousGroup);
         }
 
@@ -377,6 +393,7 @@ public void initDefaultGroupNames(Context context) throws SQLException, Authoriz
         {
             adminGroup = groupService.create(context);
             adminGroup.setName(context, Group.ADMIN);
+            adminGroup.setPermanent(true);
             groupService.update(context, adminGroup);
         }
     }

dspace-api/src/main/java/org/dspace/eperson/service/GroupService.java

@@ -198,8 +198,10 @@ public interface GroupService extends DSpaceObjectService<Group>, DSpaceObjectLe
     public boolean isEmpty(Group group);
 
     /**
-     * Initializes the group names for anymous & administrator
-     * @param context the dspace context
+     * Initializes the group names for anonymous & administrator, and marks them
+     * "permanent".
+     *
+     * @param context the DSpace context
      * @throws SQLException database exception
      * @throws AuthorizeException
      */

dspace-api/src/main/java/org/dspace/xmlworkflow/XmlWorkflowServiceImpl.java

@@ -131,9 +131,11 @@ public Group getWorkflowRoleGroup(Context context, Collection collection, String
                     authorizeService.authorizeAction(context, collection, Constants.WRITE);
                     roleGroup = groupService.create(context);
                     if(role.getScope() == Role.Scope.COLLECTION){
-                        roleGroup.setName(context, "COLLECTION_" + collection.getID().toString() + "_WORKFLOW_ROLE_" + roleName);
+                        groupService.setName(context, roleGroup,
+                                "COLLECTION_" + collection.getID().toString()
+                                        + "_WORKFLOW_ROLE_" + roleName);
                     }else{
-                        roleGroup.setName(context,  role.getName());
+                        groupService.setName(context, roleGroup,  role.getName());
                     }
                     groupService.update(context, roleGroup);
                     authorizeService.addPolicy(context, collection, Constants.ADD, roleGroup);

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2015.03.07__DS-2701_Hibernate_migration.sql

@@ -32,6 +32,11 @@ ALTER TABLE eperson ALTER COLUMN eperson_id SET NULL;
 
 
 
+UPDATE metadatavalue SET text_value='Administrator'
+  WHERE resource_type_id=6 AND resource_id=1;
+UPDATE metadatavalue SET text_value='Anonymous'
+  WHERE resource_type_id=6 AND resource_id=0;
+
 ALTER TABLE epersongroup ADD COLUMN uuid UUID DEFAULT random_uuid();
 INSERT INTO dspaceobject  (uuid) SELECT uuid FROM epersongroup;
 ALTER TABLE epersongroup ADD FOREIGN KEY (uuid) REFERENCES dspaceobject;

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V6.0_2016.01.03__DS-3024.sql

@@ -0,0 +1,27 @@
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+------------------------------------------------------
+-- DS-3024 Invent "permanent" groups
+------------------------------------------------------
+
+ALTER TABLE epersongroup
+      ADD (permanent BOOLEAN DEFAULT false);
+UPDATE epersongroup SET permanent = true
+       WHERE uuid IN (
+         SELECT dspace_object_id
+	   FROM metadataschemaregistry AS s
+	     JOIN metadatafieldregistry AS f
+	       ON (s.metadata_schema_id = f.metadata_schema_id)
+	     JOIN metadatavalue AS v
+	       ON (f.metadata_field_id = v.metadata_field_id)
+	   WHERE s.short_id = 'dc'
+	     AND f.element = 'title'
+	     AND f.qualifier IS NULL
+	     AND v.text_value IN ('Administrator', 'Anonymous')
+       );

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V6.0_2015.03.07__DS-2701_Hibernate_migration.sql

@@ -32,6 +32,11 @@ UPDATE eperson SET self_registered = '0' WHERE self_registered IS NULL;
 
 
 
+UPDATE metadatavalue SET text_value='Administrator'
+  WHERE resource_type_id=6 AND resource_id=1;
+UPDATE metadatavalue SET text_value='Anonymous'
+  WHERE resource_type_id=6 AND resource_id=0;
+
 ALTER TABLE epersongroup ADD uuid RAW(16) DEFAULT SYS_GUID();
 INSERT INTO dspaceobject  (uuid) SELECT uuid FROM epersongroup;
 ALTER TABLE epersongroup ADD FOREIGN KEY (uuid) REFERENCES dspaceobject;

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/oracle/V6.0_2016.01.03__DS-3024.sql

@@ -0,0 +1,25 @@
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+------------------------------------------------------
+-- DS-3024 Invent "permanent" groups
+------------------------------------------------------
+
+ALTER TABLE epersongroup
+      ADD (permanent NUMBER(1) DEFAULT 0);
+UPDATE epersongroup SET permanent = 1
+       WHERE uuid IN (
+         SELECT dspace_object_id
+	   FROM metadataschemaregistry s
+	     JOIN metadatafieldregistry f USING (metadata_schema_id)
+	     JOIN metadatavalue v USING (metadata_field_id)
+	   WHERE s.short_id = 'dc'
+	     AND f.element = 'title'
+	     AND f.qualifier IS NULL
+	     AND v.text_value IN ('Administrator', 'Anonymous')
+       );

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V6.0_2015.03.07__DS-2701_Hibernate_migration.sql

@@ -35,6 +35,11 @@ UPDATE eperson SET self_registered = false WHERE self_registered IS NULL;
 
 
 
+UPDATE metadatavalue SET text_value='Administrator'
+  WHERE resource_type_id=6 AND resource_id=1;
+UPDATE metadatavalue SET text_value='Anonymous'
+  WHERE resource_type_id=6 AND resource_id=0;
+
 ALTER TABLE epersongroup ADD COLUMN uuid UUID DEFAULT gen_random_uuid() UNIQUE;
 INSERT INTO dspaceobject  (uuid) SELECT uuid FROM epersongroup;
 ALTER TABLE epersongroup ADD FOREIGN KEY (uuid) REFERENCES dspaceobject;

dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V6.0_2016.01.03__DS-3024.sql

@@ -0,0 +1,25 @@
+--
+-- The contents of this file are subject to the license and copyright
+-- detailed in the LICENSE and NOTICE files at the root of the source
+-- tree and available online at
+--
+-- http://www.dspace.org/license/
+--
+
+------------------------------------------------------
+-- DS-3024 Invent "permanent" groups
+------------------------------------------------------
+
+ALTER TABLE epersongroup
+      ADD COLUMN permanent BOOLEAN DEFAULT false;
+UPDATE epersongroup SET permanent = true
+       WHERE uuid IN (
+         SELECT dspace_object_id
+	   FROM metadataschemaregistry s
+	     JOIN metadatafieldregistry f USING (metadata_schema_id)
+	     JOIN metadatavalue v USING (metadata_field_id)
+	   WHERE s.short_id = 'dc'
+	     AND f.element = 'title'
+	     AND f.qualifier IS NULL
+	     AND v.text_value IN ('Administrator', 'Anonymous')
+       );