Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix and test for GRAILS-9044 FilterConfig.methodMissing gets called f…

…or every invocation when calling a method with several arguments

Fix and test for GRAILS-9050 Grails Filters's helper methods are shared between all Filters classes
  • Loading branch information...
commit 73b248a0ecb7ee0e9b00a9273ad4952bbb55fd8f 1 parent 0f2c77b
@lhotari lhotari authored
View
133 ...rs/src/main/groovy/org/codehaus/groovy/grails/plugins/web/filters/DelegateMetaMethod.java
@@ -0,0 +1,133 @@
+package org.codehaus.groovy.grails.plugins.web.filters;
+
+import groovy.lang.MetaMethod;
+
+import org.codehaus.groovy.reflection.CachedClass;
+
+/**
+ * MetaMethod implementation that delegates to real MetaMethod implementation
+ *
+ * This can be used to efficiently proxy a metamethod from another metaClass in methodMissing.
+ * An example can be found in FilterConfig's methodMissing .
+ *
+ * Without this class it's hard to implement efficient methodMissing "caching" supporting methods with multiple signatures (same method name, different set of arguments).
+ *
+ * This class could be moved to org.codehaus.groovy.grails.commons.metaclass for reuse.
+ *
+ * @author Lari Hotari
+ *
+ */
+@SuppressWarnings("rawtypes")
+class DelegateMetaMethod extends MetaMethod {
+ static interface DelegateMetaMethodTargetStrategy {
+ public Object getTargetInstance(Object instance);
+ }
+
+ private MetaMethod delegateMethod;
+ private DelegateMetaMethodTargetStrategy targetStrategy;
+
+ public DelegateMetaMethod(MetaMethod delegateMethod, DelegateMetaMethodTargetStrategy targetStrategy) {
+ this.delegateMethod=delegateMethod;
+ this.targetStrategy=targetStrategy;
+ }
+
+ public int getModifiers() {
+ return delegateMethod.getModifiers();
+ }
+
+ public String getName() {
+ return delegateMethod.getName();
+ }
+
+ public Class getReturnType() {
+ return delegateMethod.getReturnType();
+ }
+
+ public Object invoke(Object object, Object[] arguments) {
+ return delegateMethod.invoke(targetStrategy.getTargetInstance(object), arguments);
+ }
+
+ public void checkParameters(Class[] arguments) {
+ delegateMethod.checkParameters(arguments);
+ }
+
+ public CachedClass[] getParameterTypes() {
+ return delegateMethod.getParameterTypes();
+ }
+
+ public boolean isMethod(MetaMethod method) {
+ return delegateMethod.isMethod(method);
+ }
+
+ public Class[] getNativeParameterTypes() {
+ return delegateMethod.getNativeParameterTypes();
+ }
+
+ public boolean isVargsMethod(Object[] arguments) {
+ return delegateMethod.isVargsMethod(arguments);
+ }
+
+ public String toString() {
+ return delegateMethod.toString();
+ }
+
+ public boolean equals(Object obj) {
+ return delegateMethod.equals(obj);
+ }
+
+ public Object clone() {
+ return delegateMethod.clone();
+ }
+
+ public boolean isStatic() {
+ return delegateMethod.isStatic();
+ }
+
+ public boolean isAbstract() {
+ return delegateMethod.isAbstract();
+ }
+
+ public Object[] correctArguments(Object[] argumentArray) {
+ return delegateMethod.correctArguments(argumentArray);
+ }
+
+ public boolean isCacheable() {
+ return delegateMethod.isCacheable();
+ }
+
+ public String getDescriptor() {
+ return delegateMethod.getDescriptor();
+ }
+
+ public String getSignature() {
+ return delegateMethod.getSignature();
+ }
+
+ public String getMopName() {
+ return delegateMethod.getMopName();
+ }
+
+ public Object doMethodInvoke(Object object, Object[] argumentArray) {
+ return delegateMethod.doMethodInvoke(targetStrategy.getTargetInstance(object), argumentArray);
+ }
+
+ public boolean isValidMethod(Class[] arguments) {
+ return delegateMethod.isValidMethod(arguments);
+ }
+
+ public boolean isValidExactMethod(Object[] args) {
+ return delegateMethod.isValidExactMethod(args);
+ }
+
+ public boolean isValidExactMethod(Class[] args) {
+ return delegateMethod.isValidExactMethod(args);
+ }
+
+ public boolean isValidMethod(Object[] arguments) {
+ return delegateMethod.isValidMethod(arguments);
+ }
+
+ public CachedClass getDeclaringClass() {
+ return delegateMethod.getDeclaringClass();
+ }
+}
View
42 ...ilters/src/main/groovy/org/codehaus/groovy/grails/plugins/web/filters/FilterConfig.groovy
@@ -34,7 +34,7 @@ import org.codehaus.groovy.grails.web.servlet.mvc.GrailsWebRequest
* @author mike
* @author Graeme Rocher
*/
-class FilterConfig extends ControllersApi{
+class FilterConfig extends ControllersApi {
String name
Map scope
Closure before
@@ -43,7 +43,18 @@ class FilterConfig extends ControllersApi{
// this modelAndView overrides ControllersApi's modelAndView
ModelAndView modelAndView
boolean initialised = false
-
+
+ public FilterConfig() {
+ initializeMetaClass()
+ }
+
+ void initializeMetaClass() {
+ // use per-instance metaclass
+ ExpandoMetaClass emc = new ExpandoMetaClass(getClass(), false, true)
+ emc.initialize()
+ setMetaClass(emc)
+ }
+
/**
* Redirects attempt to access an 'errors' property, so we provide
* one here with a null value.
@@ -56,7 +67,7 @@ class FilterConfig extends ControllersApi{
* delegate any missing properties or methods to it.
*/
def filtersDefinition
-
+
/**
* When the filter does not have a particular property, it passes
* the request on to the filter definition class.
@@ -65,7 +76,7 @@ class FilterConfig extends ControllersApi{
// Delegate to the parent definition if it has this property.
if (filtersDefinition.metaClass.hasProperty(filtersDefinition, propertyName)) {
def getterName = GrailsClassUtils.getGetterName(propertyName)
- metaClass."$getterName" = {-> delegate.filtersDefinition."$propertyName" }
+ metaClass."$getterName" = {-> delegate.filtersDefinition.getProperty(propertyName) }
return filtersDefinition."$propertyName"
}
@@ -78,18 +89,15 @@ class FilterConfig extends ControllersApi{
*/
def methodMissing(String methodName, args) {
// Delegate to the parent definition if it has this method.
- if (filtersDefinition.metaClass.respondsTo(filtersDefinition, methodName)) {
- if (!args) {
- // No argument method.
- metaClass."$methodName" = {-> filtersDefinition."$methodName"() }
- }
- else {
- metaClass."$methodName" = { varArgs -> filtersDefinition."$methodName"(varArgs) }
- }
-
- // We've created the forwarding method now, but we still
- // need to invoke the target method this time around.
- return filtersDefinition."$methodName"(*args)
+ List<MetaMethod> respondsTo = filtersDefinition.metaClass.respondsTo(filtersDefinition, methodName, args)
+ if (respondsTo) {
+ // Use DelegateMetaMethod to proxy calls to actual MetaMethod for subsequent calls to this method
+ DelegateMetaMethod dmm=new DelegateMetaMethod(respondsTo[0], FilterConfigDelegateMetaMethodTargetStrategy.instance)
+ // register the metamethod to EMC
+ metaClass.registerInstanceMethod(dmm)
+
+ // for this invocation we still have to make the call
+ return respondsTo[0].invoke(filtersDefinition, args)
}
// Ideally, we would throw a MissingMethodException here
@@ -104,7 +112,7 @@ class FilterConfig extends ControllersApi{
// The required method was not found on the parent filter definition either.
throw new MissingMethodException(methodName, filtersDefinition.getClass(), args)
}
-
+
String toString() {"FilterConfig[$name, scope=$scope]"}
String getActionUri() {
View
11 ...ehaus/groovy/grails/plugins/web/filters/FilterConfigDelegateMetaMethodTargetStrategy.java
@@ -0,0 +1,11 @@
+package org.codehaus.groovy.grails.plugins.web.filters;
+
+import org.codehaus.groovy.grails.plugins.web.filters.DelegateMetaMethod.DelegateMetaMethodTargetStrategy;
+
+class FilterConfigDelegateMetaMethodTargetStrategy implements DelegateMetaMethodTargetStrategy{
+ public static final FilterConfigDelegateMetaMethodTargetStrategy instance=new FilterConfigDelegateMetaMethodTargetStrategy();
+
+ public Object getTargetInstance(Object instance) {
+ return ((FilterConfig)instance).getFiltersDefinition();
+ }
+}
View
95 ...b/src/test/groovy/org/codehaus/groovy/grails/plugins/web/filters/FilterConfigTests.groovy
@@ -24,11 +24,6 @@ class FilterConfigTests extends GroovyTestCase {
private static final int INT_PROP_VALUE = 1000
private static final String STRING_PROP_VALUE = 'Test property'
- void setUp() {
- ExpandoMetaClass.enableGlobally()
- GroovySystem.metaClassRegistry.removeMetaClass(FilterConfig)
- }
-
void testPropertyMissing() {
def mockDefinition = new MockFiltersDefinition()
def testFilterConfig = new FilterConfig(name: 'Test filter', initialised: true, filtersDefinition: mockDefinition)
@@ -57,12 +52,16 @@ class FilterConfigTests extends GroovyTestCase {
void testMethodMissing() {
def mockDefinition = new MockFiltersDefinition()
- def testFilterConfig = new FilterConfig(name: 'Test filter', initialised: true, filtersDefinition: mockDefinition)
-
+ def testFilterConfig = new MethodMissingCountingFilterConfig(name: 'Test filter', initialised: true, filtersDefinition: mockDefinition)
+
// Try the 'run' method first.
testFilterConfig.run()
assert mockDefinition.runCalled
+
+ assert testFilterConfig.methodMissingCounter == 1
+ testFilterConfig.methodMissingCounter = 0
+
// Now try it a couple more times to make sure that the metaclass
// method has been registered correctly.
mockDefinition.reset()
@@ -72,6 +71,8 @@ class FilterConfigTests extends GroovyTestCase {
mockDefinition.reset()
testFilterConfig.run()
assert mockDefinition.runCalled
+
+ assert testFilterConfig.methodMissingCounter == 0
// Now try with the next method.
mockDefinition.reset()
@@ -79,6 +80,8 @@ class FilterConfigTests extends GroovyTestCase {
assert testFilterConfig.generateNumber() == 6342
assert mockDefinition.generateNumberCalled == true
+ testFilterConfig.methodMissingCounter = 0
+
mockDefinition.reset()
mockDefinition.returnValue = '101'
assert testFilterConfig.generateNumber() == '101'
@@ -88,6 +91,8 @@ class FilterConfigTests extends GroovyTestCase {
mockDefinition.returnValue = 10.232
assert testFilterConfig.generateNumber() == 10.232
assert mockDefinition.generateNumberCalled == true
+
+ assert testFilterConfig.methodMissingCounter == 0
// Now for a method with arguments.
mockDefinition.reset()
@@ -96,6 +101,8 @@ class FilterConfigTests extends GroovyTestCase {
testFilterConfig.checkArgs('Test', 1000)
assert mockDefinition.checkArgsCalled
+ testFilterConfig.methodMissingCounter = 0
+
mockDefinition.reset()
mockDefinition.expectedStringArg = 'Test two'
mockDefinition.expectedIntArg = 2000
@@ -107,11 +114,33 @@ class FilterConfigTests extends GroovyTestCase {
mockDefinition.expectedIntArg = -3423
testFilterConfig.checkArgs('Apples', -3423)
assert mockDefinition.checkArgsCalled
+
+ assert testFilterConfig.methodMissingCounter == 0
+
+ mockDefinition.reset()
+ mockDefinition.expectedStringArg = 'Oranges'
+ mockDefinition.expectedDoubleArg = 1.23d
+ testFilterConfig.checkArgs('Oranges', 1.23d)
+ assert mockDefinition.checkArgsCalled
+
+ assert testFilterConfig.methodMissingCounter == 1
+
+ testFilterConfig.methodMissingCounter = 0
+
+ mockDefinition.reset()
+ mockDefinition.expectedStringArg = 'Pears'
+ mockDefinition.expectedDoubleArg = 2.56d
+ testFilterConfig.checkArgs('Pears', 2.56d)
+ assert mockDefinition.checkArgsCalled
+
+ assert testFilterConfig.methodMissingCounter == 0
// A method that takes a list as an argument.
mockDefinition.reset()
assert testFilterConfig.sum([1, 2, 3, 4]) == 10
assert mockDefinition.sumCalled
+
+ testFilterConfig.methodMissingCounter = 0
mockDefinition.reset()
assert testFilterConfig.sum([4, 5, 1, 10]) == 20
@@ -121,6 +150,8 @@ class FilterConfigTests extends GroovyTestCase {
assert testFilterConfig.sum([12, 26, 3, 41]) == 82
assert mockDefinition.sumCalled
+ assert testFilterConfig.methodMissingCounter == 0
+
// And now make sure the 'run' method is still available.
mockDefinition.reset()
testFilterConfig.run()
@@ -131,13 +162,34 @@ class FilterConfigTests extends GroovyTestCase {
testFilterConfig.unknownMethod(23)
}
}
+
+ // test for GRAILS-9050
+ void testMethodMissingForTwoFilters() {
+ def mockDefinition = new MockFiltersDefinition()
+ def testFilterConfig = new MethodMissingCountingFilterConfig(name: 'Test filter', initialised: true, filtersDefinition: mockDefinition)
+
+ def mockDefinition2 = new MockFiltersDefinition2()
+ def testFilterConfig2 = new MethodMissingCountingFilterConfig(name: 'Test filter 2', initialised: true, filtersDefinition: mockDefinition2)
+
+ mockDefinition2.reset()
+ assert testFilterConfig2.hello() == "Hello from definition 2"
+ assert mockDefinition2.helloCalled
- void tearDown() {
- ExpandoMetaClass.disableGlobally()
- GroovySystem.metaClassRegistry.removeMetaClass(FilterConfig)
+ mockDefinition.reset()
+ assert testFilterConfig.hello() == "Hello from definition 1"
+ assert mockDefinition.helloCalled
}
}
+class MethodMissingCountingFilterConfig extends FilterConfig {
+ int methodMissingCounter=0
+
+ def methodMissing(String methodName, args) {
+ methodMissingCounter++
+ super.methodMissing(methodName, args)
+ }
+}
+
class MockFiltersDefinition {
def propOne = 1000
def prop2 = 'Test property'
@@ -147,7 +199,9 @@ class MockFiltersDefinition {
def sumCalled
def expectedStringArg
def expectedIntArg
+ def expectedDoubleArg
def returnValue
+ def helloCalled
MockFiltersDefinition() {
reset()
@@ -167,12 +221,23 @@ class MockFiltersDefinition {
assert arg1 == expectedStringArg
assert arg2 == expectedIntArg
}
+
+ void checkArgs(String arg1, double arg2) {
+ checkArgsCalled = true
+ assert arg1 == expectedStringArg
+ assert arg2 == expectedDoubleArg
+ }
def sum(items) {
sumCalled = true
items.sum()
}
+ def hello() {
+ helloCalled=true
+ "Hello from definition 1"
+ }
+
void reset() {
runCalled = false
generateNumberCalled = false
@@ -180,6 +245,16 @@ class MockFiltersDefinition {
sumCalled = false
expectedStringArg = null
expectedIntArg = null
+ expectedDoubleArg = null
returnValue = null
+ helloCalled = false
}
}
+
+class MockFiltersDefinition2 extends MockFiltersDefinition {
+
+ def hello() {
+ helloCalled=true
+ "Hello from definition 2"
+ }
+}
Please sign in to comment.
Something went wrong with that request. Please try again.