Uploaded image for project: 'Solder'
  1. Solder
  2. SOLDER-130

Handlers without qualifiers should be notified even if ExceptionToCatch has qualifiers

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 3.0.0.Beta1
    • Component/s: Core
    • Labels:
      None
    • Estimated Difficulty:
      Low

      Description

      Qualifiers are meant to narrow the handlers that are invoked (in the same way as observers). Less specific handlers (particularly handlers without any qualifiers) should be notified if the set of qualifiers on the handler are a subset of the qualifiers on the exception payload. Currently Catch fails this assertion.

      Let's assume that the following exception (of type Throwable) is sent to Catch:

      beanManager.fireEvent(new ExceptionToCatch(e, new AnnotationLiteral<WebRequest>() {}));

      The following two handlers should match:

      void handleAny(@Handles CaughtException<Throwable> caught) {}

      void handleForWebRequest(@Handles @WebRequest CaughtException<Throwable> caught) {}

      However, the following handler would not be invoked because it's qualifiers are not a subset of the qualifiers on the exception payload:

      void handleForRestRequest(@Handles @RestRequest CaughtException<Throwable> caught) {}

      For reference, see JSR-299, Section 10.2.3: Multiple event qualifiers

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            dan.j.allen Dan Allen added a comment -

            I found the definitive sentence in the JSR-299 specification that supports this (chapter 10):

            An observer method will be notified of an event if the event object is:

            1. assignable to the observed event type and
            2. all the qualifiers on the event observer are also event qualifiers of the event

            Further clarified by 5.2

            The type and qualifiers of the injection point are called the required type and required qualifiers.

            That supports my claim that the qualifiers on the observer (in our case, handler) must be a subset of the qualifiers on the event (in our case, caught exception). There is one special case, @Any, which matches any qualifier set. @Any is implicit on an observer (in our case, handler) if no qualifiers are present.

            Show
            dan.j.allen Dan Allen added a comment - I found the definitive sentence in the JSR-299 specification that supports this (chapter 10): An observer method will be notified of an event if the event object is: 1. assignable to the observed event type and 2. all the qualifiers on the event observer are also event qualifiers of the event Further clarified by 5.2 The type and qualifiers of the injection point are called the required type and required qualifiers. That supports my claim that the qualifiers on the observer (in our case, handler) must be a subset of the qualifiers on the event (in our case, caught exception). There is one special case, @Any, which matches any qualifier set. @Any is implicit on an observer (in our case, handler) if no qualifiers are present.
            Hide
            pmuir Pete Muir added a comment -

            Not sure what version of the spec you were looking at (this appears to have changed late on) but the definitive text reverses your statement.

            10.2, bullet 3 "The observer method has all the event qualifiers. An observer method has an event qualifier if it has an observed event qualifier with (a) the same type and (b) the same annotation member value for each member which is not annotated @javax.enterprise.util.Nonbinding."

            So, IOW the qualifiers on the event must be a subset of the qualifiers on the observer method. Furthermore, @Any is not implicit on the observer, but implicit on the event, so if you put @Any only on the observer method, then any event will match.

            Show
            pmuir Pete Muir added a comment - Not sure what version of the spec you were looking at (this appears to have changed late on) but the definitive text reverses your statement. 10.2, bullet 3 "The observer method has all the event qualifiers. An observer method has an event qualifier if it has an observed event qualifier with (a) the same type and (b) the same annotation member value for each member which is not annotated @javax.enterprise.util.Nonbinding." So, IOW the qualifiers on the event must be a subset of the qualifiers on the observer method. Furthermore, @Any is not implicit on the observer, but implicit on the event, so if you put @Any only on the observer method, then any event will match.
            Hide
            pmuir Pete Muir added a comment -

            Jason has this consistent with the spec.

            It is obvious that something screwy happened in the spec, as this text changed late on, and the examples do not match the rules. Please raise an issue in CDI to have this clarified.

            Show
            pmuir Pete Muir added a comment - Jason has this consistent with the spec. It is obvious that something screwy happened in the spec, as this text changed late on, and the examples do not match the rules. Please raise an issue in CDI to have this clarified.
            Hide
            dan.j.allen Dan Allen added a comment -

            Regardless of what happens with CDI-7, we can still support less-specific handlers by saying that Catch simply does the equivalent of firing multiple events, for each permutation of the qualifiers.

            For instance, if we include the @WebRequest qualifier in the ExceptionToCatch, we notify handlers that have the @WebRequest qualifier and handlers that have no qualifier. How it's accomplished is an implementation detail.

            The only open question I have is whether the catch-all observer should be required to have an explicit @Any. You could argue that an exception with the @WebRequest qualifier does not have the @Default qualifier, and therefore would not match an observer with no qualifier (as that has the @Default qualifier). This is where I'm just not sure how observers should work. I'm okay with requiring @Any for the catch-all observer.

            Thus, if the exception Throwable has two qualifiers, such as @WebRequest and @Secure, then handlers with the following qualifiers + type would be called

            • @Handles @Any CaughtException<Throwable>
            • @Handles @WebRequest CaughtException<Throwable>
            • @Handles @WebRequest @Secure CaughtException<Throwable>

            This supports using qualifiers as topic selectors.

            Show
            dan.j.allen Dan Allen added a comment - Regardless of what happens with CDI-7 , we can still support less-specific handlers by saying that Catch simply does the equivalent of firing multiple events, for each permutation of the qualifiers. For instance, if we include the @WebRequest qualifier in the ExceptionToCatch, we notify handlers that have the @WebRequest qualifier and handlers that have no qualifier. How it's accomplished is an implementation detail. The only open question I have is whether the catch-all observer should be required to have an explicit @Any. You could argue that an exception with the @WebRequest qualifier does not have the @Default qualifier, and therefore would not match an observer with no qualifier (as that has the @Default qualifier). This is where I'm just not sure how observers should work. I'm okay with requiring @Any for the catch-all observer. Thus, if the exception Throwable has two qualifiers, such as @WebRequest and @Secure, then handlers with the following qualifiers + type would be called @Handles @Any CaughtException<Throwable> @Handles @WebRequest CaughtException<Throwable> @Handles @WebRequest @Secure CaughtException<Throwable> This supports using qualifiers as topic selectors.

              People

              • Assignee:
                lightguard Jason Porter
                Reporter:
                dan.j.allen Dan Allen
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development