You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2017/10/11 15:56:05 UTC
[GitHub] activemq-artemis pull request #1584: ARTEMIS-1461 Allow changing of messages...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/1584
ARTEMIS-1461 Allow changing of messages through interceptors
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemis AMQP-interceptor
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1584.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 #1584
----
----
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
@tabish121 I have changed docs around to address your point
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
Users can do that already today. Not introducing anything new.
Not recommended. But users can do that already.
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by Matze2 <gi...@git.apache.org>.
Github user Matze2 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
I was not even aware that there is somewhere a binary snapshot to download.
I compiled it myself (had to add servlet-api dependency to artemis-rest module to be able to build it).
And I now tested the broker plugin case and this works. Thanks!
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
We would need to change putProperties to forbid it.
All we can do is document it.
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by Matze2 <gi...@git.apache.org>.
Github user Matze2 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
Ignore my last message. I was irritated by the pull request title containing the word "interceptor". The JIRA issue is named differently.
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
So I don't see any documentation explaining how this violates the AMQP specification about the immutability of the bare message part of the AMQP message, or how it can now can present a message to the receiver with the same MessageId that is not in fact the message that was sent. Also it would break anyone that uses message signing as the original Footer would likely be preserved and so again present to the receiver a message pretending to be the one that was sent but in fact not be and thereby not match any included signature.
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by harrison-tarr <gi...@git.apache.org>.
Github user harrison-tarr commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
I pulled this down and tested it with my ServerPlugin and AMQP producer/consumer code. I'm no longer seeing the issue I was before, so I think this fixes it 🎉
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
@harrison-tarr mind reviewing the doc I wrote?
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by Matze2 <gi...@git.apache.org>.
Github user Matze2 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
I tried the latest snapshot with an AmqpInterceptor implementation. This does not work.
```
@Override
public boolean intercept(final AMQPMessage message, RemotingConnection connection) {
logger.info("AMQP Interceptor gets called with message " + message.getMessageID());
logger.info("Properties before: " + message.getPropertyNames());
message.putStringProperty("newProp", "newPropValue");
logger.info("Properties after: " + message.getPropertyNames());
return true;
}
```
The listener does not see the "newProp" property added here.
Will try the broker plugin example now.
---
[GitHub] activemq-artemis pull request #1584: ARTEMIS-1461 Allow changing of messages...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1584
---
[GitHub] activemq-artemis pull request #1584: ARTEMIS-1461 Allow changing of messages...
Posted by harrison-tarr <gi...@git.apache.org>.
Github user harrison-tarr commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1584#discussion_r144161029
--- Diff: examples/features/standard/broker-plugin/src/main/java/org/apache/activemq/artemis/jms/example/BrokerPlugin.java ---
@@ -0,0 +1,52 @@
+/*
+ * 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.jms.example;
+
+import org.apache.activemq.artemis.api.core.ActiveMQException;
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.core.server.ServerSession;
+import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerPlugin;
+import org.apache.activemq.artemis.core.transaction.Transaction;
+import org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage;
+
+public class BrokerPlugin implements ActiveMQServerPlugin {
+
+ int count = 0;
+
+ @Override
+ public void beforeSend(ServerSession session,
+ Transaction tx,
+ Message message, boolean direct,
+ boolean noAutoCreateQueue) throws ActiveMQException {
+ if (message instanceof AMQPMessage) {
+ // AMQP Messages are by definition immutable.
--- End diff --
I don't think you need to treat these differently based on the type of message. I think you can just say
`message.putStringProperty()
message.reencode()`
If i remember correctly, ICoreMessage just inherits the default noop implementation of `reencode`
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
@Matze2 the broker plugin is basically intercepting the message. I used the term out of the concept instead of the class name.
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
I think the snapshot wasn’t updated yet.
---
[GitHub] activemq-artemis pull request #1584: ARTEMIS-1461 Allow changing of messages...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/1584#discussion_r144166497
--- Diff: examples/features/standard/broker-plugin/src/main/java/org/apache/activemq/artemis/jms/example/BrokerPlugin.java ---
@@ -0,0 +1,52 @@
+/*
+ * 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.jms.example;
+
+import org.apache.activemq.artemis.api.core.ActiveMQException;
+import org.apache.activemq.artemis.api.core.ICoreMessage;
+import org.apache.activemq.artemis.api.core.Message;
+import org.apache.activemq.artemis.core.server.ServerSession;
+import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerPlugin;
+import org.apache.activemq.artemis.core.transaction.Transaction;
+import org.apache.activemq.artemis.protocol.amqp.broker.AMQPMessage;
+
+public class BrokerPlugin implements ActiveMQServerPlugin {
+
+ int count = 0;
+
+ @Override
+ public void beforeSend(ServerSession session,
+ Transaction tx,
+ Message message, boolean direct,
+ boolean noAutoCreateQueue) throws ActiveMQException {
+ if (message instanceof AMQPMessage) {
+ // AMQP Messages are by definition immutable.
--- End diff --
@harrison-tarr that's right... and do you believe I wrote that? :)
---
[GitHub] activemq-artemis issue #1584: ARTEMIS-1461 Allow changing of messages throug...
Posted by harrison-tarr <gi...@git.apache.org>.
Github user harrison-tarr commented on the issue:
https://github.com/apache/activemq-artemis/pull/1584
@clebertsuconic it looks good to me. It might be worth adding a note in the https://activemq.apache.org/artemis/docs/latest/broker-plugins.html document about this example
---