http://codereview.appspot.com/11953/diff/1/15 File java/common/src/main/java/org/apache/shindig/config/AbstractContainerConfig.java (right):
http://codereview.appspot.com/11953/diff/1/15#newcode71 Line 71: public Collection<String> getContainers() { an abstract class might as well just have abstract methods, instead of UnsupportedOperationExceptions http://codereview.appspot.com/11953/diff/1/11 File java/common/src/main/java/org/apache/shindig/config/DynamicConfigProperty.java (right): http://codereview.appspot.com/11953/diff/1/11#newcode41 Line 41: @Override Java 5, shouldn't use @Override for implements http://codereview.appspot.com/11953/diff/1/11#newcode44 Line 44: Expression<String> expression = Expressions.parse(value, String.class); Expressions are immutable. Parse on creation. http://codereview.appspot.com/11953/diff/1/12 File java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java (left): http://codereview.appspot.com/11953/diff/1/12#oldcode113 Line 113: space after ) http://codereview.appspot.com/11953/diff/1/12 File java/common/src/main/java/org/apache/shindig/config/JsonContainerConfig.java (right): http://codereview.appspot.com/11953/diff/1/12#newcode93 Line 93: properties often won't start with ${} - they may be concatenations. But why do we need to support dynamic property names on lookup? The expression evaluation belongs on config.get(...).get(property), not here. http://codereview.appspot.com/11953/diff/1/12#newcode131 Line 131: json instanceof CharSequence on the prior line, but here casting to String. Change to json.toString() http://codereview.appspot.com/11953/diff/1/12#newcode140 Line 140: extract this into a separate method, and call it from createContainers() instead of jsonToConfig() - avoids the need for @suppressWarnings http://codereview.appspot.com/11953
