[ 
https://issues.apache.org/jira/browse/QPID-2834?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12906859#action_12906859
 ] 

Robbie Gemmell commented on QPID-2834:
--------------------------------------

There is a large amount of reformatting contained in the patch. Whilst I 
appreciate this was likely automated and seperating it would be desirable, the 
only bit I actually have an issue with is this:

    * /*properties.getHeaders().processOverElements(
    * new FieldTable.FieldTableElementProcessor()
    * {
      -
    * public boolean processElement(String propertyName, AMQTypedValue value)
    * {
    * Object val = value.getValue();
    * if(val instanceof AMQShortString)
    * { - val = val.toString(); - }
    * appHeaders.put(propertyName, val);
    * return true;
    * }
      -
    * public Object getResult()
    * { - return appHeaders; - }
    * });
      -
      -
    * messageProps.setApplicationHeaders(appHeaders);
      -*/
      + /*
      + * properties.getHeaders().processOverElements( new
      + * FieldTable.FieldTableElementProcessor() { public boolean
      + * processElement(String propertyName, AMQTypedValue value)
      Unknown macro: { Object+ * val = value.getValue(); if(val instanceof 
AMQShortString) { val = + * val.toString(); } appHeaders.put(propertyName, 
val); return true;+ * }
      public Object getResult() { return appHeaders; } });
      + * messageProps.setApplicationHeaders(appHeaders);
      + */

If the commented out code isnt of any use it should just be deleted.


When logging the subscription state change, the patch uses _logActor,
+ _logActor.message(_logSubject, 
SubscriptionMessages.STATE(_state.get().toString()));

The matching line in SubscriptionImpl uses CurrentActor.get() instead (probably 
because it may convey the queue whos async delivery thread caused the state 
state change, instead of the subscriptions own flusher). This applies to all of 
the logging in the Subscription's and they should be updated.

Additionally, when checking if the subscription CREATE message is enabled as 
such:
+ if (_logActor.getRootMessageLogger().isMessageEnabled(_logActor, this, 
this.getClass().getCanonicalName()))

it is not the class name that should be supplied for the log hierarchy, but the 
heirarchy associated with each individual log message (in the generated files). 
The example in SubscriptionImpl can be copied directly.

> Implement subscriptions (SUB) operational logging on 0-10 
> ----------------------------------------------------------
>
>                 Key: QPID-2834
>                 URL: https://issues.apache.org/jira/browse/QPID-2834
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Java Broker
>    Affects Versions: 0.7
>            Reporter: Sorin Suciu
>             Fix For: 0.7
>
>         Attachments: qpid-2834.patch
>
>
> This is a subset of Qpid-2801 dealing only with subscriptions. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org

Reply via email to