Move protocol handling to common

[ianboston, 2009-02-09 23:48:03]

Some comments, by no means a complete set. 
I am wondering about the json-lib configuration and if its still used. If not,
then it should be removed from the code base. If it is, then IMHO the binding to
model.Enum should be expressed in such a way as not to pull Enum into common.
Although Enum itself is a simple and almost empty concept, I think its not a
good idea to split the Opensocial model between jars (IMHO). It might be
possible to use the same approach as has been used with the xstream
configuration, where the configuration is inside the social jar, avoiding
splitting the model to undo a cyclic binding. If xstream isn't used any more,
that should also be removed entirely.

http://codereview.appspot.com/14066/diff/1/117
File
java/common/src/main/java/org/apache/shindig/protocol/conversion/jsonlib/BaseJsonLibConfig.java
(right):

http://codereview.appspot.com/14066/diff/1/117#newcode67
Line 67: morpherRegistry.registerMorpher(new
EnumMorpher(org.apache.shindig.protocol.model.Enum.Field.class));
I think this creates a cyclic binding between social and common, is that a good
idea ?

http://codereview.appspot.com/14066/diff/1/16
File java/common/src/main/java/org/apache/shindig/protocol/model/Enum.java
(right):

http://codereview.appspot.com/14066/diff/1/16#newcode22
Line 22: *
http://www.opensocial.org/Technical-Resources/opensocial-spec-v081/opensocial...
Strictly speaking this is part of the OpenSocial API and not part of common.
Although the contents of Enum is small and fairly non specific.

[louiscryan, 2009-02-10 00:47:31]

I definitely agree that xstream and json-lib should be removed if not in use
(though I believe that xstream is being used for atom support currently??) 

I'd also be fine moving Enum back into social though it requires some work in
the existing BeanConverters Id rather avoid in this CL.

http://codereview.appspot.com/14066/diff/1/117
File
java/common/src/main/java/org/apache/shindig/protocol/conversion/jsonlib/BaseJsonLibConfig.java
(right):

http://codereview.appspot.com/14066/diff/1/117#newcode67
Line 67: morpherRegistry.registerMorpher(new
EnumMorpher(org.apache.shindig.protocol.model.Enum.Field.class));
On 2009/02/09 23:48:03, ianboston wrote:
> I think this creates a cyclic binding between social and common, is that a
good
> idea ?

I wouldnt call it cyclic but I agree its not exactly ideal either. Multi-binding
in Guice 2 would solve all ills here but until then....

http://codereview.appspot.com/14066/diff/1/16
File java/common/src/main/java/org/apache/shindig/protocol/model/Enum.java
(right):

http://codereview.appspot.com/14066/diff/1/16#newcode22
Line 22: *
http://www.opensocial.org/Technical-Resources/opensocial-spec-v081/opensocial...
On 2009/02/09 23:48:03, ianboston wrote:
> Strictly speaking this is part of the OpenSocial API and not part of common.
> Although the contents of Enum is small and fairly non specific.

Mostly agreed though the notion encapsulated here isnt specific to opensocial
even though all its current uses are. It needs to remain in common because of
how our hard-coded bean conversion works. Im sure that can be fixed later I just
dont want to overload this excessively.

[awiner, 2009-02-10 00:58:02]

Started to review... but ouch... Rietveld doesn't show moves as such.  I am
defeated.

http://codereview.appspot.com/14066/diff/1/51
File java/common/pom.xml (right):

http://codereview.appspot.com/14066/diff/1/51#newcode110
Line 110: <version>2.2</version>
versions are usually specified in the root pom.xml (otherwise you run the risk
of multiple versions)

http://codereview.appspot.com/14066/diff/1/69
File
java/common/src/main/java/org/apache/shindig/protocol/model/Exportablebean.java
(right):

http://codereview.appspot.com/14066/diff/1/69#newcode32
Line 32: public @interface Exportablebean {}
ExportableBean, while you're moving

[thesmith, 2009-02-10 09:02:53]

Looks good. Have you run tried building the samples project (mvn clean install
-Pall,samples)? If you let me know which revision these patches were against
then I'll have a go for you.