Refactor BundleServiceImpl.setOrder() to be more failsafe. Update Tests to prove out (previously these new tests failed)

This commit is contained in:
Tim Donohue
2017-06-02 20:42:00 +00:00
committed by Pascal-Nicolas Becker
parent d3d8471756
commit e16f6a80d5
2 changed files with 69 additions and 22 deletions

View File

@@ -269,19 +269,49 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
public void setOrder(Context context, Bundle bundle, UUID[] bitstreamIds) throws AuthorizeException, SQLException {
authorizeService.authorizeAction(context, bundle, Constants.WRITE);
bundle.getBitstreams().clear();
List<Bitstream> currentBitstreams = bundle.getBitstreams();
List<Bitstream> updatedBitstreams = new ArrayList<Bitstream>();
// Loop through and ensure these Bitstream IDs are all valid. Add them to list of updatedBitstreams.
for (int i = 0; i < bitstreamIds.length; i++) {
UUID bitstreamId = bitstreamIds[i];
Bitstream bitstream = bitstreamService.find(context, bitstreamId);
// If we have an invalid Bitstream ID, just ignore it, but log a warning
if(bitstream == null) {
//This should never occur but just in case
log.warn(LogManager.getHeader(context, "Invalid bitstream id while changing bitstream order", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId));
continue;
}
// If we have a Bitstream not in the current list, log a warning & exit immediately
if(!currentBitstreams.contains(bitstream))
{
log.warn(LogManager.getHeader(context, "Encountered a bitstream not in this bundle while changing bitstream order. Bitstream order will not be changed.", "Bundle: " + bundle.getID() + ", bitstream id: " + bitstreamId));
return;
}
updatedBitstreams.add(bitstream);
}
// If our lists are different sizes, exit immediately
if(updatedBitstreams.size()!=currentBitstreams.size())
{
log.warn(LogManager.getHeader(context, "Size of old list and new list do not match. Bitstream order will not be changed.", "Bundle: " + bundle.getID()));
return;
}
// As long as the order has changed, update it
if(CollectionUtils.isNotEmpty(updatedBitstreams) && !updatedBitstreams.equals(currentBitstreams))
{
//First clear out the existing list of bitstreams
bundle.getBitstreams().clear();
// Now add them back in the proper order
for (Bitstream bitstream : updatedBitstreams)
{
bitstream.getBundles().remove(bundle);
bundle.getBitstreams().add(bitstream);
bitstream.getBundles().add(bundle);
bitstreamService.update(context, bitstream);
}
@@ -294,6 +324,7 @@ public class BundleServiceImpl extends DSpaceObjectServiceImpl<Bundle> implement
}
}
}
@Override
public DSpaceObject getAdminObject(Context context, Bundle bundle, int action) throws SQLException {

View File

@@ -719,26 +719,42 @@ public class BundleTest extends AbstractDSpaceObjectTest
bundleService.addBitstream(context, b, bs3);
// Assert Bitstreams are in the order added
List<Bitstream> bitstreams = b.getBitstreams();
assertTrue("testSetOrder: starting count correct", bitstreams.size() == 3);
assertThat("testSetOrder: Bitstream 1 is first", bitstreams.get(0).getName(), equalTo(bs.getName()));
assertThat("testSetOrder: Bitstream 2 is second", bitstreams.get(1).getName(), equalTo(bs2.getName()));
assertThat("testSetOrder: Bitstream 3 is third", bitstreams.get(2).getName(), equalTo(bs3.getName()));
Bitstream[] bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
assertTrue("testSetOrder: starting count correct", bitstreams.length == 3);
assertThat("testSetOrder: Bitstream 1 is first", bitstreams[0].getName(), equalTo(bs.getName()));
assertThat("testSetOrder: Bitstream 2 is second", bitstreams[1].getName(), equalTo(bs2.getName()));
assertThat("testSetOrder: Bitstream 3 is third", bitstreams[2].getName(), equalTo(bs3.getName()));
UUID bsID1 = bs.getID();
UUID bsID2 = bs2.getID();
UUID bsID3 = bs3.getID();
// Now define a new order and call setOrder()
UUID[] newBitstreamOrder = { bsID3, bsID1, bsID2 };
UUID[] newBitstreamOrder = new UUID[] { bsID3, bsID1, bsID2 };
bundleService.setOrder(context, b, newBitstreamOrder);
// Assert Bitstreams are in the new order
bitstreams = b.getBitstreams();
assertTrue("testSetOrder: new count correct", bitstreams.size() == 3);
assertThat("testSetOrder: Bitstream 3 is now first", bitstreams.get(0).getName(), equalTo(bs3.getName()));
assertThat("testSetOrder: Bitstream 1 is now second", bitstreams.get(1).getName(), equalTo(bs.getName()));
assertThat("testSetOrder: Bitstream 2 is now third", bitstreams.get(2).getName(), equalTo(bs2.getName()));
bitstreams = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
assertTrue("testSetOrder: new count correct", bitstreams.length == 3);
assertThat("testSetOrder: Bitstream 3 is now first", bitstreams[0].getName(), equalTo(bs3.getName()));
assertThat("testSetOrder: Bitstream 1 is now second", bitstreams[1].getName(), equalTo(bs.getName()));
assertThat("testSetOrder: Bitstream 2 is now third", bitstreams[2].getName(), equalTo(bs2.getName()));
// Now give only a partial list of bitstreams
newBitstreamOrder = new UUID[] { bsID1, bsID2 };
bundleService.setOrder(context, b, newBitstreamOrder);
// Assert Bitstream order is unchanged
Bitstream[] bitstreamsAfterPartialData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterPartialData, equalTo(bitstreams));
// Now give bad data in the list of bitstreams
newBitstreamOrder = new UUID[] { bsID1, null, bsID2 };
bundleService.setOrder(context, b, newBitstreamOrder);
// Assert Bitstream order is unchanged
Bitstream[] bitstreamsAfterBadData = b.getBitstreams().toArray(new Bitstream[b.getBitstreams().size()]);
assertThat("testSetOrder: Partial data doesn't change order", bitstreamsAfterBadData, equalTo(bitstreams));
}
/**