Skip to content

Commit

Permalink
DS-2086 fix. Corrects the logic to deleting Communities & Collections…
Browse files Browse the repository at this point in the history 
… such that hierarchical deletion test now succeeds. Also corrects minor permissions error in a different unit test.
  • Loading branch information
@tdonohue
tdonohue committed Aug 15, 2014
1 parent 56fda36 commit 0aa3910
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 100 deletions.
230 changes: 137 additions & 93 deletions dspace-api/src/main/java/org/dspace/content/Community.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -994,160 +994,201 @@ public void addSubcommunity(Community c) throws SQLException,
}

/**
* Remove a collection. Any items then orphaned are deleted.
* Remove a collection. If it only belongs to one parent community,
* then it is permanently deleted. If it has more than one parent community,
* it is simply unmapped from the current community.
*
* @param c
* collection to remove
* @throws SQLException
* @throws AuthorizeException
* @throws IOException
*/
public void removeCollection(Collection c) throws SQLException,
AuthorizeException, IOException
{
// Check authorisation
AuthorizeManager.authorizeAction(ourContext, this, Constants.REMOVE);

// will be the collection an orphan?
TableRow trow = DatabaseManager.querySingle(ourContext,
"SELECT COUNT(DISTINCT community_id) AS num FROM community2collection WHERE collection_id= ? ",
c.getID());
DatabaseManager.setConstraintDeferred(ourContext, "comm2coll_collection_fk");

if (trow.getLongColumn("num") == 1)
// Do the removal in a try/catch, so that we can rollback on error
try
{
// Orphan; delete it
c.delete();
}

log.info(LogManager.getHeader(ourContext, "remove_collection",
"community_id=" + getID() + ",collection_id=" + c.getID()));
// Capture ID & Handle of Collection we are removing, so we can trigger events later
int removedId = c.getID();
String removedHandle = c.getHandle();

// How many parent(s) does this collection have?
TableRow trow = DatabaseManager.querySingle(ourContext,
"SELECT COUNT(DISTINCT community_id) AS num FROM community2collection WHERE collection_id= ? ",
c.getID());
long numParents = trow.getLongColumn("num");

// Remove the parent/child mapping with this collection
// We do this before deletion, so that the deletion doesn't throw database integrity violations
DatabaseManager.updateQuery(ourContext,
"DELETE FROM community2collection WHERE community_id= ? "+
"AND collection_id= ? ", getID(), c.getID());

// As long as this Collection only had one parent, delete it
// NOTE: if it had multiple parents, we will keep it around,
// and just remove that single parent/child mapping
if (numParents == 1)
{
c.delete();
}

// Remove any mappings
DatabaseManager.updateQuery(ourContext,
"DELETE FROM community2collection WHERE community_id= ? "+
"AND collection_id= ? ", getID(), c.getID());

DatabaseManager.setConstraintImmediate(ourContext, "comm2coll_collection_fk");
// log the removal & trigger any associated event(s)
log.info(LogManager.getHeader(ourContext, "remove_collection",
"community_id=" + getID() + ",collection_id=" + removedId));

ourContext.addEvent(new Event(Event.REMOVE, Constants.COMMUNITY, getID(), Constants.COLLECTION, c.getID(), c.getHandle()));
ourContext.addEvent(new Event(Event.REMOVE, Constants.COMMUNITY, getID(), Constants.COLLECTION, removedId, removedHandle));
}
catch(SQLException|IOException e)
{
// Immediately abort the deletion, rolling back the transaction
ourContext.abort();
// Pass exception upwards for additional reporting/handling as needed
throw e;
}
}

/**
* Remove a subcommunity. Any substructure then orphaned is deleted.
* Remove a subcommunity. If it only belongs to one parent community,
* then it is permanently deleted. If it has more than one parent community,
* it is simply unmapped from the current community.
*
* @param c
* subcommunity to remove
* @throws SQLException
* @throws AuthorizeException
* @throws IOException
*/
public void removeSubcommunity(Community c) throws SQLException,
AuthorizeException, IOException
{
// Check authorisation
// Check authorisation.
AuthorizeManager.authorizeAction(ourContext, this, Constants.REMOVE);

// will be the subcommunity an orphan?
TableRow trow = DatabaseManager.querySingle(ourContext,
"SELECT COUNT(DISTINCT parent_comm_id) AS num FROM community2community WHERE child_comm_id= ? ",
c.getID());

DatabaseManager.setConstraintDeferred(ourContext, "com2com_child_fk");
if (trow.getLongColumn("num") == 1)
// Do the removal in a try/catch, so that we can rollback on error
try
{
// Orphan; delete it
c.rawDelete();
}

log.info(LogManager.getHeader(ourContext, "remove_subcommunity",
"parent_comm_id=" + getID() + ",child_comm_id=" + c.getID()));
// Capture ID & Handle of Community we are removing, so we can trigger events later
int removedId = c.getID();
String removedHandle = c.getHandle();

// Remove any mappings
DatabaseManager.updateQuery(ourContext,
"DELETE FROM community2community WHERE parent_comm_id= ? " +
" AND child_comm_id= ? ", getID(),c.getID());
// How many parent(s) does this subcommunity have?
TableRow trow = DatabaseManager.querySingle(ourContext,
"SELECT COUNT(DISTINCT parent_comm_id) AS num FROM community2community WHERE child_comm_id= ? ",
c.getID());
long numParents = trow.getLongColumn("num");

// Remove the parent/child mapping with this subcommunity
// We do this before deletion, so that the deletion doesn't throw database integrity violations
DatabaseManager.updateQuery(ourContext,
"DELETE FROM community2community WHERE parent_comm_id= ? " +
" AND child_comm_id= ? ", getID(),c.getID());

// As long as this Community only had one parent, delete it
// NOTE: if it had multiple parents, we will keep it around,
// and just remove that single parent/child mapping
if (numParents == 1)
{
c.rawDelete();
}

ourContext.addEvent(new Event(Event.REMOVE, Constants.COMMUNITY, getID(), Constants.COMMUNITY, c.getID(), c.getHandle()));

DatabaseManager.setConstraintImmediate(ourContext, "com2com_child_fk");
// log the removal & trigger any related event(s)
log.info(LogManager.getHeader(ourContext, "remove_subcommunity",
"parent_comm_id=" + getID() + ",child_comm_id=" + removedId));

ourContext.addEvent(new Event(Event.REMOVE, Constants.COMMUNITY, getID(), Constants.COMMUNITY, removedId, removedHandle));
}
catch(SQLException|IOException e)
{
// Immediately abort the deletion, rolling back the transaction
ourContext.abort();
// Pass exception upwards for additional reporting/handling as needed
throw e;
}
}

/**
* Delete the community, including the metadata and logo. Collections and
* subcommunities that are then orphans are deleted.
* Delete the community, including the metadata and logo. Any children
* (subcommunities or collections) are also deleted.
*
* @throws SQLException
* @throws AuthorizeException
* @throws IOException
*/
public void delete() throws SQLException, AuthorizeException, IOException
{
// Check authorisation
// FIXME: If this was a subcommunity, it is first removed from it's
// parent.
// This means the parentCommunity == null
// But since this is also the case for top-level communities, we would
// give everyone rights to remove the top-level communities.
// The same problem occurs in removing the logo
if (!AuthorizeManager.authorizeActionBoolean(ourContext,
// Check for a parent Community
Community parent = getParentCommunity();

// Check authorisation.
// MUST have either REMOVE permissions on parent community (if exists)
// OR have DELETE permissions on current community
if (parent!= null && !AuthorizeManager.authorizeActionBoolean(ourContext,
getParentCommunity(), Constants.REMOVE))
{
// If we don't have Parent Community REMOVE permissions, then
// we MUST at least have current Community DELETE permissions
AuthorizeManager
.authorizeAction(ourContext, this, Constants.DELETE);
}

// If not a top-level community, have parent remove me; this
// will call rawDelete() before removing the linkage
Community parent = getParentCommunity();

// Check if this is a top-level Community or not
if (parent == null)
{
// if removing a top level Community, simulate a REMOVE event at the Site.
if (getParentCommunity() == null)
{
ourContext.addEvent(new Event(Event.REMOVE, Constants.SITE, Site.SITE_ID,
Constants.COMMUNITY, getID(), getHandle()));
}
// Call rawDelete to clean up all sub-communities & collections
// under this Community, then delete the Community itself
rawDelete();

// Since this is a top level Community, simulate a REMOVE event at the Site.
ourContext.addEvent(new Event(Event.REMOVE, Constants.SITE, Site.SITE_ID,
Constants.COMMUNITY, getID(), getHandle()));
} else {
// remove the subcommunities first
Community[] subcommunities = getSubcommunities();
for (int i = 0; i < subcommunities.length; i++)
{
subcommunities[i].delete();
}
// now let the parent remove the community
// This is a subcommunity, so let the parent remove it
// NOTE: this essentially just logs event and calls "rawDelete()"
parent.removeSubcommunity(this);

return;
}

rawDelete();
}

/**
* Internal method to remove the community and all its childs from the database without aware of eventually parent
* Internal method to remove the community and all its children from the
* database, and perform any pre/post-cleanup
*
* @throws SQLException
* @throws AuthorizeException
* @throws IOException
*/
private void rawDelete() throws SQLException, AuthorizeException, IOException
{
log.info(LogManager.getHeader(ourContext, "delete_community",
"community_id=" + getID()));

ourContext.addEvent(new Event(Event.DELETE, Constants.COMMUNITY, getID(), getHandle()));
// Capture ID & Handle of object we are removing, so we can trigger events later
int deletedId = getID();
String deletedHandle = getHandle();

// Remove from cache
// Remove Community object from cache
ourContext.removeCached(this, getID());

// Remove collections
Collection[] cols = getCollections();

for (int i = 0; i < cols.length; i++)
// Remove any collections directly under this Community
for (Collection collection : getCollections())
{
removeCollection(cols[i]);
removeCollection(collection);
}

// delete subcommunities
Community[] comms = getSubcommunities();

for (int j = 0; j < comms.length; j++)
// Remove any SubCommunities under this Community
for (Community community : getSubcommunities())
{
comms[j].delete();
removeSubcommunity(community);
}

// Remove the logo
// Remove the Community logo
setLogo(null);

// Remove all authorization policies
// Remove all associated authorization policies
AuthorizeManager.removeAllPolicies(ourContext, this);

// get rid of the content count cache if it exists
Expand All @@ -1163,19 +1204,22 @@ private void rawDelete() throws SQLException, AuthorizeException, IOException
throw new IllegalStateException(e.getMessage(),e);
}

// Remove any Handle
// Unbind any handle associated with this Community
HandleManager.unbindHandle(ourContext, this);

// Delete community row
// Delete community row (which actually removes the Community)
DatabaseManager.delete(ourContext, communityRow);

// Remove administrators group - must happen after deleting community
// Remove Community administrators group (if exists)
// NOTE: this must happen AFTER deleting community
Group g = getAdministrators();

if (g != null)
{
g.delete();
}

// If everything above worked, then trigger any associated events
ourContext.addEvent(new Event(Event.DELETE, Constants.COMMUNITY, deletedId, deletedHandle));
}

/**
Expand Down
14 changes: 7 additions & 7 deletions dspace-api/src/test/java/org/dspace/content/CommunityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1027,15 +1027,15 @@ public void testRemoveSubcommunityAuth() throws Exception
{
new NonStrictExpectations(AuthorizeManager.class)
{{
// Allow current Community ADD perms
// Allow Community ADD perms (in order to add a new subcommunity to parent)
AuthorizeManager.authorizeAction((Context) any, (Community) any,
Constants.ADD); result = null;
// Allow *parent* Community ADD perms
AuthorizeManager.authorizeActionBoolean((Context) any, (Community) any,
Constants.ADD); result = true;
// Allow current Community REMOVE perms
Constants.ADD, true); result = null;
// Allow Community REMOVE perms (needed to unmap/remove subcommunity)
AuthorizeManager.authorizeAction((Context) any, (Community) any,
Constants.REMOVE); result = null;
Constants.REMOVE, true); result = null;
// Allow Community DELETE perms (needed to actually delete subcommunity)
AuthorizeManager.authorizeAction((Context) any, (Community) any,
Constants.DELETE, true); result = null;
}};

// Turn off authorization temporarily to create a new top-level community
Expand Down

0 comments on commit 0aa3910

Please sign in to comment.