[
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:[email protected]
