WAComboResponse + Zinc
Locked
 
|
I thought I'd try to make the Zinc server adaptor use WAComboResponse (like the Comanche adaptor does), but I'm running into some issues.
1. I need a handle on the underlying stream, but this is unknown to the ZnRequest once it's been read. ZnServer knows it, and the entity (if any) knows it if it's been read in a streaming fashion, but otherwise the stream isn't available to the Seaside server adaptor. 2. The Zinc server expects to write a response every time -- it doesn't have the #isCommitted plumbing that Comanche does for checking whether a response has already been committed. I was first tempted to modify Zinc itself (in blatant violation of the open/closed principle), but perhaps it'd be better to make a new subclass of ZnServer that'd handle these issues natively. The only downside: you'd lose the ability to plug your own ZnServer into your Seaside adaptor. Maybe that's OK, if everybody's basically using the "managing multi-threaded" approach anyway. Do we really need the flexibility of a pluggable threading approach? Any thoughts/direction would be greatly appreciated. Thanks, Ken _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Re: WAComboResponse + Zinc
|
|
Hi Ken,
On 10 Jan 2013, at 21:48, Ken Treis <[hidden email]> wrote: > I thought I'd try to make the Zinc server adaptor use WAComboResponse (like the Comanche adaptor does), but I'm running into some issues. > > 1. I need a handle on the underlying stream, but this is unknown to the ZnRequest once it's been read. ZnServer knows it, and the entity (if any) knows it if it's been read in a streaming fashion, but otherwise the stream isn't available to the Seaside server adaptor. > > 2. The Zinc server expects to write a response every time -- it doesn't have the #isCommitted plumbing that Comanche does for checking whether a response has already been committed. > > I was first tempted to modify Zinc itself (in blatant violation of the open/closed principle), but perhaps it'd be better to make a new subclass of ZnServer that'd handle these issues natively. The only downside: you'd lose the ability to plug your own ZnServer into your Seaside adaptor. Maybe that's OK, if everybody's basically using the "managing multi-threaded" approach anyway. Do we really need the flexibility of a pluggable threading approach? > > Any thoughts/direction would be greatly appreciated. It is great that you are interested in this subject. From your analysis, I feel that you understand the subject quite well, so it would be cool if you could contribute. I will do my best to help. Both of your conclusions are correct, but there are options within the current Zn architecture that could help. Have a look at #useConnection: (it was added for and is used by Zinc-WebSocket) it allows a response to do something with the raw stream after the response was written. Of course, you have to subclass ZnResponse for that, like ZnWebSocketResponse does. The final closing is controlled by #wantsConnectionClose. Once you have a subclass you could customize #writeOn: to work differently: just write the header, check the state, and so on. After this partial write, you can use the raw connection and write to it at will. You will probably need a ZnChunkedWriteStream (the inverse of the existing ZnChunkedReadStream), unless something similar (to write chunks) is already in Seaside - that shouldn't be very hard. [ I justed looked, there is #nextChunkPut: ] You will also have to subclass ZnZincServerAdaptor and change #responseFrom: to use your ZnResponse subclass as appropriate. Please go ahead and try to build a proof of concept. I think /hope it is possible with the current design. Use cases like this can only make Zn better. HTH, Sven PS: Also make sure you are using the bleedingEdge of Zinc, as there have been quite some changes lately, including to the Seaside adaptor. -- Sven Van Caekenberghe http://stfx.eu Smalltalk is the Red Pill _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
On Jan 10, 2013, at 2:58 PM, Sven Van Caekenberghe wrote:
> Have a look at #useConnection: (it was added for and is used by Zinc-WebSocket) it allows a response to do something with the raw stream after the response was written. Thank you for the help, Sven. I hadn't seen this hook. I will try as you suggest and see how it looks. > You will probably need a ZnChunkedWriteStream (the inverse of the existing ZnChunkedReadStream), unless something similar (to write chunks) is already in Seaside It is -- WAComboResponse can do this already. > PS: Also make sure you are using the bleedingEdge of Zinc, as there have been quite some changes lately, including to the Seaside adaptor. I'm on 2.1.3, let me know if I should be using something newer. Thanks again! -- Ken Treis Miriam Technologies, Inc_______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by Sven Van Caekenberghe-2
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:
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. 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. 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. 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. -- Ken Treis Miriam Technologies, Inc. _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
Hi all,
I have a problem in testing my code. I have a user in my session I want to use for logging changes (among other things). By default I can access the user in a WAObject by the following code: self session user. When I now write a test, this code crashes on a doesNotUnderstand on #session. In WAObject the requestContext is nil, because it is not in a request (it is in a test) and I have no session. I want to include one fix in WAObject: session "Answer the current seaside session, instance of WASesssion or a subclass." self requestContext ifNotNilDo: [ :context | ^context session ]. ^nil And the same trick in my own code saying: user "Answer the current seaside session, instance of WASesssion or a subclass." self session ifNotNilDo: [ :session | ^session user ]. ^nil In this way I have no trouble in testing my code. Does this change make sense to you too? Cheers, Diego_______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Re: WAComboResponse + Zinc
|
|
In reply to this post by Ken Treis
Hi Ken,
On 11 Jan 2013, at 07:03, Ken Treis <[hidden email]> 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 _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
Sven,
Let me know what you think of Zinc-Seaside-KenTreis.36.2. On Jan 11, 2013, at 7:16 AM, Sven Van Caekenberghe wrote: > As there is no #count (for streams) in my image (Pharo 2.0), #nextChunkPut: delegates better anyway. I was running on a bleeding-edge Seaside-Core package, and I'm guessing you were running on a Metacello load of 3.1.0 which is a little older. Either way, this is internal to Seaside proper (like you said) and doesn't require any changes to the Zinc adaptor itself. > 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 ? Sounds reasonable -- I called it ZnZincStreamingServerAdaptor, but feel free to rename as you see fit. Thanks for helping to get me going in the right direction. Ken _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by DiegoLont
Hi Diego,
On Jan 11, 2013, at 1:59 AM, Diego Lont wrote: > I have a problem in testing my code. I have a user in my session I want to use for logging changes (among other things). By default I can access the user in a WAObject by the following code: > self session user. > > When I now write a test, this code crashes on a doesNotUnderstand on #session. When writing SUnit tests on Seaside components, I usually override #performTest with something like: performTest | context | context := WARequestContext request: nil response: nil. context push: session while: [ super performTest ] ... where `session` is an instance variable that gets configured in #setUp. You can also configure the request/response if you need to. I hope that helps. You might get a better answer on the general Seaside list. -- Ken Treis Miriam Technologies, Inc. _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
|
|
In reply to this post by DiegoLont
We prefer to have an error signaled as it makes it easier to spot problems (if a nil is returned, an error might not be thrown until much later and you might not know why it's nil). You shouldn't be getting a dNU, though; you should be a getting a WARequestContextNotFound.
As Ken suggests, if your code actually depends on the session, you'd be better to provide one in your test setup. If not, you can always implement #user to use #on:do: user ^ [ self session user ] on: WARequestContextNotFound do: [ :e | nil ] Julian On Fri, Jan 11, 2013 at 9:59 AM, Diego Lont <[hidden email]> wrote: Hi all, _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Re: WAComboResponse + Zinc
|
|
In reply to this post by Ken Treis
Ken,
On 12 Jan 2013, at 01:07, Ken Treis <[hidden email]> wrote: > Sven, > > Let me know what you think of Zinc-Seaside-KenTreis.36.2. I merged your changes in the main branch, now at Zinc-Seaside-SvenVanCaekenberghe.37 > On Jan 11, 2013, at 7:16 AM, Sven Van Caekenberghe wrote: > >> As there is no #count (for streams) in my image (Pharo 2.0), #nextChunkPut: delegates better anyway. > > I was running on a bleeding-edge Seaside-Core package, and I'm guessing you were running on a Metacello load of 3.1.0 which is a little older. Either way, this is internal to Seaside proper (like you said) and doesn't require any changes to the Zinc adaptor itself. I just loaded Seaside-Core-KenTreis.784 from http://www.squeaksource.com/Seaside31 and my remark is still relevant. Should I be looking elsewhere ? >> 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 ? > > Sounds reasonable -- I called it ZnZincStreamingServerAdaptor, but feel free to rename as you see fit. Good name. > Thanks for helping to get me going in the right direction. Thank _you_ ! Sven _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
Re: WAComboResponse + Zinc
|
|
In reply to this post by Ken Treis
On 12 Jan 2013, at 01:07, Ken Treis <[hidden email]> wrote: > Let me know what you think of Zinc-Seaside-KenTreis.36.2. Ken, I forgot to mention that I loved your humorous class comments ;-) Sven -- Sven Van Caekenberghe http://stfx.eu Smalltalk is the Red Pill _______________________________________________ seaside-dev mailing list [hidden email] http://lists.squeakfoundation.org/mailman/listinfo/seaside-dev |
«
Return to Seaside Development
|
1 view|%1 views
| Free forum by Nabble | Edit this page |