You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@camel.apache.org by "Babak Vahdat (Created) (JIRA)" <ji...@apache.org> on 2011/12/11 01:02:39 UTC

[jira] [Created] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

                 Key: CAMEL-4765
                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
             Project: Camel
          Issue Type: Improvement
    Affects Versions: 2.9.0
            Reporter: Babak Vahdat
            Assignee: Babak Vahdat
            Priority: Minor


Some simple change examples on the trunk are:

From:
{code}
jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
{code}
To:
{code}
jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
{code}

Or

From:
{code}
HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
{code}
To:
{code}
HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
{code}

Here another one with the removal of @SuppressWarnings as a side effect:

From:
{code}
    @SuppressWarnings("unchecked")
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return (T)context.getBean(names[0]);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}
To:
{code}
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return context.getBean(names[0], type);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}

There're also cases where we could make a huge benefit of this so that the following code:

{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        Object value = applicationContext.getBean(name);
        assertNotNull("No spring bean found for name <" + name + ">", value);
        if (type.isInstance(value)) {
            return type.cast(value);
        } else {
            fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
            return null;
        }
    }
{code}

becomes just a one liner as:
{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        return applicationContext.getBean(name, type);
    }
{code}

I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
From:
{code}
// must type cast to work with Spring 2.5.x
SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}
To:
{code}
SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}

Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri)
before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 


There're 147 sources affected by the provided patch (one source change is just a pre-existing checkstyle violation by HtmlToPdfMojo.java).

I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):

Results :

Failed tests:
  testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>

Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 25:49.095s
[INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
[INFO] Final Memory: 20M/494M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167397#comment-13167397 ] 

Babak Vahdat commented on CAMEL-4765:
-------------------------------------

I also hit a bunch of the places (like [1]) where that was recently done using the Spring 3.0.6 API. In that concret case the caste was obsolete. Nevertheless that would be great to clean them up.

[1] http://svn.apache.org/viewvc?view=revision&revision=1212276

Babak
                
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Claus Ibsen (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Claus Ibsen resolved CAMEL-4765.
--------------------------------

    Resolution: Fixed

Thanks for the patch.
                
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167385#comment-13167385 ] 

Babak Vahdat commented on CAMEL-4765:
-------------------------------------

Yeah, exactly!
                
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Babak Vahdat updated CAMEL-4765:
--------------------------------

    Fix Version/s: 2.9.0
    
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167015#comment-13167015 ] 

Babak Vahdat commented on CAMEL-4765:
-------------------------------------

With this patch in place we would also propose a better coding-practice to the users as many of the fixes are along the lines:

{code}
// START SNIPPET: xyz
...
...
// END SNIPPET: xyz
{code}
                
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> becomes just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri)
> before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just a pre-existing checkstyle violation by HtmlToPdfMojo.java).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Babak Vahdat updated CAMEL-4765:
--------------------------------

    Description: 
Some simple change examples on the trunk are:

From:
{code}
jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
{code}
To:
{code}
jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
{code}

Or

From:
{code}
HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
{code}
To:
{code}
HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
{code}

Here another one with the removal of @SuppressWarnings as a side effect:

From:
{code}
    @SuppressWarnings("unchecked")
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return (T)context.getBean(names[0]);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}
To:
{code}
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return context.getBean(names[0], type);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}

There're also cases where we could make a huge benefit of this so that the following code:

{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        Object value = applicationContext.getBean(name);
        assertNotNull("No spring bean found for name <" + name + ">", value);
        if (type.isInstance(value)) {
            return type.cast(value);
        } else {
            fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
            return null;
        }
    }
{code}

became just a one liner as:
{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        return applicationContext.getBean(name, type);
    }
{code}

I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
From:
{code}
// must type cast to work with Spring 2.5.x
SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}
To:
{code}
SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}

Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 


There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).

I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):

Results :

Failed tests:
  testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>

Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 25:49.095s
[INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
[INFO] Final Memory: 20M/494M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

  was:
Some simple change examples on the trunk are:

From:
{code}
jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
{code}
To:
{code}
jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
{code}

Or

From:
{code}
HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
{code}
To:
{code}
HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
{code}

Here another one with the removal of @SuppressWarnings as a side effect:

From:
{code}
    @SuppressWarnings("unchecked")
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return (T)context.getBean(names[0]);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}
To:
{code}
    public <T> T lookup(Class<T> type) {
        ApplicationContext context = getApplicationContext();
        String[] names = context.getBeanNamesForType(type, true, true);
        if (names != null) {
            int count = names.length;
            if (count == 1) {
                // lets instantiate the single bean
                return context.getBean(names[0], type);
            } else if (count > 1) {
                throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
            }
        }
        throw new IllegalArgumentException("No bean available in the application context of type: " + type);
    }
{code}

There're also cases where we could make a huge benefit of this so that the following code:

{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        Object value = applicationContext.getBean(name);
        assertNotNull("No spring bean found for name <" + name + ">", value);
        if (type.isInstance(value)) {
            return type.cast(value);
        } else {
            fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
            return null;
        }
    }
{code}

becomes just a one liner as:
{code}
    public <T> T getMandatoryBean(Class<T> type, String name) {
        return applicationContext.getBean(name, type);
    }
{code}

I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
From:
{code}
// must type cast to work with Spring 2.5.x
SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}
To:
{code}
SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
{code}

Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri)
before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 


There're 147 sources affected by the provided patch (one source change is just a pre-existing checkstyle violation by HtmlToPdfMojo.java).

I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):

Results :

Failed tests:
  testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>

Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 25:49.095s
[INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
[INFO] Final Memory: 20M/494M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

    
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Babak Vahdat (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Babak Vahdat updated CAMEL-4765:
--------------------------------

    Attachment: CAMEL-4765.patch
    
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> becomes just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri)
> before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just a pre-existing checkstyle violation by HtmlToPdfMojo.java).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (CAMEL-4765) Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)

Posted by "Claus Ibsen (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/CAMEL-4765?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13167373#comment-13167373 ] 

Claus Ibsen commented on CAMEL-4765:
------------------------------------

The type casting was needed due Spring 2.x support did not have the API with the generics.
                
> Avoid explicit cast while using the CamelContext.getEndpoint() or ApplicationContext.getBean() API and better rely on the overloaded counterpart of them taking a class type as the second argument (implicit cast)
> -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: CAMEL-4765
>                 URL: https://issues.apache.org/jira/browse/CAMEL-4765
>             Project: Camel
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Babak Vahdat
>            Assignee: Babak Vahdat
>            Priority: Minor
>             Fix For: 2.9.0
>
>         Attachments: CAMEL-4765.patch
>
>
> Some simple change examples on the trunk are:
> From:
> {code}
> jpaTemplate = (JpaTemplate)applicationContext.getBean("jpaTemplate");
> {code}
> To:
> {code}
> jpaTemplate = applicationContext.getBean("jpaTemplate", JpaTemplate.class);
> {code}
> Or
> From:
> {code}
> HttpEndpoint epOut = (HttpEndpoint) getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false");
> {code}
> To:
> {code}
> HttpEndpoint epOut = getContext().getEndpoint("http://localhost:" + port1 + "?bridgeEndpoint=true&throwExceptionOnFailure=false", HttpEndpoint.class);
> {code}
> Here another one with the removal of @SuppressWarnings as a side effect:
> From:
> {code}
>     @SuppressWarnings("unchecked")
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return (T)context.getBean(names[0]);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> To:
> {code}
>     public <T> T lookup(Class<T> type) {
>         ApplicationContext context = getApplicationContext();
>         String[] names = context.getBeanNamesForType(type, true, true);
>         if (names != null) {
>             int count = names.length;
>             if (count == 1) {
>                 // lets instantiate the single bean
>                 return context.getBean(names[0], type);
>             } else if (count > 1) {
>                 throw new IllegalArgumentException("Too many beans in the application context of type: " + type + ". Found: " + count);
>             }
>         }
>         throw new IllegalArgumentException("No bean available in the application context of type: " + type);
>     }
> {code}
> There're also cases where we could make a huge benefit of this so that the following code:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         Object value = applicationContext.getBean(name);
>         assertNotNull("No spring bean found for name <" + name + ">", value);
>         if (type.isInstance(value)) {
>             return type.cast(value);
>         } else {
>             fail("Spring bean <" + name + "> is not an instanceof " + type.getName() + " but is of type " + ObjectHelper.className(value));
>             return null;
>         }
>     }
> {code}
> became just a one liner as:
> {code}
>     public <T> T getMandatoryBean(Class<T> type, String name) {
>         return applicationContext.getBean(name, type);
>     }
> {code}
> I also came across the cases where the prefered API indeed was used but there were unneccessary casts around it:
> From:
> {code}
> // must type cast to work with Spring 2.5.x
> SpringCamelContext context = (SpringCamelContext) applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> To:
> {code}
> SpringCamelContext context = applicationContext.getBeansOfType(SpringCamelContext.class).values().iterator().next();
> {code}
> Another benefit of this patch is a better test code coverage, because as one would intuitively expect DefaultCamelContext.getEndpoint(String name, Class<T> endpointType) makes use of DefaultCamelContext.getEndpoint(String uri) before casting and returning the answer. So that now through the usage of the first method API (expecting two parameters) we do call both of them while testing (one explicitly and the other one implicitly) 
> There're 147 sources affected by the provided patch (one source change is just because of an already pre-existing checkstyle violation by HtmlToPdfMojo.java on the trunk).
> I did verify *only* the tests on camel-core but *not the rest* with the following test result (Windows-Vista):
> Results :
> Failed tests:
>   testConcurrentAppend(org.apache.camel.component.file.FileConcurrentWriteAppendSameFileTest): mock://result Received message count. Expected: <5000> but was: <2655>
> Tests run: 3969, Failures: 1, Errors: 0, Skipped: 0
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD FAILURE
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 25:49.095s
> [INFO] Finished at: Sun Dec 11 00:45:35 CET 2011
> [INFO] Final Memory: 20M/494M
> [INFO] ------------------------------------------------------------------------
> [ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:2.8:test (default-test) on project camel-core: There are test failures.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira