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 <[email protected]> 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: [email protected] > To: [email protected] > 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: [email protected] > To: [email protected] > 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: [email protected] > To: [email protected] > 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 > > > > >
