Patch re ResourceManager [WAS: Re: [Yanel-dev] Cleaned src code

[Alec Bickerton]

Michael,

Sounds reasonable. FYI The chanes relate to a potential null access
which is followed by a redundant null check.

> -        String universalName = rtd.getResourceTypeUniversalName();
universal name  is never read.

> -        if (rtd != null) {
If rti was null, we've already thrown a NullPointer, so we never get here.


Michael Wechner wrote:
> Michael Wechner schrieb:
>> Alec Bickerton schrieb:
>>> Hi,
>>>
>>> I've ran a few code cleanup tools on the yanel code base and removed all
>>>  the areas that were identified as potential problems.
>>>
>>> The settings were very conservative, targeting only the following.
>>> - Unused imports
>>> - Unused variables (local and private)
>>> - Redundant null checks
>>> - Redundant if-else nesting
>>> - Indirect access to static fields.
>>>
>>> Can someone with commit access take a look.
>>>   
>>
>> I am currently reviewing the changes and will try to commit them today.
> 
> I put the following on hold, because I need to check it closer
> 
> Index: core/java/org/wyona/yanel/core/ResourceManager.java
> ===================================================================
> --- core/java/org/wyona/yanel/core/ResourceManager.java (Revision 40290)
> +++ core/java/org/wyona/yanel/core/ResourceManager.java (Arbeitskopie)
> @@ -20,10 +20,7 @@
> import java.util.Enumeration;
> 
> import javax.servlet.http.HttpServletRequest;
> -import javax.servlet.http.HttpServletResponse;
> -
> import org.apache.log4j.Category;
> -import org.wyona.yanel.core.map.Map;
> import org.wyona.yanel.core.map.Realm;
> import org.wyona.yanel.core.util.HttpServletRequestHelper;
> import org.wyona.yanel.core.util.PathUtil;
> @@ -58,24 +55,20 @@
>      * @param rti Resource type identifier (deprecated and contains
> redudant information to resource type definition. What about properties?)
>      */
>     public Resource getResource(Environment environment, Realm realm,
> String path, ResourceTypeDefinition rtd, ResourceTypeIdentifier rti)
> throws Exception {
> -        String universalName = rtd.getResourceTypeUniversalName();
> -        if (rtd != null) {
> -            Resource resource = (Resource)
> Class.forName(rtd.getResourceTypeClassname()).newInstance();
> -
> -            resource.setRTD(rtd);
> -            resource.setYanel(Yanel.getInstance());
> -            resource.setRealm(realm);
> -            resource.setPath(path);
> -            resource.setRTI(rti);
> -            resource.setEnvironment(environment);
> -
> -            passParameters(resource, environment.getRequest());
> -
> -            return resource;
> -        } else {
> -            log.error("No resource registered for rti: " + universalName);
> +        if( rtd == null ){
> +            log.error("Rtd was null");
>             return null;
>         }
> +        //String universalName = rtd.getResourceTypeUniversalName();
> +        Resource resource = (Resource)
> Class.forName(rtd.getResourceTypeClassname()).newInstance();
> +        resource.setRTD(rtd);
> +        resource.setYanel(Yanel.getInstance());
> +        resource.setRealm(realm);
> +        resource.setPath(path);
> +        resource.setRTI(rti);
> +        resource.setEnvironment(environment);
> +        passParameters(resource, environment.getRequest());
> +        return resource;
>     }
> 
>     /**
> @@ -85,9 +78,8 @@
>      */
>     public Resource getResource(Environment environment, Realm realm,
> String path, ResourceConfiguration rc) throws Exception {
>         ResourceTypeDefinition rtd =
> rtRegistry.getResourceTypeDefinition(rc.getUniversalName());
> -
>         if (rtd != null) {
> -            String universalName = rtd.getResourceTypeUniversalName();
> +//            String universalName = rtd.getResourceTypeUniversalName();
>             try {
>                 Resource resource = (Resource)
> Class.forName(rtd.getResourceTypeClassname()).newInstance();
> 
> @@ -105,10 +97,9 @@
>                 log.error("Resource class not found for " +
> rtd.getResourceTypeUniversalName());
>                 throw e;
>             }
> -        } else {
> -            log.error("Resource Type Definition cannot be determined
> for: " + realm + ", " + path + ", " + rc.getUniversalName());
> -            return null;
>         }
> +        log.error("Resource Type Definition cannot be determined for: "
> + realm + ", " + path + ", " + rc.getUniversalName());
> +        return null;
>     }
> 
>     /**
> @@ -119,7 +110,7 @@
>     public Resource getResource(Environment environment, Realm realm,
> String path) throws Exception {
>         if
> (realm.getRTIRepository().existsNode(PathUtil.getRCPath(path))) {
>             ResourceConfiguration rc = new
> ResourceConfiguration(realm.getRTIRepository().getNode(PathUtil.getRCPath(path)));
> 
> -            if (rc != null) return getResource(environment, realm,
> path, rc);
> +            return getResource(environment, realm, path, rc);
>         }
> 
>         if
> (realm.getRTIRepository().existsNode(PathUtil.getRTIPath(path))) {
> @@ -133,7 +124,7 @@
>         String rcPath = ResourceConfigurationMap.getRCPath(realm, path);
>         if (rcPath != null &&
> realm.getRTIRepository().existsNode(rcPath)) {
>             ResourceConfiguration rc = new
> ResourceConfiguration(realm.getRTIRepository().getNode(ResourceConfigurationMap.getRCPath(realm,
> path)));
> -            if (rc != null) return getResource(environment, realm,
> path, rc);
> +            return getResource(environment, realm, path, rc);
>         }
> 
>         return getResource(environment, realm, path, new
> ResourceConfiguration("file", "http://www.wyona.org/yanel/resource/1.0",
> null));
>