[Seaside-dev] WAComboResponse + Zinc
Sven Van Caekenberghe
sven at stfx.eu
Fri Jan 11 15:16:45 UTC 2013
Hi Ken,
On 11 Jan 2013, at 07:03, Ken Treis <ken at miriamtech.com> wrote:
> OK Sven, look at Zinc-Seaside-KenTreis.36.1 and let me know what you think. I saved it on a branch so that others will (hopefully) recognize it as "not yet Sven-approved". Some notes/thoughts:
Great work !
You achieved a lot while making very little changes, I think this definitively has to become part of (or an option for) 'Zinc-Seaside'. For a long time it puzzled me how to implement the streaming behaviour and you just did it.
> 1. I didn't end up needing the useConnection: hook, I latched into ZnResponse>>writeOn: instead. My new ZnSeasideResponse class could perhaps have been called ZnDeferredResponse or ZnLazyResponse. Really, WAComboResponse does all of the work.
I like the name ZnDeferredResponse, it captures the idea better.
> 2. I grouped the native ZnRequest together with its stream in a new class (ZnSeasideRequest). It's nothing more than a tuple, and I used it because it made everything fit into the WAServerAdaptor framework a little more gracefully. Originally, I tried passing the stream along in a separate argument (contextFor:stream:, responseFor:stream:, etc) but that made everything more cluttered.
Yes, that is more object oriented, the names are OK.
> 3. I had to change WAComboResponse>>binary to use a GRCountingStream. I think this may have just been missed during the original refactoring. This is the only change in Seaside-Core-KenTreis.784.
That seems like a Seaside proper issue.
I had to make a change as well in order to run your code:
WAComboResponse>>#flush
"Flush the receiver and send partial content"
committed ifFalse: [ self commit ].
"Write the partial content if any"
self nextChunkPut: bufferedStream contents.
bufferedStream reset
As there is no #count (for streams) in my image (Pharo 2.0), #nextChunkPut: delegates better anyway.
Now, your version of the adaptor moves a lot of logic from the adaptor and Zinc itself to Seaside as WAComboResponse does actually write the response completely itself. For that reason I would prefer to move your 4 changes on ZnZincServerAdaptor to a new subclass, like ZnZincComboResponseServerAdaptor or something similar. That way both version can live next to each other and users can pick the one they want. Agreed ?
With those small changes I would love to include your code. Putting this in front of more people will help in making sure nothing got broken.
Thanks again for your contribution.
Regards,
Sven
> As a quick test, I put together a component like this:
>
>> renderContentOn: html
>> html heading: 'Stream Test'.
>> (1 to: 5)
>> do: [ :i |
>> html
>> paragraph: [
>> html
>> text: i;
>> text: '. Each of these paragraphs should appear a few seconds apart.'.
>> self requestContext response flush ] ]
>> separatedBy: [ (Delay forSeconds: 2) wait ]
>
> It worked as expected, with chunks sent to the browser incrementally.
--
Sven Van Caekenberghe
http://stfx.eu
Smalltalk is the Red Pill
More information about the seaside-dev
mailing list