On Mon, Jun 14, 2010 at 9:58 PM, <[email protected]> wrote: > Some minor comments, mainly look good (tests...). > I still wonder if we have code path that do multiple getStream on the > content. > (I guess one way to test now would be to mimic the behavior in the byte > array content, and test the server) > > > http://codereview.appspot.com/1626044/diff/15001/16004 > > File > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java > (right): > > http://codereview.appspot.com/1626044/diff/15001/16004#newcode293 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:293: > public byte[] getBytes() { > Add throw >
It doesn't throw a checked exception though, just a RuntimeException. I could add "throws StreamingException" all the way up the stack for documentation purposes though... thoughts? > > http://codereview.appspot.com/1626044/diff/15001/16004#newcode304 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:304: > public InputStream getStream() { > add "throw StreamingException" > same > > http://codereview.appspot.com/1626044/diff/15001/16004#newcode328 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/MutableContent.java:328: > return stream != other.stream; > is that always true? Not if the other StreamingContentBytes is constructed w/ a different underlying stream. > > > http://codereview.appspot.com/1626044/show >
