Merge pull request #900 from tuub/DS-2497

DS-2497: Keep version numbers stable.

dspace-api/src/main/java/org/dspace/identifier/VersionedHandleIdentifierProvider.java

@@ -108,8 +108,15 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
 
                     // in case of first version we have to modify the previous metadata to be xxxx.1
                     Version version = versionService.getVersion(context, item);
-                    Version previous = versionHistoryService.getPrevious(history, version);
+                    Version previous;
-                    if (versionHistoryService.isFirstVersion(history, previous))
+                    boolean isFirstVersion;
+                    try {
+                        previous = versionHistoryService.getPrevious(context, history, version);
+                        isFirstVersion = versionHistoryService.isFirstVersion(context, history, previous);
+                    } catch (SQLException ex) {
+                        throw new RuntimeException("A problem with the database connection occured.", ex);
+                    }
+                    if (isFirstVersion)
                     {
                         try{
                             //If we have a reviewer he/she might not have the rights to edit the metadata of this item, so temporarly grant them.
@@ -153,7 +160,7 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
             VersionHistory itemHistory = getHistory(context, identifier);
             if(!identifier.matches(".*/.*\\.\\d+") && itemHistory!=null){
 
-                int newVersionNumber = versionHistoryService.getLatestVersion(itemHistory).getVersionNumber()+1;
+                int newVersionNumber = versionHistoryService.getLatestVersion(context, itemHistory).getVersionNumber()+1;
                 String canonical = identifier;
                 identifier = identifier.concat(".").concat("" + newVersionNumber);
                 restoreItAsVersion(context, dso, identifier, item, canonical, itemHistory);
@@ -204,14 +211,15 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
         return null;
     }
 
-    protected void restoreItAsVersion(Context context, DSpaceObject dso, String identifier, Item item, String canonical, VersionHistory history) throws SQLException, IOException, AuthorizeException
+    protected void restoreItAsVersion(Context context, DSpaceObject dso, String identifier, Item item, String canonical, VersionHistory history)
+            throws SQLException, IOException, AuthorizeException
     {
         createNewIdentifier(context, dso, identifier);
         populateHandleMetadata(context, item);
 
         int versionNumber = Integer.parseInt(identifier.substring(identifier.lastIndexOf(".") + 1));
         versionService.createNewVersion(context, history, item, "Restoring from AIP Service", new Date(), versionNumber);
-        Version latest = versionHistoryService.getLatestVersion(history);
+        Version latest = versionHistoryService.getLatestVersion(context, history);
 
 
         // if restoring the lastest version: needed to move the canonical
@@ -320,9 +328,15 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
 
                 // If it is the most current version occurs to move the canonical to the previous version
                 VersionHistory history = versionHistoryService.findByItem(context, item);
-                if(history!=null && versionHistoryService.getLatestVersion(history).getItem().equals(item) && history.getVersions().size() > 1)
+                if(history!=null && versionHistoryService.getLatestVersion(context, history).getItem().equals(item)
+                        && versionService.getVersionsByHistory(context, history).size() > 1)
                 {
-                    Item previous = versionHistoryService.getPrevious(history, versionHistoryService.getLatestVersion(history)).getItem();
+                    Item previous;
+                    try {
+                        previous = versionHistoryService.getPrevious(context, history, versionHistoryService.getLatestVersion(context, history)).getItem();
+                    } catch (SQLException ex) {
+                        throw new RuntimeException("A problem with our database connection occured.", ex);
+                    }
 
                     // Modify Canonical: 12345/100 will point to the new item
                     String canonical = getCanonical(context, previous);
@@ -394,9 +408,14 @@ public class VersionedHandleIdentifierProvider extends IdentifierProvider {
         //  - id.1-->old URL
         //  - id.2-->new URL
         Version version = versionService.getVersion(context, item);
-        Version previous = versionHistoryService.getPrevious(history, version);
+        Version previous;
+        try {
+            previous = versionHistoryService.getPrevious(context, history, version);
+        } catch (SQLException ex) {
+            throw new RuntimeException("A problem with our database connection occured.");
+        }
         String canonical = getCanonical(context, previous.getItem());
-        if (versionHistoryService.isFirstVersion(history, previous))
+        if (versionHistoryService.isFirstVersion(context, history, previous))
         {
             // add a new Identifier for previous item: 12345/100.1
             String identifierPreviousItem=canonical + DOT + previous.getVersionNumber();

dspace-api/src/main/java/org/dspace/versioning/DefaultItemVersionProvider.java

@@ -20,7 +20,9 @@ import org.springframework.beans.factory.annotation.Autowired;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
+import org.apache.log4j.Logger;
 import org.dspace.authorize.ResourcePolicy;
+import org.dspace.versioning.service.VersioningService;
 
 /**
  *
@@ -32,11 +34,15 @@ import org.dspace.authorize.ResourcePolicy;
 public class DefaultItemVersionProvider extends AbstractVersionProvider implements ItemVersionProvider
 {
     
+    Logger log = Logger.getLogger(DefaultItemVersionProvider.class);
+
     @Autowired(required = true)
     protected WorkspaceItemService workspaceItemService;
     @Autowired(required = true)
     protected VersionHistoryService versionHistoryService;
     @Autowired(required = true)
+    protected VersioningService versioningService;
+    @Autowired(required = true)
     protected IdentifierService identifierService;
 
     @Override
@@ -54,15 +60,17 @@ public class DefaultItemVersionProvider extends AbstractVersionProvider implemen
 
     @Override
     public void deleteVersionedItem(Context c, Version versionToDelete, VersionHistory history)
+            throws SQLException
     {
         try
         {
             // if versionToDelete is the current version we have to reinstate the previous version
             // and reset canonical
-            if(versionHistoryService.isLastVersion(history, versionToDelete) && history.getVersions().size() > 1)
+            if(versionHistoryService.isLastVersion(c, history, versionToDelete)
+                    && versioningService.getVersionsByHistory(c, history).size() > 1)
             {
                 // reset the previous version to archived
-                Item item = versionHistoryService.getPrevious(history, versionToDelete).getItem();
+                Item item = versionHistoryService.getPrevious(c, history, versionToDelete).getItem();
                 item.setArchived(true);
                 itemService.update(c, item);
             }

dspace-api/src/main/java/org/dspace/versioning/ItemVersionProvider.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.versioning;
 
+import java.sql.SQLException;
 import org.dspace.content.Item;
 import org.dspace.core.Context;
 
@@ -19,6 +20,6 @@ import org.dspace.core.Context;
  */
 public interface ItemVersionProvider {
     public Item createNewItemAndAddItInWorkspace(Context c, Item item);
-    public void deleteVersionedItem(Context c, Version versionToDelete, VersionHistory history);
+    public void deleteVersionedItem(Context c, Version versionToDelete, VersionHistory history) throws SQLException;
     public Item updateItemState(Context c, Item itemNew, Item previousItem);
 }

dspace-api/src/main/java/org/dspace/versioning/VersionHistory.java

@@ -13,6 +13,7 @@ import org.hibernate.proxy.HibernateProxyHelper;
 import javax.persistence.*;
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.log4j.Logger;
 
 /**
  *
@@ -20,11 +21,14 @@ import java.util.List;
  * @author Fabio Bolognesi (fabio at atmire dot com)
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  */
 @Entity
 @Table(name="versionhistory")
 public class VersionHistory {
     
+    private static final Logger log = Logger.getLogger(VersionHistory.class);
+
     @Id
     @Column(name="versionhistory_id")
     @GeneratedValue(strategy = GenerationType.SEQUENCE ,generator="versionhistory_seq")
@@ -50,7 +54,14 @@ public class VersionHistory {
         return id;
     }
 
-    public List<Version> getVersions() {
+    /**
+     * Please use {@link VersioningService#getVersionsByHistory(Context, VersionHistory)} instead.
+     * 
+     * To keep version number stables we keep information about deleted Versions.
+     * {@code VersioningService.getVersionsByHistory(Context, VersionHistory)} filters
+     * such versions and returns only active versions.
+     */
+    protected List<Version> getVersions() {
         return versions;
     }
 

dspace-api/src/main/java/org/dspace/versioning/VersionHistoryServiceImpl.java

@@ -17,6 +17,7 @@ import org.springframework.beans.factory.annotation.Autowired;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
+import org.dspace.versioning.service.VersioningService;
 
 /**
  *
@@ -24,12 +25,16 @@ import java.util.List;
  * @author Fabio Bolognesi (fabio at atmire dot com)
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  */
 public class VersionHistoryServiceImpl implements VersionHistoryService
 {
     @Autowired(required = true)
     protected VersionHistoryDAO versionHistoryDAO;
     
+    @Autowired(required = true)
+    private VersioningService versioningService;
+
     protected VersionHistoryServiceImpl()
     {
 
@@ -57,36 +62,43 @@ public class VersionHistoryServiceImpl implements VersionHistoryService
 
     // LIST order: descending
     @Override
-    public Version getPrevious(VersionHistory versionHistory, Version version) {
+    public Version getPrevious(Context context, VersionHistory versionHistory, Version version)
-        List<Version> versions = versionHistory.getVersions();
+    throws SQLException
+    {
+        List<Version> versions = versioningService.getVersionsByHistory(context, versionHistory);
         int index = versions.indexOf(version);
-
+        if (index + 1 < versions.size())
-        if( (index+1)==versions.size()) return null;
+        {
-
             return versions.get(index+1);
         }
         
+        return null;
+    }
+
     // LIST order: descending
     @Override
-    public Version getNext(VersionHistory versionHistory, Version version)
+    public Version getNext(Context c, VersionHistory versionHistory, Version version)
+        throws SQLException
     {
-        List<Version> versions = versionHistory.getVersions();
+        List<Version> versions = versioningService.getVersionsByHistory(c, versionHistory);
-        int index = versionHistory.getVersions().indexOf(version);
+        int index = versions.indexOf(version);
 
-        if(index==0)
+        if (index -1 >= 0)
         {
-            return null;
-        }
-
             return versions.get(index -1);
         }
         
+        return null;
+    }
+
     @Override
-    public Version getVersion(VersionHistory versionHistory, Item item) {
+    public Version getVersion(Context context, VersionHistory versionHistory, Item item)
-       List<Version> versions = versionHistory.getVersions();
+            throws SQLException
-       for(Version v : versions)
     {
-           if(v.getItem().getID()==item.getID())
+        Version v = versioningService.getVersion(context, item);
+        if (v != null);
+        {
+            if (versionHistory.equals(v.getVersionHistory()))
             {
                 return v;
             }
@@ -95,20 +107,34 @@ public class VersionHistoryServiceImpl implements VersionHistoryService
     }
     
     @Override
-    public boolean hasNext(VersionHistory versionHistory, Item item)
+    public boolean hasNext(Context context, VersionHistory versionHistory, Item item)
+        throws SQLException
     {
-        Version version = getVersion(versionHistory, item);
+        Version version = getVersion(context, versionHistory, item);
-        return hasNext(versionHistory, version);
+        if (version == null)
+        {
+            return false;
+        }
+        return hasNext(context, versionHistory, version);
     }
 
     @Override
-    public boolean hasNext(VersionHistory versionHistory, Version version)
+    public boolean hasNext(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException
     {
-        return getNext(versionHistory, version)!=null;
+        return getNext(context, versionHistory, version)!=null;
     }
 
     @Override
-    public void add(VersionHistory versionHistory, Version version)
+    public boolean hasVersionHistory(Context context, Item item)
+            throws SQLException
+    {
+        return findByItem(context, item) != null;
+    }
+    
+    @Override
+    public void add(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException
     {
         List<Version> versions = versionHistory.getVersions();
         if(versions==null) versions=new ArrayList<Version>();
@@ -116,44 +142,72 @@ public class VersionHistoryServiceImpl implements VersionHistoryService
     }
 
     @Override
-    public Version getLatestVersion(VersionHistory versionHistory)
+    public Version getLatestVersion(Context context, VersionHistory versionHistory)
+            throws SQLException
     {
-        List<Version> versions = versionHistory.getVersions();
+        List<Version> versions = versioningService.getVersionsByHistory(context, versionHistory);
-        if(versions==null || versions.size()==0)
+        if(versions != null && !versions.isEmpty())
         {
-            return null;
-        }
-
             return versions.get(0);
         }
+        return null;
+    }
 
     @Override
-    public Version getFirstVersion(VersionHistory versionHistory)
+    public Version getFirstVersion(Context context, VersionHistory versionHistory)
+            throws SQLException
     {
-        List<Version> versions = versionHistory.getVersions();
+        List<Version> versions = versioningService.getVersionsByHistory(context, versionHistory);
-        if(versions==null || versions.size()==0)
+
+        if (versions == null)
         {
             return null;
         }
         
+        if (versions.size()-1 >= 0)
+        {
             return versions.get(versions.size()-1);
         }
         
-
+        return null;
-    @Override
-    public boolean isFirstVersion(VersionHistory versionHistory, Version version)
-    {
-        List<Version> versions = versionHistory.getVersions();
-        Version first = versions.get(versions.size()-1);
-        return first.equals(version);
     }
 
     @Override
-    public boolean isLastVersion(VersionHistory versionHistory, Version version)
+    public boolean isFirstVersion(Context context, Item item) throws SQLException
     {
-        List<Version> versions = versionHistory.getVersions();
+        VersionHistory vh = findByItem(context, item);
-        Version last = versions.get(0);
+        if (vh == null)
-        return last.equals(version);
+        {
+            return true;
+        }
+        Version version = versioningService.getVersion(context, item);
+        return isFirstVersion(context, vh, version);
+    }
+    
+    @Override
+    public boolean isFirstVersion(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException
+    {
+        return getFirstVersion(context, versionHistory).equals(version);
+    }
+    
+    @Override
+    public boolean isLastVersion(Context context, Item item) throws SQLException
+    {
+        VersionHistory vh = findByItem(context, item);
+        if (vh == null)
+        {
+            return true;
+        }
+        Version version = versioningService.getVersion(context, item);
+        return isLastVersion(context, vh, version);
+    }
+
+    @Override
+    public boolean isLastVersion(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException
+    {
+        return getLatestVersion(context, versionHistory).equals(version);
     }
 
     @Override

dspace-api/src/main/java/org/dspace/versioning/VersioningConsumer.java

@@ -18,7 +18,6 @@ import org.dspace.versioning.factory.VersionServiceFactory;
 import org.dspace.versioning.service.VersionHistoryService;
 import org.dspace.versioning.service.VersioningService;
 
-import java.sql.SQLException;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -60,10 +59,10 @@ public class VersioningConsumer implements Consumer {
         if(st == Constants.ITEM && et == Event.INSTALL){
             Item item = (Item) event.getSubject(ctx);
             if (item != null && item.isArchived()) {
-                VersionHistory history = retrieveVersionHistory(ctx, item);
+                VersionHistory history = versionHistoryService.findByItem(ctx, item);
                 if (history != null) {
-                    Version latest = versionHistoryService.getLatestVersion(history);
+                    Version latest = versionHistoryService.getLatestVersion(ctx, history);
-                    Version previous = versionHistoryService.getPrevious(history, latest);
+                    Version previous = versionHistoryService.getPrevious(ctx, history, latest);
                     if(previous != null){
                         Item previousItem = previous.getItem();
                         if(previousItem != null){
@@ -98,8 +97,4 @@ public class VersioningConsumer implements Consumer {
         itemsToProcess = null;
     }
 
-
-    protected org.dspace.versioning.VersionHistory retrieveVersionHistory(Context c, Item item) throws SQLException {
-        return versioningService.findVersionHistory(c, item);
-    }
 }

dspace-api/src/main/java/org/dspace/versioning/VersioningServiceImpl.java

@@ -22,7 +22,6 @@ import org.springframework.beans.factory.annotation.Required;
 import java.sql.SQLException;
 import java.util.Date;
 import java.util.List;
-import org.apache.log4j.Logger;
 
 /**
  *
@@ -30,6 +29,7 @@ import org.apache.log4j.Logger;
  * @author Fabio Bolognesi (fabio at atmire dot com)
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
 */
 public class VersioningServiceImpl implements VersioningService {
 
@@ -41,6 +41,10 @@ public class VersioningServiceImpl implements VersioningService {
     private ItemService itemService;
     private DefaultItemVersionProvider provider;
 
+    @Required
+    public void setProvider(DefaultItemVersionProvider provider) {
+        this.provider = provider;
+    }
 
     protected VersioningServiceImpl()
     {
@@ -59,7 +63,7 @@ public class VersioningServiceImpl implements VersioningService {
             VersionHistory vh = versionHistoryService.findByItem(c, item);
             if(vh==null)
             {
-                // first time: create 2 versions, .1(old version) and .2(new version)
+                // first time: create 2 versions: old and new one
                 vh= versionHistoryService.create(c);
 
                 // get dc:date.accessioned to be set as first version date...
@@ -89,17 +93,40 @@ public class VersioningServiceImpl implements VersioningService {
     @Override
     public void removeVersion(Context c, Version version) throws SQLException {
         try{
+            // we will first delete the version and then the item
+            // after deletion of the version we cannot find the item anymore
+            // so we need to get the item now
+            Item item = version.getItem();
+
             VersionHistory history = version.getVersionHistory();
+            if (item != null)
+            {
+                // take care of the item identifiers
                 provider.deleteVersionedItem(c, version, history);
-            versionDAO.delete(c, version);
+            }
             
-            history.removeVersion(version);
+            // to keep version number stables, we do not delete versions,
+            // we set all fields null except versionnumber and versionhistory
+            version.setItem(null);
+            version.setSummary(null);
+            version.setVersionDate(null);
+            version.setePerson(null);
+            versionDAO.save(c, version);
 
-            if(CollectionUtils.isEmpty(history.getVersions())){
+            // if all versions of a version history were deleted,
+            // we delete the version history.
+            if (this.getVersionsByHistory(c, history) == null
+                    || this.getVersionsByHistory(c, history).isEmpty())
+            {
+                // hard delete the previously soft deleted versions
+                for (Version v : history.getVersions())
+                {
+                    versionDAO.delete(c, v);
+                }
+                // delete the version history
                 versionHistoryService.delete(c, history);
             }
-            //Delete the item linked to the version
+            
-            Item item = version.getItem();
             // Completely delete the item
             if (item != null) {
                 itemService.delete(c, item);
@@ -134,11 +161,6 @@ public class VersioningServiceImpl implements VersioningService {
         return null;
     }
 
-    @Override
-    public VersionHistory findVersionHistory(Context c, Item item) throws SQLException {
-        return versionHistoryService.findByItem(c, item);
-    }
-
     @Override
     public Version updateVersion(Context c, Item item, String summary) throws SQLException {
         Version version = versionDAO.findByItem(c, item);
@@ -157,34 +179,43 @@ public class VersioningServiceImpl implements VersioningService {
         try {
             Version version = versionDAO.create(context, new Version());
 
-            version.setVersionNumber(getNextVersionNumer(versionHistoryService.getLatestVersion(history)));
+            version.setVersionNumber(getNextVersionNumer(context, history));
             version.setVersionDate(date);
             version.setePerson(item.getSubmitter());
             version.setItem(item);
             version.setSummary(summary);
             version.setVersionHistory(history);
             versionDAO.save(context, version);
-            versionHistoryService.add(history, version);
+            versionHistoryService.add(context, history, version);
             return version;
         } catch (SQLException e) {
             throw new RuntimeException(e.getMessage(), e);
         }
     }
     
+    @Override
+    public List<Version> getVersionsByHistory(Context c, VersionHistory vh) throws SQLException {
+        List<Version> versions = versionDAO.findVersionsWithItems(c, vh);
+        return versions;
+    }
+
+
 // **** PROTECTED METHODS!!
 
-    protected Version createVersion(Context c, VersionHistory vh, Item item, String summary, Date date) {
+    protected Version createVersion(Context c, VersionHistory vh, Item item, String summary, Date date) throws SQLException {
-        return createNewVersion(c, vh, item, summary, date, getNextVersionNumer(versionHistoryService.getLatestVersion(vh)));
+        return createNewVersion(c, vh, item, summary, date, getNextVersionNumer(c, vh));
     }
 
-    protected int getNextVersionNumer(Version latest){
+    protected int getNextVersionNumer(Context c, VersionHistory vh) throws SQLException{
-        if(latest==null) return 1;
+        int next = versionDAO.getNextVersionNumber(c, vh);
         
-        return latest.getVersionNumber()+1;
+        // check if we have uncommited versions in DSpace's cache
+        if (versionHistoryService.getLatestVersion(c, vh) != null 
+                && versionHistoryService.getLatestVersion(c, vh).getVersionNumber() >= next)
+        {
+            next = versionHistoryService.getLatestVersion(c, vh).getVersionNumber() + 1;
         }
         
-    @Required
+        return next;
-    public void setProvider(DefaultItemVersionProvider provider) {
-        this.provider = provider;
     }
 }

dspace-api/src/main/java/org/dspace/versioning/dao/VersionDAO.java

@@ -13,6 +13,8 @@ import org.dspace.core.GenericDAO;
 import org.dspace.versioning.Version;
 
 import java.sql.SQLException;
+import java.util.List;
+import org.dspace.versioning.VersionHistory;
 
 /**
  * Database Access Object interface class for the Version object.
@@ -24,4 +26,17 @@ import java.sql.SQLException;
 public interface VersionDAO extends GenericDAO<Version>
 {
     public Version findByItem(Context context, Item item) throws SQLException;
+    
+    /**
+     * This method returns all versions of an version history that have items
+     * assigned. We do not delete versions to keep version numbers stable. To
+     * remove a version we set the item, date, summary and eperson null. This
+     * method returns only versions that aren't soft deleted and have items
+     * assigned.
+     */
+    public List<Version> findVersionsWithItems(Context context, VersionHistory versionHistory)
+            throws SQLException;
+
+    public int getNextVersionNumber(Context c, VersionHistory vh) throws SQLException;
+    
 }

dspace-api/src/main/java/org/dspace/versioning/dao/VersionHistoryDAO.java

@@ -13,6 +13,8 @@ import org.dspace.core.GenericDAO;
 import org.dspace.versioning.VersionHistory;
 
 import java.sql.SQLException;
+import java.util.List;
+import org.dspace.versioning.Version;
 
 /**
  * Database Access Object interface class for the VersionHistory object.

dspace-api/src/main/java/org/dspace/versioning/dao/impl/VersionDAOImpl.java

@@ -16,6 +16,10 @@ import org.hibernate.Criteria;
 import org.hibernate.criterion.Restrictions;
 
 import java.sql.SQLException;
+import java.util.List;
+import org.dspace.versioning.VersionHistory;
+import org.hibernate.Query;
+import org.hibernate.criterion.Order;
 
 /**
  * Hibernate implementation of the Database Access Object interface class for the Version object.
@@ -26,6 +30,7 @@ import java.sql.SQLException;
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
  * @author kevinvandevelde at atmire.com
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  */
 public class VersionDAOImpl extends AbstractHibernateDAO<Version> implements VersionDAO
 {
@@ -40,4 +45,26 @@ public class VersionDAOImpl extends AbstractHibernateDAO<Version> implements Ver
         criteria.add(Restrictions.eq("item", item));
         return singleResult(criteria);
     }
+
+    @Override
+    public int getNextVersionNumber(Context c, VersionHistory vh) throws SQLException {
+        Query q = this.createQuery(c, 
+                "SELECT (COALESCE(MAX(versionNumber), 0) + 1) "
+                        + "FROM Version WHERE versionHistory.id = :historyId");
+        q.setParameter("historyId", vh.getId());
+
+        int next = (Integer) q.uniqueResult();
+        return next;
+    }
+    
+    @Override
+    public List<Version> findVersionsWithItems(Context context, VersionHistory versionHistory)
+            throws SQLException
+    {
+        Criteria criteria = createCriteria(context, Version.class);
+        criteria.add(Restrictions.eq("versionHistory", versionHistory));
+        criteria.add(Restrictions.and(Restrictions.isNotNull("item")));
+        criteria.addOrder(Order.desc("versionNumber"));
+        return list(criteria);
+    }
 }

dspace-api/src/main/java/org/dspace/versioning/dao/impl/VersionHistoryDAOImpl.java

@@ -16,6 +16,9 @@ import org.hibernate.Criteria;
 import org.hibernate.criterion.Restrictions;
 
 import java.sql.SQLException;
+import java.util.List;
+import org.dspace.versioning.Version;
+import org.hibernate.criterion.Order;
 
 /**
  * Hibernate implementation of the Database Access Object interface class for the VersionHistory object.
@@ -39,6 +42,7 @@ public class VersionHistoryDAOImpl extends AbstractHibernateDAO<VersionHistory>
         Criteria criteria = createCriteria(context, VersionHistory.class);
         criteria.createAlias("versions", "v");
         criteria.add(Restrictions.eq("v.item", item));
+        criteria.addOrder(Order.desc("v.versionNumber"));
         return singleResult(criteria);
     }
 }

dspace-api/src/main/java/org/dspace/versioning/service/VersionHistoryService.java

@@ -14,6 +14,7 @@ import org.dspace.versioning.Version;
 import org.dspace.versioning.VersionHistory;
 
 import java.sql.SQLException;
+import java.util.List;
 
 /**
  *
@@ -21,31 +22,52 @@ import java.sql.SQLException;
  * @author Fabio Bolognesi (fabio at atmire dot com)
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  */
 public interface VersionHistoryService extends DSpaceCRUDService<VersionHistory> {
     
-    public Version getLatestVersion(VersionHistory versionHistory);
+    public void add(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
     
-    public Version getFirstVersion(VersionHistory versionHistory);
+    public VersionHistory findByItem(Context context, Item item)
+            throws SQLException;
     
-    public Version getPrevious(VersionHistory versionHistory, Version version);
+    public Version getFirstVersion(Context context, VersionHistory versionHistory)
+            throws SQLException;
     
-    public Version getNext(VersionHistory versionHistory, Version version);
+    public Version getLatestVersion(Context context, VersionHistory versionHistory)
+            throws SQLException;
     
-    public boolean hasNext(VersionHistory versionHistory, Version version);
+    public Version getNext(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
     
-    public void add(VersionHistory versionHistory, Version version);
+    public Version getPrevious(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
     
-    public Version getVersion(VersionHistory versionHistory, Item item);
+    public Version getVersion(Context context, VersionHistory versionHistory, Item item)
+            throws SQLException;
     
-    public boolean hasNext(VersionHistory versionHistory, Item item);
+    public boolean hasNext(Context context, VersionHistory versionHistory, Item item)
+            throws SQLException;
 
-    public boolean isFirstVersion(VersionHistory versionHistory, Version version);
+    public boolean hasNext(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
     
-    public boolean isLastVersion(VersionHistory versionHistory, Version version);
+    public boolean hasVersionHistory(Context context, Item item)
+            throws SQLException;
+    
+    public boolean isFirstVersion(Context context, Item item)
+            throws SQLException;
+
+    public boolean isFirstVersion(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
+
+    public boolean isLastVersion(Context context, Item item)
+            throws SQLException;
+
+    public boolean isLastVersion(Context context, VersionHistory versionHistory, Version version)
+            throws SQLException;
 
     public void remove(VersionHistory versionHistory, Version version);
 
-    public VersionHistory findByItem(Context context, Item item) throws SQLException;
-
 }

dspace-api/src/main/java/org/dspace/versioning/service/VersioningService.java

@@ -14,6 +14,7 @@ import org.dspace.versioning.VersionHistory;
 
 import java.sql.SQLException;
 import java.util.Date;
+import java.util.List;
 
 /**
  *
@@ -28,6 +29,15 @@ public interface VersioningService {
 
     Version createNewVersion(Context c, Item itemId, String summary);
     
+    /**
+     * Returns all versions of a version history.
+     * To keep version numbers stable we do not delete versions, we do only set 
+     * the item, date, summary and eperson null. This methods returns only those
+     * versions that have an item assigned.
+     * @return All versions of a version history that have an item assigned.
+     */
+    List<Version> getVersionsByHistory(Context c, VersionHistory vh) throws SQLException;
+
     void removeVersion(Context c, Version version) throws SQLException;
 
     void removeVersion(Context c, Item item) throws SQLException;
@@ -38,8 +48,6 @@ public interface VersioningService {
 
     Version restoreVersion(Context c, Version version, String summary);
 
-    VersionHistory findVersionHistory(Context c, Item itemId) throws SQLException;
-
     Version updateVersion(Context c, Item itemId, String summary) throws SQLException;
 
     Version getVersion(Context c, Item item) throws SQLException;

dspace-api/src/test/java/org/dspace/content/VersioningTest.java

@@ -122,7 +122,7 @@ public class VersioningTest extends AbstractUnitTest {
     {
         VersionHistory versionHistory = versionHistoryService.findByItem(context, originalItem);
         assertThat("testFindVersionHistory", versionHistory, notNullValue());
-        Version version = versionHistoryService.getVersion(versionHistory, versionedItem);
+        Version version = versionHistoryService.getVersion(context, versionHistory, versionedItem);
         assertThat("testFindVersion", version, notNullValue());
     }
 
@@ -134,7 +134,7 @@ public class VersioningTest extends AbstractUnitTest {
     {
         //Start by creating a new item !
         VersionHistory versionHistory = versionHistoryService.findByItem(context, originalItem);
-        Version version = versionHistoryService.getVersion(versionHistory, versionedItem);
+        Version version = versionHistoryService.getVersion(context, versionHistory, versionedItem);
         assertThat("Test_version_summary", summary, equalTo(version.getSummary()));
     }
 

dspace-jspui/src/main/java/org/dspace/app/webui/components/VersioningItemHome.java

@@ -28,6 +28,9 @@ import org.dspace.plugin.ItemHomeProcessor;
 import org.dspace.plugin.PluginException;
 import org.dspace.versioning.Version;
 import org.dspace.versioning.VersionHistory;
+import org.dspace.versioning.factory.VersionServiceFactory;
+import org.dspace.versioning.service.VersionHistoryService;
+import org.dspace.versioning.service.VersioningService;
 
 public class VersioningItemHome implements ItemHomeProcessor {
 
@@ -38,9 +41,15 @@ public class VersioningItemHome implements ItemHomeProcessor {
 
     private HandleService handleService;
     
+    private VersionHistoryService versionHistoryService;
+    
+    private VersioningService versioningService;
+
     public VersioningItemHome() {
         handleService = HandleServiceFactory.getInstance().getHandleService();
         itemService = ContentServiceFactory.getInstance().getItemService();
+        versioningService = VersionServiceFactory.getInstance().getVersionService();
+        versionHistoryService = VersionServiceFactory.getInstance().getVersionHistoryService();
     }
 
     @Override
@@ -61,24 +70,22 @@ public class VersioningItemHome implements ItemHomeProcessor {
         if (versioningEnabled) {
             try {
                 if (itemService.canEdit(context, item)) {
-					if (VersionUtil.isLatest(context, item) && item.isArchived()) {
+                    if (versionHistoryService.isLastVersion(context, item) 
+                            && item.isArchived()) {
                         hasVersionButton = true;
                     }
                 }
-				if (VersionUtil.hasVersionHistory(context, item)) {
+                if (versionHistoryService.hasVersionHistory(context, item)) {
                     hasVersionHistory = true;
-					history = VersionUtil.retrieveVersionHistory(context, item);
+                    history = versionHistoryService.findByItem(context, item);
-					for(Version versRow : history.getVersions()) {  
+                    for (Version versRow : versioningService.getVersionsByHistory(context, history)) {
                         //Skip items currently in submission
-						if(VersionUtil.isItemInSubmission(context, versRow.getItem()))
+                        if (VersionUtil.isItemInSubmission(context, versRow.getItem())) {
-						{
                             continue;
                         }
-						else {
                         historyVersions.add(versRow);
                     }
                 }
-				}
             } catch (SQLException e) {
                 throw new PluginException(e.getMessage());
             }
@@ -91,7 +98,8 @@ public class VersioningItemHome implements ItemHomeProcessor {
                 throw new PluginException(e.getMessage());
             }
             
-			if (latestVersion != null && latestVersion.getItem() != null
+            if (latestVersion != null) {
+                if (latestVersion.getItem() != null
                         && !latestVersion.getItem().getID().equals(item.getID())) {
                     // We have a newer version
                     Item latestVersionItem = latestVersion.getItem();
@@ -112,6 +120,7 @@ public class VersioningItemHome implements ItemHomeProcessor {
                     }
                 }
             }
+        }
 
         request.setAttribute("versioning.enabled", versioningEnabled);
         request.setAttribute("versioning.hasversionbutton", hasVersionButton);

dspace-jspui/src/main/java/org/dspace/app/webui/servlet/VersionHistoryServlet.java

@@ -7,6 +7,7 @@
  */
 package org.dspace.app.webui.servlet;
 
+import com.hp.hpl.jena.sparql.vocabulary.DOAP;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.UUID;
@@ -30,6 +31,7 @@ import org.dspace.versioning.Version;
 import org.dspace.versioning.VersionHistory;
 import org.dspace.versioning.factory.VersionServiceFactory;
 import org.dspace.versioning.service.VersionHistoryService;
+import org.dspace.versioning.service.VersioningService;
 
 /**
  * Servlet for handling the operations in the version history page
@@ -49,6 +51,9 @@ public class VersionHistoryServlet extends DSpaceServlet
     private final transient VersionHistoryService versionHistoryService
              = VersionServiceFactory.getInstance().getVersionHistoryService();
     
+    private final transient VersioningService versioningService
+            = VersionServiceFactory.getInstance().getVersionService();
+
     @Override
     protected void doDSGet(Context context, HttpServletRequest request,
             HttpServletResponse response) throws ServletException, IOException,
@@ -87,11 +92,10 @@ public class VersionHistoryServlet extends DSpaceServlet
         request.setAttribute("showSubmitter", show_submitter);
         
         // manage if versionID is not came by request
-        VersionHistory history = VersionUtil.retrieveVersionHistory(context,
+        VersionHistory history = versionHistoryService.findByItem(context, item);
-                item);
         if (versionID == null || versionID.isEmpty())
         {
-            Version version = versionHistoryService.getVersion(history, item);
+            Version version = versionHistoryService.getVersion(context, history, item);
             if (version != null)
             {
                 versionID = String.valueOf(version.getId());
@@ -142,6 +146,7 @@ public class VersionHistoryServlet extends DSpaceServlet
         request.setAttribute("item", item);
         request.setAttribute("itemID", itemID);
         request.setAttribute("versionID", versionID);
+        request.setAttribute("allVersions", versioningService.getVersionsByHistory(context, history));
         JSPManager.showJSP(request, response, "/tools/version-history.jsp");
     }
 

dspace-jspui/src/main/java/org/dspace/app/webui/util/VersionUtil.java

@@ -38,6 +38,7 @@ import org.dspace.workflow.factory.WorkflowServiceFactory;
  * Item level versioning feature utility method
  * 
  * @author Luigi Andrea Pascarelli
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  * 
  */
 public class VersionUtil
@@ -196,8 +197,7 @@ public class VersionUtil
         try
         {
             Item item = itemService.find(context, itemId);
-            VersionHistory versionHistory = versioningService
+            VersionHistory versionHistory = versionHistoryService.findByItem(context, item);
-                    .findVersionHistory(context, item);
 
             for (String id : versionIDs)
             {
@@ -206,7 +206,7 @@ public class VersionUtil
 
             // Retrieve the latest version of our history (IF any is even
             // present)
-            Version latestVersion = versionHistoryService.getLatestVersion(versionHistory);
+            Version latestVersion = versionHistoryService.getLatestVersion(context, versionHistory);
             if (latestVersion == null)
             {
                 return null;
@@ -228,38 +228,6 @@ public class VersionUtil
 
     }
 
-    /**
-     * Check if the item is the last version builded
-     * 
-     * @param context
-     * @param item
-     * @return true or false
-     * @throws SQLException 
-     */
-    public static boolean isLatest(Context context, Item item) throws SQLException
-    {
-    	initialize();
-        VersionHistory history = retrieveVersionHistory(context, item);
-		return (history == null
-				|| versionHistoryService.getLatestVersion(history).getItem().getID()
-					.equals(item.getID()));
-    }
-
-    /**
-     * Check if the item have a version history
-     * 
-     * @param context
-     * @param item
-     * @return true or false
-     * @throws SQLException 
-     */
-    public static boolean hasVersionHistory(Context context, Item item) throws SQLException
-    {
-    	initialize();
-        VersionHistory history = retrieveVersionHistory(context, item);
-        return (history != null);
-    }
-
     /**
      * Return the latest version, if there isn't or the user not have permission
      * then return null.
@@ -273,11 +241,11 @@ public class VersionUtil
             throws SQLException
     {
     	initialize();
-        VersionHistory history = retrieveVersionHistory(context, item);
+        VersionHistory history = versionHistoryService.findByItem(context, item);
 
         if (history != null)
         {
-            List<Version> allVersions = history.getVersions();
+            List<Version> allVersions = versioningService.getVersionsByHistory(context, history);
             for (Version version : allVersions)
             {
                 if (version.getItem().isArchived()
@@ -293,23 +261,6 @@ public class VersionUtil
         return null;
     }
 
-    /**
-     * Retrieve the version history of the item
-     * 
-     * @param context
-     * @param item
-     * @return history
-     * @throws SQLException 
-     */
-    public static VersionHistory retrieveVersionHistory(Context context,
-            Item item) throws SQLException
-    {
-    	initialize();
-        VersioningService versioningService = new DSpace()
-                .getSingletonService(VersioningService.class);
-        return versioningService.findVersionHistory(context, item);
-    }
-
     /**
      * Check item if it is in workspace or workflow
      * 

dspace-jspui/src/main/webapp/display-item.jsp

@@ -182,7 +182,7 @@
                 	<% if(hasVersionHistory) { %>			                
                 	<form method="get" action="<%= request.getContextPath() %>/tools/history">
                     	<input type="hidden" name="itemID" value="<%= item.getID() %>" />
-                    	<input type="hidden" name="versionID" value="<%= versionHistoryService.getVersion(history, item)!=null?versionHistoryService.getVersion(history, item).getId():null %>" />
+                    	<input type="hidden" name="versionID" value="<%= versionHistoryService.getVersion(context, history, item)!=null?versionHistoryService.getVersion(context, history, item).getId():null %>" />
                     	<input class="btn btn-info col-md-12" type="submit" name="submit" value="<fmt:message key="jsp.general.version.history.button"/>" />
                 	</form>         	         	
 					<% } %>

dspace-jspui/src/main/webapp/tools/version-history.jsp

@@ -7,6 +7,7 @@
     http://www.dspace.org/license/
 
 --%>
+<%@page import="java.util.List"%>
 <%--
   - Version history table with functionalities
   -
@@ -20,6 +21,7 @@
 <%@page import="org.dspace.versioning.Version"%>
 <%@page import="org.dspace.app.webui.util.VersionUtil"%>
 <%@page import="org.dspace.versioning.VersionHistory"%>
+<%@page import="org.dspace.versioning.factory.VersionServiceFactory"%>
 <%@ page import="java.util.UUID" %>
 <%@ page contentType="text/html;charset=UTF-8" %>
 
@@ -74,7 +76,7 @@ var j = jQuery.noConflict();
     <%-- Versioning table --%>
 <%
                 
-	VersionHistory history = VersionUtil.retrieveVersionHistory(context, item);
+        List<Version> allVersions = (List<Version>) request.getAttribute("allVersions");
 						 
 %>
 	<div id="versionHistory">
@@ -97,8 +99,7 @@ var j = jQuery.noConflict();
 				id="t5" class="oddRowEvenCol"><fmt:message key="jsp.version.history.column5"/> </th>
 		</tr>
 		
-		<% for(Version versRow : history.getVersions()) {  
+                <% for(Version versRow : allVersions) {
-		
 			EPerson versRowPerson = versRow.getEPerson();
 			String[] identifierPath = VersionUtil.addItemIdentifier(item, versRow);
 

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/versioning/Navigation.java

@@ -51,6 +51,7 @@ import java.util.UUID;
  * @author Fabio Bolognesi (fabio at atmire dot com)
  * @author Mark Diggory (markd at atmire dot com)
  * @author Ben Bosman (ben at atmire dot com)
+ * @author Pascal-Nicolas Becker (dspace at pascal dash becker dot de)
  */
 public class Navigation extends AbstractDSpaceTransformer implements CacheableProcessingComponent
 {
@@ -172,14 +173,14 @@ public class Navigation extends AbstractDSpaceTransformer implements CacheablePr
             if(authorizeService.isAdmin(this.context, item.getOwningCollection()))
             {
                 boolean headAdded=false;
-                if(isLatest(item) && item.isArchived())
+                if(versionHistoryService.isLastVersion(this.context, item) && item.isArchived())
                 {
                     context.setHead(T_context_head);
                     headAdded=true;
                     context.addItem().addXref(contextPath+"/item/version?itemID="+item.getID(), T_context_create_version);
                 }
 
-                if(hasVersionHistory(item))
+                if(versionHistoryService.hasVersionHistory(this.context, item))
                 {
                     if(!headAdded)
                     {
@@ -213,17 +214,4 @@ public class Navigation extends AbstractDSpaceTransformer implements CacheablePr
         super.recycle();
     }
 
-    private boolean isLatest(Item item) throws SQLException
-    {
-        org.dspace.versioning.VersionHistory history = versionHistoryService.findByItem(context, item);
-        return (history==null || versionHistoryService.getLatestVersion(history).getItem().equals(item));
-    }
-
-
-    private boolean hasVersionHistory(Item item) throws SQLException
-    {
-        org.dspace.versioning.VersionHistory history = versionHistoryService.findByItem(context, item);
-        return (history!=null);
-    }
-
 }

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/versioning/VersionHistoryForm.java

@@ -32,6 +32,7 @@ import org.dspace.workflow.factory.WorkflowServiceFactory;
 
 import java.sql.SQLException;
 import java.util.*;
+import org.dspace.versioning.service.VersionHistoryService;
 
 /**
  *
@@ -57,6 +58,7 @@ public class VersionHistoryForm extends AbstractDSpaceTransformer {
 
     protected AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService();
     protected VersioningService versioningService = VersionServiceFactory.getInstance().getVersionService();
+    protected VersionHistoryService versionHistoryService = VersionServiceFactory.getInstance().getVersionHistoryService();
     protected ItemService itemService = ContentServiceFactory.getInstance().getItemService();
     protected WorkspaceItemService workspaceItemService = ContentServiceFactory.getInstance().getWorkspaceItemService();
     protected WorkflowItemService workflowItemService = WorkflowServiceFactory.getInstance().getWorkflowItemService();
@@ -84,7 +86,7 @@ public class VersionHistoryForm extends AbstractDSpaceTransformer {
 
 
 
-        VersionHistory versionHistory = retrieveVersionHistory(item);
+        VersionHistory versionHistory = versionHistoryService.findByItem(context, item);
         if(versionHistory!=null)
         {
             Division main = createMain(body);
@@ -125,12 +127,6 @@ public class VersionHistoryForm extends AbstractDSpaceTransformer {
 
     }
 
-    private VersionHistory retrieveVersionHistory(Item item) throws WingException, SQLException
-    {
-        return versioningService.findVersionHistory(context, item);
-    }
-
-
     private Division createMain(Body body) throws WingException
     {
         Division main = body.addInteractiveDivision("view-verion-history", contextPath+"/item/versionhistory", Division.METHOD_POST, "view version history");
@@ -160,9 +156,8 @@ public class VersionHistoryForm extends AbstractDSpaceTransformer {
 
         if(history != null)
         {
-            for(Version version : history.getVersions())
+            for(Version version : versioningService.getVersionsByHistory(context, history))
             {
-
                 // Skip items currently in submission
                 if(isItemInSubmission(version.getItem()))
                 {
@@ -226,10 +221,13 @@ public class VersionHistoryForm extends AbstractDSpaceTransformer {
         }
     }
 
-    private void addButtons(Division main, VersionHistory history) throws WingException {
+    private void addButtons(Division main, VersionHistory history)
+            throws WingException, SQLException
+    {
         Para actions = main.addPara();
 
-        if(history!=null && history.getVersions().size() > 0)
+        if(history!=null 
+                && versioningService.getVersionsByHistory(context, history).size() > 0)
         {
             actions.addButton("submit_delete").setValue(T_submit_delete);
         }

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/versioning/VersionItemForm.java

@@ -101,7 +101,7 @@ public class VersionItemForm extends AbstractDSpaceTransformer {
         Para actions = main.addPara();
 
         org.dspace.versioning.VersionHistory history = retrieveVersionHistory(item);
-        if(history!=null && versionHistoryService.hasNext(history ,item))
+        if(history!=null && versionHistoryService.hasNext(context, history ,item))
         {
             actions.addButton("submit_update_version").setValue(T_submit_update_version);
         }

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/versioning/VersionManager.java

@@ -155,14 +155,14 @@ public class VersionManager {
 
             Item item = itemService.find(context, itemId);
 
-            VersionHistory versionHistory = versioningService.findVersionHistory(context, item);
+            VersionHistory versionHistory = versionHistoryService.findByItem(context, item);
 
             for (String id : versionIDs) {
                 versioningService.removeVersion(context, versioningService.getVersion(context, Integer.parseInt(id)));
             }
 
             //Retrieve the latest version of our history (IF any is even present)
-            Version latestVersion = versionHistoryService.getLatestVersion(versionHistory);
+            Version latestVersion = versionHistoryService.getLatestVersion(context, versionHistory);
             if(latestVersion == null){
                 result.setParameter("itemID", null);
             }else{

dspace-xmlui/src/main/java/org/dspace/app/xmlui/aspect/versioning/VersionNoticeTransformer.java

@@ -31,6 +31,7 @@ import org.xml.sax.SAXException;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.List;
+import org.dspace.versioning.service.VersioningService;
 
 /**
  * Adds a notice to item page in the following conditions
@@ -51,6 +52,7 @@ public class VersionNoticeTransformer extends AbstractDSpaceTransformer {
 
     protected AuthorizeService authorizeService = AuthorizeServiceFactory.getInstance().getAuthorizeService();
     protected HandleService handleService = HandleServiceFactory.getInstance().getHandleService();
+    protected VersioningService versioningService = VersionServiceFactory.getInstance().getVersionService();
     protected VersionHistoryService versionHistoryService = VersionServiceFactory.getInstance().getVersionHistoryService();
 
     @Override
@@ -104,7 +106,7 @@ public class VersionNoticeTransformer extends AbstractDSpaceTransformer {
 
     private Version retrieveLatestVersion(VersionHistory history, Item item) throws SQLException {
         //Attempt to retrieve the latest version
-        List<Version> allVersions = history.getVersions();
+        List<Version> allVersions = versioningService.getVersionsByHistory(context, history);
         for (Version version : allVersions) {
             if (version.getItem().isArchived() || authorizeService.isAdmin(context, item.getOwningCollection()))
             {