Support expressions in ContainerConfig

[awiner, 2009-01-22 01:11:50]

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

[etnu00, 2009-01-22 09:53:59]

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() {
On 2009/01/22 01:11:50, awiner wrote:
> an abstract class might as well just have abstract methods, instead of
> UnsupportedOperationExceptions

There are a lot of tests that rely on this, but don't actually care about these
methods. A testing variant might work here.

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
On 2009/01/22 01:11:50, awiner wrote:
> Java 5, shouldn't use @Override for implements

This isn't @Override for implements, it's @Override for replacing Object's
toString.

http://codereview.appspot.com/11953/diff/1/11#newcode44
Line 44: Expression<String> expression = Expressions.parse(value, String.class);
On 2009/01/22 01:11:50, awiner wrote:
> Expressions are immutable.  Parse on creation.

Done.

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: 
On 2009/01/22 01:11:50, awiner wrote:
> 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.

See javadoc -- this is to support nested property access. For instance, if I
want to get the property "views" under "gadgets.features", I can access it via
expressions (${gadgets\.features.views}, which is nice and consistent. This
replaces the old ad-hoc query system that used / as a nesting separator and
wasn't nearly as flexible.