From: Aron Roberts Date: Sat, 17 Oct 2009 00:38:44 +0000 (+0000) Subject: CSPACE-520,CSPACE-534: Removed IllegalArgumentExceptions from several core ID Service... X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=4708bf7c3d4fb1b5d00e9f638a76adf5c614f355;p=tmp%2Fjakarta-migration.git CSPACE-520,CSPACE-534: Removed IllegalArgumentExceptions from several core ID Service classes and tests. Minor streamlining of pretty printing code in IDResource, and removal of block to catch an unreachable condition. --- diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/IDResource.java b/services/id/service/src/main/java/org/collectionspace/services/id/IDResource.java index 3609c5325..8fe8d9a67 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/IDResource.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/IDResource.java @@ -177,10 +177,6 @@ public class IDResource { response = Response.status(Response.Status.BAD_REQUEST) .entity(ise.getMessage()).type(MediaType.TEXT_PLAIN).build(); - } catch (IllegalArgumentException iae) { - response = Response.status(Response.Status.BAD_REQUEST) - .entity(iae.getMessage()).type(MediaType.TEXT_PLAIN).build(); - // This is guard code that should never be reached. } catch (Exception e) { response = Response.status(Response.Status.INTERNAL_SERVER_ERROR) @@ -258,27 +254,7 @@ public class IDResource { // instance, and attach it to the root element. root = appendDetailedIDGeneratorInformation(root, instance); - try { - resourceRepresentation = prettyPrintXML(doc); - } catch(Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Error pretty-printing XML: " + e.getMessage()); - } - resourceRepresentation = doc.asXML(); - } - - if ( - resourceRepresentation == null || - resourceRepresentation.trim().isEmpty() - ) { - response = - Response.status(Response.Status.INTERNAL_SERVER_ERROR) - .entity("ID Service returned null or empty ID Generator") - .type(MediaType.TEXT_PLAIN) - .build(); - return response; - } - + resourceRepresentation = prettyPrintXML(doc); response = Response.status(Response.Status.OK) .entity(resourceRepresentation) @@ -302,14 +278,7 @@ public class IDResource { .type(MediaType.TEXT_PLAIN) .build(); - } catch (IllegalArgumentException iae) { - response = - Response.status(Response.Status.BAD_REQUEST) - .entity(iae.getMessage()) - .type(MediaType.TEXT_PLAIN) - .build(); - - // This is guard code that should never be reached. + // This is guard code that should never be reached. } catch (Exception e) { response = Response.status(Response.Status.INTERNAL_SERVER_ERROR) @@ -518,17 +487,7 @@ public class IDResource { listitem = appendSummaryIDGeneratorInformation(listitem, csid); } - String summaryList = ""; - try { - summaryList = prettyPrintXML(doc); - } catch(Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Error pretty-printing XML: " + e.getMessage()); - } - summaryList = doc.asXML(); - } - - return summaryList; + return prettyPrintXML(doc); } ////////////////////////////////////////////////////////////////////// @@ -558,17 +517,7 @@ public class IDResource { generators.get(csid)); } - String fullList = ""; - try { - fullList = prettyPrintXML(doc); - } catch(Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Error pretty-printing XML: " + e.getMessage()); - } - fullList = doc.asXML(); - } - - return fullList; + return prettyPrintXML(doc); } ////////////////////////////////////////////////////////////////////// @@ -673,14 +622,20 @@ public class IDResource { * * @return A pretty printed String representation of an XML document. */ - private String prettyPrintXML(Document doc) throws Exception { + private String prettyPrintXML(Document doc) { String xmlStr = ""; try { xmlStr = formatXML(doc, PRETTY_PRINT_OUTPUT_FORMAT); + // If an error occurs during pretty printing, fall back to + // returning a default String representation of the XML document. } catch (Exception e) { - throw e; + if (logger.isDebugEnabled()) { + logger.debug("Error pretty-printing XML: " + e.getMessage()); + } + xmlStr = doc.asXML(); } + return xmlStr; } @@ -695,6 +650,9 @@ public class IDResource { * * @return A String representation of an XML document, * formatted according to the specified output format. + * + * @throws An Exception if an error occurs in printing + * the XML document to a String. */ private String formatXML(Document doc, OutputFormat outformat) throws Exception { diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/IDService.java b/services/id/service/src/main/java/org/collectionspace/services/id/IDService.java index d93a2a04c..76b850691 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/IDService.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/IDService.java @@ -45,11 +45,11 @@ public interface IDService { // Generates and returns a new ID from the specified ID generator. public String createID(String csid) throws DocumentNotFoundException, - BadRequestException, IllegalArgumentException, IllegalStateException; + BadRequestException, IllegalStateException; // Returns the last-generated ID associated with the specified ID generator. public String readLastID(String csid) - throws IllegalArgumentException, IllegalStateException; + throws DocumentNotFoundException, IllegalStateException; // Read a list of objects (aka read multiple) @@ -61,12 +61,11 @@ public interface IDService { // Adds a new ID generator. public void createIDGenerator(String csid, String serializedIDGenerator) - throws IllegalArgumentException, IllegalStateException; + throws BadRequestException, IllegalStateException; // Read single object public IDGeneratorInstance readIDGenerator(String csid) - throws DocumentNotFoundException, IllegalArgumentException, - IllegalStateException; + throws DocumentNotFoundException, IllegalStateException; // Read a list of objects (aka read multiple) // and return a list (map) of those objects and their identifiers. diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/IDServiceJdbcImpl.java b/services/id/service/src/main/java/org/collectionspace/services/id/IDServiceJdbcImpl.java index 2a50a78ca..024d0d4ea 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/IDServiceJdbcImpl.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/IDServiceJdbcImpl.java @@ -339,12 +339,13 @@ public class IDServiceJdbcImpl implements IDService { * * @return The last ID generated that corresponds to the requested ID generator. * - * @throws IllegalArgumentException if the requested ID generator could not be found. + * @throws DocumentNotFoundException if the requested ID generator + * could not be found. * * @throws IllegalStateException if a storage-related error occurred. */ @Override - public String readLastID(String csid) throws IllegalArgumentException, + public String readLastID(String csid) throws DocumentNotFoundException, IllegalStateException { logger.debug("> in readLastID"); @@ -367,7 +368,7 @@ public class IDServiceJdbcImpl implements IDService { boolean moreRows = rs.next(); if (! moreRows) { - throw new IllegalArgumentException( + throw new DocumentNotFoundException( "ID generator " + "\'" + csid + "\'" + " could not be found."); } @@ -401,24 +402,28 @@ public class IDServiceJdbcImpl implements IDService { ////////////////////////////////////////////////////////////////////// /** - * Adds a new ID generator to persistent storage. + * Adds a new ID generator instance to persistent storage. * * @param csid An identifier for an ID generator. * * @param generator An ID generator, reflecting its current state, * including the values of its constituent parts. * + * @throws BadRequestException if the provided representation of an + * ID generator instance is not in the correct format, contains + * invalid values, or otherwise cannot be used. + * * @throws IllegalStateException if a storage-related error occurred. */ public void createIDGenerator(String csid, SettableIDGenerator generator) - throws IllegalArgumentException, IllegalStateException { + throws BadRequestException, IllegalStateException { logger.debug("> in createIDGenerator(String, SettableIDGenerator)"); // @TODO: Add checks for authorization to perform this operation. if (generator == null) { - throw new IllegalArgumentException("New ID generator " + + throw new BadRequestException("New ID generator " + "to add cannot be null."); } @@ -426,7 +431,7 @@ public class IDServiceJdbcImpl implements IDService { try { serializedGenerator = IDGeneratorSerializer.serialize(generator); } catch (IllegalArgumentException e) { - throw e; + throw new BadRequestException(e); } try { @@ -439,7 +444,7 @@ public class IDServiceJdbcImpl implements IDService { } - ////////////////////////////////////////////////////////////////////// + ////////////////////////////////////////////////////////////////////// /** * Adds a new ID generator to persistent storage, from a serialization * of that generator. @@ -453,18 +458,22 @@ public class IDServiceJdbcImpl implements IDService { * @param serializedGenerator A serialized ID generator, reflecting its current state, * including the values of its constituent parts. * - * @throws IllegalStateException if a storage-related error occurred. + * @throws BadRequestException if the provided representation of an + * ID generator instance is not in the correct format, contains + * invalid values, or otherwise cannot be used. + * + * @throws IllegalStateException if a storage-related error occurred. */ @Override public void createIDGenerator(String csid, String serializedGenerator) - throws IllegalArgumentException, IllegalStateException { + throws BadRequestException, IllegalStateException { logger.debug("> in createIDGenerator(String, String)"); // @TODO Add checks for authorization to perform this operation. if (serializedGenerator == null || serializedGenerator.equals("")) { - throw new IllegalArgumentException( + throw new BadRequestException( "Could not understand or parse this representation " + "of an ID generator."); } @@ -504,7 +513,8 @@ public class IDServiceJdbcImpl implements IDService { csid + "' to the database."); - // Otherwise, add this new ID generator, as a new record to the database. + // Otherwise, add this new ID generator, as a new record to + // the database. } else { final String SQL_STATEMENT_STRING = @@ -555,15 +565,14 @@ public class IDServiceJdbcImpl implements IDService { * * @return A serialized representation of the requested ID generator. * - * @throws IllegalArgumentException if the requested ID generator could + * @throws DocumentNotFoundException if the requested ID generator could * not be found. * * @throws IllegalStateException if a storage-related error occurred. */ @Override public IDGeneratorInstance readIDGenerator (String csid) throws - DocumentNotFoundException, IllegalArgumentException, - IllegalStateException { + DocumentNotFoundException, IllegalStateException { logger.debug("> in readIDGenerator"); @@ -617,7 +626,7 @@ public class IDServiceJdbcImpl implements IDService { } - ////////////////////////////////////////////////////////////////////// + ////////////////////////////////////////////////////////////////////// /** * Returns a list of ID generator instances from persistent storage. * @@ -690,10 +699,18 @@ public class IDServiceJdbcImpl implements IDService { * @param generator An ID generator, reflecting its current state, * including the values of its constituent parts. * - * @throws IllegalStateException if a storage-related error occurred. + * @throws DocumentNotFoundException if the requested ID generator could + * not be found. + * + * @throws BadRequestException if the provided representation of an + * ID generator instance is not in the correct format, contains + * invalid values, or otherwise cannot be used. + * + * @throws IllegalStateException if a storage-related error occurred. */ public void updateIDGenerator(String csid, SettableIDGenerator generator) - throws BadRequestException, IllegalArgumentException, IllegalStateException, DocumentNotFoundException { + throws BadRequestException, DocumentNotFoundException, + IllegalStateException { logger.debug("> in updateIDGenerator(String, SettableIDGenerator)"); @@ -708,7 +725,7 @@ public class IDServiceJdbcImpl implements IDService { try { serializedGenerator = IDGeneratorSerializer.serialize(generator); } catch (IllegalArgumentException e) { - throw e; + throw new BadRequestException(e); } try { @@ -738,12 +755,19 @@ public class IDServiceJdbcImpl implements IDService { * A serialized ID generator, reflecting its current state, * including the values of its constituent parts. * - * @throws IllegalStateException if a storage-related error occurred. + * @throws DocumentNotFoundException if the requested ID generator could + * not be found. + * + * @throws BadRequestException if the provided representation of an + * ID generator instance is not in the correct format, contains + * invalid values, or otherwise cannot be used. + * + * @throws IllegalStateException if a storage-related error occurred. */ @Override public void updateIDGenerator(String csid, String serializedGenerator) throws DocumentNotFoundException, BadRequestException, - IllegalArgumentException, IllegalStateException { + IllegalStateException { logger.debug("> in updateIDGenerator(String, String)"); @@ -834,11 +858,13 @@ public class IDServiceJdbcImpl implements IDService { * * @param csid An identifier for an ID generator. * - * @throws IllegalStateException if a storage-related error occurred. + * @throws DocumentNotFoundException if the requested ID generator could + * not be found. + * + * @throws IllegalStateException if a storage-related error occurred. */ public void deleteIDGenerator(String csid) - throws DocumentNotFoundException, IllegalArgumentException, - IllegalStateException { + throws DocumentNotFoundException, IllegalStateException { logger.debug("> in deleteIDGenerator"); @@ -966,7 +992,7 @@ public class IDServiceJdbcImpl implements IDService { } - ////////////////////////////////////////////////////////////////////// + ////////////////////////////////////////////////////////////////////// /** * Identifies whether a specified table exists in the database. * @@ -1023,5 +1049,4 @@ public class IDServiceJdbcImpl implements IDService { } - } \ No newline at end of file diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/test/IDServiceJdbcImplTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/test/IDServiceJdbcImplTest.java index e99a023d0..6d9ce14e3 100644 --- a/services/id/service/src/test/java/org/collectionspace/services/id/test/IDServiceJdbcImplTest.java +++ b/services/id/service/src/test/java/org/collectionspace/services/id/test/IDServiceJdbcImplTest.java @@ -68,7 +68,7 @@ public class IDServiceJdbcImplTest { @Test(dependsOnMethods = {"hasRequiredDatabaseTable"}) public void createIDGenerator() throws DocumentNotFoundException, - IllegalArgumentException, IllegalStateException { + BadRequestException, IllegalStateException { try { jdbc.deleteIDGenerator(DEFAULT_CSID); } catch (Exception e) { @@ -119,7 +119,7 @@ public class IDServiceJdbcImplTest { @Test(dependsOnMethods = {"hasRequiredDatabaseTable", "createIDGenerator", "readIDGenerator"}) public void updateIDGenerator() throws DocumentNotFoundException, - BadRequestException, IllegalArgumentException, IllegalStateException { + BadRequestException, IllegalStateException { final String NEW_DESCRIPTION = "new description"; @@ -150,8 +150,7 @@ public class IDServiceJdbcImplTest { @Test(dependsOnMethods = {"hasRequiredDatabaseTable", "createIDGenerator", "readIDGenerator", "deleteIDGenerator"}) public void createID() throws DocumentNotFoundException, - BadRequestException, IllegalArgumentException, - IllegalStateException { + BadRequestException, IllegalStateException { try { jdbc.deleteIDGenerator(DEFAULT_CSID);