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, <[EMAIL PROTECTED]> 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/
