[
http://jira.amdatu.org/jira/browse/AMDATUOPENSOCIAL-38?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=11319#comment-11319
]
Ivo Ladage - van Doorn commented on AMDATUOPENSOCIAL-38:
--------------------------------------------------------
CassandraAppDataServiceStore is not a class (at least, not anymore) and is not
referenced by TenantHostnameDispatchExtenderFilter.
Nullpointers should be fixed
try/finally should be added
> Bugs/concerns from quick code review on shindig extender filter
> ---------------------------------------------------------------
>
> Key: AMDATUOPENSOCIAL-38
> URL: http://jira.amdatu.org/jira/browse/AMDATUOPENSOCIAL-38
> Project: Amdatu OpenSocial
> Issue Type: Task
> Reporter: Bram de Kruijff
> Fix For: 0.2.0
>
>
> I have some concerns with regard to
> org.amdatu.opensocial.shindig.service.TenantHostnameDispatchExtenderFilter
> besides the fact that it introduces redundant resolving as described in
> AMDATU-288. Specifically the use of a static method to grant access to the
> mostly unrelated class CassandraAppDataServiceStore is obviosly a quick hack
> en smells.... Besides the potential exceptions indicated below some
> fundamental questions:
> 1) The static method access couples the availibility to the classloader
> lifecycle while the availibility of bundlecontext (etc) is coupled to service
> lifecycle
> 2) The useradmin lookup code (should it be here anyway?) uses
> getAllServiceReferences which does not check compatibility restrictions
> Annotated code:
> private static ThreadLocal<Tenant> m_properties = null;
> public static Tenant getTenant() {
> return m_properties.get();
>
> <====== potential NullPointerException
> }
> public void doFilter(ServletRequest request, ServletResponse response,
> FilterChain chain) throws
> IOException, ServletException {
> ....
> }
> public static UserAdmin getUserAdmin() throws InvalidSyntaxException {
> Tenant tenant = getTenant();
> ServiceReference[] refs =
> m_bundleContext.getAllServiceReferences(UserAdmin.class.getName(), null);
> <====== potential NullPointerException
> for (ServiceReference ref : refs) {
>
> <====== potential NullPointerException
> if
> (tenant.getId().equals(ref.getProperty(Tenant.TENANT_ID_SERVICEPROPERTY))) {
> <====== potential NullPointerException
> return (UserAdmin)
> m_bundleContext.getService(ref);
> <====== potential ClassCastException
> }
> }
> return null;
> }
--
This message is automatically generated by JIRA.
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
