Author: dblevins
Date: Tue Aug 11 06:43:22 2009
New Revision: 803009
URL: http://svn.apache.org/viewvc?rev=803009&view=rev
Log:
Improved Options support so that all properties that accept a Set of enum
values now automatically accept ALL and NONE to have an easy way for users to
imply those values without us having to add NONE or ALL enum items explicitly
which leads to strange code.
Additionally TRUE is an alias for ALL and FALSE an alias for NONE. This allows
options that used to support only true/false values to be further defined in
the future without breaking compatibility.
Modified:
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/util/OptionsTest.java
openejb/trunk/openejb3/container/openejb-loader/src/main/java/org/apache/openejb/loader/Options.java
Modified:
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/util/OptionsTest.java
URL:
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/util/OptionsTest.java?rev=803009&r1=803008&r2=803009&view=diff
==============================================================================
---
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/util/OptionsTest.java
(original)
+++
openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/util/OptionsTest.java
Tue Aug 11 06:43:22 2009
@@ -19,6 +19,9 @@
import junit.framework.TestCase;
import java.util.Properties;
+import java.util.Set;
+import java.util.ArrayList;
+import java.util.List;
import org.apache.openejb.loader.Options;
@@ -26,20 +29,255 @@
* @version $Rev$ $Date$
*/
public class OptionsTest extends TestCase {
+ private Properties properties;
+ private Options options;
+ private TestLog log;
- public void testEnum() throws Exception {
- Properties properties = new Properties();
+ public void testEnumCase() throws Exception {
properties.setProperty("caseSensitive", Colors.RED.toString());
properties.setProperty("caseInsensitive", "blue");
- Options options = new Options(properties);
-
assertSame(Colors.RED, options.get("caseSensitive", Colors.GREEN));
assertSame(Colors.BLUE, options.get("caseInsensitive", Colors.GREEN));
assertSame(Colors.GREEN, options.get("default", Colors.GREEN));
}
+ public void testNoneNone() throws Exception {
+ // User specified NONE
+ String userValue = "NONE";
+
+ properties.setProperty("colors", userValue);
+
+ // Default is NONE
+ Set<Colors> colors = options.getAll("colors", Colors.class);
+
+ assertNotNull(colors);
+ assertEquals("size", 0, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors="+userValue);
+ }
+
+ public void testNoneAll() throws Exception {
+ // User specified ALL
+ String userValue = "ALL";
+ properties.setProperty("colors", userValue);
+
+ // Default is NONE
+ Set<Colors> colors = options.getAll("colors", Colors.class);
+
+ assertNotNull(colors);
+ assertEquals("size", Colors.values().length, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors="+userValue);
+ }
+
+ public void testNoneSome() throws Exception {
+ // User specified RED
+ String userValue = "red";
+ properties.setProperty("colors", userValue);
+
+ // Default is NONE
+ Set<Colors> colors = options.getAll("colors", Colors.class);
+
+ assertNotNull(colors);
+ assertEquals("size", 1, colors.size());
+ assertEquals("size", Colors.RED, colors.iterator().next());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors=red");
+ }
+
+ public void testNoneDefault() throws Exception {
+ // User specified nothing
+
+ // Default is NONE
+ Set<Colors> colors = options.getAll("colors", Colors.class);
+
+ assertNotNull(colors);
+ assertEquals("size", 0, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Debug.class,
log.messages.get(0).getClass());
+
+ String message = log.messages.get(0).message;
+
+
+ assertContains(message, "colors=NONE");
+ assertContains(message, "Possible values");
+
+ message = message.substring(message.indexOf("Possible values"));
+
+ for (Colors color : colors) {
+ assertContains(message, color.name().toLowerCase());
+ }
+
+ assertContains(message, "NONE");
+ assertContains(message, "ALL");
+ }
+
+ public void testAllAll() throws Exception {
+ // User specified ALL
+ String userValue = "ALL";
+
+ properties.setProperty("colors", userValue);
+
+ // Default is ALL
+ Set<Colors> colors = options.getAll("colors", Colors.values());
+
+ assertNotNull(colors);
+ assertEquals("size", Colors.values().length, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors="+userValue);
+ }
+
+ public void testAllNone() throws Exception {
+ // User specified NONE
+ String userValue = "NONE";
+ properties.setProperty("colors", userValue);
+
+ // Default is ALL
+ Set<Colors> colors = options.getAll("colors", Colors.values());
+
+ assertNotNull(colors);
+ assertEquals("size", 0, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors="+userValue);
+ }
+
+ public void testAllSome() throws Exception {
+ // User specified NONE
+ String userValue = "red";
+ properties.setProperty("colors", userValue);
+
+ // Default is ALL
+ Set<Colors> colors = options.getAll("colors", Colors.values());
+
+ assertNotNull(colors);
+ assertEquals("size", 1, colors.size());
+ assertEquals("size", Colors.RED, colors.iterator().next());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Info.class,
log.messages.get(0).getClass());
+ assertContains(log.messages.get(0).message, "colors=red");
+ }
+
+ public void testAllDefault() throws Exception {
+ // User specified nothing
+
+ // Default is ALL
+ Set<Colors> colors = options.getAll("colors", Colors.values());
+
+ assertNotNull(colors);
+ assertEquals("size", Colors.values().length, colors.size());
+
+ assertEquals("messages.size", 1, log.messages.size());
+ assertEquals("messages level", Debug.class,
log.messages.get(0).getClass());
+
+ String message = log.messages.get(0).message;
+
+
+ assertContains(message, "colors=ALL");
+ assertContains(message, "Possible values");
+
+ message = message.substring(message.indexOf("Possible values"));
+
+ for (Colors color : colors) {
+ assertContains(message, color.name().toLowerCase());
+ }
+
+ assertContains(message, "ALL");
+ assertContains(message, "NONE");
+ }
+
+ private void assertContains(String message, String expected) {
+ assertTrue("Expected ["+expected+"], actual ["+message+"]",
message.contains(expected));
+ }
+
+ public void setUp() {
+ properties = new Properties();
+ options = new Options(properties);
+ log = new TestLog();
+ options.setLogger(log);
+ }
+
public static enum Colors {
RED, GREEN, BLUE;
}
+
+ // Just to test if the logging works as expected
+ private static class TestLog implements Options.Log {
+ private final List<Message> messages = new ArrayList<Message>();
+
+ public boolean isDebugEnabled() {
+ return isInfoEnabled();
+ }
+
+ public boolean isInfoEnabled() {
+ return isWarningEnabled();
+ }
+
+ public boolean isWarningEnabled() {
+ return true;
+ }
+
+ public void warning(String message, Throwable t) {
+ messages.add(new Warning(message));
+ }
+
+ public void warning(String message) {
+ messages.add(new Warning(message));
+ }
+
+ public void info(String message, Throwable t) {
+ messages.add(new Info(message));
+ }
+
+ public void info(String message) {
+ messages.add(new Info(message));
+ }
+
+ public void debug(String message, Throwable t) {
+ messages.add(new Debug(message));
+ }
+
+ public void debug(String message) {
+ messages.add(new Debug(message));
+ }
+
+ }
+
+ private abstract static class Message {
+ private final String message;
+
+ private Message(String message) {
+ this.message = message;
+ }
+ }
+
+ private static class Debug extends Message {
+ private Debug(String message) {
+ super(message);
+ }
+ }
+
+ private static class Info extends Message {
+ private Info(String message) {
+ super(message);
+ }
+ }
+
+ private static class Warning extends Message {
+ private Warning(String message) {
+ super(message);
+ }
+ }
}
Modified:
openejb/trunk/openejb3/container/openejb-loader/src/main/java/org/apache/openejb/loader/Options.java
URL:
http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-loader/src/main/java/org/apache/openejb/loader/Options.java?rev=803009&r1=803008&r2=803009&view=diff
==============================================================================
---
openejb/trunk/openejb3/container/openejb-loader/src/main/java/org/apache/openejb/loader/Options.java
(original)
+++
openejb/trunk/openejb3/container/openejb-loader/src/main/java/org/apache/openejb/loader/Options.java
Tue Aug 11 06:43:22 2009
@@ -24,8 +24,7 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
-import java.util.logging.Logger;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.Collections;
/**
* The purpose of this class is to provide a more strongly typed version of a
@@ -44,6 +43,29 @@
* - When a property is found: the property name and value. Info level.
* - When a property value cannot be parsed: the property name and invalid
value. Warn level.
*
+ * Logging the user supplied values onto INFO is really nice as it shows up in
the standard
+ * log output and allows us to easily see which values the user has changed
from the default.
+ * It's rather impossible to diagnose issues without this information.
+ *
+ * ENUM SETS:
+ *
+ * Properties that accept a Set of enum values automatically accept ALL and
NONE in
+ * addition to the explicitly created enum items.
+ *
+ * Using ALL. This allows users to have an easy way to imply "all" without
having to
+ * hardcode an the entire list of enum items and protects against the case
where that
+ * list may grow in the future.
+ *
+ * Using NONE. This allows users an alternative to using an empty string when
explicitly
+ * specifying that none of the options should be used.
+ *
+ * In the internal code, this allows us to have these concepts in all enum
options
+ * without us having to add NONE or ALL enum items explicitly which leads to
strange code.
+ *
+ * Additionally TRUE is an alias for ALL and FALSE an alias for NONE. This
allows options
+ * that used to support only true/false values to be further defined in the
future without
+ * breaking compatibility.
+ *
* @version $Rev$ $Date$
*/
public class Options {
@@ -150,15 +172,10 @@
public <T extends Enum<T>> Set<T> getAll(String property, T...
defaultValue) {
EnumSet<T> defaults = EnumSet.copyOf(Arrays.asList(defaultValue));
- return get(property, defaults);
+ return getAll(property, defaults);
}
- public <T extends Enum<T>> Set<T> get(String property, Set<T>
defaultValue) {
-
- String value = properties.getProperty(property);
-
- if (value == null) return parent.get(property, (Set) defaultValue);
-
+ public <T extends Enum<T>> Set<T> getAll(String property, Set<T>
defaultValue) {
Class<T> enumType;
try {
T t = defaultValue.iterator().next();
@@ -167,6 +184,28 @@
throw new IllegalArgumentException("Must supply a default for
property " + property);
}
+ return getAll(property, defaultValue, enumType);
+ }
+
+ public <T extends Enum<T>> Set<T> getAll(String property, Class<T>
enumType) {
+ return getAll(property, Collections.EMPTY_SET, enumType);
+ }
+
+ protected <T extends Enum<T>> Set<T> getAll(String property, Set<T>
defaultValue, Class<T> enumType) {
+ String value = properties.getProperty(property);
+
+ if (value == null) return parent.getAll(property, defaultValue,
enumType);
+
+ // Shorthand for specifying ALL or NONE for any option
+ // that allows for multiple values of the enum
+ if ("all".equalsIgnoreCase(value) || "true".equalsIgnoreCase(value)) {
+ log(property, value);
+ return EnumSet.allOf(enumType);
+ } else if ("none".equalsIgnoreCase(value) ||
"false".equalsIgnoreCase(value)) {
+ log(property, value);
+ return EnumSet.noneOf(enumType);
+ }
+
try {
String[] values = value.split(",");
EnumSet<T> set = EnumSet.noneOf(enumType);
@@ -175,10 +214,10 @@
s = s.trim();
set.add(valueOf(enumType, s.toUpperCase()));
}
- return set;
+ return logAll(property, set);
} catch (IllegalArgumentException e) {
warn(property, value);
- return parent.get(property, (Set) defaultValue);
+ return parent.getAll(property, defaultValue, enumType);
}
}
@@ -214,6 +253,8 @@
}
private <V> V log(String property, V value) {
+ if (!getLogger().isInfoEnabled()) return value;
+
if (value instanceof Class) {
Class clazz = (Class) value;
getLogger().info("Using \'" + property + "=" + clazz.getName() +
"\'");
@@ -223,6 +264,14 @@
return value;
}
+ public <T extends Enum<T>> Set<T> logAll(String property, Set<T> value) {
+ if (!getLogger().isInfoEnabled()) return value;
+
+ getLogger().info("Using \'" + property + "=" + join(", ",
lowercase(value)) + "\'");
+
+ return value;
+ }
+
protected static <T extends Enum<T>> String[] lowercase(T... items) {
String[] values = new String[items.length];
@@ -310,16 +359,21 @@
}
@Override
- public <T extends Enum<T>> Set<T> get(String property, Set<T>
defaults) {
+ protected <T extends Enum<T>> Set<T> getAll(String property, Set<T>
defaults, Class<T> enumType) {
if (getLogger().isDebugEnabled()) {
- Iterator<T> iterator = defaults.iterator();
- String possibleValues = "";
- if (iterator.hasNext()) {
- T v = iterator.next();
- possibleValues = " Possible values are: " +
possibleValues(v);
- }
+ String possibleValues = " Possible values are: " +
possibleValues(enumType);
- String defaultValues = join(", ", lowercase(defaults));
+ possibleValues += " or NONE or ALL";
+
+ String defaultValues;
+
+ if (defaults.size() == 0) {
+ defaultValues = "NONE";
+ } else if (defaults.size() ==
enumType.getEnumConstants().length) {
+ defaultValues = "ALL";
+ } else {
+ defaultValues = join(", ", lowercase(defaults));
+ }
getLogger().debug("Using default \'" + property + "=" +
defaultValues + "\'" + possibleValues);
}
