[
http://jira.amdatu.org/jira/browse/AMDATU-517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12884#comment-12884
]
Matthijs Hendriks edited comment on AMDATU-517 at 3/20/12 2:35 PM:
-------------------------------------------------------------------
My findings
* *general remark: the purpose of this bundle is not entirely clear. What
problem does it try to solve that isn't possible with the regular
ExtHttpService of Felix?*
I honestly do not know.
* *HandlerServletContext has SuppressWarnings("deprecation") for some
deprecated methods?*
* _ExtServletContext_ requires the class to implement these methods. However,
for _ServletContext_, these methods are deprecated (without replacement for
most). I have 'fixed' this by deprecating the methods that function as a
'reference' to the deprecated method and, if possible, replacing deprecated
calls by non-deprecated ones.
* *DispatchInterceptFilter tries to avoid duplicate initializations, but this
is still possible as no synchronization takes place in this method; aside that,
why is this necessary, as the Filter API clearly defines that it will call the
init method exactly once only?*
* You are correct, this check is redundant & broken. I have removed it.
* *DispatchInterceptFilter has an unused method (getLogMessage);*
* It has not in my version.
* *AbstractHandler is not thread-safe wrt its state member;*
* _m_state_ is now volatile, fixing this.
* *DispatcherServiceImpl has redundant service-lifecycle methods & log
statements;*
* I have removed the redundant log statements. I have not found any redundant
service-lifecycle methods.
* *DispatcherServiceImpl#start manually creates HandlerRegistry services which
is feeds OSGi services without respecting their life cycles, make them managed
components by DM should solve this.*
* I might have solved this. Not entirely sure.
A patch/pull request for all this will follow shortly.
was (Author: matt):
My findings
* *general remark: the purpose of this bundle is not entirely clear. What
problem does it try to solve that isn't possible with the regular
ExtHttpService of Felix?*
* I honestly do not know.
* *HandlerServletContext has SuppressWarnings("deprecation") for some
deprecated methods?*
* _ExtServletContext_ requires the class to implement these methods. However,
for _ServletContext_, these methods are deprecated (without replacement for
most). I have 'fixed' this by deprecating the methods that function as a
'reference' to the deprecated method and, if possible, replacing deprecated
calls by non-deprecated ones.
* *DispatchInterceptFilter tries to avoid duplicate initializations, but this
is still possible as no synchronization takes place in this method; aside that,
why is this necessary, as the Filter API clearly defines that it will call the
init method exactly once only?*
* You are correct, this check is redundant & broken. I have removed it.
* *DispatchInterceptFilter has an unused method (getLogMessage);*
* It has not in my version.
* *AbstractHandler is not thread-safe wrt its state member;*
* _m_state_ is now volatile, fixing this.
* *DispatcherServiceImpl has redundant service-lifecycle methods & log
statements;*
* I have removed the redundant log statements. I have not found any redundant
service-lifecycle methods.
* *DispatcherServiceImpl#start manually creates HandlerRegistry services which
is feeds OSGi services without respecting their life cycles, make them managed
components by DM should solve this.*
* I might have solved this. Not entirely sure.
A patch/pull request for all this will follow shortly.
> Code review: web.dispatcher
> ---------------------------
>
> Key: AMDATU-517
> URL: http://jira.amdatu.org/jira/browse/AMDATU-517
> Project: Amdatu
> Issue Type: Improvement
> Components: Amdatu Web
> Reporter: Jan Willem Janssen
> Labels: code_review
> Fix For: Backlog
>
>
> My comments:
> * general remark: the purpose of this bundle is not entirely clear. What
> problem does it try to solve that isn't possible with the regular
> ExtHttpService of Felix?
> * HandlerServletContext appears to be tightly coupled to Felix' HTTP
> implementation;
> * HandlerServletContext has SuppressWarnings("deprecation") for some
> deprecated methods?
> * DispatchInterceptFilter tries to avoid duplicate initializations, but this
> is still possible as no synchronization takes place in this method; aside
> that, why is this necessary, as the Filter API clearly defines that it will
> call the init method exactly once only?
> * DispatchInterceptFilter has an unused method (getLogMessage);
> * AbstractHandler is not thread-safe wrt its state member;
> * DispatcherServiceImpl has redundant service-lifecycle methods & log
> statements;
> * DispatcherServiceImpl#start manually creates HandlerRegistry services which
> is feeds OSGi services without respecting their life cycles, make them
> managed components by DM should solve this.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
http://jira.amdatu.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
_______________________________________________
Amdatu-developers mailing list
[email protected]
http://lists.amdatu.org/mailman/listinfo/amdatu-developers
