[Seaside-dev] WAComboResponse + Zinc

[Sven Van Caekenberghe]

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