[GitHub] activemq-artemis issue #2256: ARTEMIS-2045 Add support for setting delivery ...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2256
  
@jbertram yes, the broker can add additional message annotations as long as 
those being added follow the rules from the spec, if the user adds something 
that the remote doesn't understand and it is not prefixed with "x-opt" the 
remote can just kill the link.


http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-annotations


---

[GitHub] activemq-artemis pull request #2452: ARTEMIS-1938 Update Qpid JMS and proton...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2452

ARTEMIS-1938 Update Qpid JMS and proton-j to latest

Updates Qpid JMS to v0.39.0 and proton-j to v0.31.0

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1938-0.39.0

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2452.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2452


commit a05df2e59238e3781c921a1e43c4f5f7478cdfb2
Author: Timothy Bish 
Date:   2018-12-06T17:40:49Z

ARTEMIS-1938 Update Qpid JMS and proton-j to latest

Updates Qpid JMS to v0.39.0 and proton-j to v0.31.0




---

[GitHub] activemq-artemis pull request #2448: ARTEMIS-2189 allow deleting temp dest w...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2448#discussion_r239486511
  
--- Diff: 
artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java
 ---
@@ -386,14 +386,27 @@ public void setSimpleAddress(SimpleString address) {
 
public void delete() throws JMSException {
   if (session != null) {
+ ActiveMQSession sessionToUse = session;
+ boolean temporary = false;
  if (session.getCoreSession().isClosed()) {
-// Temporary queues will be deleted when the connection is 
closed.. nothing to be done then!
-return;
+/**
--- End diff --

Looks ok to me now, shouldn't be racing with session close now.


---

[GitHub] activemq-artemis pull request #2448: ARTEMIS-2189 allow deleting temp dest w...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2448#discussion_r238785616
  
--- Diff: 
artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQDestination.java
 ---
@@ -386,14 +386,27 @@ public void setSimpleAddress(SimpleString address) {
 
public void delete() throws JMSException {
   if (session != null) {
+ ActiveMQSession sessionToUse = session;
+ boolean temporary = false;
  if (session.getCoreSession().isClosed()) {
-// Temporary queues will be deleted when the connection is 
closed.. nothing to be done then!
-return;
+/**
--- End diff --

@jbertram This code is still a bit racey in that the session could become 
closed right after checking that it isn't and thereby still allow a violation 
in what the spec says should be possible.  It might make more sense to just use 
the approach of creating a session for this operation each time given this 
isn't something people are going to be doing a great deal and so doesn't need 
to be highly performant.


---

[GitHub] activemq-artemis pull request #2448: ARTEMIS-2189 allow deleting temp dest w...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2448#discussion_r238416943
  
--- Diff: 
tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/TemporaryDestinationTest.java
 ---
@@ -265,6 +266,126 @@ public void testTemporaryQueueDeleted() throws 
Exception {
   }
}
 
+   @Test
+   public void testTemporaryQueueDeletedAfterSessionClosed() throws 
Exception {
--- End diff --

For AMQP the handling of dynamic nodes is tested here:

https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpTempDestinationTest.java
And here:

https://github.com/apache/activemq-artemis/blob/master/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSTemporaryDestinationTest.java

For OpenWire you'd want to look for a test of the RemoveInfo command that 
carries a DestintionInfo object.

Other tests that are specifically targeted the JMS spec (or other) 
compliant behaviour of a client really belong to the project that maintains the 
client in question.


---

[GitHub] activemq-artemis pull request #2433: ARTEMIS-1938 Update proton-j to 0.30.0 ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2433

ARTEMIS-1938 Update proton-j to 0.30.0 and Qpid JMS 0.37.0

Update to latest proton-j release and refactor the dispostion code to use
the new type enums to better deal with the dispistions.  Updates to Qpid JMS
0.37.0 which still uses the current netty 4.1.28.Final dependency.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1938-0.30.0

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2433.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2433


commit ec35e9c512ca247866f3716f2b86826aafdab576
Author: Timothy Bish 
Date:   2018-11-14T21:17:00Z

ARTEMIS-1938 Update proton-j to 0.30.0 and Qpid JMS 0.37.0

Update to latest proton-j release and refactor the dispostion code to use
the new type enums to better deal with the dispistions.  Updates to Qpid JMS
0.37.0 which still uses the current netty 4.1.28.Final dependency.




---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
@michaelandrepearce I agree, that expiry bit qualifies as a bug and should 
also be fixed to disallow modification


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
I'd say none of them are really the right way to go.  I'd suggest your 
organization create a custom broker plugin that does whatever protocol 
violating things are acceptable to you and your team and not allow for the 
inevitable issues that arise from violating the specification from being 
dropped directly into the broker.  You could also (if you really care about the 
message user id no matching) reject incoming messages there are close the 
connection of the offending client.  


---

[GitHub] activemq-artemis issue #2423: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2423
  
-1

This is actually worse that before because now you've not only violated the 
AMQP spec in regards to changing the User ID portion of the properties section 
but also opened it up to even more spec abuse and probably lost fidelity on the 
message such that what was sent will not be faithfully reproduced to an AMQP 
consumer reading the message. 


---

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2422
  
@michaelandrepearce I may have worked due to a bug that allowed the broker 
to violate the AMQP specification but was fixed later 
(https://issues.apache.org/jira/browse/ARTEMIS-1092) and should continue to not 
allow the broker to violate the AMQP 1.0 specification. 


---

[GitHub] activemq-artemis issue #2422: ARTEMIS-2168 Fix "populate-validated-user" fea...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2422
  
-1

The properties section of an AMQP message is immutable and cannot be 
changed by the broker as per the AMQP 1.0 specification (Section 3.2)


http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#section-message-format



---

[GitHub] activemq-artemis issue #2383: NO-JIRA Adding a test for ARTEMIS-2096

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2383
  
This isn't really a NO-JIRA commit if you are adding a test against a 
specific JIRA is it?  No reason I can see to put a NO-JIRA on this instead of 
just tagging against the JIRA that it is related to.  


---

[GitHub] activemq-artemis pull request #2329: ARTEMIS-2096 Refactor AMQMessage abstra...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2329

ARTEMIS-2096 Refactor AMQMessage abstraction

Major refactoring of the AMQPMessage abstraction to resolve
some issue of message corruption still present in the code and
improve the API handling of message changes and re-encoding.

Improves handling of decoding of message sections limiting the
work to only the portions needed and ensuring the state data
is always updated with what has been done.  Fixes issues of
corrupt state on copy of message or other changes in filters.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-2096

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2329.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2329


commit ed7e7f206287db56f0c44c01fa2133c1fbd5c50d
Author: Timothy Bish 
Date:   2018-09-25T16:22:19Z

ARTEMIS-2096 Refactor AMQMessage abstraction

Major refactoring of the AMQPMessage abstraction to resolve
some issue of message corruption still present in the code and
improve the API handling of message changes and re-encoding.

Improves handling of decoding of message sections limiting the
work to only the portions needed and ensuring the state data
is always updated with what has been done.  Fixes issues of
corrupt state on copy of message or other changes in filters.




---

[GitHub] activemq-artemis pull request #2309: NO-JIRA Fix condition timeouts to avoid...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2309

NO-JIRA Fix condition timeouts to avoid spurious failures

The waits for expiration are set at the same value as the expiration
interval default to in the test support setup so on some runs there is a
race when waiting for the message to expire and the expiry task.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis expiry-test-fixes

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2309.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2309


commit 6c48380ecfc8cafcbb3f99abe1e9987b7833cbf2
Author: Timothy Bish 
Date:   2018-09-12T20:43:57Z

NO-JIRA Fix condition timeouts to avoid spurious failures

The waits for expiration are set at the same value as the expiration
interval default to in the test support setup so on some runs there is a
race when waiting for the message to expire and the expiry task.




---

[GitHub] activemq-artemis pull request #2308: ARTEMIS-2083 Decode only the relavent p...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2308#discussion_r217179064
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPMessage.java
 ---
@@ -380,83 +395,63 @@ private synchronized void 
partialDecode(ReadableBuffer buffer) {
   decoder.setBuffer(buffer.rewind());
 
   _header = null;
+  expiration = 0;
+  headerEnds = 0;
+  messagePaylodStart = 0;
   _deliveryAnnotations = null;
   _messageAnnotations = null;
   _properties = null;
   applicationProperties = null;
-  Section section = null;
+  appLocation = -1;
+  deliveryAnnotationsPosition = -1;
 
   try {
- if (buffer.hasRemaining()) {
-section = (Section) decoder.readObject();
- }
-
- if (section instanceof Header) {
-_header = (Header) section;
-headerEnds = buffer.position();
-messagePaylodStart = headerEnds;
-this.durable = _header.getDurable();
-
-if (_header.getTtl() != null) {
-   this.expiration = System.currentTimeMillis() + 
_header.getTtl().intValue();
-}
-
-if (buffer.hasRemaining()) {
-   section = (Section) decoder.readObject();
-} else {
-   section = null;
-}
-
- } else {
-// meaning there is no header
-headerEnds = 0;
- }
- if (section instanceof DeliveryAnnotations) {
-_deliveryAnnotations = (DeliveryAnnotations) section;
-
-// Advance the start beyond the delivery annotations so they 
are not written
-// out on send of the message.
-messagePaylodStart = buffer.position();
-
-if (buffer.hasRemaining()) {
-   section = (Section) decoder.readObject();
-} else {
-   section = null;
-}
- }
- if (section instanceof MessageAnnotations) {
-_messageAnnotations = (MessageAnnotations) section;
-
-if (buffer.hasRemaining()) {
-   section = (Section) decoder.readObject();
-} else {
-   section = null;
-}
- }
- if (section instanceof Properties) {
-_properties = (Properties) section;
-
-if (_properties.getAbsoluteExpiryTime() != null && 
_properties.getAbsoluteExpiryTime().getTime() > 0) {
-   this.expiration = 
_properties.getAbsoluteExpiryTime().getTime();
-}
-
-// We don't read the next section on purpose, as we will parse 
ApplicationProperties
-// lazily
-section = null;
- }
-
- if (section instanceof ApplicationProperties) {
-applicationProperties = (ApplicationProperties) section;
- } else {
-if (buffer.hasRemaining()) {
-   this.appLocation = buffer.position();
+ while (buffer.hasRemaining()) {
+int constructorPos = buffer.position();
+TypeConstructor constructor = decoder.readConstructor();
+if (Header.class.equals(constructor.getTypeClass())) {
+   _header = (Header) constructor.readValue();
+   headerEnds = messagePaylodStart = buffer.position();
+   durable = _header.getDurable();
+   if (_header.getTtl() != null) {
+  expiration = System.currentTimeMillis() + 
_header.getTtl().intValue();
+   }
+} else if 
(DeliveryAnnotations.class.equals(constructor.getTypeClass())) {
+   // Don't decode these as they are not used by the broker at 
all and are
+   // discarded on send, mark for lazy decode if ever needed.
+   constructor.skipValue();
--- End diff --

These were added as part of codec improvements in v0.24.0 of proton-j so 
they've been around for a little while now.  


---

[GitHub] activemq-artemis pull request #2306: ENTMQBR-1569 - make sure to send credit...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2306#discussion_r217160287
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java
 ---
@@ -573,20 +575,38 @@ public void offerProducerCredit(final SimpleString 
address,
final int threshold,
final Receiver receiver) {
   try {
+ /*
+ * The credit runnable will always be run in this thread unless 
the address or disc is full. If this is the case the
+ * runnable is run once the memory or disc is free, if this 
happens we don't want to keep adding runnables as this
+ * may cause a memory leak, one is enough.
+ * */
+ if (creditRunnable != null && !creditRunnable.isRun())
+return;
  PagingManager pagingManager = 
manager.getServer().getPagingManager();
- Runnable creditRunnable = () -> {
-connection.lock();
-try {
-   if (receiver.getCredit() <= threshold) {
-  int topUp = credits - receiver.getCredit();
-  if (topUp > 0) {
- receiver.flow(topUp);
+ creditRunnable = new CreditRunnable() {
+boolean isRun = false;
+@Override
+public boolean isRun() {
+   return isRun;
+}
+
+@Override
+public void run() {
+   connection.lock();
+   try {
+  if (receiver.getCredit() <= threshold) {
+ int topUp = credits - receiver.getCredit();
+ System.out.println("topUp = " + topUp);
--- End diff --

Either use a logger or remove this I'd think.


---

[GitHub] activemq-artemis issue #2306: ENTMQBR-1569 - make sure to send credits on re...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2306
  
This should be referencing some ActiveMQ Artemis JIRA issue and not an JIRA 
from an external tracker.


---

[GitHub] activemq-artemis pull request #2308: ARTEMIS-2083 Decode only the relavent p...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2308

ARTEMIS-2083 Decode only the relavent portions of the message

Ensure that the Body of the message is never decoded in the partial
decode phase of the message processing and also gaurd against the
decode of ApplicationProperties which should be done lazily.  Add
lazy decode of DeliveryAnnotations as they are not used at present.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-2083

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2308.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2308


commit cfe63355ddd3153d6107be8c1f2a8d3fdf411d02
Author: Timothy Bish 
Date:   2018-09-12T16:42:45Z

ARTEMIS-2083 Decode only the relavent portions of the message

Ensure that the Body of the message is never decoded in the partial
decode phase of the message processing and also gaurd against the
decode of ApplicationProperties which should be done lazily.  Add
lazy decode of DeliveryAnnotations as they are not used at present.




---

[GitHub] activemq-artemis issue #2284: ARTEMIS-2067 Clean up some code in the AMQP pr...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2284
  
Sync'd with master, test failure seems unrelated to AMQP


---

[GitHub] activemq-artemis pull request #2304: ARTEMIS-2082 Reset buffer valid flag af...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2304

ARTEMIS-2082 Reset buffer valid flag after re-encoding the message

Once a re-encode of the message is done the buffer is not being marked
as valid and so subsequent checks on the buffer are all assuming the
message data is not valid and re-encoding over and over.  This can lead
to poor performance in some cases and corrupted data in others.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-2082

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2304.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2304


commit e065e3e960ac8cc6505a71ec4f67b58b4ffb5990
Author: Timothy Bish 
Date:   2018-09-11T19:42:24Z

ARTEMIS-2082 Reset buffer valid flag after re-encoding the message

Once a re-encode of the message is done the buffer is not being marked
as valid and so subsequent checks on the buffer are all assuming the
message data is not valid and re-encoding over and over.  This can lead
to poor performance in some cases and corrupted data in others.




---

[GitHub] activemq-artemis issue #2284: ARTEMIS-2067 Clean up some code in the AMQP pr...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2284
  
I targeted 2.7.0 in the JIRA as I don't think it is a critical fix can add 
it on next cycle if you want. 


---

[GitHub] activemq-artemis pull request #2284: ARTEMIS-2067 Clean up some code in the ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2284

ARTEMIS-2067 Clean up some code in the AMQP protocol handling paths

Cleans up some of the code on the proton event handler, most noteable:

1. Fix IOCallback creation on each outbound send, use single instance
as the handler only ever does a flush and has no attached state.
2. Fix redundent locking and unlocking of connection lock on the event
path that already ensures that lock is held.
3. Set presettle state on the server sender at attach as it cannot
change afterwards so checking on every message is not needed.
4. Improve buffer type checking on receive to reduce amount of work

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis proton-cleanups

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2284.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2284


commit 4349bd16d406dd883b077ba3339bd89a70d4acd0
Author: Timothy Bish 
Date:   2018-08-30T18:32:01Z

ARTEMIS-2067 Clean up some code in the AMQP protocol handling paths

Cleans up some of the code on the proton event handler, most noteable:

1. Fix IOCallback creation on each outbound send, use single instance
as the handler only ever does a flush and has no attached state.
2. Fix redundent locking and unlocking of connection lock on the event
path that already ensures that lock is held.
3. Set presettle state on the server sender at attach as it cannot
change afterwards so checking on every message is not needed.
4. Improve buffer type checking on receive to reduce amount of work




---

[GitHub] activemq-artemis pull request #2277: ARTEMIS-2062 Only attempt to refill cre...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2277

ARTEMIS-2062 Only attempt to refill credit when needed

Avoid firing the offerProducerCredit code when we know that the credit
is low enough that a refill is needed which avoids lock contention and
garbage creation as each inbound message is processed.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-2062

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2277.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2277


commit 04657c323f2f3235320aa5b14130acb07fc13c01
Author: Timothy Bish 
Date:   2018-08-28T20:11:14Z

ARTEMIS-2062 Only attempt to refill credit when needed

Avoid firing the offerProducerCredit code when we know that the credit
is low enough that a refill is needed which avoids lock contention and
garbage creation as each inbound message is processed.




---

[GitHub] activemq-artemis issue #2272: ARTEMIS-2057 Fix runaway credit grants

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2272
  
@gemmellr I've pushed some updates based on comments, thanks for reviewing 


---

[GitHub] activemq-artemis pull request #2272: ARTEMIS-2057 Fix runaway credit grants

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2272

ARTEMIS-2057 Fix runaway credit grants

Ensure the broker looks at local receiver credit when checking for
credit top off threshold and then do a proper top off back to the high
water mark to sync with how client receivers manage their credit.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-2057

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2272.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2272


commit 480e41c41d8c869014d5fe799a13914e659adf17
Author: Timothy Bish 
Date:   2018-08-27T19:19:23Z

ARTEMIS-2057 Fix runaway credit grants

Ensure the broker looks at local receiver credit when checking for
credit top off threshold and then do a proper top off back to the high
water mark to sync with how client receivers manage their credit.




---

[GitHub] activemq-artemis issue #2269: ARTEMIS-1938 Update Qpid JMS along with Proton...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2269
  
@michaelandrepearce Seems the tcnative dependency used for those tests was 
to old, fixed now. 


---

[GitHub] activemq-artemis pull request #2269: ARTEMIS-1938 Update Qpid JMS along with...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2269

ARTEMIS-1938 Update Qpid JMS along with Proton and Netty

Update the Qpid JMS and Proton dependencies to lastest and sync Netty
with the 4.1.28.Final version used by Qpid JMS to avoid clash that
breaks a test.  Adds override of new Proton-J WritableBuffer API that
allows it to use the Netty String encoder when needed instead of the
slower default version.

Update Qpid JMS to v0.36.0
Proton-J to v0.29.0
Netty to 4.1.28.Final

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1938

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2269.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2269


commit 0eb2859ab1c642f285dbd8c98444dca97bac013f
Author: Timothy Bish 
Date:   2018-08-24T17:55:29Z

ARTEMIS-1938 Update Qpid JMS along with Proton and Netty

Update the Qpid JMS and Proton dependencies to lastest and sync Netty
with the 4.1.28.Final version used by Qpid JMS to avoid clash that
breaks a test.  Adds override of new Proton-J WritableBuffer API that
allows it to use the Netty String encoder when needed instead of the
slower default version.

Update Qpid JMS to v0.36.0
Proton-J to v0.29.0
Netty to 4.1.28.Final




---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207644233
  
--- Diff: src/example/csharp/HelloWorld/HelloWorld.cs ---
@@ -0,0 +1,331 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Specialized;
+using Apache.NMS;
+using CommandLine;
+
+namespace HelloWorld
+{
+class CommandLineOpts
+{
+// URI for message broker. Must be of the format 
amqp://: or amqps://:
+[Option("uri", Required = true, HelpText = "The URI for the AMQP 
Message Broker")]
+public string host { get; set; }
+// Connection Request Timeout
+[Option("ct", Default = 15000, HelpText = "the connection request 
timeout in milliseconds.")]
+public long connTimeout { get; set; }
+// UserName for authentication with the broker.
+[Option("cu", Default = null, HelpText = "The Username for 
authentication with the message broker")]
+public string username { get; set; }
+// Password for authentication with the broker
+[Option("cpwd", Default = null, HelpText = "The password for 
authentication with the message broker")]
+public string password { get; set; }
+[Option("cid", Default = null, HelpText = "The Client ID on the 
connection")]
+public string clientId { get; set; }
+// Logging Level
+[Option("log", Default = "warn", HelpText = "Sets the log level 
for the application and NMS Library. The levels are (from highest verbosity): 
debug,info,warn,error,fatal.")]
+public string logLevel { get; set; }
+//
+[Option("topic", Default = null, HelpText = "Topic to publish 
messages to. Can not be used with --queue.")]
+public string topic { get; set; }
+//
+[Option("queue", Default = null, HelpText = "Queue to publish 
messages to. Can not be used with --topic.")]
+public string queue { get; set; }
+//
+[Option('n', "messages", Default = 5, HelpText = "Number of 
messages to send.")]
+public int NUM_MSG { get; set; }
+//
+[Option("deliveryMode", Default = 5, HelpText = "Message Delivery 
Mode, Persistnent(0) and Non Persistent(1). The default is Persistent(0).")]
+public int mode { get; set; }
+}
+class Program
+{
+
+private static void RunWithOptions (CommandLineOpts opts)
+{
+ITrace logger = new Logger(Logger.ToLogLevel(opts.logLevel));
+Tracer.Trace = logger;
+
+string ip = opts.host;
+Uri providerUri = new Uri(ip);
+Console.WriteLine("scheme: {0}", providerUri.Scheme);
+
+StringDictionary properties = new StringDictionary();
+//properties["clientId"] = "guest";
+if(opts.username !=null)
+properties["NMS.username"] = opts.username;
+if(opts.password !=null)
+properties["nms.password"] = opts.password;
+if (opts.clientId != null)
+properties["NMS.CLIENTID"] = opts.clientId;
+//properties["nms.clientid"] = "myclientid1";
+properties["NMS.sendtimeout"] = opts.connTimeout+"";
+IConnection conn = null;
+if (opts.topic == null && opts.queue == null)
+{
+Console.WriteLine("ERROR: Must specify a topic or queue 
destination");
+return;
+}
+try
 

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207644379
  
--- Diff: src/example/csharp/HelloWorld/HelloWorld.cs ---
@@ -0,0 +1,331 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Specialized;
+using Apache.NMS;
+using CommandLine;
+
+namespace HelloWorld
+{
+class CommandLineOpts
+{
+// URI for message broker. Must be of the format 
amqp://: or amqps://:
+[Option("uri", Required = true, HelpText = "The URI for the AMQP 
Message Broker")]
+public string host { get; set; }
+// Connection Request Timeout
+[Option("ct", Default = 15000, HelpText = "the connection request 
timeout in milliseconds.")]
+public long connTimeout { get; set; }
+// UserName for authentication with the broker.
+[Option("cu", Default = null, HelpText = "The Username for 
authentication with the message broker")]
+public string username { get; set; }
+// Password for authentication with the broker
+[Option("cpwd", Default = null, HelpText = "The password for 
authentication with the message broker")]
+public string password { get; set; }
+[Option("cid", Default = null, HelpText = "The Client ID on the 
connection")]
+public string clientId { get; set; }
+// Logging Level
+[Option("log", Default = "warn", HelpText = "Sets the log level 
for the application and NMS Library. The levels are (from highest verbosity): 
debug,info,warn,error,fatal.")]
+public string logLevel { get; set; }
+//
+[Option("topic", Default = null, HelpText = "Topic to publish 
messages to. Can not be used with --queue.")]
+public string topic { get; set; }
+//
+[Option("queue", Default = null, HelpText = "Queue to publish 
messages to. Can not be used with --topic.")]
+public string queue { get; set; }
+//
+[Option('n', "messages", Default = 5, HelpText = "Number of 
messages to send.")]
+public int NUM_MSG { get; set; }
+//
+[Option("deliveryMode", Default = 5, HelpText = "Message Delivery 
Mode, Persistnent(0) and Non Persistent(1). The default is Persistent(0).")]
+public int mode { get; set; }
+}
+class Program
+{
+
+private static void RunWithOptions (CommandLineOpts opts)
+{
+ITrace logger = new Logger(Logger.ToLogLevel(opts.logLevel));
+Tracer.Trace = logger;
+
+string ip = opts.host;
+Uri providerUri = new Uri(ip);
+Console.WriteLine("scheme: {0}", providerUri.Scheme);
+
+StringDictionary properties = new StringDictionary();
+//properties["clientId"] = "guest";
+if(opts.username !=null)
+properties["NMS.username"] = opts.username;
+if(opts.password !=null)
+properties["nms.password"] = opts.password;
+if (opts.clientId != null)
+properties["NMS.CLIENTID"] = opts.clientId;
+//properties["nms.clientid"] = "myclientid1";
+properties["NMS.sendtimeout"] = opts.connTimeout+"";
+IConnection conn = null;
+if (opts.topic == null && opts.queue == null)
+{
+Console.WriteLine("ERROR: Must specify a topic or queue 
destination");
+return;
+}
+try
 

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207643922
  
--- Diff: src/example/csharp/HelloWorld/HelloWorld.cs ---
@@ -0,0 +1,331 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Specialized;
+using Apache.NMS;
+using CommandLine;
+
+namespace HelloWorld
+{
+class CommandLineOpts
+{
+// URI for message broker. Must be of the format 
amqp://: or amqps://:
+[Option("uri", Required = true, HelpText = "The URI for the AMQP 
Message Broker")]
+public string host { get; set; }
+// Connection Request Timeout
+[Option("ct", Default = 15000, HelpText = "the connection request 
timeout in milliseconds.")]
+public long connTimeout { get; set; }
+// UserName for authentication with the broker.
+[Option("cu", Default = null, HelpText = "The Username for 
authentication with the message broker")]
+public string username { get; set; }
+// Password for authentication with the broker
+[Option("cpwd", Default = null, HelpText = "The password for 
authentication with the message broker")]
+public string password { get; set; }
+[Option("cid", Default = null, HelpText = "The Client ID on the 
connection")]
+public string clientId { get; set; }
+// Logging Level
+[Option("log", Default = "warn", HelpText = "Sets the log level 
for the application and NMS Library. The levels are (from highest verbosity): 
debug,info,warn,error,fatal.")]
+public string logLevel { get; set; }
+//
+[Option("topic", Default = null, HelpText = "Topic to publish 
messages to. Can not be used with --queue.")]
+public string topic { get; set; }
+//
+[Option("queue", Default = null, HelpText = "Queue to publish 
messages to. Can not be used with --topic.")]
+public string queue { get; set; }
+//
+[Option('n', "messages", Default = 5, HelpText = "Number of 
messages to send.")]
+public int NUM_MSG { get; set; }
+//
+[Option("deliveryMode", Default = 5, HelpText = "Message Delivery 
Mode, Persistnent(0) and Non Persistent(1). The default is Persistent(0).")]
+public int mode { get; set; }
+}
+class Program
+{
+
+private static void RunWithOptions (CommandLineOpts opts)
+{
+ITrace logger = new Logger(Logger.ToLogLevel(opts.logLevel));
+Tracer.Trace = logger;
+
+string ip = opts.host;
+Uri providerUri = new Uri(ip);
+Console.WriteLine("scheme: {0}", providerUri.Scheme);
+
+StringDictionary properties = new StringDictionary();
+//properties["clientId"] = "guest";
--- End diff --

Some of these commented out bits in the example make it a bit less useful 
as a clean sample, this client Id and the one below here seem confusing


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207644112
  
--- Diff: src/example/csharp/HelloWorld/HelloWorld.cs ---
@@ -0,0 +1,331 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Specialized;
+using Apache.NMS;
+using CommandLine;
+
+namespace HelloWorld
+{
+class CommandLineOpts
+{
+// URI for message broker. Must be of the format 
amqp://: or amqps://:
+[Option("uri", Required = true, HelpText = "The URI for the AMQP 
Message Broker")]
+public string host { get; set; }
+// Connection Request Timeout
+[Option("ct", Default = 15000, HelpText = "the connection request 
timeout in milliseconds.")]
+public long connTimeout { get; set; }
+// UserName for authentication with the broker.
+[Option("cu", Default = null, HelpText = "The Username for 
authentication with the message broker")]
+public string username { get; set; }
+// Password for authentication with the broker
+[Option("cpwd", Default = null, HelpText = "The password for 
authentication with the message broker")]
+public string password { get; set; }
+[Option("cid", Default = null, HelpText = "The Client ID on the 
connection")]
+public string clientId { get; set; }
+// Logging Level
+[Option("log", Default = "warn", HelpText = "Sets the log level 
for the application and NMS Library. The levels are (from highest verbosity): 
debug,info,warn,error,fatal.")]
+public string logLevel { get; set; }
+//
+[Option("topic", Default = null, HelpText = "Topic to publish 
messages to. Can not be used with --queue.")]
+public string topic { get; set; }
+//
+[Option("queue", Default = null, HelpText = "Queue to publish 
messages to. Can not be used with --topic.")]
+public string queue { get; set; }
+//
+[Option('n', "messages", Default = 5, HelpText = "Number of 
messages to send.")]
+public int NUM_MSG { get; set; }
+//
+[Option("deliveryMode", Default = 5, HelpText = "Message Delivery 
Mode, Persistnent(0) and Non Persistent(1). The default is Persistent(0).")]
+public int mode { get; set; }
+}
+class Program
+{
+
+private static void RunWithOptions (CommandLineOpts opts)
+{
+ITrace logger = new Logger(Logger.ToLogLevel(opts.logLevel));
+Tracer.Trace = logger;
+
+string ip = opts.host;
+Uri providerUri = new Uri(ip);
+Console.WriteLine("scheme: {0}", providerUri.Scheme);
+
+StringDictionary properties = new StringDictionary();
+//properties["clientId"] = "guest";
+if(opts.username !=null)
+properties["NMS.username"] = opts.username;
+if(opts.password !=null)
+properties["nms.password"] = opts.password;
+if (opts.clientId != null)
+properties["NMS.CLIENTID"] = opts.clientId;
+//properties["nms.clientid"] = "myclientid1";
+properties["NMS.sendtimeout"] = opts.connTimeout+"";
+IConnection conn = null;
+if (opts.topic == null && opts.queue == null)
+{
+Console.WriteLine("ERROR: Must specify a topic or queue 
destination");
+return;
+}
+try
+{
+
+
--- End diff --

There's a decent amount of scattered extra white space in here that could 
be cleaned up while making other changes


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207643986
  
--- Diff: src/example/csharp/HelloWorld/HelloWorld.cs ---
@@ -0,0 +1,331 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Specialized;
+using Apache.NMS;
+using CommandLine;
+
+namespace HelloWorld
+{
+class CommandLineOpts
+{
+// URI for message broker. Must be of the format 
amqp://: or amqps://:
+[Option("uri", Required = true, HelpText = "The URI for the AMQP 
Message Broker")]
+public string host { get; set; }
+// Connection Request Timeout
+[Option("ct", Default = 15000, HelpText = "the connection request 
timeout in milliseconds.")]
+public long connTimeout { get; set; }
+// UserName for authentication with the broker.
+[Option("cu", Default = null, HelpText = "The Username for 
authentication with the message broker")]
+public string username { get; set; }
+// Password for authentication with the broker
+[Option("cpwd", Default = null, HelpText = "The password for 
authentication with the message broker")]
+public string password { get; set; }
+[Option("cid", Default = null, HelpText = "The Client ID on the 
connection")]
+public string clientId { get; set; }
+// Logging Level
+[Option("log", Default = "warn", HelpText = "Sets the log level 
for the application and NMS Library. The levels are (from highest verbosity): 
debug,info,warn,error,fatal.")]
+public string logLevel { get; set; }
+//
+[Option("topic", Default = null, HelpText = "Topic to publish 
messages to. Can not be used with --queue.")]
+public string topic { get; set; }
+//
+[Option("queue", Default = null, HelpText = "Queue to publish 
messages to. Can not be used with --topic.")]
+public string queue { get; set; }
+//
+[Option('n', "messages", Default = 5, HelpText = "Number of 
messages to send.")]
+public int NUM_MSG { get; set; }
+//
+[Option("deliveryMode", Default = 5, HelpText = "Message Delivery 
Mode, Persistnent(0) and Non Persistent(1). The default is Persistent(0).")]
+public int mode { get; set; }
+}
+class Program
+{
+
+private static void RunWithOptions (CommandLineOpts opts)
+{
+ITrace logger = new Logger(Logger.ToLogLevel(opts.logLevel));
+Tracer.Trace = logger;
+
+string ip = opts.host;
+Uri providerUri = new Uri(ip);
+Console.WriteLine("scheme: {0}", providerUri.Scheme);
+
+StringDictionary properties = new StringDictionary();
+//properties["clientId"] = "guest";
+if(opts.username !=null)
+properties["NMS.username"] = opts.username;
+if(opts.password !=null)
+properties["nms.password"] = opts.password;
+if (opts.clientId != null)
+properties["NMS.CLIENTID"] = opts.clientId;
+//properties["nms.clientid"] = "myclientid1";
--- End diff --

See above


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r207643307
  
--- Diff: src/main/csharp/ConnectionMetaData.cs ---
@@ -0,0 +1,213 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Apache.NMS;
+using System.Reflection;
+using System.Diagnostics;
+
+namespace Apache.NMS.AMQP
+{
+/// 
+/// NMS.AMQP.ConnectionMetaData implements 
Apache.NMS.IConnectionMetaData
--- End diff --

I spotted a few places where the API docs have the wrong package names 
based on the latest rounds of updates.  Since it isn't a real reference to a 
type it isn't as bad as it wouldn't break any doc generator but would be nice 
for the reader if they all used the right name.


---

[GitHub] activemq-nms-amqp issue #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-nms-amqp/pull/2
  
I would just make the changes and amend the commit and force push it to the 
PR branch, you can also commit things locally and squash later if you prefer 
that route.  Basically until this goes into the project tree there isn't a need 
to preserve these intermediate code changes in the history 


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204159506
  
--- Diff: README.md ---
@@ -0,0 +1,112 @@
+# NMS-AMQP
--- End diff --

Apache-NMS-AMQP


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204161653
  
--- Diff: src/main/csharp/Connection.cs ---
@@ -0,0 +1,1158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using System.Collections.Concurrent;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Apache.NMS;
+using Amqp;
+using Amqp.Framing;
+using NMS.AMQP.Util;
+using NMS.AMQP.Transport;
+using System.Reflection;
+using Apache.NMS.Util;
+
+namespace NMS.AMQP
+{
+using Message.Factory;
+enum ConnectionState
+{
+UNKNOWN = -1,
+INITIAL = 0,
+CONNECTING = 1,
+CONNECTED = 2,
+CLOSING = 3,
+CLOSED = 4,
+
+}
+
+/// 
+/// NMS.AMQP.Connection facilitates management and creates the 
underlying Amqp.Connection protocol engine object.
+/// NMS.AMQP.Connection is also the NMS.AMQP.Session Factory.
+/// 
+class Connection : NMSResource, Apache.NMS.IConnection
+{
+public static readonly string MESSAGE_OBJECT_SERIALIZATION_PROP = 
PropertyUtil.CreateProperty("Message.Serialization", 
ConnectionFactory.ConnectionPropertyPrefix);
+
+private IRedeliveryPolicy redeliveryPolicy;
+private Amqp.IConnection impl;
+private ProviderCreateConnection implCreate;
+private ConnectionInfo connInfo;
+private readonly IdGenerator clientIdGenerator;
+private Atomic clientIdCanSet = new Atomic(true);
+private Atomic closing = new Atomic(false);
+private Atomic state = new 
Atomic(ConnectionState.INITIAL);
+private CountDownLatch latch;
+private ConcurrentDictionary sessions = new 
ConcurrentDictionary();
+private IdGenerator sesIdGen = null;
+private IdGenerator tempTopicIdGen = null;
+private IdGenerator tempQueueIdGen = null;
+private StringDictionary properties;
+private TemporaryLinkCache temporaryLinks = null;
+private IProviderTransportContext transportContext = null;
+private DispatchExecutor exceptionExecutor = null;
+
+#region Contructor
+
+internal Connection(Uri addr, IdGenerator clientIdGenerator)
+{
+connInfo = new ConnectionInfo();
+connInfo.remoteHost = addr;
+Info = connInfo;
+this.clientIdGenerator = clientIdGenerator;
+latch = new CountDownLatch(1);
+temporaryLinks = new TemporaryLinkCache(this);
+}
+
+#endregion
+
+#region Internal Properties
+
+internal Amqp.IConnection InnerConnection { get { return 
this.impl; } }
+
+internal IdGenerator SessionIdGenerator
+{
+get
+{
+IdGenerator sig = sesIdGen;
+lock (this)
+{
+if (sig == null)
+{
+sig = new NestedIdGenerator("ID:ses", connInfo.Id, 
true);
+sesIdGen = sig;
+}
+}
+return sig;
+}
+}
+
+internal IdGenerator TemporaryTopicGenerator
+{
+get
+{
+IdGenerator ttg = tempTopicIdGen;
+lock (this)
+{
+if (ttg == null)
+{
+ttg = new NestedIdGenerator("ID:nms-temp-topic", 
Info.Id, true);

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204159065
  
--- Diff: NMS.AMQP/NMS-AMQP.csproj ---
@@ -0,0 +1,124 @@
+
+
+
+  
+  
+Debug
+AnyCPU
+{0D8CF699-9702-4EC6-8719-C2968D32F09A}
+Library
+Properties
+NMS.AMQP
--- End diff --

The project should carry the Apache name e.g.  Apache.NMS.AMQP


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160011
  
--- Diff: src/main/csharp/Connection.cs ---
@@ -0,0 +1,1158 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using System.Collections.Concurrent;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+using Apache.NMS;
+using Amqp;
+using Amqp.Framing;
+using NMS.AMQP.Util;
+using NMS.AMQP.Transport;
+using System.Reflection;
+using Apache.NMS.Util;
+
+namespace NMS.AMQP
--- End diff --

Use namespace Apache.NMS.AMQP for all source in this library


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160492
  
--- Diff: NMS.AMQP/App.config ---
@@ -0,0 +1,16 @@
+
--- End diff --

missing license header



---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160181
  
--- Diff: NMS.AMQP/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,37 @@
+using System.Reflection;
--- End diff --

Missing license header.


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160128
  
--- Diff: README.md ---
@@ -0,0 +1,112 @@
+# NMS-AMQP
--- End diff --

Should be Apache NMS AMQP


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204159401
  
--- Diff: NMS.AMQP/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,37 @@
+using System.Reflection;
--- End diff --

License header missing


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160391
  
--- Diff: NMS.AMQP/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,37 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+// General Information about an assembly is controlled through the 
following 
+// set of attributes. Change these attribute values to modify the 
information
+// associated with an assembly.
+[assembly: AssemblyTitle("NMS-AMQP")]
+[assembly: AssemblyDescription("")]
+[assembly: AssemblyConfiguration("")]
+[assembly: AssemblyCompany("Apache")]
+[assembly: AssemblyProduct("NMS-AMQP")]
--- End diff --

Same as above, add Apache to these names


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204160304
  
--- Diff: NMS.AMQP/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,37 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+// General Information about an assembly is controlled through the 
following 
+// set of attributes. Change these attribute values to modify the 
information
+// associated with an assembly.
+[assembly: AssemblyTitle("NMS-AMQP")]
--- End diff --

The assembly should carry the Apache name "Apache-NMS-AMQP"


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204159176
  
--- Diff: NMS.AMQP/NMS-AMQP.csproj ---
@@ -0,0 +1,124 @@
+
+
+
+  
+  
+Debug
+AnyCPU
+{0D8CF699-9702-4EC6-8719-C2968D32F09A}
+Library
+Properties
+NMS.AMQP
+NMS.AMQP
--- End diff --

The assembly should also carry the Apache name.


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204159358
  
--- Diff: NMS.AMQP/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,37 @@
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+// General Information about an assembly is controlled through the 
following 
+// set of attributes. Change these attribute values to modify the 
information
+// associated with an assembly.
+[assembly: AssemblyTitle("NMS-AMQP")]
--- End diff --

Assembly should carry the Apache name Apache-NMS-AMQP


---

[GitHub] activemq-nms-amqp pull request #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-nms-amqp/pull/2#discussion_r204158230
  
--- Diff: HelloWorld/Properties/AssemblyInfo.cs ---
@@ -0,0 +1,36 @@
+using System.Reflection;
--- End diff --

Missing License header here.


---

[GitHub] activemq-nms-amqp issue #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-nms-amqp/pull/2
  
If you can't work out the squash then another way to fix things is to back 
out the commit using a "git reset --soft HEAD~X" where 'X' is the number of 
commits and then create a new commit from there and push -f to overwrite the PR 
branch.  


---

[GitHub] activemq-nms-amqp issue #2: AMQNET-575: NMS AMQP Client Rework

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-nms-amqp/pull/2
  
It is generally best practice to squash commits into one on a PR to make 
life easier for the reviews and to make the commit history simpler.  We don't 
really need to preserve a commit removing files that shouldn't have been 
included in the first commit for instance.  


---

[GitHub] activemq-nms-amqp issue #1: AMQNET-576: Clean out old code base for NMS AMQP...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-nms-amqp/pull/1
  
This looks like it could be closed now given a new PR(#2) with the client 
code is now open 


---

[GitHub] activemq-artemis pull request #2174: ARTEMIS-1941 fix failing tests

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2174#discussion_r201156531
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/test/java/org/apache/activemq/artemis/protocol/amqp/converter/message/JMSMappingOutboundTransformerTest.java
 ---
@@ -361,7 +361,7 @@ public void 
testConvertEmptyObjectMessageToAmqpMessageWithAmqpValueBody() throws
   assertNotNull(amqp.getBody());
   assertTrue(amqp.getBody() instanceof AmqpValue);
   assertTrue(((AmqpValue) amqp.getBody()).getValue() instanceof 
Binary);
-  assertEquals(0, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
+  assertEquals(5, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
--- End diff --

Reviewing the changes and tests and all looks good, the unexpected checks 
for content length in the object message bodies seems to be a side effect of 
the way the test constructs the message and not related to any new bugs.  


---

[GitHub] activemq-nms-amqp issue #1: AMQNET-576: Clean out old code base for NMS AMQP...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-nms-amqp/pull/1
  
The move of legacy code was done properly now on the Apache repo, this PR 
needs to be rebased and squashed into one commit for review.  


---

[GitHub] activemq-artemis pull request #2174: ARTEMIS-1941 fix failing tests

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2174#discussion_r201007171
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/test/java/org/apache/activemq/artemis/protocol/amqp/converter/message/JMSMappingOutboundTransformerTest.java
 ---
@@ -361,7 +361,7 @@ public void 
testConvertEmptyObjectMessageToAmqpMessageWithAmqpValueBody() throws
   assertNotNull(amqp.getBody());
   assertTrue(amqp.getBody() instanceof AmqpValue);
   assertTrue(((AmqpValue) amqp.getBody()).getValue() instanceof 
Binary);
-  assertEquals(0, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
+  assertEquals(5, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
--- End diff --

@clebertsuconic once you have something better that is tested and working, 
sure you can revert this, but for now those of us getting messages that fall 
into the "large" message trap see unreliable transformations so this fix is 
needed.  Once the broker does it better then you can removed these workarounds 
if you know for sure that conversions are only every one way.  


---

[GitHub] activemq-artemis pull request #2174: ARTEMIS-1941 fix failing tests

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2174#discussion_r200759349
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/test/java/org/apache/activemq/artemis/protocol/amqp/converter/message/JMSMappingOutboundTransformerTest.java
 ---
@@ -361,7 +361,7 @@ public void 
testConvertEmptyObjectMessageToAmqpMessageWithAmqpValueBody() throws
   assertNotNull(amqp.getBody());
   assertTrue(amqp.getBody() instanceof AmqpValue);
   assertTrue(((AmqpValue) amqp.getBody()).getValue() instanceof 
Binary);
-  assertEquals(0, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
+  assertEquals(5, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
--- End diff --

I'm back to work on Monday and can look closer, a quite look at the 
equivalent tests in 5.x show that they all test for 0 as the expected length so 
I'm not sure why these are checking for 5 now, that seems wrong to me but I'd 
have to look closer when I'm back.  


---

[GitHub] activemq-artemis pull request #2174: ARTEMIS-1941 fix failing tests

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2174#discussion_r200754867
  
--- Diff: 
artemis-protocols/artemis-amqp-protocol/src/test/java/org/apache/activemq/artemis/protocol/amqp/converter/message/JMSMappingOutboundTransformerTest.java
 ---
@@ -361,7 +361,7 @@ public void 
testConvertEmptyObjectMessageToAmqpMessageWithAmqpValueBody() throws
   assertNotNull(amqp.getBody());
   assertTrue(amqp.getBody() instanceof AmqpValue);
   assertTrue(((AmqpValue) amqp.getBody()).getValue() instanceof 
Binary);
-  assertEquals(0, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
+  assertEquals(5, ((Binary) ((AmqpValue) 
amqp.getBody()).getValue()).getLength());
--- End diff --

At first glance this looks wrong as I'd have expected the sender to have 
sent a zero length Binary and so the resulting message should also contain a 
zero length Binary as the value in the encoded AmqpValue.  


---

[GitHub] activemq-artemis pull request #2168: RTEMIS-1941 Preserve AMQP body section ...

[tabish121]

Github user tabish121 closed the pull request at:

https://github.com/apache/activemq-artemis/pull/2168


---

[GitHub] activemq-artemis pull request #2168: RTEMIS-1941 Preserve AMQP body section ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2168

RTEMIS-1941 Preserve AMQP body section type on "large" messages

When "large" messages are converted to / from core in order to be stored
in the large message store the type of the AMQP body section is being
lost and reconstituted incorrectly in some cases.  The message needs to
be annotated with the original AMQP type for the body and that used to
manage the conversion back to AMQP from Core.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1941

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2168.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2168


commit 9780b275ea03e4c587a6e808e4e5214ff7bb2548
Author: Timothy Bish 
Date:   2018-06-29T19:22:05Z

ARTEMIS-1941 Preserve AMQP body section type on "large" messages

When "large" messages are converted to / from core in order to be stored
in the large message store the type of the AMQP body section is being
lost and reconstituted incorrectly in some cases.  The message needs to
be annotated with the original AMQP type for the body and that used to
manage the conversion back to AMQP from Core.




---

[GitHub] activemq-artemis issue #2145: ARTEMIS-1938 Update to Qpid JMS 0.33.0

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2145
  
There are no updates to proton-j at this time, only client fixes


---

[GitHub] activemq-artemis issue #2145: ARTEMIS-1938 Update to Qpid JMS 0.33.0

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2145
  
The only reason you might want to take it is that it contains several fixes 
for failover handling and artemis provides a Karaf feature for an AMQP client 
so your users could benefit from that, otherwise it has no impact on the broker 
as it is only used in testing otherwise.  


---

[GitHub] activemq-artemis issue #2145: ARTEMIS-1938 Update to Qpid JMS 0.33.0

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2145
  
I targeted the JIRA at 2.7.0 for that reason. 


---

[GitHub] activemq-artemis pull request #2145: ARTEMIS-1938 Update to Qpid JMS 0.33.0

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2145

ARTEMIS-1938 Update to Qpid JMS 0.33.0

Update to latest release of Qpid JMS

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1938

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2145.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2145


commit 952bf12eeb9a6f2fe3528d46fd748adeb6be5d16
Author: Timothy Bish 
Date:   2018-06-15T21:10:18Z

ARTEMIS-1938 Update to Qpid JMS 0.33.0

Update to latest release of Qpid JMS




---

[GitHub] activemq-artemis pull request #2135: ARTEMIS-1924 Add amqpIdleTimeout

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2135#discussion_r194568008
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpNoHearbeatsTest.java
 ---
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.amqp;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Map;
+
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.transport.amqp.client.AmqpClient;
+import org.apache.activemq.transport.amqp.client.AmqpConnection;
+import org.apache.activemq.transport.amqp.client.AmqpValidator;
+import org.apache.qpid.proton.engine.Connection;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@RunWith(Parameterized.class)
+public class AmqpNoHearbeatsTest extends AmqpClientTestSupport {
+
+   @Parameterized.Parameters(name = "useOverride={0}")
+   public static Collection parameters() {
+  return Arrays.asList(new Object[][] {
+ {true}, {false}
+  });
+   }
+
+   @Parameterized.Parameter(0)
+   public boolean useOverride;
+
+   @Override
+   protected void addConfiguration(ActiveMQServer server) {
+  if (useOverride) {
+ server.getConfiguration().setConnectionTTLOverride(0);
+  } else {
+ server.getConfiguration().setConnectionTtlCheckInterval(500);
+  }
+   }
+
+
+   @Override
+   protected void configureAMQPAcceptorParameters(Map 
params) {
+  if (!useOverride) {
+ params.put("amqpIdleTimeout", "0");
+  }
+   }
+
+
+   @Test(timeout = 6)
+   public void testBrokerSendsHalfConfiguredIdleTimeout() throws Exception 
{
--- End diff --

The name of the test doesn't appear to match what is being tested here so 
I'd name it something that actually describes that to avoid confusing folks.  


---

[GitHub] activemq-artemis issue #2089: ARTEMIS-1866 Make Quorum vote result wait time...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2089
  
Doesn't that sort of defeat the point of using source control ?


---

[GitHub] activemq-artemis issue #2106: [ARTEMIS-1885]max-consumers attribute in queue...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2106
  
If it is fixing something then it needs a test.  If it is not fixing 
something than the JIRA description needs to be changed as it makes it sound 
like it is fixing something, and probably still should have a test if it 
affects some broker behavior.  Regardless of all that the JIRA and the commit 
message should have a more meaningful description of what is going on as it 
they are both so sparse as to make it utterly impossible for the casual 
observer to figure out what they are for.  

Specifically if there is no AMQP issue here then the message and the JIRA 
should not imply there is. 


---

[GitHub] activemq-artemis issue #2106: [ARTEMIS-1885]max-consumers attribute in queue...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2106
  
There's no tests here, how do we know this does or does not fix an issue 
that may or may not exist given there's no test provided to observe it ?


---

[GitHub] activemq-artemis pull request #2087: ARTEMIS-1861 Set a max-frame-size on AM...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2087

ARTEMIS-1861 Set a max-frame-size on AMQP connections by default

Configure a value of 128KB for AMQP max frame size by default to improve
overall performance and provide a limit on delivery size before chunking
begins.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1861

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2087.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2087


commit 9e008523b54c31f0d7ad7912405d9636ac68c998
Author: Timothy Bish <tabish121@...>
Date:   2018-05-11T16:57:22Z

ARTEMIS-1861 Set a max-frame-size on AMQP connections by default

Configure a value of 128KB for AMQP max frame size by default to improve
overall performance and provide a limit on delivery size before chunking
begins.




---

[GitHub] activemq-artemis pull request #2084: ARTEMIS-1859 Adding testAnonymousProduc...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2084#discussion_r187643834
  
--- Diff: 
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/JMSMessageProducerTest.java
 ---
@@ -70,10 +70,31 @@ public void testAnonymousProducer() throws Exception {
}
 
@Test(timeout = 3)
-   public void testAnonymousProducerWithAutoCreation() throws Exception {
-  Connection connection = createConnection();
+   public void testAnonymousProducerWithQueueAutoCreation() throws 
Exception {
+  try (Connection connection = createConnection()) {
+ Session session = connection.createSession(false, 
Session.AUTO_ACKNOWLEDGE);
+ Queue queue = session.createQueue(UUID.randomUUID().toString());
+ MessageProducer p = session.createProducer(null);
 
-  try {
+ TextMessage message = session.createTextMessage();
+ message.setText("hello");
+ // this will auto-create the address
+ p.send(queue, message);
--- End diff --

I believe it is the second send you don't need, the first on create the 
address which result in a MULTICAST address being created which sets up the 
scene for the create consumer to fail because the consumer is creating a JMS 
Queue and will get clobbered because that address is already MULTICAST or 
something.  


---

[GitHub] activemq-artemis pull request #2067: ARTEMIS-1847 Update to Netty 4.1.24.Fin...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2067

ARTEMIS-1847 Update to Netty 4.1.24.Final



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1847

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2067.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2067


commit c62e467971216539fe82da7e00be1bf2972d211b
Author: Timothy Bish <tabish121@...>
Date:   2018-05-03T16:57:15Z

ARTEMIS-1847 Update to Netty 4.1.24.Final




---

[GitHub] activemq-artemis pull request #2064: ARTEMIS-1843 Update Qpid JMS 0.32.0 and...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/2064

ARTEMIS-1843 Update Qpid JMS 0.32.0 and Proton-j 0.27.1

Use new no copy variants for the delivery send and receive and make
use of the ReadableBuffer type that is now used to convery tranfer
payloads without a copy.  Also set max outgoing frame size to match
the configured maxFrameSize for the AMQP protocol head to avoid the
case where an overly large frame can be written instead of chunking
a large message.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1843

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/2064.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #2064


commit 04d5d6a33b6cf589ea96fd325e15b678aadc49c8
Author: Timothy Bish <tabish121@...>
Date:   2018-03-27T21:09:48Z

ARTEMIS-1843 Update Qpid JMS 0.32.0  and Proton-j 0.27.1

Use new no copy variants for the delivery send and receive and make
use of the ReadableBuffer type that is now used to convery tranfer
payloads without a copy.  Also set max outgoing frame size to match
the configured maxFrameSize for the AMQP protocol head to avoid the
case where an overly large frame can be written instead of chunking
a large message.




---

[GitHub] activemq-artemis issue #2053: ARTEMIS-1838 Fixing AMQP->Core Race on durable...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2053
  
@clebertsuconic ok, I was about to fix it myself.  I confirmed that the fix 
does seem to resolve the issue, I can no longer reproduce it here using the 
test and an external broker or with the smoke test.


---

[GitHub] activemq-artemis issue #2053: ARTEMIS-1838 Fixing AMQP->Core Race on durable...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2053
  
Now you have a checkstyle violation :)


---

[GitHub] activemq-artemis pull request #2053: ARTEMIS-1838 Fixing AMQP->Core Race on ...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/2053#discussion_r185075306
  
--- Diff: 
tests/smoke-tests/src/test/java/org/apache/activemq/artemis/tests/smoke/crossprotocol/MultiThreadConvertTest.java
 ---
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.activemq.artemis.tests.smoke.crossprotocol;
+
+import javax.jms.Connection;
+import javax.jms.DeliveryMode;
+import javax.jms.IllegalStateException;
+import javax.jms.Message;
+import javax.jms.MessageConsumer;
+import javax.jms.MessageProducer;
+import javax.jms.Session;
+import javax.jms.TextMessage;
+import javax.jms.Topic;
+import java.util.HashMap;
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.api.core.TransportConfiguration;
+import 
org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory;
+import org.apache.activemq.artemis.tests.smoke.common.SmokeTestBase;
+import org.apache.qpid.jms.JmsConnectionFactory;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class MultiThreadConvertTest extends SmokeTestBase {
+
+   private static final String SERVER_NAME_0 = "standard";
+
+   private static final Logger LOG = 
LoggerFactory.getLogger(MultiThreadConvertTest.class);
+
+   protected String queueName = "amqTestQueue1";
+   private SimpleString coreQueue;
--- End diff --

This coreQueue variable is never used and should be removed. 


---

[GitHub] activemq-artemis issue #2031: ARTEMIS-1816 OpenWire should avoid ByteArrayOu...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2031
  
Ok, I'll be "the decider".  Will check with franz to see how much it's 
worth to him


---

[GitHub] activemq-artemis issue #2031: ARTEMIS-1816 OpenWire should avoid ByteArrayOu...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2031
  
The change as it is seems acceptable as it does reduce unnecessary 
allocations when ByteMessages types cross to OpenWire.  I've reviewed the 
changes as it relates to the OpenWire objects and the way the converter uses it 
and I don't see anything adverse happening from this.  As always a run through 
the tests would be good for some validation.  


---

[GitHub] activemq-artemis issue #2031: ARTEMIS-1816 OpenWire should avoid ByteArrayOu...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2031
  
That would be the more ideal thing to do yes. 


---

[GitHub] activemq-artemis issue #2031: ARTEMIS-1816 OpenWire should avoid ByteArrayOu...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2031
  
@franz1981 So I think that you could avoid subclassing the 
ActiveMQBytesMessage if you built the properties yourself instead of calling 
setObjectProperty in setAMQMsgObjectProperties in the converter as that is 
where the trigger point is for allocating the bytes stream (see 
initializeWriting and its uses in ActiveMQBytesMessage).  In a manner similar 
to how the converter puts the body into the message using setContent you could 
create and marshal the properties and store them using setMarshalledProperties 
in the OpenWire base Message class.  The properties are marshalled using the 
utility class MarshallingSupport#marshalPrimitiveMap so it is possible to roll 
your own and then bypass that bit of the ActiveMQMessage's normal handling.  


---

[GitHub] activemq-artemis issue #1985: ARTEMIS-1780 Handle conversion of large Object...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1985
  
That's twice now today I've rebased.  Test failure doesn't seem related to 
AMQP


---

[GitHub] activemq-artemis issue #1985: ARTEMIS-1780 Handle conversion of large Object...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1985
  
I updated the patch to remove the duplicate decode which seems to have been 
the root cause of the other test failure. 


---

[GitHub] activemq-artemis pull request #1985: ARTEMIS-1780 Handle conversion of large...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1985

ARTEMIS-1780 Handle conversion of large ObjectMessage types

Make sure the correct buffer is used when decoding the stored Core
message that originated from the conversion of an AMQP message sent and
annotated as a JMS ObjectMessage which trips the large message boundary.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1780

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1985.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1985


commit 12cdc5c743d07524e1efa68dee4e6fb740f40aaf
Author: Timothy Bish <tabish121@...>
Date:   2018-04-02T21:57:54Z

ARTEMIS-1780 Handle conversion of large ObjectMessage types

Make sure the correct buffer is used when decoding the stored Core
message that originated from the conversion of an AMQP message sent and
annotated as a JMS ObjectMessage which trips the large message boundary.




---

[GitHub] activemq-artemis pull request #1980: ARTEMIS-1777 Adding Protocol specific i...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1980#discussion_r178144111
  
--- Diff: artemis-distribution/pom.xml ---
@@ -102,6 +102,11 @@
artemis-amqp-protocol
${project.version}

+  
+ org.apache.activemq
+ artemis-new-amqp
+ ${project.version}
+  

--- End diff --

Think maybe you didn't mean to add this particular dependency


---

[GitHub] activemq-artemis issue #1961: [ARTEMIS-1758] support SASL EXTERNAL with Text...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1961
  
When running in the IDE the new test works fine, however when I run the 
AMQP tests from the command line the new test produces an error as below which 
seems to indicate the test environment isn't getting configured correctly. 

> Test

testConnection(org.apache.activemq.artemis.tests.integration.amqp.JMSSaslExternalTest)
  Time elapsed: 0.773 sec  <<< ERROR!
javax.jms.JMSException: 
file:/home/timothy/.m2/repository/org/apache/activemq/tests/unit-tests/2.6.0-SNAPSHOT/unit-tests-2.6.0-SNAPSHOT-tests.jar!/client-side-keystore.jks
 (No such file or directory)
at 
org.apache.qpid.jms.exceptions.JmsExceptionSupport.create(JmsExceptionSupport.java:86)
at 
org.apache.qpid.jms.exceptions.JmsExceptionSupport.create(JmsExceptionSupport.java:108)
at org.apache.qpid.jms.JmsConnection.connect(JmsConnection.java:168)
at 
org.apache.qpid.jms.JmsConnectionFactory.createConnection(JmsConnectionFactory.java:203)
at 
org.apache.activemq.artemis.tests.integration.amqp.JMSSaslExternalTest.testConnection(JMSSaslExternalTest.java:116)
Caused by: java.io.FileNotFoundException: 
file:/home/timothy/.m2/repository/org/apache/activemq/tests/unit-tests/2.6.0-SNAPSHOT/unit-tests-2.6.0-SNAPSHOT-tests.jar!/client-side-keystore.jks
 (No such file or directory)
at java.io.FileInputStream.open0(Native Method)
at java.io.FileInputStream.open(FileInputStream.java:195)
at java.io.FileInputStream.(FileInputStream.java:138)
at 
org.apache.qpid.jms.transports.TransportSupport.loadStore(TransportSupport.java:287)
at 
org.apache.qpid.jms.transports.TransportSupport.loadKeyManagers(TransportSupport.java:250)
at 
org.apache.qpid.jms.transports.TransportSupport.createSslContext(TransportSupport.java:99)
at 
org.apache.qpid.jms.transports.TransportSupport.createSslHandler(TransportSupport.java:74)
at 
org.apache.qpid.jms.transports.netty.NettyTcpTransport.connect(NettyTcpTransport.java:136)
at 
org.apache.qpid.jms.provider.amqp.AmqpProvider$1.run(AmqpProvider.java:203)
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
at 
java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)



---

[GitHub] activemq-artemis issue #1928: ARTEMIS-1722 Don't copy message bytes unless n...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1928
  
Need @clebertsuconic to give it a look 


---

[GitHub] activemq-artemis issue #1928: ARTEMIS-1722 Don't copy message bytes unless n...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1928
  
@franz1981 I've updated this PR with changes to the common path based on 
your suggestions to me over on my fork.  Thanks for the review and ideas, even 
better now.  


---

[GitHub] activemq-artemis issue #1918: ARTEMIS-1722 Reduce pooled Netty ByteBuf usage

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1918
  
@franz1981 @clebertsuconic I've sent an alternate PR #1928 which performs 
the send without any buffer copy unless necessary due to message redelivery or 
existence of original delivery annotations.  Proton will be copying those bytes 
anyway so an intermediate isn't really needed on the common path.  

It could be argued that you could further change things beyond what I did 
and just keep a single ReadableBuffer instance around in the consumer and give 
it the outbound buffer to send as the proton send method will take that 
readable buffer and call get(array, offset, length) on it to get a copy which 
means netty should perform the most efficient write.  


---

[GitHub] activemq-artemis pull request #1928: ARTEMIS-1722 Don't copy message bytes u...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1928

ARTEMIS-1722 Don't copy message bytes unless needed

Alternate patch that doesn't copy the message bytes unless doing a
redelivery or skipping delivery annotations in the original version of
the message.  Proton-J will copy the bytes provided to the Sender's send
method so a copy isn't necessary on most common sends.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1722

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1928.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1928


commit 63b2e84fce76a9bf6c7cc3c9cef00b775457453c
Author: Timothy Bish <tabish121@...>
Date:   2018-03-02T20:03:08Z

ARTEMIS-1722 Don't copy message bytes unless needed

Alternate patch that doesn't copy the message bytes unless doing a
redelivery or skipping delivery annotations in the original version of
the message.  Proton-J will copy the bytes provided to the Sender's send
method so a copy isn't necessary on most common sends.




---

[GitHub] activemq-artemis pull request #1925: ARTEMIS-1727 - Make sure transport is s...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1925#discussion_r171918979
  
--- Diff: 
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java
 ---
@@ -641,13 +647,37 @@ public void fail(ActiveMQException me, String 
message) {
  }
   }
   try {
- protocolManager.removeConnection(this.getConnectionInfo(), me);
+ if (this.getConnectionInfo() != null) {
+protocolManager.removeConnection(this.getConnectionInfo(), me);
+ }
   } catch (InvalidClientIDException e) {
  ActiveMQServerLogger.LOGGER.warn("Couldn't close connection 
because invalid clientID", e);
   }
   shutdown(true);
}
 
+   private void delayedStop(final int waitTime, final String reason, 
Throwable cause) {
+  if (waitTime > 0) {
+ try {
+server.getExecutorFactory().getExecutor().execute(new 
Runnable() {
+   @Override
+   public void run() {
+  try {
+ Thread.sleep(waitTime);
--- End diff --

Sleeping here seems like it might have a ripple on the other resources 
using the executor, maybe use a scheduled executor from the pool in 
ActiveMQServer ?


---

[GitHub] activemq-artemis issue #1918: ARTEMIS-1722 Reduce pooled Netty ByteBuf usage

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1918
  
@franz1981 It doesn't look like the buffer could be contended unless 
perhaps the consumer was able to be subscribed to more than one queue at a time 
via some sort of composite type subscription but I don't think that is possible 
?  otherwise the broker should be dispatching serially if I understand 
correctly but I'd defer to @clebertsuconic on that one.  

It isn't entirely uncommon for consumers to be subscribed to low volume 
queues or topics where they might sit idle for a longer period of time so 
having each consumer hold onto memory like that indefinitely could lead to 
quick resource exhaustion on the broker in the case of 1000s of connections 
(think IoT)




---

[GitHub] activemq-artemis pull request #1916: ARTEMIS-1504 Update Qpid JMS to 0.30.0 ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1916

ARTEMIS-1504 Update Qpid JMS to 0.30.0 and proton-j to 0.26.0

Updates to latest Qpid JMS and the latest Proton-J release, use new 
proton-j setting to disable read-only buffers use to avoid netty issue with 
unnecessary extra copy and pool creation when use that types of buffers

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1504

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1916.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1916


commit 977ff75cec8ac8e3ee721011fc5c54ae7eb305e5
Author: Timothy Bish <tabish121@...>
Date:   2018-03-02T00:12:36Z

ARTEMIS-1504 Update Qpid JMS to 0.30.0 and proton-j to 0.26.0

Updates to latest Qpid JMS and the latest Proton-J release




---

[GitHub] activemq-artemis pull request #1909: ARTEMIS-1712 Add additional null check ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1909

ARTEMIS-1712 Add additional null check in TX rollback handler

Check for AMQSession being null before handling various TX state checks
in order to ensure the correct errors are thrown and TX rollback is
handled properly.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1712

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1909.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1909


commit 8fc1e00d88fed9419e566d1fb69b46175585e9f1
Author: Timothy Bish <tabish121@...>
Date:   2018-02-28T15:46:31Z

ARTEMIS-1712 Add additional null check in TX rollback handler

Check for AMQSession being null before handling various TX state checks
in order to ensure the correct errors are thrown and TX rollback is
handled properly.




---

[GitHub] activemq-artemis issue #1903: ARTEMIS-1706 - Add support for wantClientAuth

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1903
  
Looks good.  +1


---

[GitHub] activemq-artemis pull request #1905: ARTEMIS-1707 Check for already exists e...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1905

ARTEMIS-1707 Check for already exists exceptions and ignore

For address and queue create check for the already exists exceptions and
ignore as another client might have just created the same

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1707

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1905.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1905


commit 38fb17703b3479d1ef933b8f5a99e75f72166c96
Author: Timothy Bish <tabish121@...>
Date:   2018-02-27T21:30:06Z

ARTEMIS-1707 Check for already exists exceptions and ignore

For address and queue create check for the already exists exceptions and
ignore as another client might have just created the same




---

[GitHub] activemq-artemis pull request #1903: ARTEMIS-1706 - Add support for wantClie...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1903#discussion_r171066273
  
--- Diff: docs/user-manual/en/configuring-transports.md ---
@@ -403,6 +403,15 @@ following additional properties:
 This property is only for an `acceptor`. It tells a client
 connecting to this acceptor that 2-way SSL is required. Valid values
 are `true` or `false`. Default is `false`.
+
+-   `wantClientAuth`
+
+This property is only for an `acceptor`. It tells a client
+connecting to this acceptor that 2-way SSL is requested but not 
required. 
+Valid values are `true` or `false`. Default is `false`. 
+
+Note that this setting will override `needClientAuth` if both 
properties 
+are set to true.
 
--- End diff --

I failed to read this before my previous comment but as before I'd have 
opted to go the other way on the choice and defaulted to need if both are 
there.  For reference the 5.x code makes that choice as well when both are 
present on the URI


---

[GitHub] activemq-artemis pull request #1903: ARTEMIS-1706 - Add support for wantClie...

[tabish121]

Github user tabish121 commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1903#discussion_r171060932
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/NettyAcceptor.java
 ---
@@ -468,8 +473,12 @@ public synchronized SslHandler 
getSslHandler(ByteBufAllocator alloc) throws Exce
 
   engine.setUseClientMode(false);
 
-  if (needClientAuth)
+  if (needClientAuth) {
  engine.setNeedClientAuth(true);
+  }
+  if (wantClientAuth) {
--- End diff --

I would probably do this in an else as the setting would override the need 
option and if someone did something silly like specify both I'd generally 
prefer it chose the stronger of the two and setting want after need overrides 
it.  


---

[GitHub] activemq-artemis pull request #:

[tabish121]

Github user tabish121 commented on the pull request:


https://github.com/apache/activemq-artemis/commit/6470e7e64d883282624619adc9d4816fa1688a1e#commitcomment-27609794
  
Merged it, looked ok to me as well. 


---

[GitHub] activemq-artemis pull request #1802: ARTEMIS-1504 Update Qpid JMS to 0.29.0 ...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1802

ARTEMIS-1504 Update Qpid JMS to 0.29.0 and proton-j to 0.25.0

Updates to latest Qpid JMS and the latest Proton-J release

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1504

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1802.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1802


commit e626b01243980a8194276760abbcf324880ddf8a
Author: Timothy Bish <tabish121@...>
Date:   2018-01-22T17:09:07Z

ARTEMIS-1504 Update Qpid JMS to 0.29.0 and proton-j to 0.25.0

Updates to latest Qpid JMS and the latest Proton-J release




---

[GitHub] activemq-artemis issue #1773: ARTEMIS-1589 ActiveMQProtonRemotingConnection#...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1773
  
@jbertram @jostbg As far as I can see the patch looks correct, it would 
seem to make more sense to return the remote container ID here than the locally 
generated server side value as that provides more context into the remote 
connection.  That is of course if the getClientID API in RemotingConnection is 
referring to the value specified by the remote, which would make the most 
sense.  


---

[GitHub] activemq-artemis pull request #1769: ARTEMIS-1576 Fix test that was broken w...

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1769

ARTEMIS-1576 Fix test that was broken with changes on this issue

The test is using the wrong indices for the destinations it uses so they
don't match the one's created in the test support class.  Because the
code is now using the default routing type the test fails when it tries
to send a message on a JMS Queue when the auto created address default
to the multicast routing type.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1576

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1769.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1769


commit 8bba8dcea3428eb043213146f1bcdcc460930693
Author: Timothy Bish <tabish121@...>
Date:   2018-01-10T21:44:25Z

ARTEMIS-1576 Fix test that was broken with changes on this issue

The test is using the wrong indices for the destinations it uses so they
don't match the one's created in the test support class.  Because the
code is now using the default routing type the test fails when it tries
to send a message on a JMS Queue when the auto created address default
to the multicast routing type.




---

[GitHub] activemq-artemis pull request #1709: ARTEMIS-1557 Updates Netty to v 4.1.18....

[tabish121]

GitHub user tabish121 opened a pull request:

https://github.com/apache/activemq-artemis/pull/1709

ARTEMIS-1557 Updates Netty to v 4.1.18.Final



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/tabish121/activemq-artemis ARTEMIS-1557

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1709.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1709


commit 97a584fe52798c9e0b775c547d38f127829d63dc
Author: Timothy Bish <tabish...@gmail.com>
Date:   2017-12-13T21:01:55Z

ARTEMIS-1557 Updates Netty to v 4.1.18.Final




---

[GitHub] activemq-artemis issue #1695: ARTEMIS-1545 Ensure JMS security exceptions oc...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1695
  
@michaelandrepearce it depends on what other URI options you've set on the 
5.x client, it will at times switch back to sync based on other configuration.  
For alternate reference the Qpid JMS AMQP 1.0 client will also send 
non-persistent messages async by default, errors would be reported to the 
connection exception listener for JMS 1.1 sends and to the CompletionListener 
for JMS 2.0 async sends as one would expect.  


---

[GitHub] activemq-artemis issue #1695: ARTEMIS-1545 Ensure JMS security exceptions oc...

[tabish121]

Github user tabish121 commented on the issue:

https://github.com/apache/activemq-artemis/pull/1695
  
Actually I was mostly referring to JMS 1.1 sends where many JMS client opt 
to send non-persistent messages async and indeed won't report an error back 
unless you've explicitly configured the client to do so. 



---