From 4f12b1efc29165947b15bcad2a19cf5f3cabef7e Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Wed, 15 Jun 2016 17:38:29 +0900 Subject: [PATCH 01/16] Generalize multi-lingual support of Stapler to include routing Originally this capability was added to support JRuby, and so far it was only used to find @PostConstruct method. But if we expand this Klass and KlassNavigator abstraction to request routing, then we can use this to build a parallel type hierarchy. See issue #76 --- .../org/kohsuke/stapler/KlassDescriptor.java | 29 +++++++++++ .../java/org/kohsuke/stapler/MetaClass.java | 19 ++++--- .../kohsuke/stapler/lang/AnnotatedRef.java | 17 ++++++ .../org/kohsuke/stapler/lang/FieldRef.java | 52 +++++++++++++++++++ .../java/org/kohsuke/stapler/lang/Klass.java | 10 ++++ .../kohsuke/stapler/lang/KlassNavigator.java | 23 ++++++++ .../org/kohsuke/stapler/lang/MethodRef.java | 18 +++++-- .../jelly/jruby/RubyKlassNavigator.java | 18 +++++++ 8 files changed, 173 insertions(+), 13 deletions(-) create mode 100644 core/src/main/java/org/kohsuke/stapler/KlassDescriptor.java create mode 100644 core/src/main/java/org/kohsuke/stapler/lang/AnnotatedRef.java create mode 100644 core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java diff --git a/core/src/main/java/org/kohsuke/stapler/KlassDescriptor.java b/core/src/main/java/org/kohsuke/stapler/KlassDescriptor.java new file mode 100644 index 0000000000..1b669cf7eb --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/KlassDescriptor.java @@ -0,0 +1,29 @@ +package org.kohsuke.stapler; + +import org.kohsuke.stapler.lang.FieldRef; +import org.kohsuke.stapler.lang.Klass; + +import java.util.List; + +/** + * Reflection information of a {@link Klass} that drives the request routing. + * + *

+ * Born as a generalization of {@link ClassDescriptor} to {@link Klass}. + * @author Kohsuke Kawaguchi + */ +class KlassDescriptor { + final Klass clazz; + final FunctionList methods; + final List fields; + + /** + * @param klazz + * The class to build a descriptor around. + */ + public KlassDescriptor(Klass klazz) { + this.clazz = klazz; + this.fields = klazz.getFields(); + this.methods = new FunctionList(klazz.getFunctions()); + } +} diff --git a/core/src/main/java/org/kohsuke/stapler/MetaClass.java b/core/src/main/java/org/kohsuke/stapler/MetaClass.java index d90a786363..7eb8ee425d 100644 --- a/core/src/main/java/org/kohsuke/stapler/MetaClass.java +++ b/core/src/main/java/org/kohsuke/stapler/MetaClass.java @@ -26,13 +26,13 @@ import net.sf.json.JSONArray; import org.apache.commons.io.IOUtils; import org.kohsuke.stapler.bind.JavaScriptMethod; +import org.kohsuke.stapler.lang.FieldRef; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.MethodRef; import javax.annotation.PostConstruct; import javax.servlet.ServletException; import java.io.IOException; -import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Type; import java.util.ArrayList; @@ -103,7 +103,7 @@ public class MetaClass extends TearOffSupport { */ /*package*/ void buildDispatchers() { this.dispatchers.clear(); - ClassDescriptor node = new ClassDescriptor(clazz,null/*TODO:support wrappers*/); + KlassDescriptor node = new KlassDescriptor(klass); // check action .do(...) and other WebMethods for( final Function f : node.methods.webMethods() ) { @@ -162,7 +162,7 @@ public String toString() { } // check public properties of the form NODE.TOKEN - for (final Field f : node.fields) { + for (final FieldRef f : node.fields) { dispatchers.add(new NameBasedDispatcher(f.getName()) { final String role = getProtectedRole(f); public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException { @@ -175,7 +175,7 @@ public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws return true; } public String toString() { - return String.format("%1$s.%2$s for url=/%2$s/...",f.getDeclaringClass().getName(),f.getName()); + return String.format("%1$s for url=/%2$s/...",f.getQualifiedName(),f.getName()); } }); } @@ -278,7 +278,8 @@ public String toString() { }); } - if(node.clazz.isArray()) { + // TODO: Klass needs to be able to define its array like access + if(node.clazz.toJavaClass().isArray()) { dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { if(!req.tokens.hasMore()) @@ -299,7 +300,8 @@ public String toString() { }); } - if(List.class.isAssignableFrom(node.clazz)) { + // TODO: Klass needs to be able to define its list like access + if(List.class.isAssignableFrom(node.clazz.toJavaClass())) { dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { if(!req.tokens.hasMore()) @@ -328,7 +330,8 @@ public String toString() { }); } - if(Map.class.isAssignableFrom(node.clazz)) { + // TODO: Klass needs to be able to define its map like access + if(Map.class.isAssignableFrom(node.clazz.toJavaClass())) { dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { if(!req.tokens.hasMore()) @@ -430,7 +433,7 @@ public SingleLinkedList getPostConstructMethods() { return postConstructMethods; } - private String getProtectedRole(Field f) { + private String getProtectedRole(FieldRef f) { try { LimitedTo a = f.getAnnotation(LimitedTo.class); return (a!=null)?a.value():null; diff --git a/core/src/main/java/org/kohsuke/stapler/lang/AnnotatedRef.java b/core/src/main/java/org/kohsuke/stapler/lang/AnnotatedRef.java new file mode 100644 index 0000000000..dea8b64a90 --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/lang/AnnotatedRef.java @@ -0,0 +1,17 @@ +package org.kohsuke.stapler.lang; + +import java.lang.annotation.Annotation; + +/** + * @author Kohsuke Kawaguchi + */ +public abstract class AnnotatedRef { + // no subtyping outside the package + /*package*/ AnnotatedRef() {} + + public abstract T getAnnotation(Class type); + + public boolean hasAnnotation(Class type) { + return getAnnotation(type)!=null; + } +} diff --git a/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java b/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java new file mode 100644 index 0000000000..d7d7c4e682 --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java @@ -0,0 +1,52 @@ +package org.kohsuke.stapler.lang; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Field; + +/** + * @author Kohsuke Kawaguchi + */ +public abstract class FieldRef extends AnnotatedRef { + /** + * Name of the method. + * + * @see Field#getName() + */ + public abstract String getName(); + + /** + * Obtains the value of the field of the instance. + */ + public abstract Object get(Object instance) throws IllegalAccessException; + + /** + * Gets a fully qualified name of this field that includes the declaring type. + */ + public abstract String getQualifiedName(); + + public static FieldRef wrap(final Field f) { + f.setAccessible(true); + + return new FieldRef() { + @Override + public T getAnnotation(Class type) { + return f.getAnnotation(type); + } + + @Override + public String getName() { + return f.getName(); + } + + @Override + public Object get(Object instance) throws IllegalAccessException { + return f.get(instance); + } + + @Override + public String getQualifiedName() { + return f.getDeclaringClass().getName()+"."+getName(); + } + }; + } +} diff --git a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java index 92f3afbeaf..08c4b32b4d 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java @@ -1,5 +1,7 @@ package org.kohsuke.stapler.lang; +import org.kohsuke.stapler.Function; + import java.net.URL; import java.util.List; @@ -50,6 +52,14 @@ public List getDeclaredMethods() { return navigator.getDeclaredMethods(clazz); } + public List getFields() { + return navigator.getDeclaredFields(clazz); + } + + public List getFunctions() { + return navigator.getFunctions(clazz); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index ae6290aae6..fcd7debf6b 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -1,5 +1,7 @@ package org.kohsuke.stapler.lang; +import org.kohsuke.stapler.ClassDescriptor; +import org.kohsuke.stapler.Function; import org.kohsuke.stapler.MetaClassLoader; import java.lang.reflect.Method; @@ -71,6 +73,15 @@ public abstract class KlassNavigator { */ public abstract List getDeclaredMethods(C clazz); + /** + * List fields of this class. + * + * This list excludes fields from super classes. + */ + public abstract List getDeclaredFields(C clazz); + + public abstract List getFunctions(C clazz); + public static final KlassNavigator JAVA = new KlassNavigator() { @Override public URL getResource(Class clazz, String resourceName) { @@ -125,5 +136,17 @@ public int size() { } }; } + + @Override + public List getDeclaredFields(Class clazz) { + return null; + } + + @Override + public List getFunctions(Class clazz) { + // Historically ClassDescriptor used to own this non-trivial logic of computing + // valid functions for the class, so we'll keep it there. + return new ClassDescriptor(clazz).methods; + } }; } diff --git a/core/src/main/java/org/kohsuke/stapler/lang/MethodRef.java b/core/src/main/java/org/kohsuke/stapler/lang/MethodRef.java index 2221dbea3e..cbe237f95c 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/MethodRef.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/MethodRef.java @@ -3,16 +3,18 @@ import java.lang.annotation.Annotation; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; /** * @author Kohsuke Kawaguchi * @since 1.220 */ -public abstract class MethodRef { - public abstract T getAnnotation(Class type); - - public boolean hasAnnotation(Class type) { - return getAnnotation(type)!=null; +public abstract class MethodRef extends AnnotatedRef { + /** + * Returns true if this method is a 'public' method that should be used for routing requests. + */ + public boolean isRoutable() { + return true; } public abstract Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException; @@ -26,6 +28,12 @@ public T getAnnotation(Class type) { return m.getAnnotation(type); } + @Override + public boolean isRoutable() { + if (m.isBridge()) return false; + return (m.getModifiers() & Modifier.PUBLIC)!=0; + } + @Override public Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException { return m.invoke(_this,args); diff --git a/jruby/src/main/java/org/kohsuke/stapler/jelly/jruby/RubyKlassNavigator.java b/jruby/src/main/java/org/kohsuke/stapler/jelly/jruby/RubyKlassNavigator.java index f4ad549b39..2fb03bb7d5 100644 --- a/jruby/src/main/java/org/kohsuke/stapler/jelly/jruby/RubyKlassNavigator.java +++ b/jruby/src/main/java/org/kohsuke/stapler/jelly/jruby/RubyKlassNavigator.java @@ -5,17 +5,23 @@ import org.jruby.RubyModule; import org.jruby.RubyObject; import org.jruby.internal.runtime.methods.DynamicMethod; +import org.kohsuke.stapler.ClassDescriptor; +import org.kohsuke.stapler.Function; import org.kohsuke.stapler.MetaClassLoader; +import org.kohsuke.stapler.lang.FieldRef; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.KlassNavigator; import org.kohsuke.stapler.lang.MethodRef; import java.net.URL; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Locale; /** + * {@link KlassNavigator} implementation for JRuby. + * * @author Kohsuke Kawaguchi */ public class RubyKlassNavigator extends KlassNavigator { @@ -72,6 +78,18 @@ public List getDeclaredMethods(RubyModule clazz) { return r; } + @Override + public List getDeclaredFields(RubyModule clazz) { + // IIUC, Ruby doesn't have statically defined instance fields + return Collections.emptyList(); + } + + @Override + public List getFunctions(RubyModule clazz) { + // implemented as a fallback to Java through reified class, but maybe there's a better way to do this + return new ClassDescriptor(toJavaClass(clazz)).methods; + } + @Override public Class toJavaClass(RubyModule clazz) { if (clazz instanceof RubyClass) { From 8541922a1469a2965175afae74ee70d947a11327 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Wed, 15 Jun 2016 17:53:35 +0900 Subject: [PATCH 02/16] Implemented a place holder --- .../java/org/kohsuke/stapler/lang/Klass.java | 25 ++++++++++++++++++- .../kohsuke/stapler/lang/KlassNavigator.java | 14 ++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java index 08c4b32b4d..465f504856 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java @@ -2,8 +2,12 @@ import org.kohsuke.stapler.Function; +import java.lang.reflect.Field; import java.net.URL; +import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; /** * Abstraction of class-like object, agnostic to languages. @@ -52,10 +56,29 @@ public List getDeclaredMethods() { return navigator.getDeclaredMethods(clazz); } - public List getFields() { + public List getDeclaredFields() { return navigator.getDeclaredFields(clazz); } + /** + * Gets all the public fields defined in this type, including super types. + * + * @see Class#getFields() + */ + public List getFields() { + Map fields = new LinkedHashMap(); + for (Klass k = this; k!=null; k=k.getSuperClass()) { + for (FieldRef f : getDeclaredFields()) { + String name = f.getName(); + if (!fields.containsKey(name)) { + fields.put(name,f); + } + } + } + + return new ArrayList(fields.values()); + } + public List getFunctions() { return navigator.getFunctions(clazz); } diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index fcd7debf6b..b81d2ac958 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -4,6 +4,7 @@ import org.kohsuke.stapler.Function; import org.kohsuke.stapler.MetaClassLoader; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.net.URL; import java.util.AbstractList; @@ -139,7 +140,18 @@ public int size() { @Override public List getDeclaredFields(Class clazz) { - return null; + final Field[] fields = clazz.getDeclaredFields(); + return new AbstractList() { + @Override + public FieldRef get(int index) { + return FieldRef.wrap(fields[index]); + } + + @Override + public int size() { + return fields.length; + } + }; } @Override From 8ad9ffb141cf94154be073dbed584f1ae81c8093 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Wed, 15 Jun 2016 18:37:27 +0900 Subject: [PATCH 03/16] Test case as PoC --- .../main/java/org/kohsuke/stapler/WebApp.java | 8 ++ .../org/kohsuke/stapler/lang/FieldRef.java | 3 + .../org/kohsuke/stapler/lang/KInstance.java | 21 +++++ .../kohsuke/stapler/lang/KlassNavigator.java | 2 +- .../stapler/lang/util/FieldRefFilter.java | 44 +++++++++ .../stapler/lang/util/MethodRefFilter.java | 40 ++++++++ .../kohsuke/stapler/jelly/issue76/Eye.java | 12 +++ .../kohsuke/stapler/jelly/issue76/Head.java | 7 ++ .../stapler/jelly/issue76/Issue76Test.java | 33 +++++++ .../stapler/jelly/issue76/ProtectedClass.java | 93 +++++++++++++++++++ .../stapler/jelly/issue76/Protection.java | 28 ++++++ .../kohsuke/stapler/jelly/issue76/Robot.java | 19 ++++ .../stapler/jelly/issue76/Head/index.jelly | 1 + 13 files changed, 310 insertions(+), 1 deletion(-) create mode 100644 core/src/main/java/org/kohsuke/stapler/lang/KInstance.java create mode 100644 core/src/main/java/org/kohsuke/stapler/lang/util/FieldRefFilter.java create mode 100644 core/src/main/java/org/kohsuke/stapler/lang/util/MethodRefFilter.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java create mode 100644 jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Head/index.jelly diff --git a/core/src/main/java/org/kohsuke/stapler/WebApp.java b/core/src/main/java/org/kohsuke/stapler/WebApp.java index ac868b03e6..1fa88c5055 100644 --- a/core/src/main/java/org/kohsuke/stapler/WebApp.java +++ b/core/src/main/java/org/kohsuke/stapler/WebApp.java @@ -25,6 +25,7 @@ import net.sf.json.JSONObject; import org.kohsuke.stapler.bind.BoundObjectTable; +import org.kohsuke.stapler.lang.KInstance; import org.kohsuke.stapler.lang.Klass; import javax.servlet.Filter; @@ -218,6 +219,13 @@ public MetaClass getMetaClass(Object o) { } public Klass getKlass(Object o) { + if (o instanceof KInstance) { + KInstance ki = (KInstance) o; + Klass k = ki.getKlass(); + if (k!=null) + return k; + } + for (Facet f : facets) { Klass k = f.getKlass(o); if (k!=null) diff --git a/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java b/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java index d7d7c4e682..076e835037 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/FieldRef.java @@ -4,6 +4,9 @@ import java.lang.reflect.Field; /** + * Fields of {@link Klass}. + * + * @see Klass#getDeclaredFields() * @author Kohsuke Kawaguchi */ public abstract class FieldRef extends AnnotatedRef { diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KInstance.java b/core/src/main/java/org/kohsuke/stapler/lang/KInstance.java new file mode 100644 index 0000000000..143ae0ac1d --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/lang/KInstance.java @@ -0,0 +1,21 @@ +package org.kohsuke.stapler.lang; + +import org.kohsuke.stapler.Facet; + +/** + * Objects can implement this interface to designate its own {@link Klass}. + * + * This allows specific classes or instances to take over the routing rules without + * going through the trouble of creating a new {@link Facet} + * + * @author Kohsuke Kawaguchi + */ +public interface KInstance { + /** + * @return + * null if there's no designated {@link Klass} for this instance, in which + * case Stapler treats this instance normally as if it didn't implement + * {@link KInstance} to begin with (for example, by calling {@link Object#getClass()} + */ + Klass getKlass(); +} diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index b81d2ac958..ee54c13aad 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -103,7 +103,7 @@ public URL getResource(Class clazz, String resourceName) { } @Override - public Klass getSuperClass(Class clazz) { + public Klass getSuperClass(Class clazz) { return Klass.java(clazz.getSuperclass()); } diff --git a/core/src/main/java/org/kohsuke/stapler/lang/util/FieldRefFilter.java b/core/src/main/java/org/kohsuke/stapler/lang/util/FieldRefFilter.java new file mode 100644 index 0000000000..43f5693dfa --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/lang/util/FieldRefFilter.java @@ -0,0 +1,44 @@ +package org.kohsuke.stapler.lang.util; + +import org.kohsuke.stapler.lang.FieldRef; + +import java.lang.annotation.Annotation; +import java.lang.reflect.Field; + +/** + * {@link FieldRef} filter as a convenience class. + * + * @author Kohsuke Kawaguchi + */ +public abstract class FieldRefFilter extends FieldRef { + protected abstract FieldRef getBase(); + + @Override + public String getName() { + return getBase().getName(); + } + + @Override + public Object get(Object instance) throws IllegalAccessException { + return getBase().get(instance); + } + + @Override + public String getQualifiedName() { + return getBase().getQualifiedName(); + } + + public static FieldRef wrap(Field f) { + return FieldRef.wrap(f); + } + + @Override + public T getAnnotation(Class type) { + return getBase().getAnnotation(type); + } + + @Override + public boolean hasAnnotation(Class type) { + return getBase().hasAnnotation(type); + } +} diff --git a/core/src/main/java/org/kohsuke/stapler/lang/util/MethodRefFilter.java b/core/src/main/java/org/kohsuke/stapler/lang/util/MethodRefFilter.java new file mode 100644 index 0000000000..70ecf3c24f --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/lang/util/MethodRefFilter.java @@ -0,0 +1,40 @@ +package org.kohsuke.stapler.lang.util; + +import org.kohsuke.stapler.lang.MethodRef; + +import java.lang.annotation.Annotation; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +/** + * {@link MethodRef} filter as a convenience class. + * + * @author Kohsuke Kawaguchi + */ +public abstract class MethodRefFilter extends MethodRef { + protected abstract MethodRef getBase(); + + @Override + public boolean isRoutable() { + return getBase().isRoutable(); + } + + @Override + public Object invoke(Object _this, Object... args) throws InvocationTargetException, IllegalAccessException { + return getBase().invoke(_this, args); + } + + public static MethodRef wrap(Method m) { + return MethodRef.wrap(m); + } + + @Override + public T getAnnotation(Class type) { + return getBase().getAnnotation(type); + } + + @Override + public boolean hasAnnotation(Class type) { + return getBase().hasAnnotation(type); + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java new file mode 100644 index 0000000000..0c6e0bcdb7 --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java @@ -0,0 +1,12 @@ +package org.kohsuke.stapler.jelly.issue76; + +/** + * @author Kohsuke Kawaguchi + */ +public class Eye { + public final int i; + + public Eye(int i) { + this.i = i; + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java new file mode 100644 index 0000000000..21710fcaaf --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java @@ -0,0 +1,7 @@ +package org.kohsuke.stapler.jelly.issue76; + +/** + * @author Kohsuke Kawaguchi + */ +public class Head { +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java new file mode 100644 index 0000000000..6e4a44570f --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -0,0 +1,33 @@ +package org.kohsuke.stapler.jelly.issue76; + +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.WebClient; +import com.gargoylesoftware.htmlunit.html.HtmlPage; +import org.kohsuke.stapler.test.JettyTestCase; + +import java.io.IOException; +import java.net.URL; + +/** + * @author Kohsuke Kawaguchi + */ +public class Issue76Test extends JettyTestCase { + // bound to /robot + public final Robot robot = new Robot(); + + public final Protection protectedRobot = new Protection(robot); + + public void testRouting() throws Exception { + WebClient wc = new WebClient(); + HtmlPage p = wc.getPage(new URL(url, "robot/head/")); + assertTrue(p.getWebResponse().getContentAsString().contains("This is head")); + + // protected parts do not expose Jelly views + try { + wc.getPage(new URL(url, "protectedRobot/head/")); + fail("Expected 404"); + } catch (FailingHttpStatusCodeException e) { + assertEquals(404,e.getStatusCode()); + } + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java new file mode 100644 index 0000000000..66e8a9da63 --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -0,0 +1,93 @@ +package org.kohsuke.stapler.jelly.issue76; + +import org.kohsuke.stapler.Function; +import org.kohsuke.stapler.lang.FieldRef; +import org.kohsuke.stapler.lang.Klass; +import org.kohsuke.stapler.lang.KlassNavigator; +import org.kohsuke.stapler.lang.MethodRef; +import org.kohsuke.stapler.lang.util.FieldRefFilter; + +import java.net.URL; +import java.util.ArrayList; +import java.util.List; + +/** + * @author Kohsuke Kawaguchi + */ +public class ProtectedClass { + private final Class c; + + public ProtectedClass(Class c) { + this.c = c; + } + + public static KlassNavigator NAVIGATOR = new KlassNavigator() { + private Klass protect(Klass c) { + if (c==null) return null; + return new Klass(new ProtectedClass((Class)c.clazz), NAVIGATOR); + } + + // no view + @Override + public URL getResource(ProtectedClass clazz, String resourceName) { + return null; + } + + @Override + public Iterable> getAncestors(ProtectedClass clazz) { + List> r = new ArrayList>(); + for (Klass c : JAVA.getAncestors(clazz.c)) { + r.add(protect(c)); + } + return r; + } + + @Override + public Klass getSuperClass(ProtectedClass clazz) { + return protect(JAVA.getSuperClass(clazz.c)); + } + + @Override + public Class toJavaClass(ProtectedClass clazz) { + return ProtectedClass.class; + } + + @Override + public List getDeclaredMethods(ProtectedClass clazz) { + return JAVA.getDeclaredMethods(clazz.c); + } + + @Override + public List getDeclaredFields(ProtectedClass clazz) { + List r = new ArrayList(); + for (final FieldRef f : JAVA.getDeclaredFields(clazz.c)) { + r.add(new FieldRefFilter() { + @Override + protected FieldRef getBase() { + return f; + } + + @Override + public Object get(Object instance) throws IllegalAccessException { + // as we route requests, keep protecting objects + Object o = super.get(unwrap(instance)); + if (o!=null) + o = new Protection(o); + return o; + } + }); + } + return r; + } + + private Object unwrap(Object instance) { + if (instance==null) return null; + return ((Protection)instance).o; + } + + @Override + public List getFunctions(ProtectedClass clazz) { + return JAVA.getFunctions(clazz.c); + } + }; +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java new file mode 100644 index 0000000000..f422410c2c --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java @@ -0,0 +1,28 @@ +package org.kohsuke.stapler.jelly.issue76; + +import org.kohsuke.stapler.lang.KInstance; +import org.kohsuke.stapler.lang.Klass; + +/** + * Wraps various parts of robots and tweaks its routing behaviour. + * + * @author Kohsuke Kawaguchi + */ +public class Protection implements KInstance { + /** + * Object being wrapped. + */ + public final Object o; + + public Protection(Object o) { + this.o = o; + } + + /** + * Fool the reflection so that Stapler sees routes exposed by {@code o.getClass()}. + */ + @Override + public Klass getKlass() { + return new Klass(new ProtectedClass(o.getClass()),ProtectedClass.NAVIGATOR); + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java new file mode 100644 index 0000000000..d5e9aec3d3 --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java @@ -0,0 +1,19 @@ +package org.kohsuke.stapler.jelly.issue76; + +/** + * Root of the model object. + * + *

+ * Test ground of various traversal logic. + * + * @author Kohsuke Kawaguchi + */ +public class Robot { + public Eye getEye(int i) { + return new Eye(i); + } + + public final Head head = new Head(); + + // TODO: more of this +} diff --git a/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Head/index.jelly b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Head/index.jelly new file mode 100644 index 0000000000..4628ca1fc2 --- /dev/null +++ b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Head/index.jelly @@ -0,0 +1 @@ +This is head \ No newline at end of file From 94113342cf42c2b0e8a732613fc392f82d17f985 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:09:08 +0900 Subject: [PATCH 04/16] doc improvement --- .../src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index ee54c13aad..bff7de401b 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -81,6 +81,9 @@ public abstract class KlassNavigator { */ public abstract List getDeclaredFields(C clazz); + /** + * Reports all the methods that can be used for routing requests on this class. + */ public abstract List getFunctions(C clazz); public static final KlassNavigator JAVA = new KlassNavigator() { From 6e98eb7a1cc932a96986d50d0f3822102d9dd24b Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:21:17 +0900 Subject: [PATCH 05/16] Exposing it public This class doesn't seem to expose any complicated contract and fairly independent from everything else & easy to keep compatible --- core/src/main/java/org/kohsuke/stapler/FunctionList.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/kohsuke/stapler/FunctionList.java b/core/src/main/java/org/kohsuke/stapler/FunctionList.java index 7256e3469e..54ea26ba03 100644 --- a/core/src/main/java/org/kohsuke/stapler/FunctionList.java +++ b/core/src/main/java/org/kohsuke/stapler/FunctionList.java @@ -31,7 +31,7 @@ * * @author Kohsuke Kawaguchi */ -final class FunctionList extends AbstractList { +public final class FunctionList extends AbstractList { private final Function[] functions; public FunctionList(Function... functions) { From ac22f0110463a44afbdc256ea8d284f93b858fe5 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:22:55 +0900 Subject: [PATCH 06/16] Override the index route --- .../kohsuke/stapler/jelly/issue76/Issue76Test.java | 13 ++++--------- .../stapler/jelly/issue76/ProtectedClass.java | 9 ++++++++- .../kohsuke/stapler/jelly/issue76/Protection.java | 9 +++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index 6e4a44570f..4858da025f 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -1,11 +1,10 @@ package org.kohsuke.stapler.jelly.issue76; -import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; +import com.gargoylesoftware.htmlunit.TextPage; import com.gargoylesoftware.htmlunit.WebClient; import com.gargoylesoftware.htmlunit.html.HtmlPage; import org.kohsuke.stapler.test.JettyTestCase; -import java.io.IOException; import java.net.URL; /** @@ -22,12 +21,8 @@ public void testRouting() throws Exception { HtmlPage p = wc.getPage(new URL(url, "robot/head/")); assertTrue(p.getWebResponse().getContentAsString().contains("This is head")); - // protected parts do not expose Jelly views - try { - wc.getPage(new URL(url, "protectedRobot/head/")); - fail("Expected 404"); - } catch (FailingHttpStatusCodeException e) { - assertEquals(404,e.getStatusCode()); - } + // protected parts have different index view + TextPage tp = wc.getPage(new URL(url, "protectedRobot/head/")); + assertTrue(tp.getContent().startsWith("protected")); } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index 66e8a9da63..dcf67a1d1b 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -1,6 +1,7 @@ package org.kohsuke.stapler.jelly.issue76; import org.kohsuke.stapler.Function; +import org.kohsuke.stapler.FunctionList; import org.kohsuke.stapler.lang.FieldRef; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.KlassNavigator; @@ -12,6 +13,8 @@ import java.util.List; /** + * Used as 'C' of {@code Klass} to represents a protected version of a {@link Class}. + * * @author Kohsuke Kawaguchi */ public class ProtectedClass { @@ -87,7 +90,11 @@ private Object unwrap(Object instance) { @Override public List getFunctions(ProtectedClass clazz) { - return JAVA.getFunctions(clazz.c); + List r = new ArrayList(); + // insert this at the top to make sure that shadows doIndex in subtypes + r.addAll(new FunctionList(JAVA.getFunctions(Protection.class)).name("doIndex")); + r.addAll(JAVA.getFunctions(clazz.c)); + return r; } }; } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java index f422410c2c..f80a66a5d1 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Protection.java @@ -1,5 +1,7 @@ package org.kohsuke.stapler.jelly.issue76; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.lang.KInstance; import org.kohsuke.stapler.lang.Klass; @@ -25,4 +27,11 @@ public Protection(Object o) { public Klass getKlass() { return new Klass(new ProtectedClass(o.getClass()),ProtectedClass.NAVIGATOR); } + + /** + * Override doIndex. + */ + public HttpResponse doIndex() { + return HttpResponses.plainText("protected "+o); + } } From 5782c833b96d1402413c1430badf1470f936f2c5 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:28:36 +0900 Subject: [PATCH 07/16] Protect invocation through method call --- .../kohsuke/stapler/jelly/issue76/Eye.java | 5 ++++ .../kohsuke/stapler/jelly/issue76/Head.java | 3 +++ .../stapler/jelly/issue76/Issue76Test.java | 9 ++++--- .../stapler/jelly/issue76/ProtectedClass.java | 26 ++++++++++++++++++- .../kohsuke/stapler/jelly/issue76/Robot.java | 4 --- .../stapler/jelly/issue76/Eye/index.jelly | 1 + 6 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Eye/index.jelly diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java index 0c6e0bcdb7..1ce6fdb38a 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Eye.java @@ -9,4 +9,9 @@ public class Eye { public Eye(int i) { this.i = i; } + + @Override + public String toString() { + return "eye #"+i; + } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java index 21710fcaaf..abc163e306 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java @@ -4,4 +4,7 @@ * @author Kohsuke Kawaguchi */ public class Head { + public Eye getEye(int i) { + return new Eye(i); + } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index 4858da025f..e9ee2a748c 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -14,15 +14,16 @@ public class Issue76Test extends JettyTestCase { // bound to /robot public final Robot robot = new Robot(); + // protected version of /robot which mimics the URL structure but tweaks its routes public final Protection protectedRobot = new Protection(robot); public void testRouting() throws Exception { WebClient wc = new WebClient(); - HtmlPage p = wc.getPage(new URL(url, "robot/head/")); - assertTrue(p.getWebResponse().getContentAsString().contains("This is head")); + HtmlPage p = wc.getPage(new URL(url, "robot/head/eye/3/")); + assertTrue(p.getWebResponse().getContentAsString().contains("This is eye 3")); // protected parts have different index view - TextPage tp = wc.getPage(new URL(url, "protectedRobot/head/")); - assertTrue(tp.getContent().startsWith("protected")); + TextPage tp = wc.getPage(new URL(url, "protectedRobot/head/eye/3/")); + assertTrue(tp.getContent().startsWith("protected eye #3")); } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index dcf67a1d1b..6dddcbdb06 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -1,17 +1,24 @@ package org.kohsuke.stapler.jelly.issue76; +import org.kohsuke.stapler.ForwardingFunction; import org.kohsuke.stapler.Function; import org.kohsuke.stapler.FunctionList; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.lang.FieldRef; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.KlassNavigator; import org.kohsuke.stapler.lang.MethodRef; import org.kohsuke.stapler.lang.util.FieldRefFilter; +import javax.servlet.ServletException; +import java.lang.reflect.InvocationTargetException; import java.net.URL; import java.util.ArrayList; import java.util.List; +import static javafx.scene.input.KeyCode.F; + /** * Used as 'C' of {@code Klass} to represents a protected version of a {@link Class}. * @@ -88,13 +95,30 @@ private Object unwrap(Object instance) { return ((Protection)instance).o; } + private Object wrap(Object instance) { + if (instance==null) return null; + return new Protection(instance); + } + @Override public List getFunctions(ProtectedClass clazz) { List r = new ArrayList(); // insert this at the top to make sure that shadows doIndex in subtypes r.addAll(new FunctionList(JAVA.getFunctions(Protection.class)).name("doIndex")); - r.addAll(JAVA.getFunctions(clazz.c)); + // expose all the functions from the base type + for (Function f : JAVA.getFunctions(clazz.c)) { + r.add(protect(f)); + } return r; } + + private Function protect(Function f) { + return new ForwardingFunction(f) { + @Override + public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object... args) throws IllegalAccessException, InvocationTargetException, ServletException { + return wrap(super.invoke(req, rsp, unwrap(o), args)); + } + }; + } }; } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java index d5e9aec3d3..4b35693da4 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java @@ -9,10 +9,6 @@ * @author Kohsuke Kawaguchi */ public class Robot { - public Eye getEye(int i) { - return new Eye(i); - } - public final Head head = new Head(); // TODO: more of this diff --git a/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Eye/index.jelly b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Eye/index.jelly new file mode 100644 index 0000000000..27abc41086 --- /dev/null +++ b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Eye/index.jelly @@ -0,0 +1 @@ +This is eye ${it.i} \ No newline at end of file From 9b2033c292206a7b6173c9b93c8af7c1f33c14f5 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:29:20 +0900 Subject: [PATCH 08/16] Incorrect auto-completion put this line --- .../java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index 6dddcbdb06..e6921ef282 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -17,8 +17,6 @@ import java.util.ArrayList; import java.util.List; -import static javafx.scene.input.KeyCode.F; - /** * Used as 'C' of {@code Klass} to represents a protected version of a {@link Class}. * From 390517cdee21284e38f9dd47dbc7139252fc8f02 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:41:38 +0900 Subject: [PATCH 09/16] Adding a doXyz method and investigating the semantics of protection filetering. --- .../org/kohsuke/stapler/jelly/issue76/Head.java | 7 +++++++ .../kohsuke/stapler/jelly/issue76/Issue76Test.java | 3 +++ .../stapler/jelly/issue76/ProtectedClass.java | 13 ++++++++----- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java index abc163e306..2c7bf50d5f 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Head.java @@ -1,5 +1,8 @@ package org.kohsuke.stapler.jelly.issue76; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; + /** * @author Kohsuke Kawaguchi */ @@ -7,4 +10,8 @@ public class Head { public Eye getEye(int i) { return new Eye(i); } + + public HttpResponse doNose() { + return HttpResponses.plainText("nose"); + } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index e9ee2a748c..ba8369d1e9 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -25,5 +25,8 @@ public void testRouting() throws Exception { // protected parts have different index view TextPage tp = wc.getPage(new URL(url, "protectedRobot/head/eye/3/")); assertTrue(tp.getContent().startsWith("protected eye #3")); + + tp = wc.getPage(new URL(url, "protectedRobot/head/nose")); + assertEquals("nose",tp.getContent()); } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index e6921ef282..361c2ef04d 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -78,10 +78,7 @@ protected FieldRef getBase() { @Override public Object get(Object instance) throws IllegalAccessException { // as we route requests, keep protecting objects - Object o = super.get(unwrap(instance)); - if (o!=null) - o = new Protection(o); - return o; + return wrap(super.get(unwrap(instance))); } }); } @@ -93,7 +90,7 @@ private Object unwrap(Object instance) { return ((Protection)instance).o; } - private Object wrap(Object instance) { + private Protection wrap(Object instance) { if (instance==null) return null; return new Protection(instance); } @@ -110,10 +107,16 @@ public List getFunctions(ProtectedClass clazz) { return r; } + /** + * Decorates {@link Function} so that it can be invoked on {@link Protection} and the + * return value gets protected as well. + */ private Function protect(Function f) { return new ForwardingFunction(f) { @Override public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object... args) throws IllegalAccessException, InvocationTargetException, ServletException { + // TODO: either invoke needs to get a context in which the invocation is done (rendering vs invoking?) + // or we rely on the naming convention & annotations to determine how to handle this return wrap(super.invoke(req, rsp, unwrap(o), args)); } }; From f990de3a56750911dfc6be281977d2c4416b0996 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 09:44:07 +0900 Subject: [PATCH 10/16] Doc improvement --- core/src/main/java/org/kohsuke/stapler/Function.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/org/kohsuke/stapler/Function.java b/core/src/main/java/org/kohsuke/stapler/Function.java index d7c941ff75..68b41fa52e 100644 --- a/core/src/main/java/org/kohsuke/stapler/Function.java +++ b/core/src/main/java/org/kohsuke/stapler/Function.java @@ -217,6 +217,9 @@ public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object.. }); } + /** + * @see StaticFunction#RETURN_NULL + */ public static Object returnNull() { return null; } /** From e37ee65bdfe811db34e9986044ccbf016a6a304f Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 10:38:02 +0900 Subject: [PATCH 11/16] Adding a generic mechanism to pass contextual information about the use of Function. (This allows issue #76 to use contextual information to perform to the right interception --- .../kohsuke/stapler/ForwardingFunction.java | 6 ++ .../java/org/kohsuke/stapler/Function.java | 8 ++ .../stapler/JavaScriptMethodContext.java | 27 ++++++ .../java/org/kohsuke/stapler/MetaClass.java | 88 ++++++++++--------- .../stapler/PreInvokeInterceptedFunction.java | 7 ++ .../stapler/TraversalMethodContext.java | 29 ++++++ .../org/kohsuke/stapler/WebMethodContext.java | 29 ++++++ 7 files changed, 154 insertions(+), 40 deletions(-) create mode 100644 core/src/main/java/org/kohsuke/stapler/JavaScriptMethodContext.java create mode 100644 core/src/main/java/org/kohsuke/stapler/TraversalMethodContext.java create mode 100644 core/src/main/java/org/kohsuke/stapler/WebMethodContext.java diff --git a/core/src/main/java/org/kohsuke/stapler/ForwardingFunction.java b/core/src/main/java/org/kohsuke/stapler/ForwardingFunction.java index 1c53741919..0eb55ed7ed 100644 --- a/core/src/main/java/org/kohsuke/stapler/ForwardingFunction.java +++ b/core/src/main/java/org/kohsuke/stapler/ForwardingFunction.java @@ -40,6 +40,12 @@ public Class getReturnType() { return next.getReturnType(); } + // can't really call next.contextualize() + @Override + public Function contextualize(Object usage) { + return this; + } + @Override public Type[] getGenericParameterTypes() { return next.getGenericParameterTypes(); diff --git a/core/src/main/java/org/kohsuke/stapler/Function.java b/core/src/main/java/org/kohsuke/stapler/Function.java index 68b41fa52e..b0c6c8201e 100644 --- a/core/src/main/java/org/kohsuke/stapler/Function.java +++ b/core/src/main/java/org/kohsuke/stapler/Function.java @@ -86,6 +86,14 @@ public abstract class Function { */ public abstract Class getReturnType(); + /** + * Caller uses this method to tell {@link Function} about how it is being used. + * By default, this methods ignores the given context by returning {@code this}. + */ + public Function contextualize(Object usage) { + return this; + } + /** * Calls {@link #bindAndInvoke(Object, StaplerRequest, StaplerResponse, Object...)} and then * optionally serve the response object. diff --git a/core/src/main/java/org/kohsuke/stapler/JavaScriptMethodContext.java b/core/src/main/java/org/kohsuke/stapler/JavaScriptMethodContext.java new file mode 100644 index 0000000000..5300812d1f --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/JavaScriptMethodContext.java @@ -0,0 +1,27 @@ +package org.kohsuke.stapler; + +import org.kohsuke.stapler.bind.JavaScriptMethod; + +/** + * {@link Function#contextualize(Object)} parameter that indicates + * the function is called to serve JavaScript method invocation from a proxy. + * + * @author Kohsuke Kawaguchi + * @see JavaScriptMethod + */ +public final class JavaScriptMethodContext { + private final String name; + + // instantiation restricted to this class + /*package*/ JavaScriptMethodContext(String name) { + this.name = name; + } + + /** + * Name of the web method. "" for index route. + */ + public String getName() { + return name; + } +} + diff --git a/core/src/main/java/org/kohsuke/stapler/MetaClass.java b/core/src/main/java/org/kohsuke/stapler/MetaClass.java index 7eb8ee425d..dd05289dc5 100644 --- a/core/src/main/java/org/kohsuke/stapler/MetaClass.java +++ b/core/src/main/java/org/kohsuke/stapler/MetaClass.java @@ -41,6 +41,7 @@ import java.util.Map; import static javax.servlet.http.HttpServletResponse.*; +import static org.kohsuke.stapler.TraversalMethodContext.DYNAMIC; /** * Created one instance each for a {@link Klass}, @@ -52,12 +53,12 @@ public class MetaClass extends TearOffSupport { /** * This meta class wraps this class - * + * * @deprecated as of 1.177 * Use {@link #klass}. If you really want the Java class representation, use {@code klass.toJavaClass()}. */ public final Class clazz; - + public final Klass klass; /** @@ -99,33 +100,34 @@ public class MetaClass extends TearOffSupport { * *

* This is the meat of URL dispatching. It looks at the class - * via reflection and figures out what URLs are handled by who. + * via reflection and figures out what URLs are handled by who. */ /*package*/ void buildDispatchers() { this.dispatchers.clear(); KlassDescriptor node = new KlassDescriptor(klass); // check action .do(...) and other WebMethods - for( final Function f : node.methods.webMethods() ) { + for (Function f : node.methods.webMethods()) { WebMethod a = f.getAnnotation(WebMethod.class); - + String[] names; if(a!=null && a.name().length>0) names=a.name(); else names=new String[]{camelize(f.getName().substring(2))}; // 'doFoo' -> 'foo' for (String name : names) { + final Function ff = f.contextualize(new WebMethodContext(name)); if (name.length()==0) { - dispatchers.add(new IndexDispatcher(f)); + dispatchers.add(new IndexDispatcher(ff)); } else { dispatchers.add(new NameBasedDispatcher(name) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IllegalAccessException, InvocationTargetException, ServletException, IOException { if (traceable()) - trace(req, rsp, "-> <%s>.%s(...)", node, f.getName()); - return f.bindAndInvokeAndServeResponse(node, req, rsp); + trace(req, rsp, "-> <%s>.%s(...)", node, ff.getName()); + return ff.bindAndInvokeAndServeResponse(node, req, rsp); } public String toString() { - return f.getQualifiedName() + "(...) for url=/" + name + "/..."; + return ff.getQualifiedName() + "(...) for url=/" + name + "/..."; } }); } @@ -134,9 +136,9 @@ public String toString() { // JavaScript proxy method invocations for js // reacts only to a specific content type - for( final Function f : node.methods.prefix("js") ) { + for (Function f : node.methods.prefix("js") ) { String name = camelize(f.getName().substring(2)); // jsXyz -> xyz - + f = f.contextualize(new JavaScriptMethodContext(name)); dispatchers.add(new JavaScriptProxyMethodDispatcher(name, f)); } @@ -150,7 +152,7 @@ public String toString() { else names=new String[]{f.getName()}; for (String name : names) - dispatchers.add(new JavaScriptProxyMethodDispatcher(name,f)); + dispatchers.add(new JavaScriptProxyMethodDispatcher(name,f.contextualize(new JavaScriptMethodContext(name)))); } for (Facet f : webApp.facets) @@ -158,7 +160,7 @@ public String toString() { // check action .doIndex(...) for (Function f : node.methods.name("doIndex")) { - dispatchers.add(new IndexDispatcher(f)); + dispatchers.add(new IndexDispatcher(f.contextualize(new WebMethodContext("")))); } // check public properties of the form NODE.TOKEN @@ -183,97 +185,102 @@ public String toString() { FunctionList getMethods = node.methods.prefix("get"); // check public selector methods of the form NODE.getTOKEN() - for( final Function f : getMethods.signature() ) { + for (Function f : getMethods.signature()) { if(f.getName().length()<=3) continue; String name = camelize(f.getName().substring(3)); // 'getFoo' -> 'foo' + final Function ff = f.contextualize(new TraversalMethodContext(name)); dispatchers.add(new NameBasedDispatcher(name) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException, InvocationTargetException { if(traceable()) - traceEval(req,rsp,node,f.getName()+"()"); - req.getStapler().invoke(req,rsp, f.invoke(req, rsp, node)); + traceEval(req,rsp,node,ff.getName()+"()"); + req.getStapler().invoke(req,rsp, ff.invoke(req, rsp, node)); return true; } public String toString() { - return String.format("%1$s() for url=/%2$s/...",f.getQualifiedName(),name); + return String.format("%1$s() for url=/%2$s/...",ff.getQualifiedName(),name); } }); } // check public selector methods of the form static NODE.getTOKEN(StaplerRequest) - for( final Function f : getMethods.signature(StaplerRequest.class) ) { + for (Function f : getMethods.signature(StaplerRequest.class)) { if(f.getName().length()<=3) continue; String name = camelize(f.getName().substring(3)); // 'getFoo' -> 'foo' + final Function ff = f.contextualize(new TraversalMethodContext(name)); dispatchers.add(new NameBasedDispatcher(name) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException, InvocationTargetException { if(traceable()) - traceEval(req,rsp,node,f.getName()+"(...)"); - req.getStapler().invoke(req,rsp, f.invoke(req, rsp, node, req)); + traceEval(req,rsp,node,ff.getName()+"(...)"); + req.getStapler().invoke(req,rsp, ff.invoke(req, rsp, node, req)); return true; } public String toString() { - return String.format("%1$s(StaplerRequest) for url=/%2$s/...",f.getQualifiedName(),name); + return String.format("%1$s(StaplerRequest) for url=/%2$s/...",ff.getQualifiedName(),name); } }); } // check public selector methods .get(String) - for( final Function f : getMethods.signature(String.class) ) { + for (Function f : getMethods.signature(String.class)) { if(f.getName().length()<=3) continue; String name = camelize(f.getName().substring(3)); // 'getFoo' -> 'foo' + final Function ff = f.contextualize(new TraversalMethodContext(name)); dispatchers.add(new NameBasedDispatcher(name,1) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException, InvocationTargetException { String token = req.tokens.next(); if(traceable()) - traceEval(req,rsp,node,f.getName()+"(\""+token+"\")"); - req.getStapler().invoke(req,rsp, f.invoke(req, rsp, node,token)); + traceEval(req,rsp,node,ff.getName()+"(\""+token+"\")"); + req.getStapler().invoke(req,rsp, ff.invoke(req, rsp, node,token)); return true; } public String toString() { - return String.format("%1$s(String) for url=/%2$s/TOKEN/...",f.getQualifiedName(),name); + return String.format("%1$s(String) for url=/%2$s/TOKEN/...",ff.getQualifiedName(),name); } }); } // check public selector methods .get(int) - for( final Function f : getMethods.signature(int.class) ) { + for (Function f : getMethods.signature(int.class)) { if(f.getName().length()<=3) continue; String name = camelize(f.getName().substring(3)); // 'getFoo' -> 'foo' + final Function ff = f.contextualize(new TraversalMethodContext(name)); dispatchers.add(new NameBasedDispatcher(name,1) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException, InvocationTargetException { int idx = req.tokens.nextAsInt(); if(traceable()) - traceEval(req,rsp,node,f.getName()+"("+idx+")"); - req.getStapler().invoke(req,rsp, f.invoke(req, rsp, node,idx)); + traceEval(req,rsp,node,ff.getName()+"("+idx+")"); + req.getStapler().invoke(req,rsp, ff.invoke(req, rsp, node,idx)); return true; } public String toString() { - return String.format("%1$s(int) for url=/%2$s/N/...",f.getQualifiedName(),name); + return String.format("%1$s(int) for url=/%2$s/N/...",ff.getQualifiedName(),name); } }); } // check public selector methods .get(long) // TF: I'm sure these for loop blocks could be dried out in some way. - for( final Function f : getMethods.signature(long.class) ) { + for (Function f : getMethods.signature(long.class)) { if(f.getName().length()<=3) continue; String name = camelize(f.getName().substring(3)); // 'getFoo' -> 'foo' + final Function ff = f.contextualize(new TraversalMethodContext(name)); dispatchers.add(new NameBasedDispatcher(name,1) { public boolean doDispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException, IllegalAccessException, InvocationTargetException { long idx = req.tokens.nextAsLong(); if(traceable()) - traceEval(req,rsp,node,f.getName()+"("+idx+")"); - req.getStapler().invoke(req,rsp, f.invoke(req, rsp, node,idx)); + traceEval(req,rsp,node,ff.getName()+"("+idx+")"); + req.getStapler().invoke(req,rsp, ff.invoke(req, rsp, node,idx)); return true; } public String toString() { - return String.format("%1$s(long) for url=/%2$s/N/...",f.getQualifiedName(),name); + return String.format("%1$s(long) for url=/%2$s/N/...",ff.getQualifiedName(),name); } }); } @@ -369,7 +376,8 @@ public String toString() { f.buildFallbackDispatchers(this, dispatchers); // check public selector methods .getDynamic(,...) - for( final Function f : getMethods.signatureStartsWith(String.class).name("getDynamic")) { + for (Function f : getMethods.signatureStartsWith(String.class).name("getDynamic")) { + final Function ff = f.contextualize(new TraversalMethodContext(TraversalMethodContext.DYNAMIC)); dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IllegalAccessException, InvocationTargetException, IOException, ServletException { if(!req.tokens.hasMore()) @@ -378,7 +386,7 @@ public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws I if(traceable()) traceEval(req,rsp,node,"getDynamic(\""+token+"\",...)"); - Object target = f.bindAndInvoke(node, req,rsp, token); + Object target = ff.bindAndInvoke(node, req,rsp, token); if(target!=null) { req.getStapler().invoke(req,rsp, target); return true; @@ -391,22 +399,22 @@ public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws I } } public String toString() { - return String.format("%s(String,StaplerRequest,StaplerResponse) for url=/TOKEN/...",f.getQualifiedName()); + return String.format("%s(String,StaplerRequest,StaplerResponse) for url=/TOKEN/...",ff.getQualifiedName()); } }); } // check action .doDynamic(...) - for( final Function f : node.methods.name("doDynamic") ) { - + for (Function f : node.methods.name("doDynamic")) { + final Function ff = f.contextualize(new WebMethodContext(WebMethodContext.DYNAMIC)); dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IllegalAccessException, InvocationTargetException, ServletException, IOException { if(traceable()) trace(req,rsp,"-> <%s>.doDynamic(...)",node); - return f.bindAndInvokeAndServeResponse(node,req,rsp); + return ff.bindAndInvokeAndServeResponse(node,req,rsp); } public String toString() { - return String.format("%s(StaplerRequest,StaplerResponse) for any URL",f.getQualifiedName()); + return String.format("%s(StaplerRequest,StaplerResponse) for any URL",ff.getQualifiedName()); } }); } diff --git a/core/src/main/java/org/kohsuke/stapler/PreInvokeInterceptedFunction.java b/core/src/main/java/org/kohsuke/stapler/PreInvokeInterceptedFunction.java index a3c2550ff2..315fddd97c 100644 --- a/core/src/main/java/org/kohsuke/stapler/PreInvokeInterceptedFunction.java +++ b/core/src/main/java/org/kohsuke/stapler/PreInvokeInterceptedFunction.java @@ -25,4 +25,11 @@ final class PreInvokeInterceptedFunction extends ForwardingFunction { public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object... args) throws IllegalAccessException, InvocationTargetException, ServletException { return interceptor.invoke(req, rsp, o, args); } + + @Override + public Function contextualize(Object usage) { + Function f = next.contextualize(usage); + if (f==next) return this; // the base function didn't care + return new PreInvokeInterceptedFunction(f,interceptor); + } } diff --git a/core/src/main/java/org/kohsuke/stapler/TraversalMethodContext.java b/core/src/main/java/org/kohsuke/stapler/TraversalMethodContext.java new file mode 100644 index 0000000000..91e30caeed --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/TraversalMethodContext.java @@ -0,0 +1,29 @@ +package org.kohsuke.stapler; + +/** + * {@link Function#contextualize(Object)} parameter that indicates + * the function is called to traverse an object graph. + * + * @author Kohsuke Kawaguchi + * @see WebMethod + */ +public final class TraversalMethodContext { + private final String name; + + // instantiation restricted to this class + /*package*/ TraversalMethodContext(String name) { + this.name = name; + } + + /** + * Name of the web method. "" for index route. + */ + public String getName() { + return name; + } + + /** + * Used as a special name that represents {@code getDynamic(...)} that does dynamic traversal. + */ + public static final String DYNAMIC = "\u0000"; +} diff --git a/core/src/main/java/org/kohsuke/stapler/WebMethodContext.java b/core/src/main/java/org/kohsuke/stapler/WebMethodContext.java new file mode 100644 index 0000000000..231a42399a --- /dev/null +++ b/core/src/main/java/org/kohsuke/stapler/WebMethodContext.java @@ -0,0 +1,29 @@ +package org.kohsuke.stapler; + +/** + * {@link Function#contextualize(Object)} parameter that indicates + * the function is called to serve request, such as {@code doFoo(...)} or {@code doIndex(...)} + * + * @author Kohsuke Kawaguchi + * @see WebMethod + */ +public final class WebMethodContext { + private final String name; + + // instantiation restricted to this class + /*package*/ WebMethodContext(String name) { + this.name = name; + } + + /** + * Name of the web method. "" for index route. + */ + public String getName() { + return name; + } + + /** + * Used as a special name that represents {@code doDynamic(...)} that does dynamic traversal. + */ + public static final String DYNAMIC = "\u0000"; +} From 14179d45ee7183c80636e4d4fa27469ecc8c0a46 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 10:47:20 +0900 Subject: [PATCH 12/16] Updated the test case to take advantage of the previous commit ... to correcty distinguish traversal and service --- .../stapler/jelly/issue76/Issue76Test.java | 2 +- .../stapler/jelly/issue76/ProtectedClass.java | 33 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index ba8369d1e9..7a7e4a46b7 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -27,6 +27,6 @@ public void testRouting() throws Exception { assertTrue(tp.getContent().startsWith("protected eye #3")); tp = wc.getPage(new URL(url, "protectedRobot/head/nose")); - assertEquals("nose",tp.getContent()); + assertEquals("nose",tp.getContent().trim()); } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index 361c2ef04d..f0e713dae0 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -5,6 +5,7 @@ import org.kohsuke.stapler.FunctionList; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.TraversalMethodContext; import org.kohsuke.stapler.lang.FieldRef; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.KlassNavigator; @@ -78,19 +79,19 @@ protected FieldRef getBase() { @Override public Object get(Object instance) throws IllegalAccessException { // as we route requests, keep protecting objects - return wrap(super.get(unwrap(instance))); + return w(super.get(u(instance))); } }); } return r; } - private Object unwrap(Object instance) { + private Object u(Object instance) { if (instance==null) return null; return ((Protection)instance).o; } - private Protection wrap(Object instance) { + private Protection w(Object instance) { if (instance==null) return null; return new Protection(instance); } @@ -110,16 +111,34 @@ public List getFunctions(ProtectedClass clazz) { /** * Decorates {@link Function} so that it can be invoked on {@link Protection} and the * return value gets protected as well. + * + *

+ * If the function is used for object traversal, the return value needs to be wrapped to {@link Protection}. */ private Function protect(Function f) { - return new ForwardingFunction(f) { + final Function traversal = new ForwardingFunction(f) { @Override public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object... args) throws IllegalAccessException, InvocationTargetException, ServletException { - // TODO: either invoke needs to get a context in which the invocation is done (rendering vs invoking?) - // or we rely on the naming convention & annotations to determine how to handle this - return wrap(super.invoke(req, rsp, unwrap(o), args)); + return w(super.invoke(req, rsp, u(o), args)); } }; + final Function service = new ForwardingFunction(f) { + @Override + public Function contextualize(Object usage) { + if (usage instanceof TraversalMethodContext) { + return traversal; + } else { + return super.contextualize(usage); + } + } + + @Override + public Object invoke(StaplerRequest req, StaplerResponse rsp, Object o, Object... args) throws IllegalAccessException, InvocationTargetException, ServletException { + return super.invoke(req, rsp, u(o), args); + } + }; + + return service; } }; } From aa5e5861223988291b44cd8dcfd430296745b1bc Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 10:51:43 +0900 Subject: [PATCH 13/16] Make sure test case handles getDynamic/doDynamic routes correctly --- .../kohsuke/stapler/jelly/issue76/Arm.java | 19 +++++++++++++++++++ .../stapler/jelly/issue76/Issue76Test.java | 6 ++++++ .../kohsuke/stapler/jelly/issue76/Robot.java | 6 +++++- 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Arm.java diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Arm.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Arm.java new file mode 100644 index 0000000000..41b18b8d75 --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Arm.java @@ -0,0 +1,19 @@ +package org.kohsuke.stapler.jelly.issue76; + +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; +import org.kohsuke.stapler.StaplerRequest; + +/** + * @author Kohsuke Kawaguchi + */ +public class Arm { + public HttpResponse doDynamic(StaplerRequest req) { + return HttpResponses.plainText(req.getRestOfPath()); + } + + @Override + public String toString() { + return "arm"; + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index 7a7e4a46b7..26c3012cf9 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -28,5 +28,11 @@ public void testRouting() throws Exception { tp = wc.getPage(new URL(url, "protectedRobot/head/nose")); assertEquals("nose",tp.getContent().trim()); + + tp = wc.getPage(new URL(url, "protectedRobot/arm/hand/nail")); + assertEquals("/hand/nail",tp.getContent().trim()); + + tp = wc.getPage(new URL(url, "protectedRobot/arm")); + assertEquals("protected arm",tp.getContent().trim()); } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java index 4b35693da4..f070616cc4 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java @@ -11,5 +11,9 @@ public class Robot { public final Head head = new Head(); - // TODO: more of this + public Object getDynamic(String part) { + if (part.equals("arm")) + return new Arm(); + return null; + } } From e90c10ebb7038a6ae90ee130c515c9c7b0bcc8c2 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 13:27:36 +0900 Subject: [PATCH 14/16] Generalized array access into Klass and KlassNavigator --- .../java/org/kohsuke/stapler/MetaClass.java | 42 ++++--------------- .../java/org/kohsuke/stapler/lang/Klass.java | 11 ++++- .../kohsuke/stapler/lang/KlassNavigator.java | 21 ++++++++++ 3 files changed, 38 insertions(+), 36 deletions(-) diff --git a/core/src/main/java/org/kohsuke/stapler/MetaClass.java b/core/src/main/java/org/kohsuke/stapler/MetaClass.java index dd05289dc5..ea226ccd90 100644 --- a/core/src/main/java/org/kohsuke/stapler/MetaClass.java +++ b/core/src/main/java/org/kohsuke/stapler/MetaClass.java @@ -285,54 +285,28 @@ public String toString() { }); } - // TODO: Klass needs to be able to define its array like access - if(node.clazz.toJavaClass().isArray()) { + if (klass.isArray()) { dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { if(!req.tokens.hasMore()) return false; try { int index = req.tokens.nextAsInt(); - if(traceable()) - traceEval(req,rsp,node,"((Object[])",")["+index+"]"); - req.getStapler().invoke(req,rsp, ((Object[]) node)[index]); + if (traceable()) + traceEval(req, rsp, node, "", "[" + index + "]"); + req.getStapler().invoke(req, rsp, klass.getArrayElement(node, index)); return true; - } catch (NumberFormatException e) { - return false; // try next - } - } - public String toString() { - return "Array look-up for url=/N/..."; - } - }); - } - - // TODO: Klass needs to be able to define its list like access - if(List.class.isAssignableFrom(node.clazz.toJavaClass())) { - dispatchers.add(new Dispatcher() { - public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { - if(!req.tokens.hasMore()) - return false; - try { - int index = req.tokens.nextAsInt(); + } catch (IndexOutOfBoundsException e) { if(traceable()) - traceEval(req,rsp,node,"((List)",").get("+index+")"); - List list = (List) node; - if (0<=index && index IndexOutOfRange [0,%d)",list.size()); - rsp.sendError(SC_NOT_FOUND); - } - + trace(req,rsp,"-> IndexOutOfRange"); + rsp.sendError(SC_NOT_FOUND); return true; } catch (NumberFormatException e) { return false; // try next } } public String toString() { - return "List.get(int) look-up for url=/N/..."; + return "Array look-up for url=/N/..."; } }); } diff --git a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java index 465f504856..653fbeff85 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java @@ -2,7 +2,6 @@ import org.kohsuke.stapler.Function; -import java.lang.reflect.Field; import java.net.URL; import java.util.ArrayList; import java.util.LinkedHashMap; @@ -24,7 +23,7 @@ * * @author Kohsuke Kawaguchi */ -public class Klass { +public final class Klass { public final C clazz; public final KlassNavigator navigator; @@ -83,6 +82,14 @@ public List getFunctions() { return navigator.getFunctions(clazz); } + public boolean isArray() { + return navigator.isArray(clazz); + } + + public Object getArrayElement(Object o, int index) throws IndexOutOfBoundsException { + return navigator.getArrayElement(o,index); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index bff7de401b..85becd4c65 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -4,6 +4,7 @@ import org.kohsuke.stapler.Function; import org.kohsuke.stapler.MetaClassLoader; +import java.lang.reflect.Array; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.net.URL; @@ -86,6 +87,26 @@ public abstract class KlassNavigator { */ public abstract List getFunctions(C clazz); + /** + * If the given type is an array that supports index retrieval. + * @see #getArrayElement(Object, int) + */ + public boolean isArray(C clazz) { + Class j = toJavaClass(clazz); + return j.isArray() || List.class.isAssignableFrom(j); + } + + /** + * Given an instance for which the type reported {@code isArray()==true}, obtains the element + * of the specified index. + * @see #isArray(Object) + */ + public Object getArrayElement(Object o, int index) throws IndexOutOfBoundsException { + if (o instanceof List) + return ((List)o).get(index); + return Array.get(o,index); + } + public static final KlassNavigator JAVA = new KlassNavigator() { @Override public URL getResource(Class clazz, String resourceName) { From dea6393b132771dee30e2648b0a3d0949df530ec Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 13:32:47 +0900 Subject: [PATCH 15/16] Generalized map support --- .../main/java/org/kohsuke/stapler/MetaClass.java | 11 +++-------- .../java/org/kohsuke/stapler/lang/Klass.java | 8 ++++++++ .../org/kohsuke/stapler/lang/KlassNavigator.java | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/kohsuke/stapler/MetaClass.java b/core/src/main/java/org/kohsuke/stapler/MetaClass.java index ea226ccd90..aa13b938f8 100644 --- a/core/src/main/java/org/kohsuke/stapler/MetaClass.java +++ b/core/src/main/java/org/kohsuke/stapler/MetaClass.java @@ -38,10 +38,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Map; import static javax.servlet.http.HttpServletResponse.*; -import static org.kohsuke.stapler.TraversalMethodContext.DYNAMIC; /** * Created one instance each for a {@link Klass}, @@ -311,8 +309,7 @@ public String toString() { }); } - // TODO: Klass needs to be able to define its map like access - if(Map.class.isAssignableFrom(node.clazz.toJavaClass())) { + if(klass.isMap()) { dispatchers.add(new Dispatcher() { public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws IOException, ServletException { if(!req.tokens.hasMore()) @@ -320,9 +317,9 @@ public boolean dispatch(RequestImpl req, ResponseImpl rsp, Object node) throws I try { String key = req.tokens.peek(); if(traceable()) - traceEval(req,rsp,"((Map)",").get(\""+key+"\")"); + traceEval(req,rsp,"",".get(\""+key+"\")"); - Object item = ((Map)node).get(key); + Object item = klass.getMapElement(node,key); if(item!=null) { req.tokens.next(); req.getStapler().invoke(req,rsp,item); @@ -477,6 +474,4 @@ public String toString() { // ignore. } } - - private static final Object NONE = "none"; } diff --git a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java index 653fbeff85..4199623bea 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/Klass.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/Klass.java @@ -90,6 +90,14 @@ public Object getArrayElement(Object o, int index) throws IndexOutOfBoundsExcept return navigator.getArrayElement(o,index); } + public boolean isMap() { + return navigator.isMap(clazz); + } + + public Object getMapElement(Object o, String key) { + return navigator.getMapElement(o,key); + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java index 85becd4c65..00bf6ec4b6 100644 --- a/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java +++ b/core/src/main/java/org/kohsuke/stapler/lang/KlassNavigator.java @@ -11,6 +11,7 @@ import java.util.AbstractList; import java.util.ArrayList; import java.util.List; +import java.util.Map; /** * Strategy pattern to provide navigation across class-like objects in other languages of JVM. @@ -107,6 +108,21 @@ public Object getArrayElement(Object o, int index) throws IndexOutOfBoundsExcept return Array.get(o,index); } + /** + * If the given type is a map/associative array type that supports lookup by a string key + */ + public boolean isMap(C clazz) { + return Map.class.isAssignableFrom(toJavaClass(clazz)); + } + + /** + * Given an instance for which the type reported {@code isMap()==true}, obtains the element + * of the specified index. + */ + public Object getMapElement(Object o, String key) { + return ((Map)o).get(key); + } + public static final KlassNavigator JAVA = new KlassNavigator() { @Override public URL getResource(Class clazz, String resourceName) { From c2ac0d1deceb2a35aaa6b5eda42ad2eaea0d497a Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Jun 2016 13:42:39 +0900 Subject: [PATCH 16/16] Added a test case for list/map lookup --- .../kohsuke/stapler/jelly/issue76/Button.java | 16 ++++++++++++++ .../stapler/jelly/issue76/Issue76Test.java | 16 ++++++++++++++ .../kohsuke/stapler/jelly/issue76/Leg.java | 16 ++++++++++++++ .../stapler/jelly/issue76/ProtectedClass.java | 21 +++++++++++++++++++ .../kohsuke/stapler/jelly/issue76/Robot.java | 14 +++++++++++++ .../stapler/jelly/issue76/Button/index.jelly | 1 + .../stapler/jelly/issue76/Leg/index.jelly | 1 + 7 files changed, 85 insertions(+) create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Button.java create mode 100644 jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Leg.java create mode 100644 jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Button/index.jelly create mode 100644 jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Leg/index.jelly diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Button.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Button.java new file mode 100644 index 0000000000..a89c76586c --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Button.java @@ -0,0 +1,16 @@ +package org.kohsuke.stapler.jelly.issue76; + +/** + * @author Kohsuke Kawaguchi + */ +public class Button { + public final String color; + public Button(String color) { + this.color = color; + } + + @Override + public String toString() { + return color+" button"; + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java index 26c3012cf9..469da094d6 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Issue76Test.java @@ -34,5 +34,21 @@ public void testRouting() throws Exception { tp = wc.getPage(new URL(url, "protectedRobot/arm")); assertEquals("protected arm",tp.getContent().trim()); + + {// list lookup + p = wc.getPage(new URL(url, "robot/legs/0/")); + assertTrue(p.getWebResponse().getContentAsString().contains("left leg")); + + tp = wc.getPage(new URL(url, "protectedRobot/legs/1/")); + assertEquals("protected right leg", tp.getContent().trim()); + } + + {// map lookup + p = wc.getPage(new URL(url, "robot/buttons/red/")); + assertTrue(p.getWebResponse().getContentAsString().contains("This is a button")); + + tp = wc.getPage(new URL(url, "protectedRobot/buttons/red/")); + assertEquals("protected red button", tp.getContent().trim()); + } } } diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Leg.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Leg.java new file mode 100644 index 0000000000..800d4dca4d --- /dev/null +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Leg.java @@ -0,0 +1,16 @@ +package org.kohsuke.stapler.jelly.issue76; + +/** + * @author Kohsuke Kawaguchi + */ +public class Leg { + public final String name; + public Leg(String name) { + this.name = name; + } + + @Override + public String toString() { + return name+" leg"; + } +} diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java index f0e713dae0..0d6d46de83 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/ProtectedClass.java @@ -31,6 +31,27 @@ public ProtectedClass(Class c) { } public static KlassNavigator NAVIGATOR = new KlassNavigator() { + + @Override + public boolean isArray(ProtectedClass clazz) { + return JAVA.isArray(clazz.c); + } + + @Override + public Object getArrayElement(Object o, int index) throws IndexOutOfBoundsException { + return w(JAVA.getArrayElement(u(o),index)); + } + + @Override + public boolean isMap(ProtectedClass clazz) { + return JAVA.isMap(clazz.c); + } + + @Override + public Object getMapElement(Object o, String key) { + return w(JAVA.getMapElement(u(o),key)); + } + private Klass protect(Klass c) { if (c==null) return null; return new Klass(new ProtectedClass((Class)c.clazz), NAVIGATOR); diff --git a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java index f070616cc4..3f936f632b 100644 --- a/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java +++ b/jelly/src/test/java/org/kohsuke/stapler/jelly/issue76/Robot.java @@ -1,5 +1,10 @@ package org.kohsuke.stapler.jelly.issue76; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + /** * Root of the model object. * @@ -11,6 +16,15 @@ public class Robot { public final Head head = new Head(); + public final List legs = Arrays.asList(new Leg("left"),new Leg("right")); + + public final Map buttons = new HashMap(); + + public Robot() { + buttons.put("red", new Button("red")); + buttons.put("blue",new Button("blue")); + } + public Object getDynamic(String part) { if (part.equals("arm")) return new Arm(); diff --git a/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Button/index.jelly b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Button/index.jelly new file mode 100644 index 0000000000..28d4e2f44a --- /dev/null +++ b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Button/index.jelly @@ -0,0 +1 @@ +This is a button \ No newline at end of file diff --git a/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Leg/index.jelly b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Leg/index.jelly new file mode 100644 index 0000000000..b821c4122a --- /dev/null +++ b/jelly/src/test/resources/org/kohsuke/stapler/jelly/issue76/Leg/index.jelly @@ -0,0 +1 @@ +${it.name} leg \ No newline at end of file