Merge pull request DSpace#1768 from tdonohue/DS-3406-master

Forward port of DS-3406 and DS-3599 (Collection/Community alpha sorting) to master

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

@@ -7,16 +7,19 @@
  */
 package org.dspace.content;
 
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.CollectionService;
 import org.dspace.core.*;
 import org.dspace.eperson.Group;
+import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortType;
 import org.hibernate.proxy.HibernateProxyHelper;
 
 import javax.persistence.*;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
 
 /**
  * Class representing a collection.
@@ -83,7 +86,7 @@ public class Collection extends DSpaceObject implements DSpaceObjectLegacySuppor
             joinColumns = {@JoinColumn(name = "collection_id") },
             inverseJoinColumns = {@JoinColumn(name = "community_id") }
     )
-    private final List<Community> communities = new ArrayList<>();
+    private Set<Community> communities = new HashSet<>();
 
     @Transient
     private transient CollectionService collectionService;
@@ -263,15 +266,19 @@ void setTemplateItem(Item template) {
      */
     public List<Community> getCommunities() throws SQLException
     {
-        return communities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = communities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCommunity(Community community) {
         this.communities.add(community);
         setModified();
     }
 
-    void removeCommunity(Community community){
+    void removeCommunity(Community community) {
         this.communities.remove(community);
         setModified();
     }
@@ -345,4 +352,4 @@ private CollectionService getCollectionService() {
         }
         return collectionService;
     }
-}
+}

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

@@ -750,8 +750,8 @@ public void delete(Context context, Collection collection) throws SQLException,
         while (owningCommunities.hasNext())
         {
             Community owningCommunity = owningCommunities.next();
-            owningCommunities.remove();
-            owningCommunity.getCollections().remove(collection);
+            collection.removeCommunity(owningCommunity);
+            owningCommunity.removeCollection(collection);
         }
 
         collectionDAO.delete(context, collection);

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

@@ -9,10 +9,14 @@
 
 import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.apache.log4j.Logger;
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.CommunityService;
 import org.dspace.core.*;
 import org.dspace.eperson.Group;
+import org.hibernate.annotations.CacheConcurrencyStrategy;
+import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortType;
 import org.hibernate.proxy.HibernateProxyHelper;
 
 import javax.persistence.*;
@@ -44,13 +48,13 @@ public class Community extends DSpaceObject implements DSpaceObjectLegacySupport
             joinColumns = {@JoinColumn(name = "parent_comm_id") },
             inverseJoinColumns = {@JoinColumn(name = "child_comm_id") }
     )
-    private final List<Community> subCommunities = new ArrayList<>();
+    private Set<Community> subCommunities = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "subCommunities")
-    private List<Community> parentCommunities = new ArrayList<>();
+    private Set<Community> parentCommunities = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "communities", cascade = {CascadeType.PERSIST})
-    private final List<Collection> collections = new ArrayList<>();
+    private Set<Collection> collections = new HashSet<>();
 
     @OneToOne
     @JoinColumn(name = "admin")
@@ -85,13 +89,13 @@ protected Community()
 
     void addSubCommunity(Community subCommunity)
     {
-        getSubcommunities().add(subCommunity);
+        subCommunities.add(subCommunity);
         setModified();
     }
 
     void removeSubCommunity(Community subCommunity)
     {
-        getSubcommunities().remove(subCommunity);
+        subCommunities.remove(subCommunity);
         setModified();
     }
 
@@ -140,17 +144,21 @@ void setAdmins(Group admins) {
      */
     public List<Collection> getCollections()
     {
-        return collections;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Collection[] output = collections.toArray(new Collection[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCollection(Collection collection)
     {
-        getCollections().add(collection);
+        collections.add(collection);
     }
 
     void removeCollection(Collection collection)
     {
-        getCollections().remove(collection);
+        collections.remove(collection);
     }
 
     /**
@@ -162,7 +170,11 @@ void removeCollection(Collection collection)
      */
     public List<Community> getSubcommunities()
     {
-        return subCommunities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = subCommunities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     /**
@@ -173,16 +185,25 @@ public List<Community> getSubcommunities()
      */
     public List<Community> getParentCommunities()
     {
-        return parentCommunities;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Community[] output = parentCommunities.toArray(new Community[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addParentCommunity(Community parentCommunity) {
-        getParentCommunities().add(parentCommunity);
+        parentCommunities.add(parentCommunity);
     }
 
     void clearParentCommunities(){
-        this.parentCommunities.clear();
-        this.parentCommunities = null;
+        parentCommunities.clear();
+    }
+
+    public void removeParentCommunity(Community parentCommunity)
+    {
+        parentCommunities.remove(parentCommunity);
+        setModified();
     }
 
     /**
@@ -250,4 +271,4 @@ private CommunityService getCommunityService() {
         }
         return communityService;
     }
-}
+}

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

@@ -455,7 +455,7 @@ public void removeSubcommunity(Context context, Community parentCommunity, Commu
 
         rawDelete(context, childCommunity);
 
-        childCommunity.getParentCommunities().remove(parentCommunity);
+        childCommunity.removeParentCommunity(parentCommunity);
         parentCommunity.removeSubCommunity(childCommunity);
 
         log.info(LogManager.getHeader(context, "remove_subcommunity",
@@ -492,7 +492,7 @@ public void delete(Context context, Community community) throws SQLException, Au
             Iterator<Community> subcommunities = community.getSubcommunities().iterator();
             while (subcommunities.hasNext()) {
                 Community subCommunity = subcommunities.next();
-                subcommunities.remove();
+                community.removeSubCommunity(subCommunity);
                 delete(context, subCommunity);
             }
             // now let the parent remove the community
@@ -535,7 +535,7 @@ protected void rawDelete(Context context, Community community) throws SQLExcepti
         while (collections.hasNext())
         {
             Collection collection = collections.next();
-            collections.remove();
+            community.removeCollection(collection);
             removeCollection(context, community, collection);
         }
         // delete subcommunities
@@ -544,7 +544,7 @@ protected void rawDelete(Context context, Community community) throws SQLExcepti
         while (subCommunities.hasNext())
         {
             Community subComm = subCommunities.next();
-            subCommunities.remove();
+            community.removeSubCommunity(subComm);
             delete(context, subComm);
         }
 

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

@@ -7,17 +7,18 @@
  */
 package org.dspace.content;
 
+import org.dspace.content.comparator.NameAscendingComparator;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.ItemService;
 import org.dspace.core.Constants;
 import org.dspace.core.Context;
 import org.dspace.eperson.EPerson;
+import org.hibernate.annotations.Sort;
+import org.hibernate.annotations.SortType;
 import org.hibernate.proxy.HibernateProxyHelper;
 
 import javax.persistence.*;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
+import java.util.*;
 
 /**
  * Class representing an item in DSpace.
@@ -78,7 +79,7 @@ public class Item extends DSpaceObject implements DSpaceObjectLegacySupport
             joinColumns = {@JoinColumn(name = "item_id") },
             inverseJoinColumns = {@JoinColumn(name = "collection_id") }
     )
-    private final List<Collection> collections = new ArrayList<>();
+    private final Set<Collection> collections = new HashSet<>();
 
     @ManyToMany(fetch = FetchType.LAZY, mappedBy = "items")
     private final List<Bundle> bundles = new ArrayList<>();
@@ -224,23 +225,31 @@ public void setSubmitter(EPerson sub)
     }
 
     /**
-     * Get the collections this item is in. The order is indeterminate.
+     * Get the collections this item is in. The order is sorted ascending by collection name.
      *
      * @return the collections this item is in, if any.
      */
     public List<Collection> getCollections()
     {
-        return collections;
+        // We return a copy because we do not want people to add elements to this collection directly.
+        // We return a list to maintain backwards compatibility
+        Collection[] output = collections.toArray(new Collection[]{});
+        Arrays.sort(output, new NameAscendingComparator());
+        return Arrays.asList(output);
     }
 
     void addCollection(Collection collection)
     {
-        getCollections().add(collection);
+        collections.add(collection);
     }
 
     void removeCollection(Collection collection)
     {
-        getCollections().remove(collection);
+        collections.remove(collection);
+    }
+
+    public void clearCollections(){
+        collections.clear();
     }
 
     public Collection getTemplateItemOf() {

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

@@ -656,7 +656,7 @@ protected void rawDelete(Context context, Item item) throws AuthorizeException,
         }
 
         //Only clear collections after we have removed everything else from the item
-        item.getCollections().clear();
+        item.clearCollections();
         item.setOwningCollection(null);
 
         // Finally remove item row

dspace-api/src/main/java/org/dspace/content/comparator/NameAscendingComparator.java

@@ -0,0 +1,39 @@
+/**
+ * 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/
+ */
+package org.dspace.content.comparator;
+
+import org.apache.commons.lang.ObjectUtils;
+import org.apache.commons.lang.StringUtils;
+import org.dspace.content.DSpaceObject;
+
+import java.util.Comparator;
+
+public class NameAscendingComparator implements Comparator<DSpaceObject>{
+
+    @Override
+    public int compare(DSpaceObject dso1, DSpaceObject dso2) {
+        if (dso1 == dso2){
+            return 0;
+        }else if (dso1 == null){
+            return -1;
+        }else if (dso2 == null){
+            return 1;
+        }else {
+            String name1 = StringUtils.trimToEmpty(dso1.getName());
+            String name2 = StringUtils.trimToEmpty(dso2.getName());
+
+            //When two DSO's have the same name, use their UUID to put them in an order
+            if(name1.equals(name2)) {
+                return ObjectUtils.compare(dso1.getID(), dso2.getID());
+            } else {
+                return name1.compareToIgnoreCase(name2);
+            }
+        }
+    }
+
+}