From d5d3bc4a8e1f3bf34fe69e82a2803569054a41cd Mon Sep 17 00:00:00 2001 From: Aron Roberts Date: Thu, 10 Sep 2009 23:46:56 +0000 Subject: [PATCH] CSPACE-429: Minor code cleanup in ID Service following refactoring. --- .../id/AlphabeticIDGeneratorPart.java | 101 +++++++------ .../services/id/NumericIDGeneratorPart.java | 114 +++++++++------ .../services/id/SettableIDGenerator.java | 2 +- .../services/id/StringIDGeneratorPart.java | 6 +- .../services/id/UUIDGeneratorPart.java | 34 +++-- .../services/id/YearIDGeneratorPart.java | 80 +++------- .../id/test/NumericIDGeneratorPartTest.java | 138 ++++++++---------- .../id/test/StringIDGeneratorPartTest.java | 88 +++++------ .../id/test/UUIDGeneratorPartTest.java | 22 ++- 9 files changed, 272 insertions(+), 313 deletions(-) diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/AlphabeticIDGeneratorPart.java b/services/id/service/src/main/java/org/collectionspace/services/id/AlphabeticIDGeneratorPart.java index c29975c62..abcfbee65 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/AlphabeticIDGeneratorPart.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/AlphabeticIDGeneratorPart.java @@ -21,8 +21,6 @@ * limitations under the License. */ -// @TODO: Add Javadoc comments - // @TODO: When auto expanding, we'll need to allow setting of a maximum length to // which the generated IDs can grow, likely as an additional parameter to be // passed to a constructor, with a default value hard-coded in the class. @@ -81,35 +79,48 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { private Vector initialValue = new Vector(); private Vector currentValue = new Vector(); - // Constructor using defaults for character sequence and initial value. - // - // If no start and end characters are provided for the alphabetic character - // sequence, default to an 'a-z' sequence, representing the lowercase alphabetic - // characters in the USASCII character set (within Java's internal - // Unicode UTF-16 representation). - // - // Additionally defaults to an initial value of "a". + /** + * Constructor using defaults for character sequence and initial value. + * + * If no start and end characters are provided for the alphabetic character + * sequence, default to an 'a-z' sequence, representing the lowercase alphabetic + * characters in the USASCII character set (within Java's internal + * Unicode UTF-16 representation). + * + * Additionally defaults to an initial value of "a". + */ public AlphabeticIDGeneratorPart() throws IllegalArgumentException { - this(DEFAULT_START_CHAR, DEFAULT_END_CHAR, DEFAULT_INITIAL_VALUE); - } - // Constructor using defaults for character sequence. - // - // If no start and end characters are provided for the alphabetic character - // sequence, default to an 'a-z' sequence, representing the lowercase alphabetic - // characters in the USASCII character set (within Java's internal - // Unicode UTF-16 representation). - public AlphabeticIDGeneratorPart(String initial) throws IllegalArgumentException { - + /** + * Constructor using defaults for character sequence and initial value. + * + * If no start and end characters are provided for the alphabetic character + * sequence, default to an 'a-z' sequence, representing the lowercase alphabetic + * characters in the USASCII character set (within Java's internal + * Unicode UTF-16 representation). + * + * @param initial The initial value of the alphabetic ID. Must be a + * member of the valid aphabetic sequence. + */ + public AlphabeticIDGeneratorPart(String initial) + throws IllegalArgumentException { this(DEFAULT_START_CHAR, DEFAULT_END_CHAR, initial); - } - // Constructor. - public AlphabeticIDGeneratorPart(String sequenceStart, String sequenceEnd, String initial) - throws IllegalArgumentException { + /** + * Constructor + * + * @param sequenceStart The start character of the valid alphabetic sequence. + * + * @param sequenceEnd The end character of the valid alphabetic sequence. + * + * @param initial The initial value of the alphabetic ID. Must be a + * member of the valid aphabetic sequence. + */ + public AlphabeticIDGeneratorPart(String sequenceStart, String sequenceEnd, + String initial) throws IllegalArgumentException { // Validate and store the start character in the character sequence. @@ -183,26 +194,14 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { } - // Initialize the current value from the initial value. - // this.currentValue = new Vector(this.initialValue); - } - // Returns the initial value. + @Override public String getInitialID() { return getIDString(this.initialValue); } - // Returns the current value. - public String getCurrentID() { - if (this.currentValue == null || this.currentValue.size() == 0) { - return getIDString(this.initialValue); - } else { - return getIDString(this.currentValue); - } - } - - // Sets the current value. + @Override public void setCurrentID(String value) throws IllegalArgumentException { // @TODO Much of this code is copied from the main constructor, @@ -243,21 +242,23 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { } - // Reset the current value to the initial value. - public void resetID() { - this.currentValue = this.initialValue; - // Collections.copy(this.currentValue, this.initialValue); + @Override + public String getCurrentID() { + if (this.currentValue == null || this.currentValue.size() == 0) { + return getIDString(this.initialValue); + } else { + return getIDString(this.currentValue); + } } - // Returns the next alphabetic ID in the sequence. - // // Currently, the number of characters auto-expands as the // value of the most significant character rolls over. - // E.g. a call to getNextID(), where the current ID is "z", - // auto-expands to "aa", and "ZZ" auto-expands to "AAA". + // E.g. where the current ID is "z", this is auto-expanded + // to "aa". Similarly, "ZZ" is auto-expanded to "AAA". // // See the TODOs at the top of this class for additional - // functionality that needs to be implemented. + // functionality that needs to be implemented regarding auto-expansion. + @Override public String nextID() throws IllegalStateException { // Get next values for each character, from right to left @@ -303,9 +304,6 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { } - /** - * Returns a new identifier. - */ @Override public String newID() { // If there is no current value, return the initial value @@ -324,7 +322,7 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { if (id == null) return false; - // @TODO May potentially throw at least one pattern-related exception. + // @TODO May potentially throw java.util.regex.PatternSyntaxException. // We'll need to catch and handle this here, as well as in all // derived classes and test cases that invoke validation. @@ -369,5 +367,4 @@ public class AlphabeticIDGeneratorPart implements SequenceIDGeneratorPart { return sb.toString(); } - } diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/NumericIDGeneratorPart.java b/services/id/service/src/main/java/org/collectionspace/services/id/NumericIDGeneratorPart.java index 8580f0de5..94b5e1e09 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/NumericIDGeneratorPart.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/NumericIDGeneratorPart.java @@ -20,10 +20,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - -// @TODO: Add Javadoc comments - -// @TODO: Need to set and enforce maximum value. + +// @TODO Need to handle Exceptions as described in comments below. package org.collectionspace.services.id; @@ -49,35 +47,62 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { private long initialValue = DEFAULT_INITIAL_VALUE; private long currentValue = CURRENT_VALUE_NOT_SET; - // Constructor using defaults for initial value and maximum length. + /** + * Constructor using defaults for initial value and maximum length. + */ public NumericIDGeneratorPart() throws IllegalArgumentException { this(Integer.toString(DEFAULT_INITIAL_VALUE), Integer.toString(DEFAULT_MAX_LENGTH)); } - // Constructor using default maximum length. + /** + * Constructor using defaults for initial value and maximum length. + * + * @param initialValue The initial value of the numeric ID. + */ public NumericIDGeneratorPart(String initialValue) throws IllegalArgumentException { this(initialValue, Integer.toString(DEFAULT_MAX_LENGTH)); } - // Constructor. + /** + * Constructor. + * + * @param initialValue The initial value of the numeric ID. + * + * @param maxLength The maximum String length for generated IDs. + */ public NumericIDGeneratorPart(String initialValue, String maxLength) throws IllegalArgumentException { + if (maxLength == null || maxLength.equals("")) { + throw new IllegalArgumentException( + "Initial ID value must not be null or empty"); + } + try { + this.maxLength = Integer.parseInt(maxLength); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + "Maximum ID length must be parseable as a number"); + } if (initialValue == null || initialValue.equals("")) { throw new IllegalArgumentException( "Initial ID value must not be null or empty"); } try { - long l = Long.parseLong(initialValue.trim()); - if ( l < 0 ) { + long initVal = Long.parseLong(initialValue.trim()); + if ( initVal < 0 ) { throw new IllegalArgumentException( "Initial ID value should be zero (0) or greater"); } - // this.currentValue = l; - this.initialValue = l; + String initValStr = Long.toString(initVal); + if (initValStr.length() > this.maxLength) { + throw new IllegalStateException( + "Initial ID cannot exceed maximum length of " + + maxLength + "."); + } + this.initialValue = initVal; } catch (NullPointerException e) { throw new IllegalArgumentException( "Initial ID value should not be null"); @@ -86,33 +111,14 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { "Initial ID value must be parseable as a number"); } - if (maxLength == null || maxLength.equals("")) { - throw new IllegalArgumentException( - "Initial ID value must not be null or empty"); - } - - try { - this.maxLength = Integer.parseInt(maxLength); - } catch (NumberFormatException e) { - throw new IllegalArgumentException( - "Maximum ID length must be parseable as a number"); - } - } + @Override public String getInitialID() { return Long.toString(this.initialValue); } - public String getCurrentID() { - if (this.currentValue == CURRENT_VALUE_NOT_SET) { - return Long.toString(this.initialValue); - } else { - return Long.toString(this.currentValue); - } - } - - // Sets the current value of the ID. + @Override public void setCurrentID(String value) throws IllegalArgumentException { // @TODO Much of this code is copied from the main constructor, @@ -124,12 +130,18 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { } try { - long l = Long.parseLong(value.trim()); - if ( l < 0 ) { + long currVal = Long.parseLong(value.trim()); + if ( currVal < 0 ) { throw new IllegalArgumentException( "ID value should be zero (0) or greater"); } - this.currentValue = l; + String currValStr = Long.toString(currVal); + if (currValStr.length() > this.maxLength) { + throw new IllegalStateException( + "Current ID cannot exceed maximum length of " + + maxLength + "."); + } + this.currentValue = currVal; } catch (NullPointerException e) { throw new IllegalArgumentException( "ID value should not be null"); @@ -137,16 +149,13 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { throw new IllegalArgumentException( "ID value must be parseable as a number"); } - - // @TODO An expedient; we may need to check the String length of the - // provided ID and calculate a maximum length here. - this.maxLength = DEFAULT_MAX_LENGTH; } - // Returns the next ID in the sequence, and sets the current value to that ID. + + @Override public String nextID() throws IllegalStateException { if (this.currentValue == CURRENT_VALUE_NOT_SET) { - this.currentValue = this.initialValue; + this.currentValue = this.initialValue; } else { this.currentValue++; } @@ -157,10 +166,19 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { } return nextID; } - - /** - * Returns a new identifier. - */ + + @Override + public String getCurrentID() { + if (this.currentValue == CURRENT_VALUE_NOT_SET) { + return Long.toString(this.initialValue); + } else { + return Long.toString(this.currentValue); + } + } + + // @TODO Need to handle exceptions thrown by nextID(); + // currently, newID simply throws an uncaught and unreported exception. + @Override public String newID() { return nextID(); @@ -169,9 +187,11 @@ public class NumericIDGeneratorPart implements SequenceIDGeneratorPart { @Override public boolean isValidID(String id) { - if (id == null) return false; + if (id == null) { + return false; + } - // @TODO May potentially throw at least one pattern-related exception. + // @TODO May potentially throw java.util.regex.PatternSyntaxException. // We'll need to catch and handle this here, as well as in all // derived classes and test cases that invoke validation. diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/SettableIDGenerator.java b/services/id/service/src/main/java/org/collectionspace/services/id/SettableIDGenerator.java index 3ba753ad6..596bdc555 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/SettableIDGenerator.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/SettableIDGenerator.java @@ -40,7 +40,7 @@ import java.util.regex.Pattern; * * This generator extends the base ID generator to handle * instances where a full or partial ID is supplied, - * on which a new ID must be generated. + * based on which a new ID must be generated. * * $LastChangedRevision: 647 $ * $LastChangedDate$ diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/StringIDGeneratorPart.java b/services/id/service/src/main/java/org/collectionspace/services/id/StringIDGeneratorPart.java index 435162d0a..c7abc67ed 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/StringIDGeneratorPart.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/StringIDGeneratorPart.java @@ -84,9 +84,11 @@ public class StringIDGeneratorPart implements IDGeneratorPart, @Override public boolean isValidID(String id) { - if (id == null) return false; + if (id == null) { + return false; + } - // @TODO May potentially throw at least one pattern-related exception. + // @TODO May potentially throw java.util.regex.PatternSyntaxException. // We'll need to catch and handle this here, as well as in all // derived classes and test cases that invoke validation. diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/UUIDGeneratorPart.java b/services/id/service/src/main/java/org/collectionspace/services/id/UUIDGeneratorPart.java index eea702df7..905bb3509 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/UUIDGeneratorPart.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/UUIDGeneratorPart.java @@ -32,7 +32,7 @@ import java.util.regex.PatternSyntaxException; * UUIDGeneratorPart * * Generates universally unique identifiers (UUIDs) in the Version 4 format - * (random or pseudorandom numbers), described at + * (primarily consisting of random or pseudo-random numbers), described at * http://en.wikipedia.org/wiki/Universally_Unique_Identifier#Version_4_.28random.29 * * $LastChangedRevision: 625 $ @@ -65,17 +65,23 @@ public class UUIDGeneratorPart implements IDGeneratorPart { public UUIDGeneratorPart() { } - public void setCurrentID(String id) { - if (id == null || id.equals("") || (! isValidID(id) )) { - this.currentValue = newID(); - } else { - this.currentValue = id; - } + @Override + public void setCurrentID(String idValue) throws IllegalArgumentException { + if (idValue == null || idValue.equals("")) { + throw new IllegalArgumentException( + "Supplied UUID must not be null or empty"); + } + if (! isValidID(idValue)) { + throw new IllegalArgumentException( + "Supplied UUID '" + idValue + "' is not valid."); + } + this.currentValue = idValue; } + @Override public String getCurrentID() { if (this.currentValue == null || this.currentValue.equals("")) { - return newID(); + return newID(); // Will also set the current ID to the new ID. } else { return this.currentValue; } @@ -83,17 +89,19 @@ public class UUIDGeneratorPart implements IDGeneratorPart { @Override public String newID() { - String id = UUID.randomUUID().toString(); - this.currentValue = id; - return id; + String newID = UUID.randomUUID().toString(); + setCurrentID(newID); + return newID; } @Override public boolean isValidID(String id) { - if (id == null) return false; + if (id == null) { + return false; + } - // @TODO May potentially throw at least one pattern-related exception. + // @TODO May potentially throw java.util.regex.PatternSyntaxException. // We'll need to catch and handle this here, as well as in all // derived classes and test cases that invoke validation. diff --git a/services/id/service/src/main/java/org/collectionspace/services/id/YearIDGeneratorPart.java b/services/id/service/src/main/java/org/collectionspace/services/id/YearIDGeneratorPart.java index d73d28a85..3d0d33a3d 100644 --- a/services/id/service/src/main/java/org/collectionspace/services/id/YearIDGeneratorPart.java +++ b/services/id/service/src/main/java/org/collectionspace/services/id/YearIDGeneratorPart.java @@ -21,8 +21,6 @@ * limitations under the License. */ -// @TODO: Add Javadoc comments - // @TODO: Need to understand and reflect time zone issues; // what happens when a year rollover occurs: // - In the time zone of the end user. @@ -38,7 +36,8 @@ // http://joda-time.sourceforge.net/ // // There may also be a need to have a structured set of date-time -// classes related to identifier generation. +// classes related to identifier generation, which can also be +// facilitated through the use of Joda-Time. package org.collectionspace.services.id; @@ -59,7 +58,6 @@ import java.util.regex.Pattern; */ public class YearIDGeneratorPart implements IDGeneratorPart { - private String initialValue = null; private String currentValue = null; // NOTE: Currently hard-coded to accept only a range of @@ -67,78 +65,47 @@ public class YearIDGeneratorPart implements IDGeneratorPart { final static String REGEX_PATTERN = "(\\d{4})"; public YearIDGeneratorPart() throws IllegalArgumentException { - - String currentYear = getCurrentYear(); - this.initialValue = currentYear; - this.currentValue = currentYear; - + this.currentValue = getCurrentYear(); } - public YearIDGeneratorPart(String initialValue) + public YearIDGeneratorPart(String yearValue) throws IllegalArgumentException { + setCurrentID(yearValue); + } - if (initialValue == null || initialValue.equals("")) { + @Override + public void setCurrentID(String yearValue) throws IllegalArgumentException { + if (yearValue == null || yearValue.equals("")) { throw new IllegalArgumentException( - "Initial ID value must not be null or empty"); + "Supplied year must not be null or empty"); } - - // @TODO: Add regex-based validation here, by calling isValidID(). - // Consider implications for Internationalization when doing so. - - this.initialValue = initialValue; - this.currentValue = initialValue; - - } - - public String getInitialID() { - return this.initialValue; + // NOTE: Validation currently is based on a + // hard coded year format and calendar system. + if (! isValidID(yearValue)) { + throw new IllegalArgumentException( + "Supplied year '" + yearValue + "' is not valid."); + } + this.currentValue = yearValue; } + @Override public String getCurrentID() { return this.currentValue; } - // Sets the current value. - public void setCurrentID(String value) throws IllegalArgumentException { - - // @TODO This code is copied from the main constructor, - // and thus there may be an opportunity for refactoring. - - if (value == null || value.equals("")) { - throw new IllegalArgumentException("ID value must not be null or empty"); - } - - // @TODO: Add regex-based validation here, by calling isValidID(). - // Consider implications for Internationalization when doing so. - - this.currentValue = value; - - } - - public void resetID() { - this.currentValue = this.initialValue; - } - - // @TODO: We'll need to decide what a "next" ID means in the context of: - // - An initially supplied value. - // - A year value that has not changed from its previous value. - // - A year value that has changed, as a result of a rollover - // to a new instant in time. - public String nextID() { - return this.currentValue; - } - @Override public String newID() { - return getCurrentYear(); + return this.currentValue; } @Override public boolean isValidID(String id) { - if (id == null) return false; + if (id == null) { + return false; + } - // @TODO May potentially throw at least one pattern-related exception. + // @TODO May potentially throw java.util.regex.PatternSyntaxException. // We'll need to catch and handle this here, as well as in all // derived classes and test cases that invoke validation. @@ -157,6 +124,7 @@ public class YearIDGeneratorPart implements IDGeneratorPart { return REGEX_PATTERN; } + // NOTE: Currently hard-coded to use the Gregorian Calendar system. public static String getCurrentYear() { Calendar cal = GregorianCalendar.getInstance(); int year = cal.get(Calendar.YEAR); diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/test/NumericIDGeneratorPartTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/test/NumericIDGeneratorPartTest.java index eb51afa1b..b87f32382 100644 --- a/services/id/service/src/test/java/org/collectionspace/services/id/test/NumericIDGeneratorPartTest.java +++ b/services/id/service/src/test/java/org/collectionspace/services/id/test/NumericIDGeneratorPartTest.java @@ -42,8 +42,64 @@ public class NumericIDGeneratorPartTest extends TestCase { SequenceIDGeneratorPart part; - public void testNewID() { + public void testInitialID() { + part = new NumericIDGeneratorPart("0"); + assertEquals("0", part.getInitialID()); + + part = new NumericIDGeneratorPart("25"); + assertEquals("25", part.getInitialID()); + } + + public void testNullInitialValue() { + try { + part = new NumericIDGeneratorPart(null); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testNonLongParseableInitialValue() { + try { + part = new NumericIDGeneratorPart("not a long parseable value"); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testNonLongParseableMaxLength() { + try { + part = new NumericIDGeneratorPart("1", "not an int parseable value"); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testNegativeInitialValue() { + try { + part = new NumericIDGeneratorPart("-1"); + fail("Should have thrown IllegalArgumentException here"); + } catch (IllegalArgumentException expected) { + // This Exception should be thrown, and thus the test should pass. + } + } + + public void testGetCurrentID() { + part = new NumericIDGeneratorPart("0"); + assertEquals("0", part.getCurrentID()); + assertEquals("0", part.newID()); + assertEquals("1", part.newID()); + assertEquals("2", part.newID()); + assertEquals("2", part.getCurrentID()); + assertEquals("3", part.newID()); + part = new NumericIDGeneratorPart("25"); + assertEquals("25", part.getCurrentID()); + } + + public void testNewID() { part = new NumericIDGeneratorPart("0"); assertEquals("0", part.newID()); assertEquals("1", part.newID()); @@ -60,7 +116,6 @@ public class NumericIDGeneratorPartTest extends TestCase { assertEquals("1", part.newID()); assertEquals("2", part.newID()); assertEquals("3", part.newID()); - } public void testNewIDOverflow() { @@ -90,107 +145,28 @@ public class NumericIDGeneratorPartTest extends TestCase { } - public void testInitialID() { - - part = new NumericIDGeneratorPart("0"); - assertEquals("0", part.getInitialID()); - - part = new NumericIDGeneratorPart("25"); - assertEquals("25", part.getInitialID()); - - } - - public void testCurrentID() { - - part = new NumericIDGeneratorPart("0"); - assertEquals("0", part.getCurrentID()); - assertEquals("0", part.newID()); - assertEquals("1", part.newID()); - assertEquals("2", part.newID()); - assertEquals("2", part.getCurrentID()); - assertEquals("3", part.newID()); - - part = new NumericIDGeneratorPart("25"); - assertEquals("25", part.getCurrentID()); - - } - - public void testNullInitialValue() { - - try { - part = new NumericIDGeneratorPart(null); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - - } - - public void testNonLongParseableInitialValue() { - - try { - part = new NumericIDGeneratorPart("not a long parseable value"); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - - } - - public void testNonLongParseableMaxLength() { - - try { - part = new NumericIDGeneratorPart("1", "not an int parseable value"); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - - } - - public void testNegativeInitialValue() { - - try { - part = new NumericIDGeneratorPart("-1"); - fail("Should have thrown IllegalArgumentException here"); - } catch (IllegalArgumentException expected) { - // This Exception should be thrown, and thus the test should pass. - } - } - - public void testIsValidID() { - part = new NumericIDGeneratorPart("1"); assertTrue(part.isValidID("1")); - part = new NumericIDGeneratorPart("1"); assertTrue(part.isValidID("123")); - part = new NumericIDGeneratorPart("1"); assertTrue(part.isValidID("123456")); part = new NumericIDGeneratorPart("1"); assertFalse(part.isValidID("1234567")); - part = new NumericIDGeneratorPart("1", "3"); assertTrue(part.isValidID("123")); - part = new NumericIDGeneratorPart("1", "3"); assertFalse(part.isValidID("1234")); - part = new NumericIDGeneratorPart("1"); assertFalse(part.isValidID("not a parseable long")); - part = new NumericIDGeneratorPart("1", "3"); assertFalse(part.isValidID("not a parseable long")); - part = new NumericIDGeneratorPart("1", "3"); assertFalse(part.isValidID(null)); - part = new NumericIDGeneratorPart("1", "3"); assertFalse(part.isValidID("")); - } // @TODO: Add more tests of boundary conditions, exceptions ... diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/test/StringIDGeneratorPartTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/test/StringIDGeneratorPartTest.java index a13db7ab5..11d60dd06 100644 --- a/services/id/service/src/test/java/org/collectionspace/services/id/test/StringIDGeneratorPartTest.java +++ b/services/id/service/src/test/java/org/collectionspace/services/id/test/StringIDGeneratorPartTest.java @@ -38,88 +38,70 @@ public class StringIDGeneratorPartTest extends TestCase { StoredValueIDGeneratorPart part; - public void testnewID() { - - part = new StringIDGeneratorPart("E"); - assertEquals("E", part.newID()); - - part = new StringIDGeneratorPart("XYZ"); - assertEquals("XYZ", part.newID()); - - } - -/* - public void testresetID() { - - part = new StringIDGeneratorPart("."); - assertEquals(".", part.newID()); - part.resetID(); - assertEquals(".", part.newID()); - - } -*/ - - public void testInitialID() { - part = new StringIDGeneratorPart("-"); - assertEquals("-", part.getInitialID()); - } - - public void testCurrentID() { - part = new StringIDGeneratorPart("- -"); - assertEquals("- -", part.getCurrentID()); - } - public void testNullInitialValue() { - try { part = new StringIDGeneratorPart(null); fail("Should have thrown IllegalArgumentException here"); } catch (IllegalArgumentException expected) { // This Exception should be thrown, and thus the test should pass. } - } public void testEmptyInitialValue() { - try { part = new StringIDGeneratorPart(""); fail("Should have thrown IllegalArgumentException here"); } catch (IllegalArgumentException expected) { // This Exception should be thrown, and thus the test should pass. } + } + public void testGetInitialID() { + part = new StringIDGeneratorPart("-"); + assertEquals("-", part.getInitialID()); } - public void testIsValidID() { + public void testGetCurrentID() { + part = new StringIDGeneratorPart("- -"); + assertEquals("- -", part.getCurrentID()); + } - part = new StringIDGeneratorPart("-"); - assertTrue(part.isValidID("-")); + public void testNewID() { + part = new StringIDGeneratorPart("E"); + assertEquals("E", part.newID()); + part = new StringIDGeneratorPart("XYZ"); + assertEquals("XYZ", part.newID()); + } + public void testIsValidID() { part = new StringIDGeneratorPart("-"); - assertFalse(part.isValidID("--")); - - // Test chars with special meaning in regexes. - part = new StringIDGeneratorPart("."); - assertTrue(part.isValidID(".")); - + assertTrue(part.isValidID("-")); part = new StringIDGeneratorPart("TE"); assertTrue(part.isValidID("TE")); - - part = new StringIDGeneratorPart("TE"); - assertFalse(part.isValidID("T")); - - part = new StringIDGeneratorPart("T"); - assertFalse(part.isValidID("TE")); - + part = new StringIDGeneratorPart("-"); - assertFalse(part.isValidID(null)); - + assertFalse(part.isValidID(null)); part = new StringIDGeneratorPart("-"); assertFalse(part.isValidID("")); - + part = new StringIDGeneratorPart("-"); + assertFalse(part.isValidID("--")); + part = new StringIDGeneratorPart("TE"); + assertFalse(part.isValidID("T")); + part = new StringIDGeneratorPart("T"); + assertFalse(part.isValidID("TE")); } +/* + public void testIsValidIDWithRegexChars() { + // Test chars with special meaning in regexes. + // @TODO Will throw a PatternSyntaxException; we need to catch this. + part = new StringIDGeneratorPart("*"); + assertTrue(part.isValidID("*")); + part = new StringIDGeneratorPart("T*E"); + assertTrue(part.isValidID("T*E")); + } +*/ + // @TODO: Add more tests of boundary conditions, exceptions ... } diff --git a/services/id/service/src/test/java/org/collectionspace/services/id/test/UUIDGeneratorPartTest.java b/services/id/service/src/test/java/org/collectionspace/services/id/test/UUIDGeneratorPartTest.java index 2ab2116e7..15bf30b83 100644 --- a/services/id/service/src/test/java/org/collectionspace/services/id/test/UUIDGeneratorPartTest.java +++ b/services/id/service/src/test/java/org/collectionspace/services/id/test/UUIDGeneratorPartTest.java @@ -39,7 +39,6 @@ public class UUIDGeneratorPartTest extends TestCase { IDGeneratorPart part; public void testnewID() { - part = new UUIDGeneratorPart(); String firstID = part.newID(); @@ -53,23 +52,30 @@ public class UUIDGeneratorPartTest extends TestCase { assertTrue(firstID.compareTo(secondID) != 0); assertTrue(firstID.compareTo(thirdID) != 0); assertTrue(secondID.compareTo(thirdID) != 0); - + + assertTrue(part.isValidID(firstID)); + assertTrue(part.isValidID(secondID)); + assertTrue(part.isValidID(thirdID)); } public void testIsValidID() { - part = new UUIDGeneratorPart(); assertTrue(part.isValidID("2d5ef3cc-bfb2-4383-a4c6-35645cd5dd81")); - part = new UUIDGeneratorPart(); - assertFalse(part.isValidID("foo")); - part = new UUIDGeneratorPart(); assertFalse(part.isValidID(null)); - part = new UUIDGeneratorPart(); assertFalse(part.isValidID("")); - + part = new UUIDGeneratorPart(); + assertFalse(part.isValidID("not a UUID")); + part = new UUIDGeneratorPart(); + assertFalse(part.isValidID("12345")); + // Invalid character in 15th position (should be '4'). + part = new UUIDGeneratorPart(); + assertFalse(part.isValidID("4c9395a8-1669-31f9-806c-920d86e40912")); + // Invalid character in 20th position (should be '8', '9', 'a', or 'b'). + part = new UUIDGeneratorPart(); + assertFalse(part.isValidID("4c9395a8-1669-41f9-106c-920d86e40912")); } // @TODO: Add more tests of boundary conditions, exceptions ... -- 2.47.3