From 881ee30047a95a78af25f435eadd787bd573ddd9 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 1 Jun 2011 11:40:26 -0700 Subject: [PATCH 1/9] Modified toString() such that the no-box-arguments versions do not automatically traverse collections, but instead simply call toString on them. The versions that take box arguments (where the developer is clearly expressing a desire for custom formatted collections) still do the traversal. Moved the warning that Enumeration and Iterator are consumed into said methods. Switched LogBuilder to use StringUtil.toString, since it now subsumes the behavior of ArrayUtil.toString without the undesirable collection munging that motivated its original creation. Nixed ArrayUtil.toString/safeToString because they haven't been in the wild long enough to be likely to have been discovered and used by third parties. --- .../java/com/samskivert/util/ArrayUtil.java | 45 --- .../java/com/samskivert/util/LogBuilder.java | 2 +- src/main/java/com/samskivert/util/Logger.java | 2 +- .../java/com/samskivert/util/StringUtil.java | 345 ++++++++++-------- .../java/com/samskivert/util/ConfigTest.java | 4 +- .../com/samskivert/util/HashIntMapTest.java | 4 +- 6 files changed, 190 insertions(+), 212 deletions(-) diff --git a/src/main/java/com/samskivert/util/ArrayUtil.java b/src/main/java/com/samskivert/util/ArrayUtil.java index 4b9fa08f..7db9d5d7 100644 --- a/src/main/java/com/samskivert/util/ArrayUtil.java +++ b/src/main/java/com/samskivert/util/ArrayUtil.java @@ -892,51 +892,6 @@ public class ArrayUtil return dest; } - /** - * Return the String representation of the specified Object, which may or may not be an array. - */ - public static String safeToString (Object o) - { - return ((o == null) || !o.getClass().isArray()) ? String.valueOf(o) : toString(o); - } - - /** - * Return the String representation of the specified Object, which must be an array. - * - * @throws IllegalArgumentException if array is not actually an array. - */ - public static String toString (Object array) - { - if (array instanceof Object[]) { - return Arrays.deepToString((Object[])array); // go deep, baby - - } else if (array instanceof int[]) { - return Arrays.toString((int[])array); - - } else if (array instanceof byte[]) { - return Arrays.toString((byte[])array); - - } else if (array instanceof char[]) { - return Arrays.toString((char[])array); - - } else if (array instanceof short[]) { - return Arrays.toString((short[])array); - - } else if (array instanceof long[]) { - return Arrays.toString((long[])array); - - } else if (array instanceof float[]) { - return Arrays.toString((float[])array); - - } else if (array instanceof double[]) { - return Arrays.toString((double[])array); - - } else if (array instanceof boolean[]) { - return Arrays.toString((boolean[])array); - } - throw new IllegalArgumentException("Not an array: " + array); - } - /** The default random object used when shuffling an array. */ protected static final Random _rnd = new Random(); } diff --git a/src/main/java/com/samskivert/util/LogBuilder.java b/src/main/java/com/samskivert/util/LogBuilder.java index 1be1eced..be73c726 100644 --- a/src/main/java/com/samskivert/util/LogBuilder.java +++ b/src/main/java/com/samskivert/util/LogBuilder.java @@ -48,7 +48,7 @@ public class LogBuilder } _log.append(args[ii]).append('='); try { - _log.append(ArrayUtil.safeToString(args[ii + 1])); + _log.append(StringUtil.toString(args[ii + 1])); } catch (Throwable t) { _log.append("'); } diff --git a/src/main/java/com/samskivert/util/Logger.java b/src/main/java/com/samskivert/util/Logger.java index 9eab8de7..800c6f5c 100644 --- a/src/main/java/com/samskivert/util/Logger.java +++ b/src/main/java/com/samskivert/util/Logger.java @@ -142,7 +142,7 @@ public abstract class Logger } buf.append(args[ii]).append('='); try { - buf.append(ArrayUtil.safeToString(args[ii + 1])); + buf.append(StringUtil.toString(args[ii + 1])); } catch (Throwable t) { buf.append(""); } diff --git a/src/main/java/com/samskivert/util/StringUtil.java b/src/main/java/com/samskivert/util/StringUtil.java index 68349bc2..6ce021a4 100644 --- a/src/main/java/com/samskivert/util/StringUtil.java +++ b/src/main/java/com/samskivert/util/StringUtil.java @@ -355,16 +355,8 @@ public class StringUtil * Converts the supplied object to a string. Normally this is accomplished via the object's * built in toString() method, but in the case of arrays, toString() * is called on each element and the contents are listed like so: - * - * 
-     * (value, value, value)
-     *
- * - * Arrays of ints, longs, floats and doubles are also handled for convenience. - * - *

Additionally, Enumeration or Iterator objects can be passed - * and they will be enumerated and output in a similar manner to arrays. Bear in mind that this - * uses up the enumeration or iterator in question. + * 

(value, value, value)
+ * Arrays of primitive types are also handled for convenience. * *

Also note that passing null will result in the string "null" being returned. */ @@ -375,10 +367,29 @@ public class StringUtil return buf.toString(); } + /** + * Converts the supplied value to a string and appends it to the supplied string buffer. See + * {@link #toString()} for more information. + * + * @param buf the string buffer to which we will append the string. + * @param val the value from which to generate the string. + */ + public static void toString (StringBuilder buf, Object val) + { + // these boxes+sep will only be used for arrays + toString(buf, val, "(", ")", ", ", false); + } + /** * Like the single argument {@link #toString(Object)} with the additional function of - * specifying the characters that are used to box in list and array types. For example, if "[" - * and "]" were supplied, an int array might be formatted like so: [1, 3, 5]. + * specifying the characters that are used to box in collection and array types. For example, + * if "[" and "]" were supplied, an int array might be formatted like so: [1, 3, + * 5]. + * + *

Note: in this method (unlike {@link #toString()}), Enumeration or + * Iterator objects can be passed and they will be enumerated and output in a + * similar manner to arrays. Bear in mind that this uses up the enumeration or iterator in + * question. */ public static String toString (Object val, String openBox, String closeBox) { @@ -387,23 +398,16 @@ public class StringUtil return buf.toString(); } - /** - * Converts the supplied value to a string and appends it to the supplied string buffer. See - * the single argument version for more information. - * - * @param buf the string buffer to which we will append the string. - * @param val the value from which to generate the string. - */ - public static void toString (StringBuilder buf, Object val) - { - toString(buf, val, "(", ")"); - } - /** * Converts the supplied value to a string and appends it to the supplied string buffer. The * specified boxing characters are used to enclose list and array types. For example, if "[" * and "]" were supplied, an int array might be formatted like so: [1, 3, 5]. * + *

Note: in this method (unlike {@link #toString()}), Enumeration or + * Iterator objects can be passed and they will be enumerated and output in a + * similar manner to arrays. Bear in mind that this uses up the enumeration or iterator in + * question. + * * @param buf the string buffer to which we will append the string. * @param val the value from which to generate the string. * @param openBox the opening box character. @@ -419,150 +423,21 @@ public class StringUtil * specified boxing characters are used to enclose list and array types. For example, if "[" * and "]" were supplied, an int array might be formatted like so: [1, 3, 5]. * + *

Note: in this method (unlike {@link #toString()}), Enumeration or + * Iterator objects can be passed and they will be enumerated and output in a + * similar manner to arrays. Bear in mind that this uses up the enumeration or iterator in + * question. + * * @param buf the string buffer to which we will append the string. * @param val the value from which to generate the string. * @param openBox the opening box character. * @param closeBox the closing box character. * @param sep the separator string. */ - public static void toString ( - StringBuilder buf, Object val, String openBox, String closeBox, String sep) + public static void toString (StringBuilder buf, Object val, String openBox, String closeBox, + String sep) { - if (val instanceof byte[]) { - buf.append(openBox); - byte[] v = (byte[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof short[]) { - buf.append(openBox); - short[] v = (short[])val; - for (short i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof int[]) { - buf.append(openBox); - int[] v = (int[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof long[]) { - buf.append(openBox); - long[] v = (long[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof float[]) { - buf.append(openBox); - float[] v = (float[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof double[]) { - buf.append(openBox); - double[] v = (double[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i]); - } - buf.append(closeBox); - - } else if (val instanceof Object[]) { - buf.append(openBox); - Object[] v = (Object[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - toString(buf, v[i], openBox, closeBox); - } - buf.append(closeBox); - - } else if (val instanceof boolean[]) { - buf.append(openBox); - boolean[] v = (boolean[])val; - for (int i = 0; i < v.length; i++) { - if (i > 0) { - buf.append(sep); - } - buf.append(v[i] ? "t" : "f"); - } - buf.append(closeBox); - - } else if (val instanceof Iterable) { - toString(buf, ((Iterable)val).iterator(), openBox, closeBox); - - } else if (val instanceof Iterator) { - buf.append(openBox); - Iterator iter = (Iterator)val; - for (int i = 0; iter.hasNext(); i++) { - if (i > 0) { - buf.append(sep); - } - toString(buf, iter.next(), openBox, closeBox); - } - buf.append(closeBox); - - } else if (val instanceof Enumeration) { - buf.append(openBox); - Enumeration enm = (Enumeration)val; - for (int i = 0; enm.hasMoreElements(); i++) { - if (i > 0) { - buf.append(sep); - } - toString(buf, enm.nextElement(), openBox, closeBox); - } - buf.append(closeBox); - - } else if (val instanceof Point2D) { - Point2D p = (Point2D)val; - buf.append(openBox); - coordsToString(buf, (int)p.getX(), (int)p.getY()); - buf.append(closeBox); - - } else if (val instanceof Dimension2D) { - Dimension2D d = (Dimension2D)val; - buf.append(openBox); - buf.append(d.getWidth()).append("x").append(d.getHeight()); - buf.append(closeBox); - - } else if (val instanceof Rectangle2D) { - Rectangle2D r = (Rectangle2D)val; - buf.append(openBox); - buf.append(r.getWidth()).append("x").append(r.getHeight()); - coordsToString(buf, (int)r.getX(), (int)r.getY()); - buf.append(closeBox); - - } else { - buf.append(val); - } + toString(buf, val, openBox, closeBox, sep, true); } /** @@ -1378,6 +1253,154 @@ public class StringUtil return buf.toString(); } + /** + * Helper function for the various {@link #toString} methods. + */ + protected static void toString (StringBuilder buf, Object val, String openBox, String closeBox, + String sep, boolean traverseCollections) + { + if (val instanceof byte[]) { + buf.append(openBox); + byte[] v = (byte[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof short[]) { + buf.append(openBox); + short[] v = (short[])val; + for (short i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof int[]) { + buf.append(openBox); + int[] v = (int[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof long[]) { + buf.append(openBox); + long[] v = (long[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof float[]) { + buf.append(openBox); + float[] v = (float[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof double[]) { + buf.append(openBox); + double[] v = (double[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof Object[]) { + buf.append(openBox); + Object[] v = (Object[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + toString(buf, v[i], openBox, closeBox); + } + buf.append(closeBox); + + } else if (val instanceof boolean[]) { + buf.append(openBox); + boolean[] v = (boolean[])val; + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i] ? "t" : "f"); + } + buf.append(closeBox); + + } else if (val instanceof Point2D) { + Point2D p = (Point2D)val; + buf.append(openBox); + coordsToString(buf, (int)p.getX(), (int)p.getY()); + buf.append(closeBox); + + } else if (val instanceof Dimension2D) { + Dimension2D d = (Dimension2D)val; + buf.append(openBox); + buf.append(d.getWidth()).append("x").append(d.getHeight()); + buf.append(closeBox); + + } else if (val instanceof Rectangle2D) { + Rectangle2D r = (Rectangle2D)val; + buf.append(openBox); + buf.append(r.getWidth()).append("x").append(r.getHeight()); + coordsToString(buf, (int)r.getX(), (int)r.getY()); + buf.append(closeBox); + + } else if (traverseCollections) { + if (val instanceof Iterable) { + toString(buf, ((Iterable)val).iterator(), openBox, closeBox, sep, true); + + } else if (val instanceof Iterator) { + buf.append(openBox); + Iterator iter = (Iterator)val; + for (int i = 0; iter.hasNext(); i++) { + if (i > 0) { + buf.append(sep); + } + toString(buf, iter.next(), openBox, closeBox); + } + buf.append(closeBox); + + } else if (val instanceof Enumeration) { + buf.append(openBox); + Enumeration enm = (Enumeration)val; + for (int i = 0; enm.hasMoreElements(); i++) { + if (i > 0) { + buf.append(sep); + } + toString(buf, enm.nextElement(), openBox, closeBox); + } + buf.append(closeBox); + + } else { + buf.append(val); + } + + } else { + buf.append(val); + } + } + /** * Helper function for the various join methods. */ diff --git a/src/test/java/com/samskivert/util/ConfigTest.java b/src/test/java/com/samskivert/util/ConfigTest.java index 678b08f6..95ecbbff 100644 --- a/src/test/java/com/samskivert/util/ConfigTest.java +++ b/src/test/java/com/samskivert/util/ConfigTest.java @@ -56,12 +56,12 @@ public class ConfigTest slist.add(iter.nextElement().toString()); } Collections.sort(slist); - assertEquals("(sub1, sub2, sub3)", StringUtil.toString(slist)); + assertEquals("[sub1, sub2, sub3]", StringUtil.toString(slist)); // check the whole shebang List list = CollectionUtil.addAll(new ArrayList(), pconfig.keys()); Collections.sort(list); - assertEquals("(prop1, prop2, prop3, prop4, sub.sub1, sub.sub2, sub.sub3)", + assertEquals("[prop1, prop2, prop3, prop4, sub.sub1, sub.sub2, sub.sub3]", StringUtil.toString(list)); } } diff --git a/src/test/java/com/samskivert/util/HashIntMapTest.java b/src/test/java/com/samskivert/util/HashIntMapTest.java index aad02d9d..85a73e56 100644 --- a/src/test/java/com/samskivert/util/HashIntMapTest.java +++ b/src/test/java/com/samskivert/util/HashIntMapTest.java @@ -114,6 +114,6 @@ public class HashIntMapTest assertTrue(valuestr + ".equals(" + exvals + ")", valuestr.equals(exvals)); } - protected static final String TEST1 = "(10, 11, 12, 13, 14, 15, 16, 17, 18, 19)"; - protected static final String TEST2 = "(10, 11)"; + protected static final String TEST1 = "[10, 11, 12, 13, 14, 15, 16, 17, 18, 19]"; + protected static final String TEST2 = "[10, 11]"; } From 9344b51c4c41766cc8ca767177372ca7aeb3e2cd Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 1 Jun 2011 11:50:26 -0700 Subject: [PATCH 2/9] Update pom.xml with Github URLs. --- pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index b5b77ced..3cfe578e 100644 --- a/pom.xml +++ b/pom.xml @@ -35,9 +35,9 @@ - scm:svn:http://samskivert.googlecode.com/svn/trunk - scm:svn:https://samskivert.googlecode.com/svn/trunk - http://samskivert.googlecode.com/svn/trunk + scm:git:git://github.com/samskivert/samskivert.git + scm:git:git@github.com:samskivert/samskivert.git + http://github.com/samskivert/samskivert/ From 8c9c2a42b0fb1c33e93c9d9fc1e14434c674ad1a Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Wed, 1 Jun 2011 12:15:28 -0700 Subject: [PATCH 3/9] - Added missing handling for char[] in toString. - Fixed wacky use of short index variable in toString's short[] handling. - Ensured that custom separator and whether or not we're traversing collections is properly passed to recursive calls. --- .../java/com/samskivert/util/StringUtil.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/samskivert/util/StringUtil.java b/src/main/java/com/samskivert/util/StringUtil.java index 6ce021a4..a729b302 100644 --- a/src/main/java/com/samskivert/util/StringUtil.java +++ b/src/main/java/com/samskivert/util/StringUtil.java @@ -1273,7 +1273,18 @@ public class StringUtil } else if (val instanceof short[]) { buf.append(openBox); short[] v = (short[])val; - for (short i = 0; i < v.length; i++) { + for (int i = 0; i < v.length; i++) { + if (i > 0) { + buf.append(sep); + } + buf.append(v[i]); + } + buf.append(closeBox); + + } else if (val instanceof char[]) { + buf.append(openBox); + char[] v = (char[])val; + for (int i = 0; i < v.length; i++) { if (i > 0) { buf.append(sep); } @@ -1332,7 +1343,7 @@ public class StringUtil if (i > 0) { buf.append(sep); } - toString(buf, v[i], openBox, closeBox); + toString(buf, v[i], openBox, closeBox, sep, traverseCollections); } buf.append(closeBox); @@ -1377,7 +1388,7 @@ public class StringUtil if (i > 0) { buf.append(sep); } - toString(buf, iter.next(), openBox, closeBox); + toString(buf, iter.next(), openBox, closeBox, sep, true); } buf.append(closeBox); @@ -1388,7 +1399,7 @@ public class StringUtil if (i > 0) { buf.append(sep); } - toString(buf, enm.nextElement(), openBox, closeBox); + toString(buf, enm.nextElement(), openBox, closeBox, sep, true); } buf.append(closeBox); From e308ab9bf3fa6a8a7a4dc01bae1dd4d24fb9c36e Mon Sep 17 00:00:00 2001 From: David Hoover Date: Fri, 22 Apr 2011 16:38:52 -0700 Subject: [PATCH 4/9] Whitespace --- .../java/com/samskivert/net/HttpPostUtil.java | 20 +++----- .../com/samskivert/util/ServiceWaiter.java | 50 ++++++++----------- .../com/samskivert/util/ValueMarshaller.java | 7 ++- 3 files changed, 30 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/samskivert/net/HttpPostUtil.java b/src/main/java/com/samskivert/net/HttpPostUtil.java index 41c9b2cf..64d09fc5 100644 --- a/src/main/java/com/samskivert/net/HttpPostUtil.java +++ b/src/main/java/com/samskivert/net/HttpPostUtil.java @@ -21,17 +21,14 @@ import com.samskivert.util.ServiceWaiter; public class HttpPostUtil { /** - * Return the results of a form post. Note that the http request takes - * place on another thread, but this thread blocks until the results - * are returned or it times out. + * Return the results of a form post. Note that the http request takes place on another + * thread, but this thread blocks until the results are returned or it times out. * * @param url from which to make the request. * @param submission the entire submission eg "foo=bar&baz=boo&futz=foo". - * @param timeout time to wait for the response, in seconds, or -1 - * for forever. + * @param timeout time to wait for the response, in seconds, or -1 for forever. */ - public static String httpPost (final URL url, final String submission, - int timeout) + public static String httpPost (final URL url, final String submission, int timeout) throws IOException, ServiceWaiter.TimeoutException { final ServiceWaiter waiter = new ServiceWaiter( @@ -39,16 +36,13 @@ public class HttpPostUtil Thread tt = new Thread() { @Override public void run () { try { - HttpURLConnection conn = - (HttpURLConnection) url.openConnection(); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); conn.setDoInput(true); conn.setDoOutput(true); conn.setUseCaches(false); - conn.setRequestProperty( - "Content-Type", "application/x-www-form-urlencoded"); + conn.setRequestProperty("Content-Type", "application/x-www-form-urlencoded"); - DataOutputStream out = new DataOutputStream( - conn.getOutputStream()); + DataOutputStream out = new DataOutputStream(conn.getOutputStream()); out.writeBytes(submission); out.flush(); out.close(); diff --git a/src/main/java/com/samskivert/util/ServiceWaiter.java b/src/main/java/com/samskivert/util/ServiceWaiter.java index 6b5205cc..a2f7bf68 100644 --- a/src/main/java/com/samskivert/util/ServiceWaiter.java +++ b/src/main/java/com/samskivert/util/ServiceWaiter.java @@ -6,11 +6,10 @@ package com.samskivert.util; /** - * A handy base class for issuing server-side service requests and - * awaiting their responses from within a servlet. + * A handy base class for issuing server-side service requests and awaiting their responses from + * within a servlet. * - * Note: You might think that it would be keen to use this - * anonymously like so: + * Note: You might think that it would be keen to use this anonymously like so: * * 

  * ServiceWaiter waiter = new ServiceWaiter() {
@@ -21,24 +20,21 @@ package com.samskivert.util;
  * };
  *
* - * But that won't work because public methods in anonymous inner classes - * do not have public visibility and so the invocation manager will not be - * able to reflect your response methods when the time comes to deliver - * the response. Unfortunately, there appears to be no manner in which to - * instruct the Java compiler to give public scope to an anonymous inner - * class rather than default scope. Sigh. + * But that won't work because public methods in anonymous inner classes do not have public + * visibility and so the invocation manager will not be able to reflect your response methods + * when the time comes to deliver the response. Unfortunately, there appears to be no manner in + * which to instruct the Java compiler to give public scope to an anonymous inner class rather + * than default scope. Sigh. */ public class ServiceWaiter implements ResultListener { - /** Timeout to specify when you don't want a timeout. Use at your own - * risk. */ + /** Timeout to specify when you don't want a timeout. Use at your own risk. */ public static final int NO_TIMEOUT = -1; public static class TimeoutException extends Exception { - public TimeoutException () - { + public TimeoutException () { super("Timeout! Pow!"); } } @@ -62,8 +58,7 @@ public class ServiceWaiter } /** - * Change the timeout being used for this ServiceWaiter after it - * has been constructed. + * Change the timeout being used for this ServiceWaiter after it has been constructed. */ public void setTimeout (int timeout) { @@ -80,8 +75,8 @@ public class ServiceWaiter } /** - * Marks the request as successful and posts the supplied response - * argument for perusal by the caller. + * Marks the request as successful and posts the supplied response argument for perusal by the + * caller. */ public synchronized void postSuccess (T arg) { @@ -101,8 +96,7 @@ public class ServiceWaiter } /** - * Returns the argument posted by the waiter when the response - * arrived. + * Returns the argument posted by the waiter when the response arrived. */ public T getArgument () { @@ -120,8 +114,7 @@ public class ServiceWaiter /** * Blocks waiting for the response. * - * @return true if a success response was posted, false if a failure - * repsonse was posted. + * @return true if a success response was posted, false if a failure repsonse was posted. */ public boolean waitForResponse () throws TimeoutException @@ -136,8 +129,7 @@ public class ServiceWaiter } else { wait(1000L * _timeout); } - // if we get here without some sort of response, then - // we've timed out + // if we get here without some sort of response, then we've timed out if (_success == 0) { throw new TimeoutException(); } @@ -164,9 +156,8 @@ public class ServiceWaiter postFailure(cause); } - /** Whether or not the response succeeded; positive for success, - * negative for failure, zero means we haven't received the response - * yet. */ + /** Whether or not the response succeeded; positive for success, negative for failure, zero + * means we haven't received the response yet. */ protected int _success = 0; /** The argument posted by the waiter upon receipt of the response. */ @@ -178,8 +169,7 @@ public class ServiceWaiter /** How many seconds to wait before giving up the ghost. */ protected int _timeout; - /** If a response is not received within the specified timeout, an - * exception is thrown which redirects the user to an internal error - * page. */ + /** If a response is not received within the specified timeout, an exception is thrown which + * redirects the user to an internal error page. */ protected static final int DEFAULT_WAITER_TIMEOUT = 30; } diff --git a/src/main/java/com/samskivert/util/ValueMarshaller.java b/src/main/java/com/samskivert/util/ValueMarshaller.java index e5638800..99ea1745 100644 --- a/src/main/java/com/samskivert/util/ValueMarshaller.java +++ b/src/main/java/com/samskivert/util/ValueMarshaller.java @@ -36,9 +36,8 @@ public class ValueMarshaller // look up an argument parser for the field type Parser parser = _parsers.get(type); if (parser == null) { - String errmsg = "Don't know how to convert strings into " + - "values of type '" + type + "'."; - throw new Exception(errmsg); + throw new Exception( + "Don't know how to convert strings into values of type '" + type + "'."); } return parser.parse(source); } @@ -48,7 +47,7 @@ public class ValueMarshaller public Object parse (String source) throws Exception; } - protected static Map,Parser> _parsers = new HashMap,Parser>(); + protected static Map, Parser> _parsers = new HashMap, Parser>(); static { Parser p; // we can parse strings From af3c9e88f02c2c3477c9c39848f267b29101b7af Mon Sep 17 00:00:00 2001 From: David Hoover Date: Fri, 22 Apr 2011 16:39:01 -0700 Subject: [PATCH 5/9] Speeling --- src/main/java/com/samskivert/util/ServiceWaiter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/samskivert/util/ServiceWaiter.java b/src/main/java/com/samskivert/util/ServiceWaiter.java index a2f7bf68..e08872bc 100644 --- a/src/main/java/com/samskivert/util/ServiceWaiter.java +++ b/src/main/java/com/samskivert/util/ServiceWaiter.java @@ -114,7 +114,7 @@ public class ServiceWaiter /** * Blocks waiting for the response. * - * @return true if a success response was posted, false if a failure repsonse was posted. + * @return true if a success response was posted, false if a failure response was posted. */ public boolean waitForResponse () throws TimeoutException From d63f147caecb22d79f87133beba69a77e50285ab Mon Sep 17 00:00:00 2001 From: David Hoover Date: Thu, 2 Jun 2011 09:16:10 -0700 Subject: [PATCH 6/9] Unused import --- src/main/java/com/samskivert/util/ArrayUtil.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/samskivert/util/ArrayUtil.java b/src/main/java/com/samskivert/util/ArrayUtil.java index 7db9d5d7..065e04a6 100644 --- a/src/main/java/com/samskivert/util/ArrayUtil.java +++ b/src/main/java/com/samskivert/util/ArrayUtil.java @@ -7,7 +7,6 @@ package com.samskivert.util; import java.lang.reflect.Array; -import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.Random; From 4a53f9c29be568e4438a69e5ef0b31a46986adb1 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Fri, 17 Jun 2011 12:22:48 -0700 Subject: [PATCH 7/9] Revamped Interval to force the creator to explicitly indicate that they want to run on the interval timer thread, rather than allowing that to be the default behavior if they forget to supply a RunQueue to the constructor. Running on the interval timer thread is only safe if you know your interval will complete very quickly, because you'll delay the firing of all other intervals until your interval finishes. This is almost never what you want. --- .../java/com/samskivert/util/Interval.java | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/main/java/com/samskivert/util/Interval.java b/src/main/java/com/samskivert/util/Interval.java index 2fa03991..247a68e1 100644 --- a/src/main/java/com/samskivert/util/Interval.java +++ b/src/main/java/com/samskivert/util/Interval.java @@ -18,6 +18,25 @@ import static com.samskivert.Log.log; */ public abstract class Interval { + /** A marker {@link RunQueue} to supply when you intend for your interval to run directly on + * the interval timer thread, rather than having the interval processed on a separate thread. + * Warning: you must be absolutely sure your interval will complete very quickly, + * otherwise you risk delaying the firing of other intervals. */ + public static RunQueue RUN_DIRECT = new RunQueue() { + public void postRunnable (Runnable r) { + throw new UnsupportedOperationException("dummy"); + } + public boolean isDispatchThread () { + throw new UnsupportedOperationException("dummy"); + } + public boolean isRunning () { + throw new UnsupportedOperationException("dummy"); + } + public String toString () { + return ""; + } + }; + /** * An interface for entities that create, and keep track of, intervals. The intended use case * is for repeating intervals to be created via a factory that tracks all such intervals, and @@ -54,21 +73,6 @@ public abstract class Interval public String getIntervalClassName (); } - /** - * Creates an interval that executes the supplied runnable when it expires. - */ - public static Interval create (final Runnable onExpired) - { - return new Interval() { - @Override public void expired () { - onExpired.run(); - } - @Override public String toString () { - return onExpired.toString(); - } - }; - } - /** * Creates an interval that executes the supplied runnable on the specified RunQueue when it * expires. @@ -88,12 +92,9 @@ public abstract class Interval }; } - /** - * Create a simple interval that does not use a RunQueue to run the {@link #expired} method. - */ - public Interval () - { - // _runQueue stays null + /** @deprecated If direct-running is desired, pass {@link #RUN_DIRECT} explicitly. */ + @Deprecated public Interval () { + this(RUN_DIRECT); } /** @@ -102,16 +103,14 @@ public abstract class Interval */ public Interval (RunQueue runQueue) { - setRunQueue(runQueue); + if (runQueue == null) { + throw new NullPointerException("RunQueue must be non-null"); + } + _runQueue = runQueue; } - /** - * Configures the run queue to be used by this interval. This must be called before - * the interval is started and a non-null queue must be provided. This exists for situations - * where the caller needs to configure an optional run queue and thus can't easily call the - * appropriate constructor. - */ - public void setRunQueue (RunQueue runQueue) + /** @deprecated Just pass the desired run-queue to the constructor. */ + @Deprecated public void setRunQueue (RunQueue runQueue) { if (runQueue == null) { throw new IllegalArgumentException("Supplied RunQueue must be non-null"); @@ -227,7 +226,7 @@ public abstract class Interval _timer.schedule(_task, initialDelay); } else if (fixedRate) { _timer.scheduleAtFixedRate(_task, initialDelay, repeatDelay); - } else if (_runQueue != null) { + } else if (_runQueue != RUN_DIRECT) { throw new IllegalArgumentException( "Cannot schedule at a fixed delay when using a RunQueue."); } else { @@ -265,7 +264,7 @@ public abstract class Interval protected void noteRejected () { log.warning("Interval posted to shutdown RunQueue. Cancelling.", - "queue", _runQueue, "interval", this); + "queue", _runQueue, "interval", this); } protected static Timer createTimer () @@ -297,7 +296,7 @@ public abstract class Interval if (ival == null) { return; } - if (ival._runQueue == null) { + if (ival._runQueue == RUN_DIRECT) { ival.safelyExpire(this); return; } @@ -328,7 +327,7 @@ public abstract class Interval ival._runQueue.postRunnable(_runner); } catch (Exception e) { log.warning("Failed to execute interval on run-queue", - "queue", ival._runQueue, "interval", ival, e); + "queue", ival._runQueue, "interval", ival, e); } } else { @@ -352,7 +351,8 @@ public abstract class Interval } // end: static class IntervalTask - /** If non-null, the RunQueue used to run the expired() method for each Interval. */ + /** The RunQueue used to run the expired() method for this Interval, or {@link #RUN_DIRECT} to + * indicate that the interval should be executed directly on the Inteval timer thread. */ protected RunQueue _runQueue; /** The task that actually schedules our execution with the static Timer. */ From 85efd40adfd7572894c186c3e3c0ae541bd58631 Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Fri, 17 Jun 2011 12:23:09 -0700 Subject: [PATCH 8/9] Better interval behavior. If no runqueue is supplied on which to prune sessions, we don't schedule a session pruner, rather than scheduling a long running database action on the Interval timer thread, thereby booching all other Intervals. --- .../samskivert/servlet/user/UserManager.java | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/main/java/com/samskivert/servlet/user/UserManager.java b/src/main/java/com/samskivert/servlet/user/UserManager.java index b4a1fd2f..2e419eeb 100644 --- a/src/main/java/com/samskivert/servlet/user/UserManager.java +++ b/src/main/java/com/samskivert/servlet/user/UserManager.java @@ -124,25 +124,26 @@ public class UserManager } // register a cron job to prune the session table every hour - _pruner = new Interval() { - @Override public void expired () { - try { - _repository.pruneSessions(); - } catch (PersistenceException pe) { - log.warning("Error pruning session table.", pe); - } - } - }; if (pruneQueue != null) { - _pruner.setRunQueue(pruneQueue); + _pruner = new Interval(pruneQueue) { + @Override public void expired () { + try { + _repository.pruneSessions(); + } catch (PersistenceException pe) { + log.warning("Error pruning session table.", pe); + } + } + }; + _pruner.schedule(SESSION_PRUNE_INTERVAL, true); } - _pruner.schedule(SESSION_PRUNE_INTERVAL, true); } public void shutdown () { // cancel our session table pruning thread - _pruner.cancel(); + if (_pruner != null) { + _pruner.cancel(); + } } /** From 402cdfdc298d3f81a8216ada8fa58c06666f49fc Mon Sep 17 00:00:00 2001 From: Michael Bayne Date: Fri, 17 Jun 2011 12:24:24 -0700 Subject: [PATCH 9/9] Be explicit about our Interval usage. In AuditLogger, we roll over the audit log on the Interval timer thread, which is dubious, because it could block, but introducing an Invoker thread here would require a ticket on a boat that sailed a long time ago. In the case of SerialExecutor, we're just doing Thread.kill() on the Interval timer thread, so that's fast enough. --- src/main/java/com/samskivert/util/AuditLogger.java | 2 +- src/main/java/com/samskivert/util/SerialExecutor.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/samskivert/util/AuditLogger.java b/src/main/java/com/samskivert/util/AuditLogger.java index e469df58..684e1df4 100644 --- a/src/main/java/com/samskivert/util/AuditLogger.java +++ b/src/main/java/com/samskivert/util/AuditLogger.java @@ -166,7 +166,7 @@ public class AuditLogger } /** The interval that rolls over the log file. */ - protected Interval _rollover = new Interval() { + protected Interval _rollover = new Interval(Interval.RUN_DIRECT) { @Override public void expired () { checkRollOver(); } diff --git a/src/main/java/com/samskivert/util/SerialExecutor.java b/src/main/java/com/samskivert/util/SerialExecutor.java index 7045d48f..58881d64 100644 --- a/src/main/java/com/samskivert/util/SerialExecutor.java +++ b/src/main/java/com/samskivert/util/SerialExecutor.java @@ -135,9 +135,8 @@ public class SerialExecutor final ExecutorThread thread = new ExecutorThread(task); thread.start(); - // start up a timer that will abort this thread after the specified - // timeout - new Interval() { + // start up a timer that will abort this thread after the specified timeout + new Interval(Interval.RUN_DIRECT) { @Override public void expired () { // this will NOOP if the task has already completed thread.abort();