click-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Md. Jahid Shohel" <development....@gmail.com>
Subject Re: Issues or no issue...
Date Tue, 15 Jun 2010 13:05:04 GMT
Hi Bob,

Thanks for your reply.

On Tue, Jun 15, 2010 at 1:34 AM, Bob Schellink <sabob1@gmail.com> wrote:

> Hi Jahid,
>
> On 15/06/2010 02:33, Md. Jahid Shohel wrote:
>
> >
> > 2) The same way DeployUtils class is package local, and I made a true
> copy.
>
>
> I assume you needed this for #3? See my next comment.
>
>
> > 3) The org.apache.Control interface was made to be both "a generic
> > control" and "a deploy aware control". Which I felt more of mixing of
> > concern. Shouldn't these be two separate interfaces. Like -
> >
> > public interface DeployAwareControl{
> >      public void onDeploy(ServletContext context);
> > }
> >
> > And for backward compatibility we can make Control interface extending
> > DeployAwareControl.
>
>
> Click provides two ways of deploying resources. Control#onDeploy and the
> Servlet3.0 approach of
> packaging resources under /META-INF/resources. I think Control#onDeploy
> should be deprecated as it
> has some major drawbacks:
>
>  - Control has to be instantiated during startup in order to invoke
> onDeploy. At startup time there
> is no request, and thus no Context object. The net effect of this is that
> the Control cannot rely on
> Context in it's constructor. Accessing Context in the Control constructor
> is useful because one can
> pull data from the Session to retrieve state.
>  - changes to the resources have to be updated in two places, once for
> getHeadElements and again for
> onDeploy
>
>
> I still see onDeploy has more potential then making it deprecated. There
are lots of things that developer can do when the application is deploying.
I can give one example, the spring integration I have written (more details
can be found on http://code.google.com/p/doubleclick/wiki/clickspring) loads
up the spring context during the deploy time -

<controls>
    <control classname="org.apache.click.doubleclick.injectspring.SpringControl"/>
</controls>

This is just an example that what we can do on deploy. There are many other
things we can do, we can prepare services common services which need to
intercept each call without writing a filter, we can prepare services
(spring, JPA, Hibernate) which will be singleton service through out the
application. And since its singleton and so the only perfect place we can
load up the singleton service is onDeploy. But I totally agree with you,
that this does not need to be a Control. We can just make a separate one.




> > 4) The "isTemplate" method is having a bug. In the method we are not
> > checking if the file is really a file or folder, Because on my Linux
> > (Ubuntu) I was able to create a folder with .htm  or .jsp extension. I
> > know this might not happen so often, but still I think it might create
> > problem.
>
>
> isTemplate is used at runtime and would create overhead if we were to touch
> the filesystem. I don't
> see a use case for having .htm and .jsp named folders inside the webapp.
> That said what is the
> behavior of having such a folder defined?
>
>
> I agree with you, that there is no use case which can make .htm or .jsp
named folders. But logic wise it did not seems correct that we do not check
if its a file or folder. Any naughty developer can make a .htm folder inside
the / and that .htm folder will still be processed on deploy time, and will
try to figure out its variables and auto bind them.



> > 6) When the application is loading, we load all page classes, and their
> > bindable page fields. Could be a memory issue when benchmarking Click
> > against other frameworks. We can load bindable page fields on demand,
> > and cache them. That way we will not end up loading all page fields even
> > if its not needed. For an application with 100 pages this could be a
> > performance issue. As an example: say on http://click.apache.org there
> > are lots of pages which is not browsed by user (example license content
> > page, how to release, and pages like that), those pages will still be
> > loaded. I know an application running for long time might be loading all
> > pages eventually. But still, we can load them on demand, and we can
> > cache loaded classes. And we can also keep the house keeping thread to
> > eventually clear up rarely loaded pages.
>
>
> I'd like to see lazy loading as well so feel free to open a JIRA for this.
> Not only does it affect
> startup but it will allow Click to run in production mode under GAE[1]. GAE
> does not support the
> method servletContext.getResourcePaths which XmlConfigService uses
> internally. Lazy loading does
> present its own problems such as synchronizing the data structures used to
> store page classes and
> templates.
>
> I will create one JIRA issue for this, and I will also add patch about how
I see the lazy loading should work for this (which will include
synchronizing the data structure used to store page classes and templates.


>
> > 7) On "getBindablePageFields" method of XmlConfigService, this piece of
> > code -
> >
> > Field[] publicFields = pageClass.getFields();
> >     for (Field field : publicFields) {
> >         fieldMap.put(field.getName(), field);
> > }
> >
> > Though @Bindable fields are still processed, but here we again we are
> > processing all public fields (even though they have @Bindable and
> > already processed). Also there are some repetition of code (on if-else)
> > block. Possible to improve. Will help to get a better picture when
> > benchmarking.
>
>
> What optimization do you have in mind?
>
>
I will reply details about this one, I have to look into it to find out
where did I see that.


> Kind regards
>
> Bob
>
> [1]: https://issues.apache.org/jira/browse/CLK-639
>


Regards,


Jahid

Mime
View raw message