[
https://issues.apache.org/jira/browse/QPID-7633?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15839690#comment-15839690
]
Lorenz Quack edited comment on QPID-7633 at 1/26/17 1:49 PM:
-------------------------------------------------------------
WIP review comments:
* {{AMQPSession#getBlocking}} seems to be redundant.
{{Session#isProducerFlowBlocked}} seems to already serve the same purpose.
* Is there a point in re-defining abstract methods in {{AbstractAMQPSession}}
if they are already on the interfaces? Affected (some of which have
{{@Override}} while others do not): {{getChannelId}}, {{getBlocking}},
{{getConsumers}}, {{getConsumerCount}}, {{getTxnRejects}}, {{getTxnCommits}},
{{getTxnStart}}, {{getUnacknowledgedMessageCount}}, {{getEventLogger}},
{{addTicker}}, {{removeTicker}}, {{doTimeoutAction}}, {{getLogSubject}},
{{getTransactionUpdateTimeLong}}, and {{getTransactionStartTimeLong}}.
* Why do we only return {{0}} or {{1}} from
{{AbstractAMQPSession#getLocalTransactionOpen}}. That seems wrong.
* On {{ServerConnection}} we got rid of {{removeSession}} but
{{registerSession}} is still there. I'm just wondering about the asymmetry.
What about {{
* {{Session_1_0#setReceivingChannel}} and {{Session_1_0#setSendingChannel}} can
die and the functionality should move to the constructor.
* I think eventually {{ServerSession}} and {{ServerSessionDelegate}} should
die. I guess that this could potentially be done when we separate the client
from the broker but not now.
* The change to
{{org.apache.qpid.server.protocol.v0_10.Session_0_10#unblock(org.apache.qpid.server.model.Queue<?>)}}
in r1780076 seems to be a serious bug fix that we should backport.
... to be continued
was (Author: lorenz.quack):
review comments:
* {{AMQPSession#getBlocking}} seems to be redundant.
{{Session#isProducerFlowBlocked}} seems to already serve the same purpose.
* Is there a point in re-defining abstract methods in {{AbstractAMQPSession}}
if they are already on the interfaces? Affected (some of which have
{{@Override}} while others do not): {{getChannelId}}, {{getBlocking}},
{{getConsumers}}, {{getConsumerCount}}, {{getTxnRejects}}, {{getTxnCommits}},
{{getTxnStart}}, {{getUnacknowledgedMessageCount}}, {{getEventLogger}},
{{addTicker}}, {{removeTicker}}, {{doTimeoutAction}}, {{getLogSubject}},
{{getTransactionUpdateTimeLong}}, and {{getTransactionStartTimeLong}}.
* Why do we only return {{0}} or {{1}} from
{{AbstractAMQPSession#getLocalTransactionOpen}}. That seems wrong.
* On {{ServerConnection}} we got rid of {{removeSession}} but
{{registerSession}} is still there. I'm just wondering about the asymmetry.
What about {{
* {{Session_1_0#setReceivingChannel}} and {{Session_1_0#setSendingChannel}} can
die and the functionality should move to the constructor.
* I think eventually {{ServerSession}} and {{ServerSessionDelegate}} should
die. I guess that this could potentially be done when we separate the client
from the broker but not now.
* The change to
{{org.apache.qpid.server.protocol.v0_10.Session_0_10#unblock(org.apache.qpid.server.model.Queue<?>)}}
in r1780076 seems to be a serious bug fix that we should backport.
... to be continued
> Remove SessionAdapter
> ---------------------
>
> Key: QPID-7633
> URL: https://issues.apache.org/jira/browse/QPID-7633
> Project: Qpid
> Issue Type: Improvement
> Components: Java Broker
> Reporter: Keith Wall
> Assignee: Lorenz Quack
> Fix For: qpid-java-7.0
>
> Attachments: 0001-QPID-7633-Java-Broker-Remove-SessionAdapter.patch,
> QPID-7633.tar.bz2
>
>
> {{SessionAdapter}} is legacy code that predates the existing model. It
> should be removed, allowing for better code reuse between session
> implementations and better unit/smoke tests.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
