[ 
https://issues.apache.org/activemq/browse/SM-220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_38904
 ] 

Jamie Goodyear commented on SM-220:
-----------------------------------


 Hi All,

  I've reviewed the collection of minor issues described in this
 JIRA entry and have prepared a patch to resolve the remaining
 out standing issues (SM-220-fixes.patch). 

 Cheers,
 Jamie

 Issues 1,2,3: Corrected typos.
 Issue 4: Leaving getTransactionManager as public method.
 Issue 5: This was corrected in a previous revision.
 Issue 6: This was corrected in a previous revision.
 Issue 7: Modified second logging message to capture e1 message.
 Issue 8: This was corrected in a previous revision.


 Files Modified:

 
core/servicemix-core/src/main/java/org/apache/servicemix/jbi/messaging/ExchangePacket.java
 
core/servicemix-core/src/main/java/org/apache/servicemix/jbi/messaging/MessageExchangeImpl.java
 
core/servicemix-core/src/main/java/org/apache/servicemix/jbi/container/JBIContainer.java

> A number of minor code review issues
> ------------------------------------
>
>                 Key: SM-220
>                 URL: https://issues.apache.org/activemq/browse/SM-220
>             Project: ServiceMix
>          Issue Type: Bug
>          Components: servicemix-core
>    Affects Versions: 2.0.2
>            Reporter: Peter Smith
>            Priority: Minor
>
> #1 ExhchangePacket - typo
>       * @return the proerty from the exchange
> =====
> #2 MessageExhangeImpl - typo
>       * @return the proerty from the exchange
> =====
> #3 JBIContainer - typo
>       * light weight initialization - default values for mbeanSErver, 
> TransactionManager etc are null
> =====
> #4 JCAFlow - method modifier
>       public TransactionManager getTransactionManager() {
> why is this method public?
> =====
> #5 ManagementContext - member default value
>       private String jndiPath = "/jmxconnector"; 
> Isn't this value unconditionally overwritten every time during init?
> =====
> #6 SedaFlow - Use of JTA_TRANSACTION_NAME
> In doSend instead of:
>       if (me.getProperty(MessageExchange.JTA_TRANSACTION_PROPERTY_NAME) == 
> null &&...
> could say:
>       if (!me.isTransacted() &&...
>       
> =====
> #7 JBIContainer - getTransactionManager
> I think the 2nd JNDI lookup attempt is logging the wrong message if it fails.
>       log.debug("No  transaction manager found from naming context", e);
>       
> Shouldn't that be:
>       log.debug("No  transaction manager found from naming context", e1); // 
> Note: e1 instead of e            
> ======        
> #8 MessageExchangeImpl - setMessage
> There seems inefficient checking of String name. (eg if IN.equals(name) is 
> true no need to keep testing name)
>         if (IN.equals(name) && !can(CAN_SET_IN_MSG)) {
>             throw new MessagingException("In not supported");
>         }
>         if (OUT.equals(name) && !can(CAN_SET_OUT_MSG)) {
>             throw new MessagingException("Out not supported");
>         }
>         if (FAULT.equals(name) && !can(CAN_SET_FAULT_MSG)) {
>             throw new MessagingException("Fault not supported");
>         }
>         if (FAULT.equals(name) && !(message instanceof Fault)) {
>             throw new MessagingException("Setting fault, but message is not a 
> fault");
>         }
>         
> =======
> [END]

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

Reply via email to