Simplify logic for copying input stream to output stream

Simplify logic for copying input stream to output stream.  We longer pay attention to the input content length, since that functionality is not useful.  Advantage is that it significantly simplifies our code and removes a lot of unnecessary complexity from our library.

Fix content length logic in MockLowLevelHttpResponse that treated zero content length as unknown length, but it should be -1.

[Issue 41] HttpResponse.parseAsString() should respect charset Content-Type parameter

[mlinder, 2012-05-21 18:00:17]

LGTM once you have addressed those comments.

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:484:
* Parses the content of the HTTP response from {@link #getContent()} and reads
it into a string.
Add that that the response is read as UTF-8

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:82:
setContentLength(-1);
Content-Length should be 0 if no content is given, and only -1 if content-length
is unknown, right?

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:229:
* Warning: in prior version {@code 0} was used for none, but now it means
0-length content and
none and 0-length basically mean the same to me. Can we use different words for
this? -1 = content has unknown length, 0 = no content

[yanivi, 2012-05-21 19:58:06]

Implemented a fix for Issue 41.  Please review again.

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:484:
* Parses the content of the HTTP response from {@link #getContent()} and reads
it into a string.
On 2012/05/21 18:00:17, Matthias wrote:
> Add that that the response is read as UTF-8

That is actually a bug.  I will fix this in the next changeset:

http://code.google.com/p/google-http-java-client/issues/detail?id=41

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:82:
setContentLength(-1);
On 2012/05/21 18:00:17, Matthias wrote:
> Content-Length should be 0 if no content is given, and only -1 if
content-length
> is unknown, right?

Done.

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:229:
* Warning: in prior version {@code 0} was used for none, but now it means
0-length content and
On 2012/05/21 18:00:17, Matthias wrote:
> none and 0-length basically mean the same to me. Can we use different words
for
> this? -1 = content has unknown length, 0 = no content

Done.

[yanivi, 2012-05-21 20:00:16]

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/3004/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:484:
* Parses the content of the HTTP response from {@link #getContent()} and reads
it into a string.
On 2012/05/21 19:58:07, yanivi wrote:
> On 2012/05/21 18:00:17, Matthias wrote:
> > Add that that the response is read as UTF-8
> 
> That is actually a bug.  I will fix this in the next changeset:
> 
> http://code.google.com/p/google-http-java-client/issues/detail?id=41

What I meant to say: this is now fixed :)

[mlinder, 2012-05-21 20:32:53]

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:513:
static String parseCharset(String contentType) {
Add a small 1-line comment about what this does and that this defaults to
ISO-8859-1

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:515:
Matcher m =
Pattern.compile(";\\s*(\\S[^=]*)=([^;]*[^;\\p{Space}])").matcher(contentType);
Can we cache that Pattern in a static field?

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:517:
if ("charset".equals(m.group(1))) {
what about casing? shouldn't this also work with upper case chars?

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:222:
* Sets the content length or {@code -1} for unknown.
Suggestion: A static final field CONTENT_LENGTH_UNKNOWN = -1 or something might
be useful.

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/test/j...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:100:
"a b c", HttpResponse.parseCharset("application/json;a=a; charset=a b c ;
b=b"));
http://tools.ietf.org/html/rfc2616#section-3.7

Check for case-insensitivity @ charset, e.g. test for "CHARSET=" ..

[yanivi, 2012-05-21 20:47:14]

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:513:
static String parseCharset(String contentType) {
On 2012/05/21 20:32:53, Matthias wrote:
> Add a small 1-line comment about what this does and that this defaults to
> ISO-8859-1

Done.

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:515:
Matcher m =
Pattern.compile(";\\s*(\\S[^=]*)=([^;]*[^;\\p{Space}])").matcher(contentType);
On 2012/05/21 20:32:53, Matthias wrote:
> Can we cache that Pattern in a static field?

Done.

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/http/HttpResponse.java:517:
if ("charset".equals(m.group(1))) {
On 2012/05/21 20:32:53, Matthias wrote:
> what about casing? shouldn't this also work with upper case chars?

Done.

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
File
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/main/j...
google-http-client/src/main/java/com/google/api/client/testing/http/MockLowLevelHttpResponse.java:222:
* Sets the content length or {@code -1} for unknown.
On 2012/05/21 20:32:53, Matthias wrote:
> Suggestion: A static final field CONTENT_LENGTH_UNKNOWN = -1 or something
might
> be useful.

Nah.  I think -1 is fine.

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/test/j...
File
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java
(right):

http://codereview.appspot.com/6218059/diff/6003/google-http-client/src/test/j...
google-http-client/src/test/java/com/google/api/client/http/HttpResponseTest.java:100:
"a b c", HttpResponse.parseCharset("application/json;a=a; charset=a b c ;
b=b"));
On 2012/05/21 20:32:53, Matthias wrote:
> http://tools.ietf.org/html/rfc2616#section-3.7
> 
> Check for case-insensitivity @ charset, e.g. test for "CHARSET=" ..

Done.

[mlinder, 2012-05-21 20:49:48]

LGTM