[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15224332#comment-15224332
 ] 

ASF subversion and git services commented on NIFI-1686:
---

Commit b531b97a4da2fc35b16e5bed0d24028865383ee4 in nifi's branch 
refs/heads/master from [~steveyh25]
[ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=b531b97 ]

NIFI-1686 - NiFi is unable to populate over 1/4 of AMQP properties from flow 
properties


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>Assignee: Oleg Zhurakousky
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15224201#comment-15224201
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user asfgit closed the pull request at:

https://github.com/apache/nifi/pull/305


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15224195#comment-15224195
 ] 

ASF subversion and git services commented on NIFI-1686:
---

Commit e02c79975ed8db69e63d96a28e81db08bc869e54 in nifi's branch 
refs/heads/master from [~steveyh25]
[ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=e02c799 ]

NIFI-1686 - NiFi is unable to populate over 1/4 of AMQP properties from flow 
properties
This closes #305


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15224186#comment-15224186
 ] 

ASF subversion and git services commented on NIFI-1686:
---

Commit b531b97a4da2fc35b16e5bed0d24028865383ee4 in nifi's branch 
refs/heads/master from [~steveyh25]
[ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=b531b97 ]

NIFI-1686 - NiFi is unable to populate over 1/4 of AMQP properties from flow 
properties


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15215933#comment-15215933
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202872584
  
@steveyh25 

I am ok with NFE. Actually if you do some research it appears to behave a 
bit better then other approaches that may look better on the surface. I've 
added a small comment, otherwise it looks good. The only thing I'd like to ask 
is to add some tests (to validate both success and failure) conditions. Even 
the test that you modified originally just add assertions that those properties 
were indeed set since that is the logic that was modified.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15215930#comment-15215930
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57714484
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AMQPUtils.java
 ---
@@ -141,4 +142,101 @@ private static String 
extractPropertyNameFromMethod(Method method) {
 c[0] = Character.toLowerCase(c[0]);
 return AMQP_PROP_PREFIX + new String(c);
 }
+
+/**
+ * Will validate if provided amqpPropValue can be converted to a 
{@link Map}.
+ *
+ * @param amqpPropValue
+ *the value of the property
+ * @return {@link Map} if valid otherwise null
+ */
+public static Map validateAMQPHeaderProperty(String 
amqpPropValue){
+String[] strEntries = amqpPropValue.split(",");
+Map headers = new HashMap<>();
+for (String strEntry : strEntries) {
+String[] kv = strEntry.split("=");
+if (kv.length == 2) {
+headers.put(kv[0].trim(), kv[1].trim());
+} else {
+logger.warn("Malformed key value pair for AMQP header 
property: " + amqpPropValue);
+}
+}
+
+return headers;
+}
+
+/**
+ * Will validate if provided amqpPropValue can be converted to an 
{@link Integer}, and that its
+ * value is 1 or 2.
+ *
+ * @param amqpPropValue
+ *the value of the property
+ * @return {@link Integer} if valid otherwise null
+ */
+public static Integer validateAMQPDeliveryModeProperty(String 
amqpPropValue){
+Integer deliveryMode = null;
+
+try {
+deliveryMode = Integer.parseInt(amqpPropValue);
+} catch (NumberFormatException aE){
+//we will deal with the error below instead of having 
duplicate logger code
+}
--- End diff --

Consider ```private static Integer toInt(String strVal)``` method, since 
the same logic is repeated in the next one. Just trying to avoid duplication as 
much as possible.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15215106#comment-15215106
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202630454
  
@olegz,

Please see latest commit. The try-catch block has been removed and I've 
changed the validation slightly like was spoken about. It appears as though we 
don't need to worry about NPE, because if no value is provided for the flow 
attribute then it appears to be ignored anyway (i.e it is not a part of 
flowFile.getAttributes()).

I couldn't find any great way of dealing with the parsing of values other 
than catching the NumberFormatException - things like StringUtils.isNumeric() 
are nice but we can still be in a situation where a number is deemed to be 
numeric but parseInt will throw an exception anyway because the number's too 
big to store - I hope this solution is ok, unless there is a specific way of 
validating this that you would like?

I've made specific warnings occur if a property fails validation, including 
out of range values.

Let me know what you think,
Thanks


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15214226#comment-15214226
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202407396
  
@trkurc yep, that's the plan once we figure it out


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15214223#comment-15214223
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user trkurc commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202406795
  
@olegz - if you work out the intellij problems, try to ensure what  the fix 
was is memorialized on the wiki or dev list


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15214105#comment-15214105
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57563219
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
+break;
+case CORRELATION_ID:
+builder.correlationId(amqpPropValue);
+break;
+case REPLY_TO:
+builder.replyTo(amqpPropValue);
+break;
+case EXPIRATION:
+builder.expiration(amqpPropValue);
+break;
+case MESSAGE_ID:
+builder.messageId(amqpPropValue);
+break;
+case TIMESTAMP:
+builder.timestamp(new 
SimpleDateFormat("/MM/dd HH:mm:ss").parse(amqpPropValue));
--- End diff --

Hmm, so you mean Long represented as a String coming in, then converted 
back to Long and then Timestamp. If so that would be ideal.


> NiFi is unable to populate over 1/4 of AMQP properties from flow 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213615#comment-15213615
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202140044
  
@steveyh25 yes


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213606#comment-15213606
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202131835
  
@olegz you mean your e-mail?


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213578#comment-15213578
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202126065
  
@steveyh25 ping me at 'oleg at hortonworks' for the ide part. 


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213577#comment-15213577
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user petmit commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57533244
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

Yes, I agree. Probably not the most appropriate way to handle headers.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213562#comment-15213562
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202114411
  
@petmit thanks for that, will take a look and compare
@olegz I know this isn't the place to ask but I cannot get the 
nifi-ide-integration working with intelliJ - I am not sure where to add the 
real nifi sources to and it cannot find the main class to launch. Any help 
would be appreciated.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213561#comment-15213561
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57532375
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
+break;
+case CORRELATION_ID:
+builder.correlationId(amqpPropValue);
+break;
+case REPLY_TO:
+builder.replyTo(amqpPropValue);
+break;
+case EXPIRATION:
+builder.expiration(amqpPropValue);
+break;
+case MESSAGE_ID:
+builder.messageId(amqpPropValue);
+break;
+case TIMESTAMP:
+builder.timestamp(new 
SimpleDateFormat("/MM/dd HH:mm:ss").parse(amqpPropValue));
--- End diff --

From looking at the patch from @petmit would using a long not be the ideal 
thing for this? When sending a date through the builder and from viewing the 
header on the message queue it is indeed encoded as 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213558#comment-15213558
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57532227
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

@petmit would this not potentially cause a confusing issue where a flow 
tries to send a header with a key of say "userId", and it is wanted in the 
headers but actually populates the userId property? It also means that AMQP 
properties essentially become "keywords" that cannot ever be used in the 
headers - which doesn't seem ideal


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213557#comment-15213557
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user petmit commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202110923
  
I've been working on a patch to address these same issues. I've pushed my 
current progress to 
https://github.com/petmit/nifi/commit/de08d2e8966cf08a4b0b1f625fcc6f1918da5d71 
if you want to take a look. Feel free to incorporate anything that may be 
useful in to this pull request. As is apparent from this discussion, there is 
still some work to be done.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213549#comment-15213549
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531950
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
+break;
+case CORRELATION_ID:
+builder.correlationId(amqpPropValue);
+break;
+case REPLY_TO:
+builder.replyTo(amqpPropValue);
+break;
+case EXPIRATION:
+builder.expiration(amqpPropValue);
+break;
+case MESSAGE_ID:
+builder.messageId(amqpPropValue);
+break;
+case TIMESTAMP:
+builder.timestamp(new 
SimpleDateFormat("/MM/dd HH:mm:ss").parse(amqpPropValue));
--- End diff --

Yeah, as I explained before try/catch was primarily for Reflection so 
indeed it could be removed.
Naive is probably the wrong word. We definitely need it and the above 
format is a very common format and 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213548#comment-15213548
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531906
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
--- End diff --

@petmit That is correct and once again explains why we omit it during the 
initial impl. 
@steveyh25 the original try/catch was there to specifically deal with 
Reflection exceptions and not as catch all (although it kind of became a 
side-effect). In any event IMHO it's more appropriate to provide proper 
validation then rely on validation via exception logging. 


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213547#comment-15213547
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202107378
  
@steveyh25 Don't sweat it on the tone remark. All good here, just wanted to 
mention it as some may be more sensitive then others ;)


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213546#comment-15213546
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531819
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

@steveyh25 if the format is incomplete I am assuming the splitting array 
would only had one element. So I'd say encapsulate this logic into a private 
method and make consider this scenario so the appropriate IF is there. Also, 
making it a private method will pave the way to eventually extract it into some 
utility class in _nifi-util_ 
@petmit +1. I personally like that.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213544#comment-15213544
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user petmit commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531786
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
--- End diff --

Certain properties also have a limited set of valid values. DELIVERY_MODE 
needs to be set to either 1 (for non-persistent messages) or 2 (for persistent 
messages). PRIORITY i expected to be set to a value of a number in the range 
0-9. 


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213542#comment-15213542
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user petmit commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531722
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

Another option is to consider all attributes with the 'amqp$' prefix which 
aren't valid AMQP properties as candidates for property headers. E.g. a 
FlowFile attribute 'amqp$attr=value' would be added to the headers map as 
'attr=value'.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213533#comment-15213533
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531501
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
+break;
+case CORRELATION_ID:
+builder.correlationId(amqpPropValue);
+break;
+case REPLY_TO:
+builder.replyTo(amqpPropValue);
+break;
+case EXPIRATION:
+builder.expiration(amqpPropValue);
+break;
+case MESSAGE_ID:
+builder.messageId(amqpPropValue);
+break;
+case TIMESTAMP:
+builder.timestamp(new 
SimpleDateFormat("/MM/dd HH:mm:ss").parse(amqpPropValue));
--- End diff --

1 & 2: These errors will be caught by the try/catch block, so if we are 
going to fully validate each individual one the then we could remove the 
try/catch? That seems like a lot more code for not-a-lot 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213526#comment-15213526
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202099268
  
Hi @olegz,

Thanks for your feedback. It certainly wasn't my intention in the original 
bug report to come across as having a bad tone. From the comment in the code 
and the fact that the test was only testing the contentType property I thought 
it may have been an oversight, rather than an intentional limitation - my 
apologies.

I have added some thoughts to your comments,


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213525#comment-15213525
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531320
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
--- End diff --

This can be done, but any errors that occur within the block will be caught 
and log "Failed while trying to build AMQP Property" warning instead - is this 
not good enough?


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213524#comment-15213524
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user steveyh25 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57531275
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

This seems more appropriate. What should happen if the format is 
incomplete? i.e [key1=value1,key2=]. In the committed code I was making sure 
that a key and a value existed before populating the headers - because there's 
no point in sending a header without a value - it may as well not exist?


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213514#comment-15213514
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57530817
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
+break;
+case CORRELATION_ID:
+builder.correlationId(amqpPropValue);
+break;
+case REPLY_TO:
+builder.replyTo(amqpPropValue);
+break;
+case EXPIRATION:
+builder.expiration(amqpPropValue);
+break;
+case MESSAGE_ID:
+builder.messageId(amqpPropValue);
+break;
+case TIMESTAMP:
+builder.timestamp(new 
SimpleDateFormat("/MM/dd HH:mm:ss").parse(amqpPropValue));
--- End diff --

I see that the assumption here is that String representation of Date will 
always come in "/MM/dd HH:mm:ss". There are several issues here:
1. Similar to DELIVERY_MODE and PRIORITY, the value could be 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213512#comment-15213512
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57530762
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
+case DELIVERY_MODE:
+
builder.deliveryMode(Integer.parseInt(amqpPropValue));
+break;
+case PRIORITY:
+
builder.priority(Integer.parseInt(amqpPropValue));
--- End diff --

Consider adding null-value check as well as number check to avoid both NPE 
and NFE. Applies to both DELIVERY_MODE and PRIORITY.


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213511#comment-15213511
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57530736
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
+case CONTENT_TYPE:
+builder.contentType(amqpPropValue);
+break;
+case CONTENT_ENCODING:
+builder.contentEncoding(amqpPropValue);
+break;
+case HEADERS:
+String[] s = amqpPropValue.split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER);
+Map headers = new 
HashMap<>();
+
+for (int i = 0; i < s.length ; i += 2){
+if (i + 2 <= s.length){
+headers.put(s[i], s[i + 1]);
+}
+}
+
+builder.headers(headers);
+break;
--- End diff --

The reason for using '$' as a delimiter was to distinguish AMQP specific 
property names from other attributes, so 'amqp$' denotes that what follows is 
AMQP-specific property. For AMQP header values (Map) I would 
prefer to use common toString Map notation (i.e., KEY=VALUE,KEY=VALUE). I 
just feel it's more natural and intuitive then using '$'. 
```
String[] strEntries = amqpPropValue.split(",");
Map headers = new HashMap<>();
for (String strEntry : strEntries) {
String[] kv = strEntry.split("=");
headers.put(kv[0].trim(), kv[1].trim());
}
```
Thoughts?


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question 

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213510#comment-15213510
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57530690
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/PublishAMQP.java
 ---
@@ -195,29 +193,85 @@ public void process(final InputStream in) throws 
IOException {
  * Extracts AMQP properties from the {@link FlowFile} attributes. 
Attributes
  * extracted from {@link FlowFile} are considered candidates for AMQP
  * properties if their names are prefixed with
- * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., amqp$contentType=text/xml)
+ * {@link AMQPUtils#AMQP_PROP_PREFIX} (e.g., 
amqp$contentType=text/xml).
+ * The header property is an exception and requires a {@link Map}, so 
should
+ * be passed in the following format: amqp$headers=key$value$key$value 
etc.
  */
 private BasicProperties extractAmqpPropertiesFromFlowFile(FlowFile 
flowFile) {
 Map attributes = flowFile.getAttributes();
 AMQP.BasicProperties.Builder builder = new 
AMQP.BasicProperties.Builder();
 for (Entry attributeEntry : attributes.entrySet()) 
{
 if 
(attributeEntry.getKey().startsWith(AMQPUtils.AMQP_PROP_PREFIX)) {
-String amqpPropName = attributeEntry.getKey().split("\\" + 
AMQPUtils.AMQP_PROP_DELIMITER)[1];
+String amqpPropName = attributeEntry.getKey();
 String amqpPropValue = attributeEntry.getValue();
-try {
-if 
(amqpPropertyNames.contains(AMQPUtils.AMQP_PROP_PREFIX + amqpPropName)) {
-Method m = 
builder.getClass().getDeclaredMethod(amqpPropName, String.class);
-m.invoke(builder, amqpPropValue);
-} else {
-getLogger().warn("Unrecogninsed AMQP property '" + 
amqpPropName + "', will ignore.");
+
+AMQPUtils.PropertyNames propertyNames = 
AMQPUtils.PropertyNames.fromValue(amqpPropName);
+
+if (propertyNames != null) {
+try {
+switch (propertyNames){
--- End diff --

Definitely a step in the right direction. . .


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213508#comment-15213508
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on a diff in the pull request:

https://github.com/apache/nifi/pull/305#discussion_r57530669
  
--- Diff: 
nifi-nar-bundles/nifi-amqp-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/AMQPUtils.java
 ---
@@ -43,16 +41,48 @@
 
 private final static Logger logger = 
LoggerFactory.getLogger(AMQPUtils.class);
 
-private final static List propertyNames = 
Arrays.asList("amqp$contentType", "amqp$contentEncoding",
-"amqp$headers", "amqp$deliveryMode", "amqp$priority", 
"amqp$correlationId", "amqp$replyTo",
-"amqp$expiration", "amqp$messageId", "amqp$timestamp", 
"amqp$type", "amqp$userId", "amqp$appId",
-"amqp$clusterId");
-/**
- * Returns a {@link List} of AMQP property names defined in
- * {@link BasicProperties}
- */
-public static List getAmqpPropertyNames() {
-return propertyNames;
+public enum PropertyNames {
--- End diff --

+1 here, primarily due to the fact that now individual properties 
(especially non-Sttring-value type) may need additional logic to help with 
conversion and therefore would need to be individually referenced in several 
places.  


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213504#comment-15213504
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

Github user olegz commented on the pull request:

https://github.com/apache/nifi/pull/305#issuecomment-202095259
  
@steveyh25 

Thank you so much for your contribution. AMQP support is one of the most 
recent additions to NiFi and it's great to see its adoption is on the way. 
What's even more admirable is to see the 
members (such as yourself) of the community are eager to help making it 
even better by addressing some of the limitations these new components may have.

That said, keep in mind that certain limitations (especially in new 
components) may be there intentionally (as in this case). In other words this 
initial version of AMQP was not supporting _non-String-value-type_ properties 
since we wanted to give more thoughts as to the conversion strategies of such 
values to/from String given that NiFi attributes only support String as a value 
type. I'll comment more on this inline at the relevant parts of the code, but 
wanted to make sure that it is clear that such behavior was intentional and not 
a bug. I'll update JIRA classification accordingly as well. 

Also, instead of saying "1/4" consider listing _timestamp, deliveryMode, 
headers, priority_ individually. The main point here is that one only cares 
about properties he/she needs and if the other 3/4 satisfy those needs then all 
is good while it may not be all good for others who need one of the unsupported 
properties. So the emphasis here should be on specific property rather then 
1/4. 

Last but not least, consider the tone and basic ethics next time you 
interact in a public forum (JIRA/Github etc.). When writing things like "the 
author of the code mustn't have. . ." keep in mind that 
you can not possibly know _what_, _where_ and _how_ in relation to the 
"author's" _intent_, _thoughts_ and/or _motivation_ unless you had direct 
interaction with him/her which I'll assume you didn't. Without such interaction 
these words become pure speculation which personalizes the issue and therefore 
counter productive. See more here: 
http://www.apache.org/foundation/policies/conduct.html

That said, please see the comments in line as I go through the review.

Once again thank you for you contribution!


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213188#comment-15213188
 ] 

ASF GitHub Bot commented on NIFI-1686:
--

GitHub user steveyh25 opened a pull request:

https://github.com/apache/nifi/pull/305

NIFI-1686 - NiFi is unable to populate over 1/4 of AMQP properties from 
flow properties

I've changed propertyNames into an enum so that we can refer to their 
values multiple times properly instead of hard-coding string values all over 
the place. The enum includes a lookup map that is built by a static block so we 
can get the enum "by value" efficiently.

I've removed the reflective calls and so am using a setter for each 
property type - along with fixes for the 4 properties that had issues in the 
bug. Multiple headers can be passed in as follows: 
amqp$headers=foo$bar$foo2$bar2$foo3$bar3 etc. and the code will build the Map 
parameter that the Builder requires.

Removed the now unnecessary variable amqpPropertyNames, and getter 
getAmqpPropertyNames.

Fixed some spelling mistakes, amended doc comments, removed unused imports, 
made the test more robust by testing all of the properties instead of just 
contentType.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/steveyh25/nifi NIFI-1686

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/nifi/pull/305.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #305


commit c686a11cbfb2256bbd645dc78902c730f46d8628
Author: Stephen Harper 
Date:   2016-03-26T20:06:50Z

NIFI-1686 - NiFi is unable to populate over 1/4 of AMQP properties from 
flow properties




> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---
>
> Key: NIFI-1686
> URL: https://issues.apache.org/jira/browse/NIFI-1686
> Project: Apache NiFi
>  Issue Type: Bug
>  Components: Core Framework
>Affects Versions: 0.5.1
>Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that 
> forwards on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP 
> uses the method extractAmqpPropertiesFromFlowFile to populate the AMQP 
> BasicProperties if the flow attributes match a certain format (i.e 
> amqp$contentType=text/xml). 
> The method in question uses reflection to find a matching method name from 
> the AMQP.BasicProperties class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however 
> there are some that don't (specifically, headers takes a Map, 
> deliveryMode and priority take Integer, and timestamp takes a Date), and it 
> is impossible to populate these values because the invocation assumes a 
> String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really 
> never happen since it should be caught by the above IF" - however the author 
> of the code mustn't have tested all cases because this error is consistently 
> present when trying to forward flow attributes in over a quarter of the 
> available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)