Hi, Please file a ticket. We should fix that for Wicket 6.x. I understand that it is weird to migrate an app from 1.4 to 1.5 and then revert the change when migrating to 6.x but I believe its is for good.
On Fri, Jun 8, 2012 at 10:48 AM, Jesse Long <[email protected]> wrote: > Hi All, > > Martin, thanks for the response, see comments below: > > > On 08/06/2012 02:26, Martin Grigorov wrote: >> >> Hi Jesse, >> >> The signature of this method has been changed with: >> >> commit 4977f66ef55eaa5d89ed5d3751ff6aefc9652c50 >> Author: Matej Knopp<[email protected]> >> Date: Mon Jan 4 01:03:29 2010 +0000 >> >> Wild attempt to integrate new RequestCycle >> src/main/java almost builds, definitely doesn't work >> rest doesn't even build >> >> put bunch of classes that will likely be removed to src/main/disabled >> >> hopefully will continue working on it shortly >> >> >> I.e. it was changed in the very early early days of 1.5. >> I'm not sure whether the change is intentional, i.e. whether Matej >> wanted to improve the API by making it consistent with the rest of the >> code or it was just part of "make the code compiling" coding session. > > > I think it makes it more inconsistent. IResourceStream has a size, > contentType and lastModified. It also has a getInputStream() to get the > data. This can be implemented for things like java.io.File etc that have no > knowledge of what a HTTP request/response is. IResourceStreamWriter is a > simple class that is a IResourceStream, but instead of using pull to get the > data, it pushes the data to an OutputStream. Think generate data on the fly. > Without it, I would have to use a IResourceStream, write my data to memory > or a temp file, then provide a InputStream to read that temp data. I think > this is what the javadoc means when it says buffer. > > It is the job of IResource to respond to a Response. It is the job of the > IResourceStream to provide data and meta data. You cant have IResourceStream > doing IResource's job, you'll have the labour unions all over you :-) > > I wrote the original mail when I was implementing a custom IResourceStream, > that generates new data for download for each request. I didn't want to > write a buffer like described above, so I use IResourceStreamWriter. But, > now I must write to a Response. So my thoughts go like this: > > "Oh no, not I must first work out how a Response works, just to serve up > generated data. Ok, so what does a Response do? Set headers and write to an > OutputStream. OK, so I am being passed a Response, there is probably a need > to write the headers, but I already provide the header data in > IResourceStream API - lets see if they are being set for me. <dive into > wicket internals, discover that ultimately its the responibility of > ResourceStreamResource>. Oh look, its setting all the headers for me, so all > I need to do is write data to the OutputStream. So why did I need a Response > and have to dive into the internals to work out what to do with it?" > > >> >> The javadoc says that IResourceStreamWriter can stream to the client >> without buffering but looking at the current version I think this is >> no more true, i.e. the written data IS buffered. > > I cant see where it is buffered but thats not important for my situation. If the Response instance is BufferedWebResponse then either using response.write() or response.getOutputStream().write() will be buffered (kept in memory) until Response#flush() or #reset() is called. But also see https://issues.apache.org/jira/browse/WICKET-4233 This tickets explains why Response#getOutputStream() actually delegates to Response#write() > >> I personally have never needed to use this class in my apps and I'm >> not very familiar with it. >> >> @Devs: is this class broken in 1.5.x ? Should we revert the API change for >> 6.x ? >> >> >> On Thu, Jun 7, 2012 at 6:04 PM, Jesse Long<[email protected]> wrote: >>> >>> Hi Wicket Devs, >>> >>> (wicket 6.0.0-beta2 btw). >>> >>> Why does IResourceStreamWriter#write() take a Response and not an >>> OutputStream? >>> >>> As I see it, ResourceStreamResource (which is called from >>> ResourceStreamRequestHandler) is responsible for setting the headers for >>> the >>> response, and does. What more could the IResourceStreamWriter want to do >>> with the response? >>> >>> AbstractResource#respond() calls >>> ResourceStreamResource#newResourceResponse(), which returns all the >>> headers >>> in the ResourceReponse (using normal IResourceStream calls), and passes >>> back >>> a wrapped call to IResourceStreamWriter#write() as the writer callback. >>> AbstractResource#respond() then sets the headers on the Response, then >>> only >>> calls IResourceStreamWriter#write() via the write callback only if >>> AbstractResource thinks data needs to be written based on the values >>> returned from the normal IResourceStream calls. >>> >>> So, if the IResourceStreamWriter sets different headers in its write() >>> method to the values produced by calling lastModifiedTime() etc (I would >>> consider this a bad idea, and probably wrong), there is a danger that >>> unexpected behaviour may occur if AbstractResource#respond() decides NOT >>> to >>> call the writer callback (which will call IResourceStreamWriter#write()). >>> The IResourceStreamWriter implementer may wonder why his headers are not >>> being set. >>> >>> If the IResourceStreamWriter does not set any headers apart from the data >>> available via its IResourceStream methods, then it obviously does not >>> need a >>> Response object, because AbstractResource is setting all the headers >>> already. All IResourceStreamWriter needs to do is write data to the >>> OutputStream. >>> >>> Could it be a remnant of an era where Response did not have a >>> getOutputStream() method, and writing to it required using >>> Response#write()? >>> >>> Also, AbstractResource.WriteCallback#writeStream() unnecessarily creates >>> an >>> OutputStream that delegates write() calls to the Response. It could just >>> as >>> well use the one provided by Response.getOutputStream(). >>> >>> Thanks, >>> Jesse >>> >> >> > -- Martin Grigorov jWeekend Training, Consulting, Development http://jWeekend.com
