You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2019/04/02 05:46:25 UTC
[GitHub] [nifi] ijokarumawak commented on a change in pull request #3246:
NIFI-5929 Support for IBM MQ multi-instance queue managers
ijokarumawak commented on a change in pull request #3246: NIFI-5929 Support for IBM MQ multi-instance queue managers
URL: https://github.com/apache/nifi/pull/3246#discussion_r271139389
##########
File path: nifi-nar-bundles/nifi-jms-bundle/nifi-jms-processors/src/main/java/org/apache/nifi/jms/cf/JMSConnectionFactoryProvider.java
##########
@@ -184,55 +182,70 @@ public void disable() {
* invoked to set the corresponding property to a value provided by during
* service configuration. For example, 'channel' property will correspond to
* 'setChannel(..) method and 'queueManager' property will correspond to
- * setQueueManager(..) method with a single argument.
+ * setQueueManager(..) method with a single argument. The bean convention is also
+ * explained in user manual for this component with links pointing to
+ * documentation of various ConnectionFactories.
* <p>
* There are also few adjustments to accommodate well known brokers. For
* example ActiveMQ ConnectionFactory accepts address of the Message Broker
- * in a form of URL while IBMs in the form of host/port pair (more common).
- * So this method will use value retrieved from the 'BROKER_URI' static
- * property 'as is' if ConnectionFactory implementation is coming from
- * ActiveMQ and for all others (for now) the 'BROKER_URI' value will be
- * split on ':' and the resulting pair will be used to execute
- * setHostName(..) and setPort(..) methods on the provided
- * ConnectionFactory. This may need to be maintained and adjusted to
- * accommodate other implementation of ConnectionFactory, but only for
- * URL/Host/Port issue. All other properties are set as dynamic properties
- * where user essentially provides both property name and value, The bean
- * convention is also explained in user manual for this component with links
- * pointing to documentation of various ConnectionFactories.
+ * in a form of URL while IBMs in the form of host/port pair(s).
+ * <p>
+ * This method will use the value retrieved from the 'BROKER_URI' static
+ * property as is. An exception to this if ConnectionFactory implementation
+ * is coming from IBM MQ and connecting to a stand-alone queue manager. In
+ * this case the Broker URI is expected to be entered as a colon separated
+ * host/port pair, which then is split on ':' and the resulting pair will be
+ * used to execute setHostName(..) and setPort(..) methods on the provided
+ * ConnectionFactory.
+ * <p>
+ * This method may need to be maintained and adjusted to accommodate other
+ * implementation of ConnectionFactory, but only for URL/Host/Port issue.
+ * All other properties are set as dynamic properties where user essentially
+ * provides both property name and value.
*
- * @see #setProperty(String, String) method
+ * @see <a href="http://activemq.apache.org/maven/apidocs/org/apache/activemq/ActiveMQConnectionFactory.html#setBrokerURL-java.lang.String-">setBrokerURL(String brokerURL)</a>
+ * @see <a href="https://docs.tibco.com/pub/enterprise_message_service/8.1.0/doc/html/tib_ems_api_reference/api/javadoc/com/tibco/tibjms/TibjmsConnectionFactory.html#setServerUrl(java.lang.String)">setServerUrl(String serverUrl)</a>
+ * @see <a href="https://www.ibm.com/support/knowledgecenter/en/SSFKSJ_7.1.0/com.ibm.mq.javadoc.doc/WMQJMSClasses/com/ibm/mq/jms/MQConnectionFactory.html#setHostName_java.lang.String_">setHostName(String hostname)</a>
+ * @see <a href="https://www.ibm.com/support/knowledgecenter/en/SSFKSJ_7.1.0/com.ibm.mq.javadoc.doc/WMQJMSClasses/com/ibm/mq/jms/MQConnectionFactory.html#setPort_int_">setPort(int port)</a>
+ * @see <a href="https://www.ibm.com/support/knowledgecenter/en/SSFKSJ_7.1.0/com.ibm.mq.javadoc.doc/WMQJMSClasses/com/ibm/mq/jms/MQConnectionFactory.html#setConnectionNameList_java.lang.String_">setConnectionNameList(String hosts)</a>
+ * @see #setProperty(String propertyName, Object propertyValue)
*/
- private void setConnectionFactoryProperties(ConfigurationContext context) {
- for (final Entry<PropertyDescriptor, String> entry : context.getProperties().entrySet()) {
- PropertyDescriptor descriptor = entry.getKey();
- String propertyName = descriptor.getName();
- if (descriptor.isDynamic()) {
- this.setProperty(propertyName, entry.getValue());
+ void setConnectionFactoryProperties(ConfigurationContext context) {
+ String brokerValue = context.getProperty(BROKER_URI).evaluateAttributeExpressions().getValue();
+ String connectionFactoryValue = context.getProperty(CONNECTION_FACTORY_IMPL).evaluateAttributeExpressions().getValue();
+ if (connectionFactoryValue.startsWith("org.apache.activemq")) {
+ this.setProperty("brokerURL", brokerValue);
+ } else if (connectionFactoryValue.startsWith("com.tibco.tibjms")) {
+ this.setProperty("serverUrl", brokerValue);
+ } else if (connectionFactoryValue.startsWith("com.ibm.mq.jms")) {
Review comment:
Now, we don't have the else block to handle combination when BROKER_URL is specified with other JMS products. Before this PR, following block handles such situation:
```java
String[] hostPort = brokerValue.split(":");
if (hostPort.length == 2) {
this.setProperty("hostName", hostPort[0]);
this.setProperty("port", hostPort[1]);
} else if (hostPort.length != 2) {
this.setProperty("serverUrl", brokerValue); // for tibco
} else {
throw new IllegalArgumentException("Failed to parse broker url: " + brokerValue);
}
```
Since we won't be able to know if other products have been used other than the 3 above, I think we should add the `else` block at the end of this `else if` block. So that we don't break existing flow configurations in such cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services