Spring Security
  1. Spring Security
  2. SEC-1177

MethodInvocationUtils Returns Null With Valid Method String and Class

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4
    • Fix Version/s: 3.0.0 RC1
    • Component/s: Core
    • Labels:
      None

      Description

      public void testMethodInvocation()

      { MethodInvocation methodInvocation = MethodInvocationUtils. createFromClass(AuthenticationServiceImpl.class, "login"); assertNull(methodInvocation); }

      public void testFindMethod()
      {
      String methodName = "login";

      boolean methodFound = false;

      for(Method m: AuthenticationServiceImpl.class.getMethods())

      { methodFound = true; }

      assertTrue(methodFound);
      };

      Suppose we have two methods like this on the same class:

      com.example.MyClass.myMethod(String, String);
      com.example.MyClass.myMethod(String, int);

      And we call MethodInvocationUtils.create(object, methodName). It would be nice to get a MultiplePossibilityException("Which method - the one with 2 String arguments or the one with a (String, int) arguments". Use create(object, methodName, Object[] instead).

      Or if theres no method overloading, but the method can't be found a NoSuchMethodException("Method not found on class of object").

        Activity

        Hide
        Luke Taylor added a comment -

        MethodInvocationUtils currently is just a simple wrapper for Class.getMethods(), acting as a means of creating MethodInvocations for use in testing a user's privileges against an AccessDecisionManager. It's assumed that the user is aware of which method they are looking for and any required arguments. If they don't specify enough information to find the method then null will be returned rather than raising an exception.

        It's something that has been lying around in the framework for a long time and I don't think many people use it in practice - they're more likely to use the web equivalent for deciding what can be rendered in pages etc. The Javadoc could certainly be improved but I don't think this is a "blocker" issue - even if you think the class should behave differently there are trivial workarounds (like creating your own MethodInvocation instance - e.g. using the SimpleMethodInvocation class).

        BTW, your testFindMethod() doesn't look like it does anything other than set methodFound to "true" and then assert that it is so.

        Show
        Luke Taylor added a comment - MethodInvocationUtils currently is just a simple wrapper for Class.getMethods(), acting as a means of creating MethodInvocations for use in testing a user's privileges against an AccessDecisionManager. It's assumed that the user is aware of which method they are looking for and any required arguments. If they don't specify enough information to find the method then null will be returned rather than raising an exception. It's something that has been lying around in the framework for a long time and I don't think many people use it in practice - they're more likely to use the web equivalent for deciding what can be rendered in pages etc. The Javadoc could certainly be improved but I don't think this is a "blocker" issue - even if you think the class should behave differently there are trivial workarounds (like creating your own MethodInvocation instance - e.g. using the SimpleMethodInvocation class). BTW, your testFindMethod() doesn't look like it does anything other than set methodFound to "true" and then assert that it is so.
        Hide
        Ole Ersoy added a comment -

        MethodInvocationUtils would be a perfect fit for us if it behaved the way the JavaDoc describes, but you are right the simple workaround works (I think still testing with 2.0.4).

        Ooops again - Thanks - the testFindMethod is supposed to look like this:
        public void testFindMethod()
        {
        String methodName = "login";

        boolean methodFound = false;

        for(Method m: AuthenticationServiceImpl.class.getMethods())
        {
        if (methodName == m.getName())

        { methodFound = true; }

        }
        assertTrue(methodFound);
        };

        The test still passes. So it seems like, like Nickar says, null is returned when no type parameters are passed.

        Show
        Ole Ersoy added a comment - MethodInvocationUtils would be a perfect fit for us if it behaved the way the JavaDoc describes, but you are right the simple workaround works (I think still testing with 2.0.4). Ooops again - Thanks - the testFindMethod is supposed to look like this: public void testFindMethod() { String methodName = "login"; boolean methodFound = false; for(Method m: AuthenticationServiceImpl.class.getMethods()) { if (methodName == m.getName()) { methodFound = true; } } assertTrue(methodFound); }; The test still passes. So it seems like, like Nickar says, null is returned when no type parameters are passed.
        Hide
        Luke Taylor added a comment -

        I've added an extra loop to iterate through the declared method names on the supplied class and try to find one matching the requested method name. If one is found, an invocation object will be created without any argument information. If there are duplicate method names, an IllegalArgumentException will be thrown.

        Show
        Luke Taylor added a comment - I've added an extra loop to iterate through the declared method names on the supplied class and try to find one matching the requested method name. If one is found, an invocation object will be created without any argument information. If there are duplicate method names, an IllegalArgumentException will be thrown.
        Hide
        Ole Ersoy added a comment -

        Brilliant - Thanks!

        Show
        Ole Ersoy added a comment - Brilliant - Thanks!

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Ole Ersoy
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: