Merge pull request #88 from oleg-nenashev/bug/JENKINS-39414-part2

[JENKINS-39414] - Stapler 1.246 Binary Incompatibility. Part 2

core/src/main/java/org/kohsuke/stapler/lang/Klass.java

@@ -8,6 +8,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nonnull;
 
 /**
  * Abstraction of class-like object, agnostic to languages.
@@ -56,14 +57,15 @@ public List<MethodRef> getDeclaredMethods() {
         return navigator.getDeclaredMethods(clazz);
     }
 
+    /**
+     * Gets list of fields declared by the class.
+     * @return List of fields. 
+     *         May return empty list in the case of obsolete {@link #navigator}, which does not offer the method.
+     * @since 1.246
+     */
+    @Nonnull
     public List<FieldRef> getDeclaredFields() {
-        try {
-            return navigator.getDeclaredFields(clazz);
-        } catch (AbstractMethodError err) {
-            // A plugin uses obsolete version of Stapler-dependent library (e.g. JRuby), which does not offer the method (JENKINS-39414)
-            // TODO: what to do with Logging? The error must be VERY visible, but it will totally pollute system logs
-            return Collections.emptyList();
-        }
+        return navigator.getDeclaredFields(clazz);
     }
 
     /**
@@ -85,6 +87,13 @@ public List<FieldRef> getFields() {
         return new ArrayList<FieldRef>(fields.values());
     }
 
+    /**
+     * Reports all the methods that can be used for routing requests on this class.
+     * @return List of functions. 
+     *         May return empty list in the case of obsolete {@link #navigator}, which does not offer the method.
+     * @since 1.246
+     */
+    @Nonnull
     public List<Function> getFunctions() {
         return navigator.getFunctions(clazz);
     }

core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java

@@ -10,8 +10,10 @@
 import java.net.URL;
 import java.util.AbstractList;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nonnull;
 
 /**
  * Strategy pattern to provide navigation across class-like objects in other languages of JVM.
@@ -78,15 +80,28 @@ public abstract class KlassNavigator<C> {
 
     /**
      * List fields of this class.
-     *
      * This list excludes fields from super classes.
+     * @param clazz Class
+     * @return List of the fields declared for the class.
+     *         By default this list is empty, {@link KlassNavigator} implementations are responsible to implement it.
+     * @since 1.246
      */
-    public abstract List<FieldRef> getDeclaredFields(C clazz);
+    @Nonnull
+    public List<FieldRef> getDeclaredFields(C clazz) {
+        return Collections.emptyList();
+    }
 
     /**
      * Reports all the methods that can be used for routing requests on this class.
+     * @param clazz Class
+     * @return List of the fields functions declared for the class.
+     *         By default this list is empty, {@link KlassNavigator} implementations are responsible to implement it.
+     * @since 1.246
      */
-    public abstract List<Function> getFunctions(C clazz);
+    @Nonnull
+    public List<Function> getFunctions(C clazz) {
+        return Collections.emptyList();
+    }
 
     /**
      * If the given type is an array that supports index retrieval.

core/src/main/java/org/kohsuke/stapler/lang/MethodRef.java

@@ -4,6 +4,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import javax.annotation.CheckForNull;
 
 /**
  * @author Kohsuke Kawaguchi
@@ -16,6 +17,18 @@ public abstract class MethodRef extends AnnotatedRef {
     public boolean isRoutable() {
         return true;
     }
+
+    /**
+     * Retrieves the referenced method name.
+     * Some implementations (e.g. Ruby) cannot guarantee availability of names for all cases,
+     * so sometimes the name may be missing.
+     * @return Method name. {@code null} if it cannot be determined.
+     * @since 1.248
+     */
+    @CheckForNull
+    public String getName() {
+        return null;
+    }
 
     public abstract Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException;
 
@@ -33,7 +46,12 @@ public boolean isRoutable() {
                 if (m.isBridge())    return false;
                 return (m.getModifiers() & Modifier.PUBLIC)!=0;
             }
-
+
+            @Override
+            public String getName() {
+                return m.getName();
+            }
+
             @Override
             public Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException {
                 return m.invoke(_this,args);

core/src/main/java/org/kohsuke/stapler/lang/util/MethodRefFilter.java

@@ -19,6 +19,11 @@ public boolean isRoutable() {
         return getBase().isRoutable();
     }
 
+    @Override
+    public String getName() {
+        return getBase().getName();
+    }
+
     @Override
     public Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException {
         return getBase().invoke(_this, args);

core/src/test/java/org/kohsuke/stapler/lang/KlassTest.java

@@ -4,13 +4,27 @@
 import org.junit.Test;
 
 import java.util.List;
+import org.kohsuke.stapler.Function;
 
 /**
  * Contains tests for {@link Klass}.
  * @author Oleg Nenashev.
  */
 public class KlassTest {
 
+    @Test
+    public void shouldProperlyAccessJavaDeclaredMethods() throws Exception {
+        final Klass<Class> classInstance = Klass.java(FooClass.class);
+        final List<MethodRef> declaredFunctions = classInstance.getDeclaredMethods();
+        for (MethodRef ref : declaredFunctions) {
+            if ("doDynamic".equals(ref.getName())) {
+                //TODO: check field parameters once Stapler provides such info
+                return;
+            }
+        }
+        Assert.fail("Have not found the 'doDynamic' declared method for FooClass");
+    }
+
     @Test
     public void shouldProperlyAccessJavaDeclaredFields() throws Exception {
         final Klass<Class> classInstance = Klass.java(FooClass.class);
@@ -23,8 +37,26 @@ public void shouldProperlyAccessJavaDeclaredFields() throws Exception {
         }
         Assert.fail("Have not found 'fooField' in the returned field list");
     }
+
+    @Test
+    public void shouldProperlyAccessJavaDeclaredFunctions() throws Exception {
+        final Klass<Class> classInstance = Klass.java(FooClass.class);
+        final List<Function> declaredFunctions = classInstance.getFunctions();
+        for (Function ref : declaredFunctions) {
+            if ("doDynamic".equals(ref.getName())) {
+                //TODO: check field parameters once Stapler provides such info
+                return;
+            }
+        }
+        Assert.fail("Have not found 'doDynamic' function for FooClass");
+    }
 
     private static final class FooClass {
         private int fooField;
+
+        public Object doDynamic(String token) {
+            // Just return something potentially routable
+            return new Integer(0);
+        }
     }
 }

jruby/src/main/java/org/kohsuke/stapler/jelly/jruby/RubyMethodRef.java

@@ -8,20 +8,48 @@
 
 import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationTargetException;
+import javax.annotation.Nonnull;
 
 /**
  * @author Kohsuke Kawaguchi
  */
 public class RubyMethodRef extends MethodRef {
+    @Nonnull
     private final RubyModule klass;
+    @Nonnull
     private final DynamicMethod method;
 
 
-    public RubyMethodRef(RubyModule klass, DynamicMethod method) {
+    public RubyMethodRef(@Nonnull RubyModule klass, @Nonnull DynamicMethod method) {
         this.klass = klass;
         this.method = method;
     }
 
+    /**
+     * Retrieves the Ruby module (aka class), for which the method is declared.
+     * @return Ruby module, which stores the method reference
+     * @since 1.248
+     */
+    @Nonnull
+    public RubyModule getKlass() {
+        return klass;
+    }
+
+    /**
+     * Retrieves the referenced method.
+     * @return Referenced method
+     * @since 1.248
+     */
+    @Nonnull
+    public DynamicMethod getMethod() {
+        return method;
+    }
+
+    @Override
+    public String getName() {
+        return method.getName();
+    }
+
     @Override
     public <T extends Annotation> T getAnnotation(Class<T> type) {
         // TODO: what's the equivalent in JRuby?

jruby/src/test/java/org/kohsuke/stapler/jelly/jruby/RubyKlassNavigatorTest.java

@@ -16,22 +16,59 @@
 
 import static org.hamcrest.CoreMatchers.*;
 import org.hamcrest.collection.IsEmptyCollection;
+import org.jruby.RubyFixnum;
+import org.jruby.RubyInteger;
+import org.jruby.internal.runtime.methods.CallConfiguration;
+import org.jruby.internal.runtime.methods.DynamicMethod;
+import org.jruby.runtime.Block;
+import org.jruby.runtime.ThreadContext;
+import org.jruby.runtime.Visibility;
+import org.jruby.runtime.builtin.IRubyObject;
 import static org.junit.Assume.assumeThat;
+import org.kohsuke.stapler.Function;
+import org.kohsuke.stapler.lang.MethodRef;
 
 /**
  * Tests handling of Ruby classes and modules with {@link RubyKlassNavigator}.
  * @author Oleg Nenashev.
  */
 public class RubyKlassNavigatorTest {
 
+    /**
+     * Verifies that declared method retrieval works well.
+     * Effective use-case - Ruby Runtime Plugin for Jenkins.
+     * @throws Exception Test failure
+     */
+    @Test
+    public void shouldProperlyHandleDeclaredMethodsInRubyModules() throws Exception {
+        final ScriptingContainer ruby = createRubyInstance();
+        final RubyKlassNavigator navigator = new RubyKlassNavigator(ruby.getProvider().getRuntime(), ClassLoader.getSystemClassLoader());
+        final MyRubyModule myModule = new MyRubyModule(ruby.getRuntime(), new MyRubyClass(ruby.getRuntime()), true);
+
+
+        final Klass<RubyModule> classInstance = new Klass<RubyModule>(myModule, navigator);
+
+        final List<MethodRef> declaredMethods = classInstance.getDeclaredMethods();
+        for (MethodRef ref : declaredMethods) {
+            if (ref instanceof RubyMethodRef) {
+                // Ruby engine API allows creating methods with null names, not our bug BTW...
+                if ("doDynamic".equals(ref.getName())) {
+                    //TODO: More consistency checks
+                    return;
+                }
+            }
+        }
+        Assert.fail("Have not found 'doDynamic' in the returned function list");
+    }
+
     /**
      * Verifies that field retrieval do not fail horribly for {@link RubyModule}.
      * Effective use-case - Ruby Runtime Plugin for Jenkins.
      * @throws Exception Test failure
      */
     @Test
     @Issue("JENKINS-39414")
-    public void shouldProperlyHandleRubyModules() throws Exception {
+    public void shouldProperlyHandleFieldsInRubyModules() throws Exception {
         final ScriptingContainer ruby = createRubyInstance();
         final RubyKlassNavigator navigator = new RubyKlassNavigator(ruby.getProvider().getRuntime(), ClassLoader.getSystemClassLoader());
         final MyRubyModule myModule = new MyRubyModule(ruby.getRuntime(), new MyRubyClass(ruby.getRuntime()), true);
@@ -51,19 +88,66 @@ public void shouldProperlyHandleRubyModules() throws Exception {
         Assert.fail("Have not found 'fooField' in the returned field list");
     }
 
-    private static final class MyRubyModule extends RubyModule {
+    //TODO: fix the test when Ruby routing gets implemented (https://github.com/stapler/stapler/issues/87)
+    /**
+     * Verifies that function retrieval do not fail horribly for {@link RubyModule}.
+     * Effective use-case - Ruby Runtime Plugin for Jenkins.
+     * @throws Exception Test failure
+     */
+    @Test
+    @Issue("JENKINS-39414")
+    public void shouldProperlyHandleFunctionsInRubyModules() throws Exception {
+        final ScriptingContainer ruby = createRubyInstance();
+        final RubyKlassNavigator navigator = new RubyKlassNavigator(ruby.getProvider().getRuntime(), ClassLoader.getSystemClassLoader());
+        final MyRubyModule myModule = new MyRubyModule(ruby.getRuntime(), new MyRubyClass(ruby.getRuntime()), true);
 
-        private int fooField = 123;
 
+        final Klass<RubyModule> classInstance = new Klass<RubyModule>(myModule, navigator);
+
+        final List<Function> declaredFunctions = classInstance.getFunctions();
+        for (Function ref : declaredFunctions) {
+            if ("doDynamic".equals(ref.getName())) {
+                //TODO: check fields once implemented in Stapler
+                return;
+            }
+        }
+
+        assumeThat("Routing of declared routable methods is not fully implemented (See https://github.com/stapler/stapler/issues/87)", 
+                ruby, not(anything("Nothing to do in this code")));
+        Assert.fail("Have not found 'doDynamic' in the returned function list");
+    }
+
+    private static final class MyRubyModule extends RubyModule {
+
+        private int fooField = 123;
+
         MyRubyModule(Ruby runtime, RubyClass metaClass, boolean objectSpace) {
             super(runtime, metaClass, objectSpace);
+
+            // Register a Mock method for the class
+            getMethodsForWrite().put("doDynamic", new DynamicMethod(this, Visibility.PUBLIC, CallConfiguration.FrameFullScopeFull, "doDynamic") {
+                @Override
+                public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
+                    return doDynamic("foo");
+                }
+
+                @Override
+                public DynamicMethod dup() {
+                    throw new UnsupportedOperationException("Not supported yet.");
+                }
+            });
+        }
+
+        public final IRubyObject doDynamic(String token) {
+            // Just return a routable object
+            return new RubyFixnum(getRuntime(), 0);
         }
     }
 
-    private static final class MyRubyClass extends RubyClass {
+    private static final class MyRubyClass extends RubyClass {     
         MyRubyClass(Ruby runtime) {
             super(runtime);
-        }
+        }   
     }
 
     private ScriptingContainer createRubyInstance() {