Merge pull request #1277 from tdonohue/DS-3039

DS-3039 : Fix Community item counting logic. Also cleanup item counting elsewhere in code
This commit is contained in:
Tim Donohue
2016-02-19 14:05:20 -06:00
12 changed files with 301 additions and 235 deletions

View File

@@ -15,10 +15,11 @@ import org.dspace.content.service.CommunityService;
import org.dspace.content.service.ItemService; import org.dspace.content.service.ItemService;
import org.dspace.core.Context; import org.dspace.core.Context;
import org.dspace.content.DSpaceObject; import org.dspace.content.DSpaceObject;
import org.dspace.core.ConfigurationManager;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.List; import java.util.List;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
/** /**
* This class provides a standard interface to all item counting * This class provides a standard interface to all item counting
@@ -37,189 +38,191 @@ import java.util.List;
*/ */
public class ItemCounter public class ItemCounter
{ {
/** Log4j logger */ /** Log4j logger */
private static Logger log = Logger.getLogger(ItemCounter.class); private static Logger log = Logger.getLogger(ItemCounter.class);
/** DAO to use to store and retrieve data */ /** DAO to use to store and retrieve data */
private ItemCountDAO dao; private ItemCountDAO dao;
/** DSpace Context */ /** DSpace Context */
private Context context; private Context context;
protected CommunityService communityService; protected CommunityService communityService;
protected ItemService itemService; protected ItemService itemService;
protected ConfigurationService configurationService;
/** /**
* method invoked by CLI which will result in the number of items * method invoked by CLI which will result in the number of items
* in each community and collection being cached. These counts will * in each community and collection being cached. These counts will
* not update themselves until this is run again. * not update themselves until this is run again.
* *
* @param args * @param args
*/ */
public static void main(String[] args) public static void main(String[] args)
throws ItemCountException, SQLException throws ItemCountException, SQLException
{ {
Context context = new Context(); Context context = new Context();
ItemCounter ic = new ItemCounter(context); ItemCounter ic = new ItemCounter(context);
ic.buildItemCounts(); ic.buildItemCounts();
context.complete(); context.complete();
} }
/** /**
* Construct a new item counter which will use the give DSpace Context * Construct a new item counter which will use the give DSpace Context
* *
* @param context * @param context
* @throws ItemCountException * @throws ItemCountException
*/ */
public ItemCounter(Context context) public ItemCounter(Context context)
throws ItemCountException throws ItemCountException
{ {
this.context = context; this.context = context;
this.dao = ItemCountDAOFactory.getInstance(this.context); this.dao = ItemCountDAOFactory.getInstance(this.context);
this.communityService = ContentServiceFactory.getInstance().getCommunityService(); this.communityService = ContentServiceFactory.getInstance().getCommunityService();
this.itemService = ContentServiceFactory.getInstance().getItemService(); this.itemService = ContentServiceFactory.getInstance().getItemService();
} this.configurationService = DSpaceServicesFactory.getInstance().getConfigurationService();
}
/** /**
* This method does the grunt work of drilling through and iterating * This method does the grunt work of drilling through and iterating
* over all of the communities and collections in the system and * over all of the communities and collections in the system and
* obtaining and caching the item counts for each one. * obtaining and caching the item counts for each one.
* *
* @throws ItemCountException * @throws ItemCountException
*/ */
public void buildItemCounts() public void buildItemCounts()
throws ItemCountException throws ItemCountException
{ {
try try
{ {
List<Community> tlc = communityService.findAllTop(context); List<Community> tlc = communityService.findAllTop(context);
for (Community aTlc : tlc) { for (Community aTlc : tlc) {
count(aTlc); count(aTlc);
} }
} }
catch (SQLException e) catch (SQLException e)
{ {
log.error("caught exception: ", e); log.error("caught exception: ", e);
throw new ItemCountException(e); throw new ItemCountException(e);
} }
} }
/** /**
* Get the count of the items in the given container. If the configuration * Get the count of the items in the given container. If the configuration
* value webui.strengths.cache is equal to 'true' this will return the * value webui.strengths.cache is equal to 'true' this will return the
* cached value if it exists. If it is equal to 'false' it will count * cached value if it exists. If it is equal to 'false' it will count
* the number of items in the container in real time. * the number of items in the container in real time.
* *
* @param dso * @param dso
* @throws ItemCountException * @throws ItemCountException
* @throws SQLException * @throws SQLException
*/ */
public int getCount(DSpaceObject dso) public int getCount(DSpaceObject dso)
throws ItemCountException throws ItemCountException
{ {
boolean useCache = ConfigurationManager.getBooleanProperty( boolean useCache = configurationService.getBooleanProperty(
"webui.strengths.cache", true); "webui.strengths.cache", true);
if (useCache) if (useCache)
{ {
return dao.getCount(dso); return dao.getCount(dso);
} }
// if we make it this far, we need to manually count // if we make it this far, we need to manually count
if (dso instanceof Collection) if (dso instanceof Collection)
{ {
try { try {
return itemService.countItems(context, (Collection) dso); return itemService.countItems(context, (Collection) dso);
} catch (SQLException e) { } catch (SQLException e) {
log.error("caught exception: ", e); log.error("caught exception: ", e);
throw new ItemCountException(e); throw new ItemCountException(e);
} }
} }
if (dso instanceof Community) if (dso instanceof Community)
{ {
try { try {
return communityService.countItems(context, ((Community) dso)); return itemService.countItems(context, ((Community) dso));
} catch (SQLException e) { } catch (SQLException e) {
log.error("caught exception: ", e); log.error("caught exception: ", e);
throw new ItemCountException(e); throw new ItemCountException(e);
} }
} }
return 0; return 0;
} }
/** /**
* Remove any cached data for the given container * Remove any cached data for the given container
* *
* @param dso * @param dso
* @throws ItemCountException * @throws ItemCountException
*/ */
public void remove(DSpaceObject dso) public void remove(DSpaceObject dso)
throws ItemCountException throws ItemCountException
{ {
dao.remove(dso); dao.remove(dso);
} }
/** /**
* count and cache the number of items in the community. This * count and cache the number of items in the community. This
* will include all sub-communities and collections in the * will include all sub-communities and collections in the
* community. It will also recurse into sub-communities and * community. It will also recurse into sub-communities and
* collections and call count() on them also. * collections and call count() on them also.
* *
* Therefore, the count the contents of the entire system, it is * Therefore, the count the contents of the entire system, it is
* necessary just to call this method on each top level community * necessary just to call this method on each top level community
* *
* @param community * @param community
* @throws ItemCountException * @throws ItemCountException
*/ */
protected void count(Community community) protected void count(Community community)
throws ItemCountException throws ItemCountException
{ {
try try
{ {
// first count the community we are in // first count the community we are in
int count = communityService.countItems(context, community); int count = itemService.countItems(context, community);
dao.communityCount(community, count); dao.communityCount(community, count);
// now get the sub-communities // now get the sub-communities
List<Community> scs = community.getSubcommunities(); List<Community> scs = community.getSubcommunities();
for (Community sc : scs) { for (Community sc : scs) {
count(sc); count(sc);
} }
// now get the collections // now get the collections
List<Collection> cols = community.getCollections(); List<Collection> cols = community.getCollections();
for (Collection col : cols) { for (Collection col : cols) {
count(col); count(col);
} }
} }
catch (SQLException e) catch (SQLException e)
{ {
log.error("caught exception: ", e); log.error("caught exception: ", e);
throw new ItemCountException(e); throw new ItemCountException(e);
} }
} }
/** /**
* count and cache the number of items in the given collection * count and cache the number of items in the given collection
* *
* @param collection * @param collection
* @throws ItemCountException * @throws ItemCountException
*/ */
protected void count(Collection collection) protected void count(Collection collection)
throws ItemCountException throws ItemCountException
{ {
try try
{ {
int ccount = itemService.countItems(context, collection); int ccount = itemService.countItems(context, collection);
dao.collectionCount(collection, ccount); dao.collectionCount(collection, ccount);
} }
catch (SQLException e) catch (SQLException e)
{ {
log.error("caught exception: ", e); log.error("caught exception: ", e);
throw new ItemCountException(e); throw new ItemCountException(e);
} }
} }
} }

View File

@@ -576,22 +576,6 @@ public class CommunityServiceImpl extends DSpaceObjectServiceImpl<Community> imp
authorizeService.authorizeAction(context, community, Constants.WRITE); authorizeService.authorizeAction(context, community, Constants.WRITE);
} }
@Override
public int countItems(Context context, Community community) throws SQLException {
int total = 0;
// add collection counts
List<Collection> cols = community.getCollections();
for (Collection col : cols) {
total += itemService.countItems(context, col);
}
// add sub-community counts
List<Community> comms = community.getSubcommunities();
for (int j = 0; j < comms.size(); j++) {
total += countItems(context, comms.get(j));
}
return total;
}
@Override @Override
public Community findByAdminGroup(Context context, Group group) throws SQLException { public Community findByAdminGroup(Context context, Group group) throws SQLException {
return communityDAO.findByAdminGroup(context, group); return communityDAO.findByAdminGroup(context, group);

View File

@@ -1121,12 +1121,11 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
@Override @Override
public int countItems(Context context, Community community) throws SQLException { public int countItems(Context context, Community community) throws SQLException {
// First we need a list of all collections under this community in the hierarchy
List<Collection> collections = communityService.getAllCollections(context, community); List<Collection> collections = communityService.getAllCollections(context, community);
int itemCount = 0;
for(Collection collection : collections) { // Now, lets count unique items across that list of collections
itemCount += countItems(context, collection); return itemDAO.countItems(context, collections, true, false);
}
return itemCount;
} }
@Override @Override
@@ -1166,12 +1165,14 @@ public class ItemServiceImpl extends DSpaceObjectServiceImpl<Item> implements It
} }
@Override @Override
public int getNotArchivedItemsCount(Context context) throws SQLException { public int countNotArchivedItems(Context context) throws SQLException {
return itemDAO.countNotArchived(context); // return count of items not in archive and also not withdrawn
return itemDAO.countItems(context, false, false);
} }
@Override @Override
public int countWithdrawnItems(Context context) throws SQLException { public int countWithdrawnItems(Context context) throws SQLException {
return itemDAO.countWithdrawn(context); // return count of items that are in archive and withdrawn
return itemDAO.countItems(context, true, true);
} }
} }

View File

@@ -56,7 +56,31 @@ public interface ItemDAO extends DSpaceObjectLegacySupportDAO<Item>
public Iterator<Item> findAllByCollection(Context context, Collection collection) throws SQLException; public Iterator<Item> findAllByCollection(Context context, Collection collection) throws SQLException;
/**
* Count number of items in a given collection
* @param context
* @param collection the collection
* @param includeArchived whether to include archived items in count
* @param includeWithdrawn whether to include withdrawn items in count
* @return item count
* @throws SQLException
*/
public int countItems(Context context, Collection collection, boolean includeArchived, boolean includeWithdrawn) throws SQLException; public int countItems(Context context, Collection collection, boolean includeArchived, boolean includeWithdrawn) throws SQLException;
/**
* Count number of unique items across several collections at once.
* This method can be used with
* {@link org.dspace.content.service.CommunityService#getAllCollections(Context,Community)}
* to determine the unique number of items in a Community.
*
* @param context
* @param collections the list of collections
* @param includeArchived whether to include archived items in count
* @param includeWithdrawn whether to include withdrawn items in count
* @return item count
* @throws SQLException
*/
public int countItems(Context context, List<Collection> collections, boolean includeArchived, boolean includeWithdrawn) throws SQLException;
/** /**
* Get all Items installed or withdrawn, discoverable, and modified since a Date. * Get all Items installed or withdrawn, discoverable, and modified since a Date.
@@ -71,9 +95,22 @@ public interface ItemDAO extends DSpaceObjectLegacySupportDAO<Item>
boolean withdrawn, boolean discoverable, Date lastModified) boolean withdrawn, boolean discoverable, Date lastModified)
throws SQLException; throws SQLException;
/**
* Count total number of items (rows in item table)
* @param context
* @return total count
* @throws SQLException
*/
int countRows(Context context) throws SQLException; int countRows(Context context) throws SQLException;
int countNotArchived(Context context) throws SQLException; /**
* Count number of items based on specific status flags
int countWithdrawn(Context context) throws SQLException; * @param context
* @param includeArchived whether to include archived items in count
* @param includeWithdrawn whether to include withdrawn items in count
* @return count of items
* @throws SQLException
*/
int countItems(Context context, boolean includeArchived, boolean includeWithdrawn) throws SQLException;
} }

View File

@@ -11,7 +11,6 @@ import org.apache.log4j.Logger;
import org.dspace.content.Collection; import org.dspace.content.Collection;
import org.dspace.content.Item; import org.dspace.content.Item;
import org.dspace.content.MetadataField; import org.dspace.content.MetadataField;
import org.dspace.content.MetadataSchema;
import org.dspace.content.MetadataValue; import org.dspace.content.MetadataValue;
import org.dspace.content.dao.ItemDAO; import org.dspace.content.dao.ItemDAO;
import org.dspace.core.Context; import org.dspace.core.Context;
@@ -25,10 +24,8 @@ import org.hibernate.criterion.Property;
import org.hibernate.criterion.Restrictions; import org.hibernate.criterion.Restrictions;
import org.hibernate.criterion.Subqueries; import org.hibernate.criterion.Subqueries;
import org.hibernate.type.StandardBasicTypes; import org.hibernate.type.StandardBasicTypes;
import org.hibernate.type.Type;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.Date; import java.util.Date;
import java.util.Iterator; import java.util.Iterator;
@@ -244,6 +241,18 @@ public class ItemDAOImpl extends AbstractHibernateDSODAO<Item> implements ItemDA
return count(query); return count(query);
} }
@Override
public int countItems(Context context, List<Collection> collections, boolean includeArchived, boolean includeWithdrawn) throws SQLException {
Query query = createQuery(context, "select count(distinct i) from Item i " +
"join i.collections collection " +
"WHERE collection IN (:collections) AND i.inArchive=:in_archive AND i.withdrawn=:withdrawn");
query.setParameterList("collections", collections);
query.setParameter("in_archive", includeArchived);
query.setParameter("withdrawn", includeWithdrawn);
return count(query);
}
@Override @Override
public Iterator<Item> findByLastModifiedSince(Context context, Date since) public Iterator<Item> findByLastModifiedSince(Context context, Date since)
@@ -260,12 +269,10 @@ public class ItemDAOImpl extends AbstractHibernateDSODAO<Item> implements ItemDA
} }
@Override @Override
public int countNotArchived(Context context) throws SQLException { public int countItems(Context context, boolean includeArchived, boolean includeWithdrawn) throws SQLException {
return count(createQuery(context, "SELECT count(*) FROM Item i WHERE i.inArchive=false AND i.withdrawn=false")); Query query = createQuery(context, "SELECT count(*) FROM Item i WHERE i.inArchive=:in_archive AND i.withdrawn=:withdrawn");
} query.setParameter("in_archive", includeArchived);
query.setParameter("withdrawn", includeWithdrawn);
@Override return count(query);
public int countWithdrawn(Context context) throws SQLException {
return count(createQuery(context, "SELECT count(*) FROM Item i WHERE i.withdrawn=true"));
} }
} }

View File

@@ -243,13 +243,6 @@ public interface CommunityService extends DSpaceObjectService<Community>, DSpace
public void canEdit(Context context, Community community) throws AuthorizeException, SQLException; public void canEdit(Context context, Community community) throws AuthorizeException, SQLException;
/**
* counts items in this community
*
* @return total items
*/
public int countItems(Context context, Community community) throws SQLException;
public Community findByAdminGroup(Context context, Group group) throws SQLException; public Community findByAdminGroup(Context context, Group group) throws SQLException;
public List<Community> findAuthorized(Context context, List<Integer> actions) throws SQLException; public List<Community> findAuthorized(Context context, List<Integer> actions) throws SQLException;

View File

@@ -453,7 +453,7 @@ public interface ItemService extends DSpaceObjectService<Item>, DSpaceObjectLega
public Iterator<Item> findByLastModifiedSince(Context context, Date last) public Iterator<Item> findByLastModifiedSince(Context context, Date last)
throws SQLException; throws SQLException;
/** /**
* counts items in the given community * counts items in the given community
* *
* @return total items * @return total items
@@ -462,7 +462,7 @@ public interface ItemService extends DSpaceObjectService<Item>, DSpaceObjectLega
int countTotal(Context context) throws SQLException; int countTotal(Context context) throws SQLException;
int getNotArchivedItemsCount(Context context) throws SQLException; int countNotArchivedItems(Context context) throws SQLException;
int countWithdrawnItems(Context context) throws SQLException; int countWithdrawnItems(Context context) throws SQLException;
} }

View File

@@ -74,7 +74,7 @@ public class ItemCheck extends Check {
"Withdrawn items: %d\n", itemService.countWithdrawnItems(context)); "Withdrawn items: %d\n", itemService.countWithdrawnItems(context));
ret += String.format( ret += String.format(
"Not published items (in workspace or workflow mode): %d\n", "Not published items (in workspace or workflow mode): %d\n",
itemService.getNotArchivedItemsCount(context)); itemService.countNotArchivedItems(context));
for (Map.Entry<Integer, Long> row : workspaceItemService.getStageReachedCounts(context)) { for (Map.Entry<Integer, Long> row : workspaceItemService.getStageReachedCounts(context)) {
ret += String.format("\tIn Stage %s: %s\n", ret += String.format("\tIn Stage %s: %s\n",

View File

@@ -1961,6 +1961,8 @@ public class CollectionTest extends AbstractDSpaceObjectTest
{ {
//0 by default //0 by default
assertTrue("testCountItems 0", itemService.countItems(context, collection) == 0); assertTrue("testCountItems 0", itemService.countItems(context, collection) == 0);
//NOTE: a more thorough test of item counting is in ITCommunityCollection integration test
} }
/** /**

View File

@@ -1480,7 +1480,9 @@ public class CommunityTest extends AbstractDSpaceObjectTest
public void testCountItems() throws Exception public void testCountItems() throws Exception
{ {
//0 by default //0 by default
assertTrue("testCountItems 0", communityService.countItems(context, c) == 0); assertTrue("testCountItems 0", itemService.countItems(context, c) == 0);
//NOTE: a more thorough test of item counting is in ITCommunityCollection integration test
} }
/** /**

View File

@@ -96,38 +96,75 @@ public class ITCommunityCollection extends AbstractIntegrationTest
assertThat("testCreateTree 2", (Community) collectionService.getParentObject(context, col1), equalTo(child1)); assertThat("testCreateTree 2", (Community) collectionService.getParentObject(context, col1), equalTo(child1));
assertThat("testCreateTree 3", (Community) collectionService.getParentObject(context, col2), equalTo(child1)); assertThat("testCreateTree 3", (Community) collectionService.getParentObject(context, col2), equalTo(child1));
} }
/** /**
* Tests that count items works as expected * Tests the creation of items in a community/collection tree
*/ */
@Test @Test
@PerfTest(invocations = 50, threads = 1) @PerfTest(invocations = 25, threads = 1)
@Required(percentile95 = 2000, average= 1800) @Required(percentile95 = 1200, average = 700, throughput = 1)
public void testCountItems() throws SQLException, AuthorizeException, IOException { public void testCreateItems() throws SQLException, AuthorizeException
//make it an even number, not too high to reduce time during testing {
int totalitems = 4;
//we create the structure //we create the structure
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
Community parent = communityService.create(null, context); Community parent = communityService.create(null, context);
Community child1 = communityService.create(parent, context); Community child1 = communityService.create(parent, context);
Collection col1 = collectionService.create(context, child1); Collection col1 = collectionService.create(context, child1);
Collection col2 = collectionService.create(context, child1); Collection col2 = collectionService.create(context, child1);
for(int count = 0; count < totalitems/2; count++) Item item1 = installItemService.installItem(context, workspaceItemService.create(context, col1, false));
{ Item item2 = installItemService.installItem(context, workspaceItemService.create(context, col2, false));
Item item1 = installItemService.installItem(context, workspaceItemService.create(context, col1, false));
Item item2 = installItemService.installItem(context, workspaceItemService.create(context, col2, false));
}
context.restoreAuthSystemState(); context.restoreAuthSystemState();
//verify it works as expected //verify it works as expected
assertThat("testCountItems 0", itemService.countItems(context, col1), equalTo(totalitems/2)); assertThat("testCreateItems 0", (Collection) itemService.getParentObject(context, item1), equalTo(col1));
assertThat("testCountItems 1", itemService.countItems(context, col2), equalTo(totalitems/2)); assertThat("testCreateItems 1", (Collection) itemService.getParentObject(context, item2), equalTo(col2));
assertThat("testCountItems 2", communityService.countItems(context, child1), equalTo(totalitems)); }
assertThat("testCountItems 3", communityService.countItems(context, parent), equalTo(totalitems));
/**
* Tests that count items works as expected
* NOTE: Counts are currently expensive (take a while)
*/
@Test
@PerfTest(invocations = 10, threads = 1)
@Required(percentile95 = 2000, average= 1800)
public void testCountItems() throws SQLException, AuthorizeException, IOException {
int items_per_collection = 2;
//we create the structure
context.turnOffAuthorisationSystem();
Community parentCom = communityService.create(null, context);
Community childCom = communityService.create(parentCom, context);
Collection col1 = collectionService.create(context, childCom);
Collection col2 = collectionService.create(context, childCom);
// Add same number of items to each collection
for(int count = 0; count < items_per_collection; count++)
{
Item item1 = installItemService.installItem(context, workspaceItemService.create(context, col1, false));
Item item2 = installItemService.installItem(context, workspaceItemService.create(context, col2, false));
}
// Finally, let's throw in a small wrench and add a mapped item
// Add it to collection 1
Item item3 = installItemService.installItem(context, workspaceItemService.create(context, col1, false));
// Map it into collection 2
collectionService.addItem(context, col2, item3);
// Our total number of items should be
int totalitems = items_per_collection*2 + 1;
// Our collection counts should be
int collTotalItems = items_per_collection + 1;
context.restoreAuthSystemState();
//verify it works as expected
assertThat("testCountItems 0", itemService.countItems(context, col1), equalTo(collTotalItems));
assertThat("testCountItems 1", itemService.countItems(context, col2), equalTo(collTotalItems));
assertThat("testCountItems 2", itemService.countItems(context, childCom), equalTo(totalitems));
assertThat("testCountItems 3", itemService.countItems(context, parentCom), equalTo(totalitems));
} }
} }

View File

@@ -21,7 +21,7 @@ import org.dspace.authorize.AuthorizeException;
import org.dspace.content.Community; import org.dspace.content.Community;
import org.dspace.content.DSpaceObject; import org.dspace.content.DSpaceObject;
import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.factory.ContentServiceFactory;
import org.dspace.content.service.CommunityService; import org.dspace.content.service.ItemService;
import org.xml.sax.SAXException; import org.xml.sax.SAXException;
/** /**
@@ -36,7 +36,7 @@ public class CommunityRecentSubmissions extends AbstractRecentSubmissionTransfor
private static final Message T_head_recent_submissions = private static final Message T_head_recent_submissions =
message("xmlui.ArtifactBrowser.CommunityViewer.head_recent_submissions"); message("xmlui.ArtifactBrowser.CommunityViewer.head_recent_submissions");
protected CommunityService communityService = ContentServiceFactory.getInstance().getCommunityService(); protected ItemService itemService = ContentServiceFactory.getInstance().getItemService();
/** /**
* Displays the recent submissions for this community * Displays the recent submissions for this community
@@ -80,7 +80,7 @@ public class CommunityRecentSubmissions extends AbstractRecentSubmissionTransfor
Community community = (Community) dso; Community community = (Community) dso;
if (communityService.countItems(context, community) > maxRecentSubmissions) if (itemService.countItems(context, community) > maxRecentSubmissions)
addViewMoreLink(lastSubmittedDiv, dso); addViewMoreLink(lastSubmittedDiv, dso);
} }
} }