On 31.1.2012 13:55, Julien Viet wrote:
On Jan 31, 2012, at 1:46 PM, Marek Posolda wrote:
> On 30.1.2012 23:33, Julien Viet wrote:
>> On Jan 30, 2012, at 7:45 PM, Julien Viet wrote:
>>
>>> On Jan 30, 2012, at 5:22 PM, Marek Posolda wrote:
>>>
>>>> Hi,
>>>>
>>>> I think that it's not ideal how WebAppController processes static
>>>> resources like images. I have 2 things to point out:
>>>>
>>>> 1) Today I found (and fixed) a bug that images are first processed by
>>>> PortalRequestHandler, even if they should be processed by
>>>> StaticResourceRequestHandler. Please look at
>>>>
https://issues.jboss.org/browse/GTNPORTAL-2338 for more details. This is
>>>> really not good because HTTP header "Cache-Control: no-cache"
is
>>>> returned for every image, and so web browser needs to download images
>>>> for each HTTP request to portal. I fixed it by adding
>>>> StaticResourceRequestHandler before PortalRequestHandler into
>>>> navController configuration, so it should not be the issue anymore.
>>> in StaticResourceRequestHandler the request is dispatched with the servlet
API and the "default" servlet should setup the proper headers.
>>>
>>> could you explain me why having the StaticResourceRequestHandler after the
PortalRequestHandler prevents it ?
> "default" servlet dispatched from StaticResourceRequestHandler does not add
headers. Headers for images (like "Cache-control" header) are added by
ResourceRequestFilter. Problem is that header is overriden by PortalRequestHandler.
makes sense
actually at some point the ResourceRequestFilter should be removed as it is intrusive for
the war file. We have a prototype of resource serving for Javascript (spec is available
here
https://wiki-int.exoplatform.org/display/rhcollab/Resource+Controller) that makes all
serving of resource to be done by the portal for deployed scripts.
> Well, what's going during processing of HTTP request to image (like
http://localhost:8080/portal/something.jpg ) before fix of GTNPORTAL-2338 is:
> - Header "Cache-Control: max-age=2592000,s-maxage=2592000" is added by
ResourceRequestFilter
> - Processing is dispatched to WebAppController and there are two compliant handlers:
PortalRequestHandler and StaticResourceRequestHandler
> - PortalRequestHandler is first and so image is processed first by it. And this does
not makes sense as it's overriding Cache-Control header with value
"no-cache" and it's calling some operations to UserPortalConfigService and
DataStorage (really does not makes sense).
> - Processing by PortalRequestHandler returns false and so it continues to
StaticResourceRequestHandler. This handler dispatches it to "default" servlet
and image is returned to client, but with incorrect header "Cache-control:
no-cache"
>
> The result is that every browser request to portal like
"http://localhost:8080/portal/classic" needs to download all portal images as
they can't be cached on the browser side. Really bandwidth killer IMO.
correct, can you fix it in gatein trunk with the same headers ?
I've fixed it
in GateIn by adding StaticResourceRequestHandler before
PortalRequestHandler. See
https://issues.jboss.org/browse/GTNPORTAL-2338.
soon also we are going to investigate CSS sprite generation in order
to pack several images into a single one to optimize the connections made to the server.
> By adding StaticResourceRequestHandler before PortalRequestHandler is processing of
images by PortalRequestHandler omited. For details please look at
https://issues.jboss.org/browse/GTNPORTAL-2338 .
>>>> 2) Second thing is not so bad but a bit more tricky. The piece of code
>>>> in WebAppController:
>>>>
>>>> {code}
>>>> if (!started)
>>>> {
>>>>
>>>> RequestLifeCycle.begin(ExoContainerContext.getCurrentContainer());
>>>> started = true;
>>>> }
>>>> .....
>>>> processed = handler.execute(new ControllerContext(this,
>>>> router, req, res, parameters));
>>>> .....
>>>> if (started)
>>>> {
>>>> RequestLifeCycle.end();
>>>> }
>>>> {code}
>>>>
>>>> This means that we need to start RequestLifeCycle for processing of
>>>> every resource including static resource. And startup of some services
>>>> is quite expensive (like startup of OrganizationService requires startup
>>>> of Hibernate transaction). In other words, currently we are starting
>>>> Hibernate transaction for processing images and other static resources.
>>>>
>>>> It's obvious that some handlers (PortalRequestHandler,
>>>> LegacyRequestHandler and probably some others) really need to start
>>>> RequestLifecycle, but some others like StaticResourceRequestHandler
>>>> don't need it. So how about postponing the RequestLifecycle.begin
from
>>>> WebAppController to concrete handlers, which really need it? And use
>>>> some ThreadLocal variable or flag "started" on
ControllerContext, so
>>>> that we can track if RequestLifeCycle.begin has been already started
>>>> (can be useful to avoid double-start if some resource needs to be
>>>> processed by more request handlers) ?
>>> please make an abstract method on WebRequestHandler :
>>>
>>> protected abstract boolean getRequiresLifeCycle() that the WebAppController
can use in the loop to replace the condition checked by
>>>
>>> if (!started)
>>>
>>> by the condition
>>>
>>> if (!started&& handler.getRequiresLifeCycle()) { ... }
>>>
>>> (which requires to move this block in the
>>>
>>> if (handler != null)
>>> {
>>> }
>>>
>>> block
>> I just want that the whole request lifecycle stuff keeps confined in the
WebAppController class if we can, as the subclass will likely always or never need a
RequestLifecyle to be triggered.
> Thanks, initially I though that some handlers may need to start only something (for
example start only ChromatticManager but not OrganizationService) but probably does not
makes sense to complicate things because of it. So I've made it as you suggested. Jira
is
https://issues.jboss.org/browse/GTNPORTAL-2340
>
> I made it that getRequiresLifeCycle returns true for: PortalRequestHandler,
LegacyRequestHandler
> and returns false for: DefaultRequestHandler, StaticResourceRequestHandler,
DownloadHandler, UploadHandler
>
> Adding new method will require changes for other handlers in other eXo components. At
least for EPP-SP I am seeing two additional handlers mapped with names "robots"
and "sitemap". Should I create JIRA for you in some eXo project to cover it?
yes you should but you should also create a JIRA in EXOGTN to port this change to our
product branch.
I've created
https://jira.exoplatform.org/browse/SPUB-139 for
EPPSP
handlers. Seems that I don't have permission to create JIRA in EXOGTN
project.
>>>> Any other suggestions for this?
>>>>
>>>> Marek
>>>>
>>>> _______________________________________________
>>>> gatein-dev mailing list
>>>> gatein-dev(a)lists.jboss.org
>>>>
https://lists.jboss.org/mailman/listinfo/gatein-dev