Wicket
  1. Wicket
  2. WICKET-2241

Guice integration doesn't honour optional bindings

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4-RC4
    • Component/s: wicket-guice
    • Labels:
      None

      Description

      When @Inject(optional=true) is applied and the binding isn't available, wicket should skip the injection silently.
      http://groups.google.com/group/google-guice/browse_thread/thread/2abe9b55ee26a198

      To check whether the binding is available, you need to use a try/catch block:
      try

      { Binding binding = injector.getBinding(key); // use the binding }

      catch (ConfigurationException e)

      { // the binding isn't available }

        Activity

        Hide
        Guðmundur Bjarni Ólafsson added a comment -

        What Eduardo S. Nunes wrote is 100% correct. I unfortunately didn't try the patch with the Guice 2.0 snapshots, which throws a ConfigurationException (this exception has default visibility in Guice 1.0, thus catching a RuntimeException) if no binding is found.

        I've created another patch (and tested) that should be applied on top of the one that has already been committed.

        Show
        Guðmundur Bjarni Ólafsson added a comment - What Eduardo S. Nunes wrote is 100% correct. I unfortunately didn't try the patch with the Guice 2.0 snapshots, which throws a ConfigurationException (this exception has default visibility in Guice 1.0, thus catching a RuntimeException) if no binding is found. I've created another patch (and tested) that should be applied on top of the one that has already been committed.
        Hide
        Eduardo S. Nunes added a comment - - edited

        I think there is something wrong with this patch:
        // if the Inject annotation is marked optional and no binding is found
        // then skip this injection (WICKET-2241)
        if (optional && injector.getBinding(key) == null)

        { return null; }

        But here (latest version of guice) injector.getBinding(..) throws an exception instead of return null. Consider do like Jesse Wilson said.

        A solution that works for guice 1.0 and 2.0 is:
        if (optional)
        {
        try
        {
        if (injector.getBinding(key) == null)

        { return null; }

        }
        catch (RuntimeException e)

        { return null; }

        }

        Show
        Eduardo S. Nunes added a comment - - edited I think there is something wrong with this patch: // if the Inject annotation is marked optional and no binding is found // then skip this injection ( WICKET-2241 ) if (optional && injector.getBinding(key) == null) { return null; } But here (latest version of guice) injector.getBinding(..) throws an exception instead of return null. Consider do like Jesse Wilson said. A solution that works for guice 1.0 and 2.0 is: if (optional) { try { if (injector.getBinding(key) == null) { return null; } } catch (RuntimeException e) { return null; } }
        Hide
        Guðmundur Bjarni Ólafsson added a comment -

        Wrote a simple fix which adds an optional parameter to GuiceProxyTargetLocator. Before calling injector.getInstance(key) it checks whether the target is optional, if it is Guice is asked whether a binding exists (via injector.getBinding(key)) and if there is no binding a null is returned from the locator. Otherwise it just proceeds as it did before.

        Show
        Guðmundur Bjarni Ólafsson added a comment - Wrote a simple fix which adds an optional parameter to GuiceProxyTargetLocator. Before calling injector.getInstance(key) it checks whether the target is optional, if it is Guice is asked whether a binding exists (via injector.getBinding(key)) and if there is no binding a null is returned from the locator. Otherwise it just proceeds as it did before.

          People

          • Assignee:
            Igor Vaynberg
            Reporter:
            Jesse Wilson
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development