You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by jo...@gmail.com on 2010/05/01 00:02:23 UTC

Re: Add contentBytes-type data modification to MutableContent (issue1044042)

Committed, but missed Jacobo's comments. I've updated the code in
response to them, and am committing an incremental update.


http://codereview.appspot.com/1044042/diff/7001/8002
File
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java
(right):

http://codereview.appspot.com/1044042/diff/7001/8002#newcode128
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:128:
* is modified, resultant behavior is
On 2010/04/30 20:46:34, jtarrio wrote:
> undefined? :)

Done.

http://codereview.appspot.com/1044042/diff/7001/8002#newcode131
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:131:
public InputStream getContentBytes() {
On 2010/04/30 20:46:34, jtarrio wrote:
> The comment says that it's returned as a byte array, but in reality it
returns
> an InputStream.

Done.

http://codereview.appspot.com/1044042/diff/7001/8002#newcode163
java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:163:
if (contentBytes == null || Arrays.equals(contentBytes, newBytes)) {
On 2010/04/30 20:46:34, jtarrio wrote:
> !Arrays.equals

Done.

http://codereview.appspot.com/1044042/diff/7001/8001
File
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java
(right):

http://codereview.appspot.com/1044042/diff/7001/8001#newcode61
java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/MutableContentTest.java:61:
content.getBytes("UTF8"), IOUtils.toByteArray(mhc.getContentBytes())));
Good call, test added!

On 2010/04/30 20:46:34, jtarrio wrote:
> If you'd like to test that UTF-8 is really used internally for the
conversions,
> you can add some non-ascii characters, as in "DÉFÃÜLT VÌÊW" or "NËW
CØNTÈNT" :)

http://codereview.appspot.com/1044042/show