From d53bd7ae32a6d21971e7b8dfff86618fcc3daf1a Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Thu, 23 Jul 2009 20:17:10 +0000 Subject: [PATCH] CSPACE-245,CSPACE-236: Minor internal code cleanup and logging changes. Spun off serializer. --- .../services/id/IDPatternSerializer.java | 111 +++++++++++++++++ .../services/id/IDResource.java | 41 +----- .../services/id/IDServiceJdbcImpl.java | 117 ++++-------------- .../services/id/IDPatternSerializerTest.java | 91 ++++++++++++++ .../services/id/IDServiceJdbcImplTest.java | 71 ++--------- 5 files changed, 239 insertions(+), 192 deletions(-) create mode 100644 services/id/service/src/main/java/org/collectionspace/services/id/IDPatternSerializer.java create mode 100644 services/id/service/src/test/java/org/collectionspace/services/id/IDPatternSerializerTest.java diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/IDPatternSerializer.java b/services/id/service/src/main/java/org/collectionspace/services/id/IDPatternSerializer.java new file mode 100644 index 000000000..c58fd3589 --- /dev/null +++ b/services/id/service/src/main/java/org/collectionspace/services/id/IDPatternSerializer.java @@ -0,0 +1,111 @@ +/** + * IDPatternSerializer + * + * Serializer and deserializer for ID patterns. + * + * This document is a part of the source code and related artifacts + * for CollectionSpace, an open source collections management system + * for museums and related institutions: + * + * http://www.collectionspace.org + * http://wiki.collectionspace.org + * + * Copyright © 2009 Regents of the University of California + * + * Licensed under the Educational Community License (ECL), Version 2.0. + * You may not use this file except in compliance with this License. + * + * You may obtain a copy of the ECL 2.0 License at + * https://source.collectionspace.org/collection-space/LICENSE.txt + * + * Based on work by Richard Millet and Sanjay Dalal. + * + * $LastChangedBy: aron $ + * $LastChangedRevision: 414 $ + * $LastChangedDate$ + */ + +// @TODO: Revise exception handling to return custom Exceptions, +// perhaps mirroring the subset of HTTP status codes returned. +// +// We're currently overloading existing core and extension Java Exceptions +// in ways that are not consistent with their original semantic meaning. + +package org.collectionspace.services.id; + +import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.XStreamException; +import com.thoughtworks.xstream.io.xml.DomDriver; + +public class IDPatternSerializer { + + ////////////////////////////////////////////////////////////////////// + /** + * Constructor (no-argument). + */ + public void IDPatternSerializer() { + } + + ////////////////////////////////////////////////////////////////////// + /** + * Serializes an ID pattern, converting it from a Java object into an XML representation. + * + * @param pattern An ID pattern. + * + * @return A serialized representation of that ID pattern. + * + * @throws IllegalArgumentException if the ID pattern cannot be serialized. + */ + public static String serialize(IDPattern pattern) throws IllegalArgumentException { + + if (pattern == null) { + throw new IllegalArgumentException("ID pattern cannot be null."); + } + + XStream xstream = new XStream(new DomDriver()); + + String serializedPattern = ""; + try { + serializedPattern = xstream.toXML(pattern); + } catch (XStreamException e) { + throw new IllegalArgumentException( + "Could not convert ID pattern to XML for storage in database."); + } + + return serializedPattern; + + } + + ////////////////////////////////////////////////////////////////////// + /** + * Deserializes an ID pattern, converting it from an XML representation + * into a Java object. + * + * @param serializedPattern A serialized representation of an ID pattern. + * + * @return The ID pattern deserialized as a Java object. + * + * @throws IllegalArgumentException if the ID pattern cannot be deserialized. + */ + public static IDPattern deserialize(String serializedPattern) + throws IllegalArgumentException { + + if (serializedPattern == null || serializedPattern.equals("")) { + throw new IllegalArgumentException("ID pattern cannot be null or empty."); + } + + XStream xstream = new XStream(new DomDriver()); + + IDPattern pattern; + try { + pattern = (IDPattern) xstream.fromXML(serializedPattern); + } catch (XStreamException e) { + throw new IllegalArgumentException( + "Could not understand or parse this representation of an ID pattern."); + } + + return pattern; + + } + +} 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 14a3ff044..7f4729d8e 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 @@ -51,6 +51,7 @@ public class IDResource { final Logger logger = LoggerFactory.getLogger(IDResource.class); + // @TODO Determine if this is pertinent here: // Per Richard's comment in the CollectionObject Resource class, from which // this class was derived: "This should be a DI wired by a container like // Spring, Seam, or EJB3." @@ -58,7 +59,7 @@ public class IDResource { final static IDService service = new IDServiceJdbcImpl(); ////////////////////////////////////////////////////////////////////// - /* + /** * Constructor (no argument). */ public IDResource() { @@ -66,7 +67,7 @@ public class IDResource { } ////////////////////////////////////////////////////////////////////// - /* + /** * Returns the next available ID associated with a specified ID pattern. * * @param csid An identifier for an ID pattern. @@ -89,7 +90,7 @@ public class IDResource { @Produces("text/plain") public Response getNextID(@PathParam("csid") String csid) { - verbose("> in getNextID"); + logger.debug("> in getNextID(String)"); // Unless the 'response' variable is explicitly initialized here, the // compiler gives the error: "variable response might not have been initialized." @@ -137,38 +138,4 @@ public class IDResource { } - ////////////////////////////////////////////////////////////////////// - /* - * Prints a serialized ID pattern to the console (stdout). - * - * @param msg A message. - * - * @param idPattern An ID Pattern. - * - */ - protected static void verbose(String msg, IDPattern idPattern) { - - try { - verbose(msg); - JAXBContext jc = JAXBContext.newInstance(IDPattern.class); - Marshaller m = jc.createMarshaller(); - m.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE); - m.marshal(idPattern, System.out); - } catch (Exception e) { - e.printStackTrace(); - } - - } - - ////////////////////////////////////////////////////////////////////// - /* - * Prints a message to the console (stdout). - * - * @param msg A message. - * - */ - protected static void verbose(String msg) { - System.out.println("IDResource. " + msg); - } - } 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 e298cab4b..85bfc3b49 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 @@ -63,10 +63,6 @@ package org.collectionspace.services.id; -import com.thoughtworks.xstream.XStream; -import com.thoughtworks.xstream.XStreamException; -import com.thoughtworks.xstream.io.xml.DomDriver; - import java.sql.Connection; import java.sql.DriverManager; import java.sql.ResultSet; @@ -74,8 +70,13 @@ import java.sql.SQLException; import java.sql.PreparedStatement; import java.sql.Statement; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public class IDServiceJdbcImpl implements IDService { + final Logger logger = LoggerFactory.getLogger(IDServiceJdbcImpl.class); + // @TODO Get the JDBC driver classname and database URL from configuration; // better yet, substitute Hibernate for JDBC for accessing database-managed persistence. // @@ -120,7 +121,7 @@ public class IDServiceJdbcImpl implements IDService { public String nextID(String csid) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in nextID"); + logger.debug("> in nextID"); String nextId = ""; String lastId = ""; @@ -147,7 +148,7 @@ public class IDServiceJdbcImpl implements IDService { IDPattern pattern; try { - pattern = deserializeIDPattern(serializedPattern); + pattern = IDPatternSerializer.deserialize(serializedPattern); } catch (IllegalArgumentException e) { throw e; } @@ -216,7 +217,7 @@ public class IDServiceJdbcImpl implements IDService { public void updateLastID(String csid, String lastId) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in updateLastID"); + logger.debug("> in updateLastID"); try { Class.forName(JDBC_DRIVER_CLASSNAME).newInstance(); @@ -252,10 +253,9 @@ public class IDServiceJdbcImpl implements IDService { "Error updating last-generated ID in the database for ID Pattern '" + csid + "'"); } - IDResource.verbose("> successfully updated last-generated ID: " + lastId); + logger.debug("> successfully updated last-generated ID: " + lastId); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException("Error updating last-generated ID in the database: " + e.getMessage()); } finally { try { @@ -296,7 +296,7 @@ public class IDServiceJdbcImpl implements IDService { */ public String getLastID(String csid) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in getLastID"); + logger.debug("> in getLastID"); String lastId = null; @@ -337,12 +337,11 @@ public class IDServiceJdbcImpl implements IDService { lastId = rs.getString(1); - IDResource.verbose("> retrieved ID: " + lastId); + logger.debug("> retrieved ID: " + lastId); rs.close(); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException("Error retrieving last ID from the database: " + e.getMessage()); } finally { try { @@ -354,7 +353,7 @@ public class IDServiceJdbcImpl implements IDService { } } - IDResource.verbose("> returning ID: " + lastId); + logger.debug("> returning ID: " + lastId); return lastId; @@ -386,7 +385,7 @@ public class IDServiceJdbcImpl implements IDService { public void addIDPattern(String csid, IDPattern pattern) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in addIDPattern(String, IDPattern)"); + logger.debug("> in addIDPattern(String, IDPattern)"); if (pattern == null) { throw new IllegalArgumentException("New ID pattern to add cannot be null."); @@ -394,7 +393,7 @@ public class IDServiceJdbcImpl implements IDService { String serializedPattern = ""; try { - serializedPattern = serializeIDPattern(pattern); + serializedPattern = IDPatternSerializer.serialize(pattern); } catch (IllegalArgumentException e) { throw e; } @@ -439,7 +438,7 @@ public class IDServiceJdbcImpl implements IDService { public void addIDPattern(String csid, String serializedPattern) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in addIDPattern(String, String)"); + logger.debug("> in addIDPattern(String, String)"); if (serializedPattern == null || serializedPattern.equals("")) { throw new IllegalArgumentException( @@ -521,10 +520,9 @@ public class IDServiceJdbcImpl implements IDService { } // end if (idPatternFound) - IDResource.verbose("> successfully added ID Pattern: " + csid); + logger.debug("> successfully added ID Pattern: " + csid); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException("Error adding new ID pattern to the database: " + e.getMessage()); } finally { try { @@ -564,7 +562,7 @@ public class IDServiceJdbcImpl implements IDService { public void updateIDPattern(String csid, IDPattern pattern) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in updateIDPattern(String, IDPattern)"); + logger.debug("> in updateIDPattern(String, IDPattern)"); if (pattern == null) { throw new IllegalArgumentException( @@ -573,7 +571,7 @@ public class IDServiceJdbcImpl implements IDService { String serializedPattern = ""; try { - serializedPattern = serializeIDPattern(pattern); + serializedPattern = IDPatternSerializer.serialize(pattern); } catch (IllegalArgumentException e) { throw e; } @@ -619,7 +617,7 @@ public class IDServiceJdbcImpl implements IDService { public void updateIDPattern(String csid, String serializedPattern) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in updateIDPattern(String, String)"); + logger.debug("> in updateIDPattern(String, String)"); if (serializedPattern == null || serializedPattern.equals("")) { throw new IllegalArgumentException( @@ -674,7 +672,7 @@ public class IDServiceJdbcImpl implements IDService { IDPattern pattern; try { - pattern = deserializeIDPattern(serializedPattern); + pattern = IDPatternSerializer.deserialize(serializedPattern); } catch (IllegalArgumentException e) { throw e; } @@ -700,10 +698,9 @@ public class IDServiceJdbcImpl implements IDService { } // end if (idPatternFound) - IDResource.verbose("> successfully updated ID Pattern: " + csid); + logger.debug("> successfully updated ID Pattern: " + csid); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException("Error updating ID pattern in the database: " + e.getMessage()); } finally { try { @@ -740,7 +737,7 @@ public class IDServiceJdbcImpl implements IDService { public void deleteIDPattern(String csid) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in deleteIDPattern"); + logger.debug("> in deleteIDPattern"); try { Class.forName(JDBC_DRIVER_CLASSNAME).newInstance(); @@ -803,10 +800,9 @@ public class IDServiceJdbcImpl implements IDService { } // end if (idPatternFound) - IDResource.verbose("> successfully deleted ID Pattern: " + csid); + logger.debug("> successfully deleted ID Pattern: " + csid); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException("Error deleting ID pattern in database: " + e.getMessage()); } finally { try { @@ -846,7 +842,7 @@ public class IDServiceJdbcImpl implements IDService { */ public String getIDPattern (String csid) throws IllegalArgumentException, IllegalStateException { - IDResource.verbose("> in getIDPattern"); + logger.debug("> in getIDPattern"); String serializedPattern = null; @@ -892,7 +888,6 @@ public class IDServiceJdbcImpl implements IDService { rs.close(); } catch (SQLException e) { - System.err.println("Exception: " + e.getMessage()); throw new IllegalStateException( "Error retrieving ID pattern " + "\'" + csid + "\'" + @@ -907,72 +902,10 @@ public class IDServiceJdbcImpl implements IDService { } } - IDResource.verbose("> retrieved IDPattern: " + serializedPattern); + logger.debug("> retrieved IDPattern: " + serializedPattern); return serializedPattern; } - - ////////////////////////////////////////////////////////////////////// - /** - * Serializes an ID pattern, converting it from a Java object into an XML representation. - * - * @param pattern An ID pattern. - * - * @return A serialized representation of that ID pattern. - * - * @throws IllegalArgumentException if the ID pattern cannot be serialized. - */ - public static String serializeIDPattern(IDPattern pattern) throws IllegalArgumentException { - - if (pattern == null) { - throw new IllegalArgumentException("ID pattern cannot be null."); - } - - XStream xstream = new XStream(new DomDriver()); - - String serializedPattern = ""; - try { - serializedPattern = xstream.toXML(pattern); - } catch (XStreamException e) { - throw new IllegalArgumentException( - "Could not convert ID pattern to XML for storage in database."); - } - - return serializedPattern; - - } - - ////////////////////////////////////////////////////////////////////// - /** - * Deserializes an ID pattern, converting it from an XML representation - * into a Java object. - * - * @param serializedPattern A serialized representation of an ID pattern. - * - * @return The ID pattern deserialized as a Java object. - * - * @throws IllegalArgumentException if the ID pattern cannot be deserialized. - */ - public static IDPattern deserializeIDPattern(String serializedPattern) - throws IllegalArgumentException { - - if (serializedPattern == null || serializedPattern.equals("")) { - throw new IllegalArgumentException("ID pattern cannot be null or empty."); - } - - XStream xstream = new XStream(new DomDriver()); - - IDPattern pattern; - try { - pattern = (IDPattern) xstream.fromXML(serializedPattern); - } catch (XStreamException e) { - throw new IllegalArgumentException( - "Could not understand or parse this representation of an ID pattern."); - } - - return pattern; - - } } diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/IDPatternSerializerTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/IDPatternSerializerTest.java new file mode 100644 index 000000000..4d4647614 --- /dev/null +++ b/services/id/service/src/test/java/org/collectionspace/services/id/IDPatternSerializerTest.java @@ -0,0 +1,91 @@ +/** + * IDPatternSerializerTest + * + * Unit tests of the ID Service's IDPatternSerializer class. + * + * This document is a part of the source code and related artifacts + * for CollectionSpace, an open source collections management system + * for museums and related institutions: + * + * http://www.collectionspace.org + * http://wiki.collectionspace.org + * + * Copyright © 2009 Regents of the University of California + * + * Licensed under the Educational Community License (ECL), Version 2.0. + * You may not use this file except in compliance with this License. + * + * You may obtain a copy of the ECL 2.0 License at + * https://source.collectionspace.org/collection-space/LICENSE.txt + * + * $LastChangedBy: aron $ + * $LastChangedRevision: 302 $ + * $LastChangedDate$ + */ + +package org.collectionspace.services.test.id; + +import org.collectionspace.services.id.*; + +import junit.framework.TestCase; +import static org.junit.Assert.*; + +public class IDPatternSerializerTest extends TestCase { + + String serializedPattern; + IDPattern pattern; + + final static String DEFAULT_CSID = "TEST-1"; + + final static String DEFAULT_SERIALIZED_ID_PATTERN = + "\n" + + " " + DEFAULT_CSID + "\n" + + " \n" + + " \n" + + " \n" + + ""; + + // @TODO We may want to canonicalize (or otherwise normalize) the expected and + // actual XML in these tests, to avoid failures resulting from differences in + // whitespace, etc. + public void testSerializeIDPattern() { + IDPattern pattern = new IDPattern(DEFAULT_CSID); + assertEquals(DEFAULT_SERIALIZED_ID_PATTERN, IDPatternSerializer.serialize(pattern)); + } + + public void testSerializeNullIDPattern() { + try { + String serializedPattern = IDPatternSerializer.serialize(null); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testDeserializeIDPattern() { + // This test will fail with different hash codes unless we add an IDPattern.equals() + // method that explicitly defines object equality as, for instance, having identical values + // in each of its instance variables. + // IDPattern pattern = IDPatternSerializer.deserialize(DEFAULT_SERIALIZED_ID_PATTERN); + // assertEquals(pattern, new IDPattern(DEFAULT_CSID)); + } + + public void testDeserializeNullSerializedIDPattern() { + try { + IDPattern pattern = IDPatternSerializer.deserialize(null); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testDeserializeInvalidSerializedIDPattern() { + try { + IDPattern pattern = IDPatternSerializer.deserialize(""); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + +} diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/IDServiceJdbcImplTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/IDServiceJdbcImplTest.java index a98386367..9d0b23215 100644 --- a/services/id/service/src/test/java/org/collectionspace/services/id/IDServiceJdbcImplTest.java +++ b/services/id/service/src/test/java/org/collectionspace/services/id/IDServiceJdbcImplTest.java @@ -1,7 +1,7 @@ -/* +/** * IDServiceJdbcImplTest * - * Test class for the ID Service's JDBC implementation class, IDServiceJdbcImpl. + * Unit tests for the ID Service's JDBC implementation class, IDServiceJdbcImpl. * * This document is a part of the source code and related artifacts * for CollectionSpace, an open source collections management system @@ -42,57 +42,6 @@ public class IDServiceJdbcImplTest extends TestCase { final static String DEFAULT_CSID = "TEST-1"; - final static String DEFAULT_SERIALIZED_ID_PATTERN = - "\n" + - " " + DEFAULT_CSID + "\n" + - " \n" + - " \n" + - " \n" + - ""; - - // @TODO We may want to canonicalize (or otherwise normalize) the expected and - // actual XML in these tests, to avoid failures resulting from differences in - // whitespace, etc. - public void testSerializeIDPattern() { - IDPattern pattern = new IDPattern(DEFAULT_CSID); - assertEquals(DEFAULT_SERIALIZED_ID_PATTERN, jdbc.serializeIDPattern(pattern)); - } - - public void testSerializeNullIDPattern() { - try { - String serializedPattern = jdbc.serializeIDPattern(null); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - } - - public void testDeserializeIDPattern() { - // This test will fail with different hash codes unless we add an IDPattern.equals() - // method that explicitly defines object equality as, for instance, having identical values - // in each of its instance variables. - // IDPattern pattern = jdbc.deserializeIDPattern(DEFAULT_SERIALIZED_ID_PATTERN); - // assertEquals(pattern, new IDPattern(DEFAULT_CSID)); - } - - public void testDeserializeNullSerializedIDPattern() { - try { - IDPattern pattern = jdbc.deserializeIDPattern(null); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - } - - public void testDeserializeInvalidSerializedIDPattern() { - try { - IDPattern pattern = jdbc.deserializeIDPattern(""); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - } - // @TODO Read test patterns from external configuration. public String generateSpectrumEntryNumberTestPattern() { @@ -102,7 +51,7 @@ public class IDServiceJdbcImplTest extends TestCase { pattern.add(new StringIDPart("E")); pattern.add(new NumericIDPart("1")); - return jdbc.serializeIDPattern(pattern); + return IDPatternSerializer.serialize(pattern); } @@ -118,20 +67,18 @@ public class IDServiceJdbcImplTest extends TestCase { pattern.add(new StringIDPart(".")); pattern.add(new NumericIDPart("1")); - return jdbc.serializeIDPattern(pattern); + return IDPatternSerializer.serialize(pattern); } public void testAddIDPattern() { - jdbc.addIDPattern(DEFAULT_CSID, generateSpectrumEntryNumberTestPattern()); - } public void testReadIDPattern() { serializedPattern = jdbc.getIDPattern(DEFAULT_CSID); - pattern = jdbc.deserializeIDPattern(serializedPattern); + pattern = IDPatternSerializer.deserialize(serializedPattern); assertEquals(DEFAULT_CSID, pattern.getCsid()); } @@ -142,23 +89,21 @@ public class IDServiceJdbcImplTest extends TestCase { serializedPattern = jdbc.getIDPattern(DEFAULT_CSID); - pattern = jdbc.deserializeIDPattern(serializedPattern); + pattern = IDPatternSerializer.deserialize(serializedPattern); pattern.setDescription(NEW_DESCRIPTION); - serializedPattern = jdbc.serializeIDPattern(pattern); + serializedPattern = IDPatternSerializer.serialize(pattern); jdbc.updateIDPattern(DEFAULT_CSID, serializedPattern); serializedPattern = jdbc.getIDPattern(DEFAULT_CSID); - pattern = jdbc.deserializeIDPattern(serializedPattern); + pattern = IDPatternSerializer.deserialize(serializedPattern); assertEquals(NEW_DESCRIPTION, pattern.getDescription()); } public void testDeleteIDPattern() { - jdbc.deleteIDPattern(DEFAULT_CSID); - } public void testNextIDValidPattern() { -- 2.47.3