google-http-client-gson & google-http-client-jackson

Splits off the gson and jackson parts of http-client project into http-client-gson and http-client-jackson.

Created to fix the feature request http://code.google.com/p/google-http-java-client/issues/detail?id=103 : google-http-client-gson and google-http-client-jackson

[yanivi, 2012-07-30 19:30:10]

Note to self: extra commands to run after hg import:

hg cp
google-http-client/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
hg mv google-http-client/src/main/java/com/google/api/client/json/jackson 
google-http-client-jackson/src/main/java/com/google/api/client/json/jackson
hg mv google-http-client/src/main/java/com/google/api/client/json/gson 
google-http-client-gson/src/main/java/com/google/api/client/json/gson
hg mv google-http-client/src/test/java/com/google/api/client/json/gson
google-http-client-gson/src/test/java/com/google/api/client/json/gson
hg mv google-http-client/src/test/java/com/google/api/client/json/jackson
google-http-client-jackson/src/test/java/com/google/api/client/json/jackson
hg mv
google-http-client/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:15:
package com.google.api.client.json;
[optional] move AbstractJsonFactoryTest.java into the src folder of a new
google-http-client-test project to avoid making duplicate copies.

Note that I would still keep src package
com.google.api.client.testing.http in google-http-client because that's really a
test support library, whereas AbstractJsonFactoryTest is actual literal tests.

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:49:
/**
Please add a comment that there is a duplicate of this file in
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
and if you update this one you must also update the other.

please also add a similar comment in the jackson one.

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/GenericJsonTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/GenericJsonTest.java:26:
public class GenericJsonTest extends TestCase {
these tests belong in GsonFactoryTest
similarly for JsonParserTest

same comments for GenericJsonTest and JsonParserTest in  jackson tests

http://codereview.appspot.com/6408051/diff/8001/google-http-client-jackson/sr...
File
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(left):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-jackson/sr...
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:1134:
public void testObjectParserParse_entry() throws Exception {
Please don't remove these tests.  Instead, add a dependency on GSON library that
has <scope>test</scope>.  Then, add a TODO to instead use TypeToken from Guava
r12 once we've upgraded when/if they ever decide to backport a Java 5 compatible
flavor of Guava.

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java:32:
* Implementation is not thread-safe.
it is thread-safe, and in fact it must be

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java:42:
return null;
please also make MockJsonParser

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java:30:
* Implementation is not thread-safe.
it is thread-safe

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java:40:
return null;
this should not be null and instead be the instance of MockJsonFactory that
created this

http://codereview.appspot.com/6408051/diff/8001/pom.xml
File pom.xml (right):

http://codereview.appspot.com/6408051/diff/8001/pom.xml#newcode55
pom.xml:55: -->
Please update this comment with the most up-to-date accurate information.  It
should mention at least:
* release.html
* google-http-client-assembly/readme.html
* google-http-client-assembly/.classpath
* google-http-client-assembly/dependencies/*-dependencies.html

[rmistry, 2012-08-01 14:35:01]

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:15:
package com.google.api.client.json;
On 2012/07/30 19:30:10, yanivi wrote:
> [optional] move AbstractJsonFactoryTest.java into the src folder of a new
> google-http-client-test project to avoid making duplicate copies.
> 
> Note that I would still keep src package
> com.google.api.client.testing.http in google-http-client because that's really
a
> test support library, whereas AbstractJsonFactoryTest is actual literal tests.

I started off with this approach but ran into problems getting the dependencies
right, I will stick with the current approach since it is optional.

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:49:
/**
On 2012/07/30 19:30:10, yanivi wrote:
> Please add a comment that there is a duplicate of this file in
>
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
> and if you update this one you must also update the other.
> 
> please also add a similar comment in the jackson one.

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/GenericJsonTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/GenericJsonTest.java:26:
public class GenericJsonTest extends TestCase {
On 2012/07/30 19:30:10, yanivi wrote:
> these tests belong in GsonFactoryTest
> similarly for JsonParserTest
> 
> same comments for GenericJsonTest and JsonParserTest in  jackson tests

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client-jackson/sr...
File
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(left):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-jackson/sr...
google-http-client-jackson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:1134:
public void testObjectParserParse_entry() throws Exception {
On 2012/07/30 19:30:10, yanivi wrote:
> Please don't remove these tests.  Instead, add a dependency on GSON library
that
> has <scope>test</scope>.  Then, add a TODO to instead use TypeToken from Guava
> r12 once we've upgraded when/if they ever decide to backport a Java 5
compatible
> flavor of Guava.

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java:32:
* Implementation is not thread-safe.
On 2012/07/30 19:30:10, yanivi wrote:
> it is thread-safe, and in fact it must be

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonFactory.java:42:
return null;
On 2012/07/30 19:30:10, yanivi wrote:
> please also make MockJsonParser

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java:30:
* Implementation is not thread-safe.
On 2012/07/30 19:30:10, yanivi wrote:
> it is thread-safe

Done.

http://codereview.appspot.com/6408051/diff/8001/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonGenerator.java:40:
return null;
On 2012/07/30 19:30:10, yanivi wrote:
> this should not be null and instead be the instance of MockJsonFactory that
> created this

Done.

http://codereview.appspot.com/6408051/diff/8001/pom.xml
File pom.xml (right):

http://codereview.appspot.com/6408051/diff/8001/pom.xml#newcode55
pom.xml:55: -->
On 2012/07/30 19:30:10, yanivi wrote:
> Please update this comment with the most up-to-date accurate information.  It
> should mention at least:
> * release.html
> * google-http-client-assembly/readme.html
> * google-http-client-assembly/.classpath
> * google-http-client-assembly/dependencies/*-dependencies.html

Done.

[yanivi, 2012-08-01 16:03:58]

LGTM

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:15:
package com.google.api.client.json;
On 2012/08/01 14:35:01, rmistry wrote:
> On 2012/07/30 19:30:10, yanivi wrote:
> > [optional] move AbstractJsonFactoryTest.java into the src folder of a new
> > google-http-client-test project to avoid making duplicate copies.
> > 
> > Note that I would still keep src package
> > com.google.api.client.testing.http in google-http-client because that's
really
> a
> > test support library, whereas AbstractJsonFactoryTest is actual literal
tests.
> 
> I started off with this approach but ran into problems getting the
dependencies
> right, I will stick with the current approach since it is optional.

OK, but then please add a TODO.

http://codereview.appspot.com/6408051/diff/11004/google-http-client-gson/src/...
File
google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/11004/google-http-client-gson/src/...
google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java:46:
private static final String JSON_THREE_ELEMENTS =
please move this constant and all of the unit tests that use it into
AbstractJsonFactoryTest.  Change "new GsonFactory()" to "newFactory()". 
Similarly for testToString_withFactory and testFactory.

http://codereview.appspot.com/6408051/diff/11004/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonParser.java
(right):

http://codereview.appspot.com/6408051/diff/11004/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonParser.java:41:
return null;
    return factory;

[rmistry, 2012-08-01 16:46:14]

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
File
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/8001/google-http-client-gson/src/t...
google-http-client-gson/src/test/java/com/google/api/client/json/AbstractJsonFactoryTest.java:15:
package com.google.api.client.json;
On 2012/08/01 16:03:58, yanivi wrote:
> On 2012/08/01 14:35:01, rmistry wrote:
> > On 2012/07/30 19:30:10, yanivi wrote:
> > > [optional] move AbstractJsonFactoryTest.java into the src folder of a new
> > > google-http-client-test project to avoid making duplicate copies.
> > > 
> > > Note that I would still keep src package
> > > com.google.api.client.testing.http in google-http-client because that's
> really
> > a
> > > test support library, whereas AbstractJsonFactoryTest is actual literal
> tests.
> > 
> > I started off with this approach but ran into problems getting the
> dependencies
> > right, I will stick with the current approach since it is optional.
> 
> OK, but then please add a TODO.

Done.

http://codereview.appspot.com/6408051/diff/11004/google-http-client-gson/src/...
File
google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java
(right):

http://codereview.appspot.com/6408051/diff/11004/google-http-client-gson/src/...
google-http-client-gson/src/test/java/com/google/api/client/json/gson/GsonFactoryTest.java:46:
private static final String JSON_THREE_ELEMENTS =
On 2012/08/01 16:03:58, yanivi wrote:
> please move this constant and all of the unit tests that use it into
> AbstractJsonFactoryTest.  Change "new GsonFactory()" to "newFactory()". 
> Similarly for testToString_withFactory and testFactory.

Done.

http://codereview.appspot.com/6408051/diff/11004/google-http-client/src/main/...
File
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonParser.java
(right):

http://codereview.appspot.com/6408051/diff/11004/google-http-client/src/main/...
google-http-client/src/main/java/com/google/api/client/testing/http/json/MockJsonParser.java:41:
return null;
On 2012/08/01 16:03:58, yanivi wrote:
>     return factory;

Done.

[rmistry, 2012-08-01 16:56:26]

Submitting..

[rmistry, 2012-08-01 16:57:16]

Actually not submitting, let me do a sync first. This CL will make for a
complicated MOE sync by itself.