camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Claus Ibsen" <claus.ib...@gmail.com>
Subject Re: svn commit: r712662 - in /activemq/camel/trunk/components/camel-jms/src: main/java/org/apache/camel/component/jms/JmsComponent.java test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java
Date Tue, 11 Nov 2008 07:00:32 GMT
On Tue, Nov 11, 2008 at 3:42 AM, Willem Jiang <willem.jiang@gmail.com> wrote:
> Hi Claus,
>
> Thanks for reviewing the code, I will change to throw the
> IllegalArgumentException.
>
> For the "if (username == null || password == null)" , I need to exclude
> the saturation of username and password are all empty.

Ah that was it. Would you mind adding a code comment on this in the java file?
Then we know its on purpose.



>
> Willem
>
> Claus Ibsen wrote:
>> Hi
>>
>> I think you should throw an IllegalArgumentException instead of plain
>> CamelException.
>>
>> Also I am wondering if we should push for ObjectHelper.notNull or
>> other similar feature to test for mandatory options. So we do it
>> consistent.
>>
>> Also I am a bit puzzled with the else statement and the ifs.
>> I would write the 2nd if as:
>>     if (username == null || password == null)
>>
>> However why not throw a specific error message so end user know if its
>> the username or password that is missing.
>> I would write it as
>> if (username == null)
>>   throw new IAE(Username is not set)
>> if (password == null)
>>   throw new IAE(password is not set)
>>
>> But do you break anything? As now it looks like username and password
>> is mandatory. Well I can only see a small part of the code. I am sure
>> are on top of it ;)
>>
>> On Mon, Nov 10, 2008 at 3:10 PM,  <ningjiang@apache.org> wrote:
>>> Author: ningjiang
>>> Date: Mon Nov 10 06:10:16 2008
>>> New Revision: 712662
>>>
>>> URL: http://svn.apache.org/viewvc?rev=712662&view=rev
>>> Log:
>>> CAMEL-246 allow the username and password to be specified on the JMSComponent
/ JMSConfiguration
>>>
>>> Modified:
>>>    activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java
>>>    activemq/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java
>>>
>>> Modified: activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java
>>> URL: http://svn.apache.org/viewvc/activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java?rev=712662&r1=712661&r2=712662&view=diff
>>> ==============================================================================
>>> --- activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java
(original)
>>> +++ activemq/camel/trunk/components/camel-jms/src/main/java/org/apache/camel/component/jms/JmsComponent.java
Mon Nov 10 06:10:16 2008
>>> @@ -23,6 +23,7 @@
>>>  import javax.jms.Session;
>>>
>>>  import org.apache.camel.CamelContext;
>>> +import org.apache.camel.CamelException;
>>>  import org.apache.camel.Endpoint;
>>>  import org.apache.camel.HeaderFilterStrategyAware;
>>>  import org.apache.camel.component.jms.requestor.Requestor;
>>> @@ -36,6 +37,7 @@
>>>  import org.springframework.context.ApplicationContextAware;
>>>  import org.springframework.core.task.TaskExecutor;
>>>  import org.springframework.jms.connection.JmsTransactionManager;
>>> +import org.springframework.jms.connection.UserCredentialsConnectionFactoryAdapter;
>>>  import org.springframework.jms.core.JmsOperations;
>>>  import org.springframework.jms.support.converter.MessageConverter;
>>>  import org.springframework.jms.support.destination.DestinationResolver;
>>> @@ -402,6 +404,20 @@
>>>         if (selector != null) {
>>>             endpoint.setSelector(selector);
>>>         }
>>> +        String username = getAndRemoveParameter(parameters, "username", String.class);
>>> +        String password = getAndRemoveParameter(parameters, "password", String.class);
>>> +        if (username != null && password != null) {
>>> +            ConnectionFactory cf = endpoint.getConfiguration().getConnectionFactory();
>>> +            UserCredentialsConnectionFactoryAdapter ucfa = new UserCredentialsConnectionFactoryAdapter();
>>> +            ucfa.setTargetConnectionFactory(cf);
>>> +            ucfa.setPassword(password);
>>> +            ucfa.setUsername(username);
>>> +            endpoint.getConfiguration().setConnectionFactory(ucfa);
>>> +        } else {
>>> +            if (username != null || password != null) {
>>> +                throw new CamelException("The JmsComponent's username or password
is null");
>>> +            }
>>> +        }
>>>         setProperties(endpoint.getConfiguration(), parameters);
>>>         return endpoint;
>>>     }
>>>
>>> Modified: activemq/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java
>>> URL: http://svn.apache.org/viewvc/activemq/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java?rev=712662&r1=712661&r2=712662&view=diff
>>> ==============================================================================
>>> --- activemq/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java
(original)
>>> +++ activemq/camel/trunk/components/camel-jms/src/test/java/org/apache/camel/component/jms/JmsEndpointConfigurationTest.java
Mon Nov 10 06:10:16 2008
>>> @@ -22,9 +22,12 @@
>>>
>>>  import org.apache.activemq.ActiveMQConnectionFactory;
>>>  import org.apache.camel.CamelContext;
>>> +import org.apache.camel.CamelException;
>>>  import org.apache.camel.ContextTestSupport;
>>>  import org.apache.camel.Exchange;
>>>  import org.apache.camel.Processor;
>>> +import org.apache.camel.ResolveEndpointFailedException;
>>> +import org.springframework.jms.connection.UserCredentialsConnectionFactoryAdapter;
>>>  import org.springframework.jms.core.JmsOperations;
>>>  import org.springframework.jms.core.JmsTemplate;
>>>  import org.springframework.jms.listener.AbstractMessageListenerContainer;
>>> @@ -52,6 +55,31 @@
>>>         JmsEndpoint endpoint = (JmsEndpoint) resolveMandatoryEndpoint("jms:topic:Foo.Bar?durableSubscriptionName=James&clientId=ABC");
>>>         assertDurableSubscriberEndpointIsValid(endpoint);
>>>     }
>>> +
>>> +    public void testSetUsernameAndPassword() throws Exception {
>>> +        JmsEndpoint endpoint = (JmsEndpoint) resolveMandatoryEndpoint("jms:topic:Foo.Bar?username=James&password=ABC");
>>> +        ConnectionFactory cf = endpoint.getConfiguration().getConnectionFactory();
>>> +        assertNotNull("The connectionFactory should not be null", cf);
>>> +        assertTrue("The connectionFactory should be the instance of UserCredentialsConnectionFactoryAdapter",
>>> +                   cf instanceof UserCredentialsConnectionFactoryAdapter);
>>> +    }
>>> +
>>> +    public void testNotSetUsernameOrPassword() {
>>> +        try {
>>> +            JmsEndpoint endpoint = (JmsEndpoint) resolveMandatoryEndpoint("jms:topic:Foo.Bar?username=James");
>>> +            fail("Expect the exception here");
>>> +        } catch (ResolveEndpointFailedException exception) {
>>> +            // Expect the exception here
>>> +        }
>>> +
>>> +        try {
>>> +            JmsEndpoint endpoint = (JmsEndpoint) resolveMandatoryEndpoint("jms:topic:Foo.Bar?password=ABC");
>>> +            fail("Expect the exception here");
>>> +        } catch (ResolveEndpointFailedException exception) {
>>> +            // Expect the exception here
>>> +        }
>>> +
>>> +    }
>>>
>>>     public void testSelector() throws Exception {
>>>         JmsEndpoint endpoint = (JmsEndpoint) resolveMandatoryEndpoint("jms:Foo.Bar?selector=foo%3D'ABC'");
>>>
>>>
>>>
>>
>>
>>
>
>



-- 

/Claus Ibsen
Apache Camel Committer
Blog: http://davsclaus.blogspot.com/

Mime
View raw message