[
http://jira.amdatu.org/jira/browse/AMDATU-282?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ivo Ladage - van Doorn reopened AMDATU-282:
-------------------------------------------
Performed a code review, will continue with more testing/performance testing of
dispatcher stuff. My comments:
- There were some spelling errors in the javadoc, I already fixed those and
comitted the changes
- In the new dispatcher a new 'DefaultHttpContext' was introduced, however
HttpContextImpl is also still used. I'm not sure why there are two HttpContext
implementations. Also HttpContextImpl uses the Apache Sling Mimetype service
for mimetype resoving, the DefaultHttpContext doesn't. However,
HandlerServletContext seems to use the Felix http service for mimetype
resolving. If we choose to use the mimetype resolving of the Felix http
service, I would expect the HttpContextImpl to use the same resolver. However,
it would be best to prevent dependencies with the Felix implementation of the
http service, such that we can replace it by another http service
implementation if we want to (ok, we depend on it anyway for filter
registration, I know).
- AbstractHandler lines 70-72 contain unnecessary casts as the map is already
typed <String, String>
- In AbstractHandlerRegistry line 104 a new DefaultHttpContext is returned when
the service reference has no contextId property. So this method returns a
different instance each time it is invoked for the same ServiceReference. I'm
not sure if that is the intention.
- In FilterHandlerRegistry lines 88, 149 inproper exception handling
- In DispatcherServiceActivator the LogService is defined as a required
dependency, but the DispatcherServiceImpl checks if the LogService is null
before using it (sometimes and sometimes not).
- In DispatcherServiceImpl line 92 ServletException is rethrown as Exception,
why?
- I have concerns regarding the design of the way dependencies between the
DispatcherServiceImpl, ServletHandlerRegistry, FilterHandlerRegistry and
DispatchInterceptFilter are setup. The DispatcherServiceImpl is the one and
only actual service, other components like the servlet and filter handler
registry are instantiated from the DispatcherServiceImpl. There are cross
references between them, for example the DispatcherServiceImpl references the
DispatchInterceptFilter and the DispatchInterceptFilter references the
DispatcherServiceImpl. Also other service dependencies like the LogService are
passed from the constructor instead of using service dependencies. Why are
those classes not implemented as services? I guess the cross-references between
them makes that hard, but I believe this needs some refactoring such that all
those components are actual services using proper service dependencies.
- In line with the previous comment; in DispatcherServiceImpl there is a timing
issue in the destroy() of DispatcherServiceImpl; when the bundle is stopped
first the registries are set to null in the dispatcher, then the interceptor
filter is unregistered. But when a http request comes by before that, the
filter will invoke the DispatcherServiceImpl.dispatchRequest and the registries
will be null at that time, causing nullpointer exceptions.
> Integrate web dispatcher project
> --------------------------------
>
> Key: AMDATU-282
> URL: http://jira.amdatu.org/jira/browse/AMDATU-282
> Project: Amdatu
> Issue Type: New Feature
> Components: Amdatu Web
> Affects Versions: 0.1.0
> Reporter: Bram de Kruijff
> Assignee: Ivo Ladage - van Doorn
> Fix For: 0.1.0
>
>
> Implementation and integration of a web dispatcher mechanism that support
> pluggable (servlet/request) context extenders and tenant aware dispatching to
> web component. As described at
> http://lists.amdatu.org/pipermail/amdatu-developers/2011-January/002341.html
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
