Qpid
  1. Qpid
  2. QPID-4497

[Java client] Exclusive property for the subscription queue cannot be overridden using the address string

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.18, 0.20
    • Fix Version/s: 0.21
    • Component/s: Java Client
    • Labels:

      Description

      There have been requests to allow the customization of the subscription queue. However there was an issue as the exclusive property wasn't set properly due to a bug.

      amq.topic/test; {link: {name:my-queue, durable:true, x-declare:

      {exclusive:false, auto-delete:false}

        Activity

        Hide
        Rajith Attapattu added a comment -

        As to your original question about a test for this specific case (of being able to override subscription queue properties) exists ?
        The answer to that question is we do have a test.

        As to whether we have adequate coverage for assertion and error handling in general, then the answer is no. There was a gap (and possibly more) and the fact that this issue was masked is a case in point. We do have assert tests that are used as part of other tests, but not enough IMO.

        As mentioned in QPID-4496 I think we need a better set of tests for this area and I'm looking at it atm.

        Show
        Rajith Attapattu added a comment - As to your original question about a test for this specific case (of being able to override subscription queue properties) exists ? The answer to that question is we do have a test. As to whether we have adequate coverage for assertion and error handling in general, then the answer is no. There was a gap (and possibly more) and the fact that this issue was masked is a case in point. We do have assert tests that are used as part of other tests, but not enough IMO. As mentioned in QPID-4496 I think we need a better set of tests for this area and I'm looking at it atm.
        Hide
        Robbie Gemmell added a comment -

        Not really, no. I don't see that any tests have been added by any of the commits under discussion, but changes to affect whether existing tests pass or fail were made following bug fixes. This says to me that we are most likely still not properly testing something that we should be, or can't really trust that the tests are actually correct and passing because the code is working as expected too. In particular, if the assertion code wasn't reliable previously (which it couldn't have been for that test to pass when it shouldnt) then why should we trust it is reliable now if no specific testing was put in place to demonstrate the defects it had at the time are fixed now and aren't reintroduced in the future?

        Show
        Robbie Gemmell added a comment - Not really, no. I don't see that any tests have been added by any of the commits under discussion, but changes to affect whether existing tests pass or fail were made following bug fixes. This says to me that we are most likely still not properly testing something that we should be, or can't really trust that the tests are actually correct and passing because the code is working as expected too. In particular, if the assertion code wasn't reliable previously (which it couldn't have been for that test to pass when it shouldnt) then why should we trust it is reliable now if no specific testing was put in place to demonstrate the defects it had at the time are fixed now and aren't reintroduced in the future?
        Hide
        Rajith Attapattu added a comment -

        Perhaps there is a misunderstanding. The "testCustomizingSubscriptionQueue" among other tests does verify the properties of a queue using isQueueExists method.
        This method takes a second parameter called "assert". If true, will match exclusive, auto-delete, durable and queue declare arguments returned by the QueueQuerryResult to that of the node properties of a given address.
        Therefore we do have a method that asserts various queue properties as part of several tests and "testCustomizingSubscriptionQueue" for the specific issue of verifying that we do in fact allow customization of the subscription queue for topics.

        Does that answer your questions adequately ?

        Show
        Rajith Attapattu added a comment - Perhaps there is a misunderstanding. The "testCustomizingSubscriptionQueue" among other tests does verify the properties of a queue using isQueueExists method. This method takes a second parameter called "assert". If true, will match exclusive, auto-delete, durable and queue declare arguments returned by the QueueQuerryResult to that of the node properties of a given address. Therefore we do have a method that asserts various queue properties as part of several tests and "testCustomizingSubscriptionQueue" for the specific issue of verifying that we do in fact allow customization of the subscription queue for topics. Does that answer your questions adequately ?
        Hide
        Robbie Gemmell added a comment -

        Ok. We can fall back to the 'tests?' question regarding those changes then, i.e. there obviously isn't a test that verifies we can correctly assert exclusivity since the test that tried to use that functionality incorrectly passed.

        Show
        Robbie Gemmell added a comment - Ok. We can fall back to the 'tests?' question regarding those changes then, i.e. there obviously isn't a test that verifies we can correctly assert exclusivity since the test that tried to use that functionality incorrectly passed.
        Hide
        Rajith Attapattu added a comment -

        The "testCustomizingSubscriptionQueue" test covers this issue. Actually thats how I found this.
        Due to the improper handling of assertion failures this test was passing before.
        But once I fixed that, this test was failing, thus exposing this issue.

        Show
        Rajith Attapattu added a comment - The "testCustomizingSubscriptionQueue" test covers this issue. Actually thats how I found this. Due to the improper handling of assertion failures this test was passing before. But once I fixed that, this test was failing, thus exposing this issue.
        Hide
        Robbie Gemmell added a comment -
        Show
        Robbie Gemmell added a comment - Regarding http://svn.apache.org/viewvc?view=revision&revision=1418541 Did I miss the new tests?

          People

          • Assignee:
            Rajith Attapattu
            Reporter:
            Rajith Attapattu
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development