qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Martin Ritchie (JIRA)" <qpid-...@incubator.apache.org>
Subject [jira] Commented: (QPID-1440) [Java Client] - Tidy up tasks from QPID-1289
Date Thu, 08 Oct 2009 12:42:31 GMT

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

Martin Ritchie commented on QPID-1440:
--------------------------------------

There are 6 points classes with change requests:

AMQSession.java : DONE
 - Variable Rename : 
AMQConnection.java: DONE
 - Variable was protected to be reused in XAConnectionImpl.. but we have a getter. Swapped
to use getter, made variable private. No Setter is needed
ClientProperties.java:  DONE
- Set Prefetch Default to 500
XAConnectionImpl.java: DONE
- Now that code uses getter for _maxPrefetch it doesn't need to be in the XAConnectionImpl
added createXASession() that defines the default prefetch for that session. 
StreamMessageTest.java: NOT CHANGED
- True we do not care about the prefetch values but we care about setting the FieldTable argument.
This feature is used so rarely .. as in only for HeadersExchange, I don't think it is worth
expanding the qpid.jms interface and having a new method, and the implied functionality, 
we must continue to support. 
ReturnUnroutableMandatoryMessageTest.java :  NOT CHANGE
Same comment as above.



> [Java Client] - Tidy up tasks from QPID-1289
> --------------------------------------------
>
>                 Key: QPID-1440
>                 URL: https://issues.apache.org/jira/browse/QPID-1440
>             Project: Qpid
>          Issue Type: Sub-task
>          Components: Java Client
>    Affects Versions: M4
>            Reporter: Rob Godfrey
>            Assignee: Martin Ritchie
>            Priority: Minor
>
> Review points from QPID-1289:
> AMQSession.java : _defaultPrefetchHighMark; _defaultPrefetchLowMark; should probably
have the word "default" removed from their name
> AMQConnection.java: protected int _maxPrefetch; ... protected member varibales are (in
general) evil... what is the justification for this over using getters/setters?
> ClientProperties.java: + public static final String MAX_PREFETCH_DEFAULT = "5000"; What
is the reason for making the already large (1000) default even larger? The JIRA text above
says make it 500
> XAConnectionImpl.java: + return _delegate.createXASession(_maxPrefetch, _maxPrefetch
/ 2); Why put this logic in the XAConnectionImpl? Surely we should have a method on the underlying
connection which takes no arguments - and it can work out the default prefetch values?
> StreamMessageTest.java:
> - consumerSession.createConsumer(queue, AMQSession.DEFAULT_PREFETCH_LOW_MARK,
> - AMQSession.DEFAULT_PREFETCH_HIGH_MARK, false, false, (String) null, ft);
> + consumerSession.createConsumer(queue, Integer.parseInt(ClientProperties.MAX_PREFETCH_DEFAULT),
Integer.parseInt(ClientProperties.MAX_PREFETCH_DEFAULT), false, false, (String) null, ft);
> This seems like a change in the test... what it really seems to suggest is that there
should be a "createConsumer" method where you don't have to pass this in... since we clearly
do not care about it in this instance, and we are having to repeat code that should be elsewhere
(getting the defaults from client properties)
> ReturnUnroutableMandatoryMessageTest.java : same comment as for Stream message test

-- 
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-subscribe@qpid.apache.org


Mime
View raw message