ARTEMIS-3180 - fix multiple path match case in wildcard address map #3492
Conversation
|
I can confirm that the changes solve the issue we are facing with Artemis 2.0.17. |
|
thanks for verification. I should have a clean test run in a few hours and can push this fix. |
| @@ -638,7 +638,7 @@ public void testHashA() throws Exception { | |||
| assertFalse(aABCA.matches(aHashA)); | |||
| assertFalse(aHashA.matches(aABCA)); | |||
|
|
|||
| assertEquals(0, countMatchingWildcards(abcaS)); | |||
| assertEquals(1, countMatchingWildcards(abcaS)); | |||
There was a problem hiding this comment.
This doesnt seem right. I would expect existing test case to pass the same without modification
There was a problem hiding this comment.
Isn't this test essentially about if "#.a" matches "a.b.c.a"? Which I think it should.
There was a problem hiding this comment.
My issue is that would be a change of behaviour, that some users may have got used to and built systems around such quirks. As such just changing that may break many users.
If theres a change in behaviour it needs to be behind a toggle and then by default kept as original. And original behaviour tests kept as is to ensure that.
This is the same as when we aligned queue naming for amqp, so that it used the same rules as core and openwire so there wasnt a conflict and you could then have openwire core and amqp consumers share the same shares subs (queues). Whilst it would have been nice to default on the better new behaviour, to avoid least astonishment for existing users it went behind a toggle and defaulted to the original behaviour.
Only at the next major release aka 3.0.0 would we then be able to change that default behaviour (break change) with the major version change. Theres a few things like the one i mentioned thats waiting to change those defaults being switched to default true at the next major.
There was a problem hiding this comment.
@michaelandrepearce this is a bug fix for a regression, the tests verify the behaviour of the existing code but the semantic was wrong in the tests.
| @@ -650,7 +650,7 @@ public void testHashA() throws Exception { | |||
| SimpleString AHashA = new SimpleString("a.#.a"); | |||
| underTest.put(AHashA, AHashA); | |||
|
|
|||
| assertEquals(1, countMatchingWildcards(new SimpleString("a.b.c.a"))); | |||
| assertEquals(2, countMatchingWildcards(new SimpleString("a.b.c.a"))); | |||
There was a problem hiding this comment.
Ditto comment i would expect existing test case to return the same
There was a problem hiding this comment.
Here "#.a" and "a.#.a" are matched against "a.b.c.a". Both should match, given that as per doc "# match any sequence of zero or more words"
No description provided.