Hi Guys,

This is an interesting discussion and I agree that Click could do with a better IoC style services design where there is better separation of concerns.

However for the Click 2.x code line this type of change would introduce a significant risk to existing applications. As Click uses sub-classing heavily in its design, there is relatively poor encapsulation across class boundaries and the code is potentially quite brittle. This type of re-factoring would be more appropriate in a Click 3.x code line, where you can set different expectations for backward compatibility.

regards


On Sat, Apr 27, 2013 at 11:20 PM, Bob Schellink <sabob1@gmail.com> wrote:
Sounds good to me.

Kind regards

Bob


On 2013/04/27 12:40, Dennis M. J. Yerger wrote:
Subclassing ClickServlet is certainly one option, and if it fits your application's needs, that's fine. However, I'm thinking about separation of concerns and flexibility. By having each object focus on a particular role, it can be replaced with a similar object without affecting the others. Admittedly, SpringClickServlet makes for simpler configuration, but it's performing three different roles: creation, injection, and dispatching requests. If custom logic were desired for any of these roles, there is no way to plugin that in without creating another subclass. By shifting creation and injection from ClickServlet to ConfigService, the servlet is free to focus on dispatching requests and may even be replaced without affecting page creation or injection.

This wasn't really a CDI problem. My ClickServlet was left untouched when testing with CDI. I actually encountered this page creation problem when testing with Tapestry IoC, but it can happen in other cases as well. My focus is on flexibility to integrate with current and future technologies.


Date: Sat, 27 Apr 2013 10:37:56 +0200
From: sabob1@gmail.com
To: dev@click.apache.org
Subject: Re: Page factory service

Wouldn't the PageFactory complicate config? So instead of:
web.xml:
<servlet>SpringClickServlet</servlet>

one would need to do:

web.xml:
<servlet>ClickServlet</servlet>

click.xml
<click-app>
    <page-factory-service classname="org.example.SpringPageFactoryService"/>
    <page-interceptor classname="org.example.SpringPageInterceptor"/>
</click-app>

I'm not against the idea, I just don't see the use case being solved yet?

Is there something in CDI that won't work with a ClickServlet subclass eg: CDIClickServlet?

Would it help if page creation is moved to ConfigService?

Kind regards

Bob

On 2013/04/27 03:05, Dennis M. J. Yerger wrote:
Thanks for the link, Bob. After examining the SpringClickServlet source code, I believe most of the code can fit into an implementation of PageFactoryService. The only questionable part of the code would be the contents of activatePageInstance(). Subclassing ClickServlet is really only necessary if you want to override activatePageInstance() to support stateful pages. As an alternative, The activatePageInstance() contents can go into an implementation of PageInterceptor, making a subclass of ClickServlet unnecessary. Any other setup code should nicely into PageFactoryService.onInit().

An xml-based configuration might look like this:

<click-app>
    ...
    <page-factory-service classname="org.example.SpringPageFactoryService"/>
    <page-interceptor classname="org.example.SpringPageInterceptor"/>
</click-app>

The Spring application context may be stored as a servlet context attribute so it can be accessed by both the interceptor and page factory.


Date: Fri, 26 Apr 2013 20:44:04 +0200
From: sabob1@gmail.com
To: dev@click.apache.org
Subject: Re: Page factory service

It's in the extras package 'spring'.

http://svn.apache.org/repos/asf/click/trunk/click/extras/src/org/apache/click/extras/spring/SpringClickServlet.java

Kind regards

Bob

On 2013/04/26 19:03, Dennis M. J. Yerger wrote:
Hi, Bob. I have not seen the code for SpringClickServlet, so I cannot determine whether subclassing is necessary. Would you post a link to the source code?


Date: Fri, 26 Apr 2013 13:05:31 +0200
From: sabob1@gmail.com
To: dev@click.apache.org
Subject: Re: Page factory service

Hi Denis,

On 2013/04/25 22:13, Dennis M. J. Yerger wrote:
After examining the ClickServlet class, I noticed the newPageInstance() method, which is where new instances of Page objects are created. By default, newPageInstance() calls Class.newInstance() on the given class. This can be overridden by subclassing ClickServlet. However, I believe this functionality should be factored out into a separate class. A separate page factory would allow page objects to be created in more flexible ways (e.g., from Spring contexts or Tapestry IoC registries) without altering the ClickServlet. I

Looking at SpringClickServlet there seems to be quite a bit going on in there which doesn't deal only with newPageIntance. So with a PageFactory in place, wouldn't Spring still need to subclass ClickServlet to work?

Kind regards

Bob