ARTEMIS-5131 Add A Copy message button to console #5316
Conversation
andytaylor
force-pushed
the
ARTEMIS-5131
branch
from
a0d4cf8
to
88c5482
Compare
| @@ -2618,6 +2618,60 @@ public void testMoveMessage() throws Exception { | |||
| session.deleteQueue(otherQueue); | |||
| } | |||
|
|
|||
| @TestTemplate | |||
There was a problem hiding this comment.
For adding a feature like this you should ideally write a test that covers more of the protocols and messages (normal or large) as well as times when paging is happening vs not as there have been a number of cases lately where a breakage or misbehavior has been found when an AMQP or a Large message was involved in internal management code such as this.
Refer to test: tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/plugin/BeforeSendPluginTest.java for some inspiration
| throw ActiveMQMessageBundle.BUNDLE.noQueueFound(targetQueue); | ||
| } | ||
|
|
||
| return queue.copyReference(messageID, binding.getAddress(), binding); |
There was a problem hiding this comment.
The implementation details here should be part of the Queue::copyReference.
Also... what if the targetQueue was passed a topic.
Shouldn't this call PostOffice::route instead?...
on that case targetQueue here should be targetAddress...
I started writing a test to validate this test and I bumped into that
There was a problem hiding this comment.
As with the move method this is adding directly to a queue, rerouting it would send it to all queus on an address which is wrong. I could do use route and add rout-to-id of the queue but would there be any benefit to doing that?
There was a problem hiding this comment.
actually ignore, if you follow the code thru it does use rout on the binding itself so it only goes to that queue
|
I sent a PR for a test that's not working at the moment, but it would give you an idea of a test to be written... as I said before I think the method should be updated. |
|
@andytaylor also.. I just thought... how security handled on the security settings on the copy? I don't think this is checking security.. is it? and I still think it should go through the PostOffice so you can copy for a topic for instance? |
I don't sww how the topic instance makes any difference as it is just being moved between queues? |
andytaylor
force-pushed
the
ARTEMIS-5131
branch
from
88c5482
to
49f09f1
Compare
|
I have added tests for each protocol, large messages and some around paging. one paging test copies to a queue on the same address and makes sure it is also paged |
andytaylor
force-pushed
the
ARTEMIS-5131
branch
from
49f09f1
to
17d385e
Compare
This exposes a copyMessage method that simply copies a message to a different queue so the new console can add a copy button
andytaylor
force-pushed
the
ARTEMIS-5131
branch
from
17d385e
to
f238f0f
Compare
This exposes a copyMessage method that simply copies a message to a different queue so the new console can add a copy button