diff --git a/grails-async/plugin/src/main/groovy/grails/async/web/AsyncController.groovy b/grails-async/plugin/src/main/groovy/grails/async/web/AsyncController.groovy index 71d4892d526..afcb28f58a4 100644 --- a/grails-async/plugin/src/main/groovy/grails/async/web/AsyncController.groovy +++ b/grails-async/plugin/src/main/groovy/grails/async/web/AsyncController.groovy @@ -50,7 +50,7 @@ trait AsyncController { AsyncContext startAsync() { GrailsWebRequest webRequest = (GrailsWebRequest) RequestContextHolder.currentRequestAttributes() - HttpServletRequest request = webRequest.currentRequest + HttpServletRequest request = webRequest.request WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request) AsyncWebRequest asyncWebRequest = new AsyncGrailsWebRequest(request, webRequest.currentResponse, webRequest.servletContext) diff --git a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecorator.groovy b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecorator.groovy index 28bc9f1b050..3edd96a28b0 100644 --- a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecorator.groovy +++ b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/AsyncWebRequestPromiseDecorator.groovy @@ -53,7 +53,7 @@ class AsyncWebRequestPromiseDecorator implements PromiseDecorator { AsyncWebRequestPromiseDecorator(GrailsWebRequest webRequest) { this.webRequest = webRequest - HttpServletRequest currentServletRequest = webRequest.currentRequest + HttpServletRequest currentServletRequest = webRequest.request WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(currentServletRequest) AsyncGrailsWebRequest newWebRequest if (asyncManager.isConcurrentHandlingStarted()) { diff --git a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/mvc/AsyncActionResultTransformer.groovy b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/mvc/AsyncActionResultTransformer.groovy index bfbb5eda24c..ada5ec1d090 100644 --- a/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/mvc/AsyncActionResultTransformer.groovy +++ b/grails-async/plugin/src/main/groovy/org/grails/plugins/web/async/mvc/AsyncActionResultTransformer.groovy @@ -52,7 +52,7 @@ class AsyncActionResultTransformer implements ActionResultTransformer { if (actionResult instanceof Promise) { - final request = webRequest.getCurrentRequest() + final request = webRequest.getRequest() WebAsyncManager asyncManager = WebAsyncUtils.getAsyncManager(request) final response = webRequest.getResponse() diff --git a/grails-benchmarks/README.adoc b/grails-benchmarks/README.adoc index fed304575d1..d600ef421cf 100644 --- a/grails-benchmarks/README.adoc +++ b/grails-benchmarks/README.adoc @@ -88,12 +88,17 @@ Benchmarks are Java, under `src/jmh/java`. Setup fixtures are Groovy, under `src The comparison tool is Groovy in the `report` source set, under `src/report/groovy`. That split is intentional. A benchmark written in Groovy measures Groovy's dynamic call-site -machinery as much as the Grails API under test, so the measured method stays in Java. Fixtures -are Groovy only where the Grails API genuinely requires it - the URL mappings DSL and view -templates are closure-based, and `grails.artefact.Interceptor` is a Groovy trait. Fixtures live in `main` rather -than `src/jmh/groovy` because the `me.champeau.jmh` plugin puts the main source set's output on -the jmh compile classpath, which keeps the benchmark source set pure Java and preserves normal -JMH annotation processing. +machinery as much as the Grails API under test, so the measured method stays in Java. Fixtures live +in `main` rather than `src/jmh/groovy` because the `me.champeau.jmh` plugin puts the main source +set's output on the jmh compile classpath, which keeps the benchmark source set pure Java and +preserves normal JMH annotation processing. + +Setup code goes in a Groovy fixture whether or not the Grails API forces it to - the URL mappings +DSL and view templates are closure-based and `grails.artefact.Interceptor` is a Groovy trait, but +building a mock servlet context or compiling a controller does not need Groovy and lives there +anyway. Nothing a fixture does is inside the timed region, so the only fixture that is Groovy *for +the measurement* is `web/RequestPropertyFixture`, whose reader is deliberately not statically +compiled because the dynamic call site is the thing being measured. The `report` source set depends on Groovy alone and on no Grails project. The CI reporting job runs it after a bare checkout, so it must render Markdown without compiling the framework. @@ -103,10 +108,12 @@ Benchmarks are grouped by package, and the CI report aggregates per group: |=== | Package | Covers -| `urlmappings` | Request URI matching (warm and cold cache) and reverse URL creation +| `urlmappings` | Request URI matching (warm cache, cold cache, catch-all fall-through) and reverse URL creation +| `controllers` | Controller action invocation, the `redirect` and `render(template:)` response paths, and the per-request collection of controller URL mappings +| `web` | Binding a `GrailsWebRequest`, building `params`, multipart discovery, Groovy request property access | `databinding` | Binding a map onto an object, with and without type conversion | `gsp` | GSP parsing (template text to generated Groovy source) -| `interceptors` | Interceptor URI match decisions +| `interceptors` | Interceptor URI match decisions, and the interceptor handler chain per request | `views` | JSON and markup view rendering | `ruler` | Pure-JDK probes used to detect an unstable CI runner |=== diff --git a/grails-benchmarks/build.gradle b/grails-benchmarks/build.gradle index be247d4ca1e..0e00de69ec0 100644 --- a/grails-benchmarks/build.gradle +++ b/grails-benchmarks/build.gradle @@ -71,14 +71,26 @@ dependencies { implementation project(':grails-views-gson') implementation project(':grails-views-markup') implementation project(':grails-core') + // The controller AST transformer, so that benchmarked controllers are compiled by the same + // injector a real application's controllers go through. + implementation project(':grails-controllers') + // The data binding collaborators an application registers as beans, so that a command object + // action binds the way it does in a running application rather than building a binder per call. + implementation project(':grails-web-databinding') + implementation project(':grails-mimetypes') implementation 'org.apache.groovy:groovy' + // ObservationRegistry, which GrailsInterceptorHandlerInterceptorAdapter branches on + implementation 'io.micrometer:micrometer-observation' + // The framework modules declare these as compileOnly, so they are absent from the runtime // classpath a benchmark actually executes on. The modules' own test suites add them back // the same way. Without them, URL mapping, interceptor and view benchmarks fail at @Setup // with NoClassDefFoundError: jakarta/servlet/ServletContext. implementation 'jakarta.servlet:jakarta.servlet-api' implementation 'org.springframework:spring-test' + // ModelAndView, which the interceptor adapter's postHandle takes + implementation 'org.springframework:spring-webmvc' reportImplementation platform(project(':grails-bom')) reportImplementation 'org.apache.groovy:groovy' diff --git a/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerActionBenchmark.java b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerActionBenchmark.java new file mode 100644 index 00000000000..23fcd5bce2f --- /dev/null +++ b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerActionBenchmark.java @@ -0,0 +1,252 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.benchmarks.controllers; + +import java.util.concurrent.TimeUnit; + +import groovy.lang.GroovyClassLoader; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletContext; +import org.springframework.web.context.WebApplicationContext; + +import grails.core.GrailsApplication; +import grails.core.GrailsControllerClass; +import grails.util.Environment; +import grails.util.GrailsWebMockUtil; +import grails.web.databinding.DataBindingUtils; +import grails.web.databinding.GrailsWebDataBinder; +import grails.web.mime.MimeTypeResolver; +import org.apache.grails.benchmarks.web.WebContextFixture; +import org.grails.core.DefaultGrailsControllerClass; +import org.grails.web.databinding.bindingsource.DataBindingSourceRegistry; +import org.grails.web.databinding.bindingsource.DefaultDataBindingSourceRegistry; +import org.grails.web.mime.DefaultMimeTypeResolver; + +/** + * Measures one controller action invocation through {@code GrailsControllerClass.invoke}, which is + * the call {@code UrlMappingsInfoHandlerAdapter} makes for every request that reaches a controller. + * + *
Three shapes are measured, because the generated wrapper differs between them:
+ *The controllers are compiled at setup by a {@code GrailsAwareClassLoader} running the real + * {@code ControllerActionTransformer}, so the bytecode invoked is the bytecode a Grails application + * would run. Setup also prints, once per fork, how many request-attribute operations one invocation + * of each action performs, measured against a counting request outside the timed region. That count + * - not the timing - is the direct evidence of what the generated code does.
+ */ +@State(Scope.Benchmark) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"}) +public class ControllerActionBenchmark { + + private static final String PLAIN_CONTROLLER_SOURCE = """ + @grails.artefact.Artefact('Controller') + class BenchmarkPlainController { + def index() { + 'plain' + } + } + """; + + private static final String RESTRICTED_CONTROLLER_SOURCE = """ + @grails.artefact.Artefact('Controller') + class BenchmarkRestrictedController { + static allowedMethods = [index: 'GET'] + def index() { + 'restricted' + } + } + """; + + private static final String COMMAND_CONTROLLER_SOURCE = """ + class BenchmarkBookCommand { + String title + Integer pages + } + + @grails.artefact.Artefact('Controller') + class BenchmarkCommandController { + def save(BenchmarkBookCommand command) { + 'saved' + } + } + """; + + private GrailsControllerClass plainControllerClass; + + private Object plainController; + + private GrailsControllerClass restrictedControllerClass; + + private Object restrictedController; + + private GrailsControllerClass commandControllerClass; + + private Object commandController; + + @Setup + public void setup() throws Throwable { + MockServletContext servletContext = WebContextFixture.createServletContext(); + WebApplicationContext applicationContext = WebContextFixture.applicationContext(servletContext); + registerDataBindingBeans(applicationContext); + + GroovyClassLoader classLoader = ControllerFixture.createTransformingClassLoader(); + + classLoader.parseClass(PLAIN_CONTROLLER_SOURCE, "BenchmarkPlainController.groovy"); + Class> plainClass = classLoader.loadClass("BenchmarkPlainController"); + plainControllerClass = new DefaultGrailsControllerClass(plainClass); + plainController = plainClass.getDeclaredConstructor().newInstance(); + + classLoader.parseClass(RESTRICTED_CONTROLLER_SOURCE, "BenchmarkRestrictedController.groovy"); + Class> restrictedClass = classLoader.loadClass("BenchmarkRestrictedController"); + restrictedControllerClass = new DefaultGrailsControllerClass(restrictedClass); + restrictedController = restrictedClass.getDeclaredConstructor().newInstance(); + + classLoader.parseClass(COMMAND_CONTROLLER_SOURCE, "BenchmarkCommandController.groovy"); + Class> commandClass = classLoader.loadClass("BenchmarkCommandController"); + commandControllerClass = new DefaultGrailsControllerClass(commandClass); + commandController = commandClass.getDeclaredConstructor().newInstance(); + + reportAttributeCounts(servletContext, applicationContext); + + // The request all three measured invocations run against. GET, which is what the restricted + // controller's allowedMethods declares, so that benchmark measures the check passing rather + // than the 405 error path. Two request parameters are present so the command object action + // has something to bind. + MockHttpServletRequest request = new MockHttpServletRequest(servletContext, "GET", "/benchmark/index"); + request.setParameter("title", "Groovy in Action"); + request.setParameter("pages", "912"); + GrailsWebMockUtil.bindMockWebRequest(applicationContext, request, new MockHttpServletResponse()); + + assertFixtureInvokes(); + + System.out.println("[fixture] Environment.isDevelopmentMode()=" + Environment.isDevelopmentMode() + + " (false means GrailsControllerClass.invoke dispatches through a MethodHandle, as in production)"); + } + + /** + * Registers the data binding collaborators an application context normally holds as singletons, + * and points the {@code GrailsApplication} at that context so they are found. + * + *Without this, {@code DataBindingUtils} cannot find a {@code grailsWebDataBinder} bean and + * builds a whole {@code GrailsWebDataBinder} - conversion service and all - on every single + * command object binding, which costs tens of microseconds and is nothing like what a running + * application does.
+ */ + private static void registerDataBindingBeans(WebApplicationContext applicationContext) { + GrailsApplication grailsApplication = applicationContext.getBean(GrailsApplication.APPLICATION_ID, GrailsApplication.class); + grailsApplication.setApplicationContext(applicationContext); + + DefaultDataBindingSourceRegistry dataBindingSourceRegistry = new DefaultDataBindingSourceRegistry(); + dataBindingSourceRegistry.initialize(); + + ConfigurableListableBeanFactory beanFactory = ((ConfigurableApplicationContext) applicationContext).getBeanFactory(); + beanFactory.registerSingleton(DataBindingSourceRegistry.BEAN_NAME, dataBindingSourceRegistry); + beanFactory.registerSingleton(MimeTypeResolver.BEAN_NAME, new DefaultMimeTypeResolver()); + beanFactory.registerSingleton(DataBindingUtils.DATA_BINDER_BEAN_NAME, new GrailsWebDataBinder(grailsApplication)); + } + + /** + * Invokes each action once against a request that counts attribute operations, and prints the + * counts. Runs before the measured request is bound, and never inside the timed region. + */ + private void reportAttributeCounts(MockServletContext servletContext, WebApplicationContext applicationContext) throws Throwable { + AttributeCountingRequest countingRequest = ControllerFixture.createCountingRequest(servletContext, "GET", "/benchmark/index"); + countingRequest.setParameter("title", "Groovy in Action"); + countingRequest.setParameter("pages", "912"); + GrailsWebMockUtil.bindMockWebRequest(applicationContext, countingRequest, new MockHttpServletResponse()); + + // The first invocation of an action initialises metaclasses and caches, which does its own + // attribute traffic; the reported counts are from the second, steady state, invocation. + plainControllerClass.invoke(plainController, "index"); + countingRequest.resetCounts(); + plainControllerClass.invoke(plainController, "index"); + System.out.println("[fixture] plainAction request attribute ops: " + countingRequest.describeCounts()); + + restrictedControllerClass.invoke(restrictedController, "index"); + countingRequest.resetCounts(); + restrictedControllerClass.invoke(restrictedController, "index"); + System.out.println("[fixture] restrictedAction request attribute ops: " + countingRequest.describeCounts()); + + commandControllerClass.invoke(commandController, "save"); + countingRequest.resetCounts(); + commandControllerClass.invoke(commandController, "save"); + System.out.println("[fixture] commandObjectAction request attribute ops: " + countingRequest.describeCounts()); + } + + // An action that fails - a rejected HTTP method, say - still returns, so check the return value + // rather than publishing a number measured on an error path. + private void assertFixtureInvokes() throws Throwable { + require(plainControllerClass.invoke(plainController, "index"), "plain"); + require(restrictedControllerClass.invoke(restrictedController, "index"), "restricted"); + require(commandControllerClass.invoke(commandController, "save"), "saved"); + } + + private static void require(Object actual, String expected) { + if (!expected.equals(actual)) { + throw new IllegalStateException("Expected the action to return '" + expected + "' but it returned '" + actual + "'"); + } + } + + /** An action on a controller declaring no {@code allowedMethods} - the common case. */ + @Benchmark + public Object plainAction() throws Throwable { + return plainControllerClass.invoke(plainController, "index"); + } + + /** An action on a controller declaring {@code allowedMethods}, where the check has to run. */ + @Benchmark + public Object restrictedAction() throws Throwable { + return restrictedControllerClass.invoke(restrictedController, "index"); + } + + /** An action taking a command object, so the generated wrapper binds one per invocation. */ + @Benchmark + public Object commandObjectAction() throws Throwable { + return commandControllerClass.invoke(commandController, "save"); + } +} diff --git a/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerMappingCollectionBenchmark.java b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerMappingCollectionBenchmark.java new file mode 100644 index 00000000000..28c777d1fa3 --- /dev/null +++ b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/controllers/ControllerMappingCollectionBenchmark.java @@ -0,0 +1,195 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.benchmarks.controllers; + +import java.util.concurrent.TimeUnit; + +import groovy.lang.GroovyClassLoader; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.mock.web.MockServletContext; +import org.springframework.web.context.WebApplicationContext; + +import grails.core.DefaultGrailsApplication; +import grails.util.GrailsWebMockUtil; +import grails.web.mapping.UrlMapping; +import grails.web.mapping.UrlMappingInfo; +import org.apache.grails.benchmarks.web.WebContextFixture; +import org.grails.web.mapping.mvc.GrailsControllerUrlMappings; + +/** + * Measures {@code GrailsControllerUrlMappings.matchAll}, which is what + * {@code UrlMappingsHandlerMapping.getHandlerInternal} calls on every request. + * + *The delegate's own {@code matchAll} memoises its result in a Caffeine cache, but the wrapper + * around it - {@code AbstractGrailsControllerUrlMappings.collectControllerMappings} - is not cached + * and runs in full every time. For each candidate it calls {@code webRequest.resetParams()} (which + * clones the parameter map), {@code info.configure(webRequest)}, and allocates a controller key. + * Each benchmark here calls a single URI repeatedly, so the delegate cache always hits and what is + * measured is the uncached wrapper.
+ * + *Three shapes are measured:
+ *Both redirect benchmarks allocate a fresh argument map per invocation, exactly as a controller + * writing {@code redirect(action: 'show')} does. That is not cosmetic: {@code redirect} writes the + * resolved namespace back into the map it is given, so a shared map would skip namespace resolution + * on every invocation after the first and measure nothing. They also clear the + * "redirect already issued" request attribute, since a second redirect on one request is an error. + * Both costs are constant across the states being compared.
+ */ +@State(Scope.Benchmark) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"}) +public class ControllerResponseBenchmark { + + private static final String PLAIN_CONTROLLER_SOURCE = """ + @grails.artefact.Artefact('Controller') + class BenchmarkPlainRedirectController { + def index() { + 'plain' + } + } + """; + + private static final String NAMESPACED_CONTROLLER_SOURCE = """ + @grails.artefact.Artefact('Controller') + class BenchmarkNamespacedRedirectController { + static namespace = 'admin' + def index() { + 'namespaced' + } + } + """; + + private static final String TEMPLATE_CONTROLLER_SOURCE = """ + @grails.artefact.Artefact('Controller') + class BenchmarkTemplateController { + def index() { + 'template' + } + } + """; + + private Controller plainController; + + private Controller namespacedController; + + private Controller templateController; + + private MockHttpServletRequest request; + + private CountingView countingView; + + @Setup + public void setup() throws Exception { + MockServletContext servletContext = WebContextFixture.createServletContext(); + WebApplicationContext applicationContext = WebContextFixture.applicationContext(servletContext); + + this.countingView = ControllerResponseFixture.createCountingView(); + ConfigurableListableBeanFactory beanFactory = ((ConfigurableApplicationContext) applicationContext).getBeanFactory(); + beanFactory.registerSingleton(LinkGenerator.BEAN_NAME, ControllerResponseFixture.createLinkGenerator()); + beanFactory.registerSingleton(CompositeViewResolver.BEAN_NAME, ControllerResponseFixture.createViewResolver(this.countingView)); + + // The controllers have to be registered as artefacts, exactly as an application's controllers are. A + // controller that is not in the registry is a shape only a hand-built unit test produces, and measuring it + // would measure the framework's fallback rather than a request. + GrailsApplication grailsApplication = applicationContext.getBean(GrailsApplication.APPLICATION_ID, GrailsApplication.class); + grailsApplication.setApplicationContext(applicationContext); + grailsApplication.initialise(); + + GroovyClassLoader classLoader = ControllerFixture.createTransformingClassLoader(); + this.plainController = newController(grailsApplication, classLoader, PLAIN_CONTROLLER_SOURCE, "BenchmarkPlainRedirectController"); + this.namespacedController = newController(grailsApplication, classLoader, NAMESPACED_CONTROLLER_SOURCE, "BenchmarkNamespacedRedirectController"); + this.templateController = newController(grailsApplication, classLoader, TEMPLATE_CONTROLLER_SOURCE, "BenchmarkTemplateController"); + + this.request = new MockHttpServletRequest(servletContext, "GET", "/benchmark/index"); + GrailsWebMockUtil.bindMockWebRequest(applicationContext, this.request, new MockHttpServletResponse()); + + assertFixtureRedirects(); + assertFixtureRenders(); + } + + private static Controller newController(GrailsApplication grailsApplication, GroovyClassLoader classLoader, + String source, String className) throws Exception { + classLoader.parseClass(source, className + ".groovy"); + Class> controllerClass = classLoader.loadClass(className); + grailsApplication.addArtefact(ControllerArtefactHandler.TYPE, controllerClass); + if (grailsApplication.getArtefact(ControllerArtefactHandler.TYPE, controllerClass.getName()) == null) { + throw new IllegalStateException("Expected " + className + " to be retrievable from the artefact registry"); + } + return (Controller) controllerClass.getDeclaredConstructor().newInstance(); + } + + private static MapThe dominant production configuration is a no-op {@code ObservationRegistry}: an application + * that has not enabled metrics or tracing gets {@code ObservationRegistry.NOOP}, and even one that + * has, gets a registry that is no-op until a handler is registered. The observing variants are + * measured too, but the no-op numbers are the ones that describe most applications.
+ * + *The interceptors leave {@code before()} and {@code after()} at their trait defaults, so the + * score is the adapter's own per-interceptor per-phase cost - matcher evaluation, list bookkeeping, + * and whatever the adapter allocates to dispatch the callback - and not the cost of anybody's + * interceptor body.
+ */ +@State(Scope.Benchmark) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"}) +public class InterceptorChainBenchmark { + + private static final String MATCHED_INTERCEPTORS = "org.grails.web.MATCHED_INTERCEPTORS"; + + private MockHttpServletRequest request; + + private MockHttpServletResponse response; + + private ModelAndView modelAndView; + + private Object handler; + + private GrailsInterceptorHandlerInterceptorAdapter oneNoOp; + + private GrailsInterceptorHandlerInterceptorAdapter threeNoOp; + + private GrailsInterceptorHandlerInterceptorAdapter oneObserving; + + private GrailsInterceptorHandlerInterceptorAdapter threeObserving; + + @Setup + public void setup() { + ServletContext servletContext = WebContextFixture.createServletContext(); + request = new MockHttpServletRequest(servletContext, "GET", "/book/show/42"); + response = new MockHttpServletResponse(); + modelAndView = new ModelAndView("/book/show"); + handler = new Object(); + + oneNoOp = createAdapter(1, ObservationRegistry.NOOP); + threeNoOp = createAdapter(3, ObservationRegistry.NOOP); + oneObserving = createAdapter(1, createObservingRegistry()); + threeObserving = createAdapter(3, createObservingRegistry()); + + assertFixtureMatches(oneNoOp, 1); + assertFixtureMatches(threeNoOp, 3); + assertFixtureMatches(oneObserving, 1); + assertFixtureMatches(threeObserving, 3); + } + + private static GrailsInterceptorHandlerInterceptorAdapter createAdapter(int count, ObservationRegistry registry) { + GrailsInterceptorHandlerInterceptorAdapter adapter = new GrailsInterceptorHandlerInterceptorAdapter(); + Interceptor[] interceptors = InterceptorChainFixture.createMatchingInterceptors(count); + adapter.setInterceptors(interceptors); + adapter.setObservationRegistry(registry); + return adapter; + } + + /** + * @return a registry that is not no-op, because a handler is registered on it - the shape an + * application running Micrometer tracing or metrics has + */ + private static ObservationRegistry createObservingRegistry() { + ObservationRegistry registry = ObservationRegistry.create(); + registry.observationConfig().observationHandler(new ObservationHandlerThe overwhelmingly common case is a request that is not multipart, so the miss path + * matters more than the hit path. Both the flat and the wrapped shapes are measured: in a real + * application the Grails request filter sits under several servlet filters, so the request handed + * to Grails is normally a couple of {@code HttpServletRequestWrapper}s deep.
+ */ +@State(Scope.Benchmark) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"}) +public class MultipartResolutionBenchmark { + + private HttpServletRequest plain; + + private HttpServletRequest plainBehindTwoWrappers; + + private HttpServletRequest multipartBehindTwoWrappers; + + private HttpServletRequest multipartByAttribute; + + @Setup + public void setup() { + ServletContext servletContext = WebContextFixture.createServletContext(); + + MockHttpServletRequest plainRequest = new MockHttpServletRequest(servletContext, "POST", "/book/save"); + plainRequest.setContentType("application/x-www-form-urlencoded"); + plain = plainRequest; + plainBehindTwoWrappers = wrapTwice(plainRequest); + + MockMultipartHttpServletRequest multipartRequest = new MockMultipartHttpServletRequest(servletContext); + multipartRequest.setMethod("POST"); + multipartRequest.setRequestURI("/book/save"); + multipartRequest.addFile(new MockMultipartFile("cover", "cover.png", "image/png", + "not-really-a-png".getBytes(StandardCharsets.UTF_8))); + multipartBehindTwoWrappers = wrapTwice(multipartRequest); + + // Spring's StandardServletMultipartResolver publishes the resolved multipart request as a + // request attribute rather than as a wrapper, so the attribute fallback is a real path too. + MockHttpServletRequest attributeCarrier = new MockHttpServletRequest(servletContext, "POST", "/book/save"); + attributeCarrier.setAttribute(MultipartHttpServletRequest.class.getName(), multipartRequest); + multipartByAttribute = attributeCarrier; + + assertFixtureResolves(); + } + + // A resolution that silently found nothing would leave the multipart benchmarks timing the + // miss path under a name that claims a hit. + private void assertFixtureResolves() { + if (WebUtils.resolveMultipartRequest(plain) != null) { + throw new IllegalStateException("A plain request must not resolve to a multipart request"); + } + if (WebUtils.resolveMultipartRequest(multipartBehindTwoWrappers) == null) { + throw new IllegalStateException("A wrapped multipart request must resolve through the wrappers"); + } + if (WebUtils.resolveMultipartRequest(multipartByAttribute) == null) { + throw new IllegalStateException("A multipart request published as an attribute must resolve"); + } + } + + private static HttpServletRequest wrapTwice(HttpServletRequest request) { + return new HttpServletRequestWrapper(new HttpServletRequestWrapper(request)); + } + + @Benchmark + public MultipartHttpServletRequest resolvePlain() { + return WebUtils.resolveMultipartRequest(plain); + } + + @Benchmark + public MultipartHttpServletRequest resolvePlainBehindTwoWrappers() { + return WebUtils.resolveMultipartRequest(plainBehindTwoWrappers); + } + + @Benchmark + public MultipartHttpServletRequest resolveMultipartBehindTwoWrappers() { + return WebUtils.resolveMultipartRequest(multipartBehindTwoWrappers); + } + + @Benchmark + public MultipartHttpServletRequest resolveMultipartByAttribute() { + return WebUtils.resolveMultipartRequest(multipartByAttribute); + } +} diff --git a/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/web/RequestPropertyAccessBenchmark.java b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/web/RequestPropertyAccessBenchmark.java new file mode 100644 index 00000000000..ddf3e790607 --- /dev/null +++ b/grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/web/RequestPropertyAccessBenchmark.java @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.benchmarks.web; + +import java.util.concurrent.TimeUnit; + +import jakarta.servlet.ServletContext; +import jakarta.servlet.http.HttpServletRequest; + +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.Setup; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; + +import org.springframework.mock.web.MockHttpServletRequest; + +/** + * Measures Groovy property access on an {@code HttpServletRequest}, which application code performs + * on every request that reads a request attribute. + * + *{@code request.someAttribute} in application code is not a field read: the property is unknown + * to the request class, so it goes through the metaclass, falls through to the {@code getProperty} + * / {@code propertyMissing} methods contributed by {@code HttpServletRequestExtension} (registered + * as a Groovy extension module by {@code grails-web-core}), and that implementation performs a + * further {@code metaClass.getMetaProperty(name)} lookup before reading the request attribute.
+ * + *{@link #groovyAttributeCall()} and {@link #javaGetAttribute()} bracket that cost with the + * explicit {@code getAttribute} call from Groovy and from Java respectively.
+ */ +@State(Scope.Benchmark) +@Threads(1) +@BenchmarkMode(Mode.AverageTime) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS) +@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS) +@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"}) +public class RequestPropertyAccessBenchmark { + + private HttpServletRequest request; + + private DynamicRequestPropertyReader reader; + + @Setup + public void setup() { + ServletContext servletContext = WebContextFixture.createServletContext(); + MockHttpServletRequest mockRequest = new MockHttpServletRequest(servletContext, "GET", "/book/show/42"); + mockRequest.setAttribute("someAttribute", "someValue"); + request = mockRequest; + reader = RequestPropertyFixture.createReader(); + assertFixtureResolves(); + } + + // Without the extension module on the classpath the unknown property raises rather than + // resolving, so check it here instead of publishing a number for the wrong path. + private void assertFixtureResolves() { + if (!"someValue".equals(reader.readUnknownProperty(request))) { + throw new IllegalStateException( + "request.someAttribute did not resolve through HttpServletRequestExtension"); + } + } + + /** {@code request.someAttribute} - metaclass miss, extension {@code getProperty}, attribute read. */ + @Benchmark + public Object groovyUnknownProperty() { + return reader.readUnknownProperty(request); + } + + /** {@code request.method} - metaclass hit on a real getter. */ + @Benchmark + public Object groovyGetterBackedProperty() { + return reader.readGetterBackedProperty(request); + } + + /** {@code request.getAttribute('someAttribute')} from Groovy - a dynamic call, not a property. */ + @Benchmark + public Object groovyAttributeCall() { + return reader.readAttributeDirectly(request); + } + + /** The floor: the same attribute read straight from Java. */ + @Benchmark + public Object javaGetAttribute() { + return request.getAttribute("someAttribute"); + } +} diff --git a/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerFixture.groovy b/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerFixture.groovy new file mode 100644 index 00000000000..46253d22342 --- /dev/null +++ b/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerFixture.groovy @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.benchmarks.controllers + +import org.codehaus.groovy.control.CompilationUnit + +import jakarta.servlet.ServletContext + +import org.springframework.mock.web.MockHttpServletRequest + +import grails.compiler.ast.ClassInjector +import org.grails.compiler.injection.GrailsAwareClassLoader +import org.grails.compiler.web.ControllerActionTransformer + +/** + * Compiles controller sources through the real {@code ControllerActionTransformer}, so that the + * bytecode a benchmark invokes is the bytecode a Grails application would run. + * + * This is the same arrangement {@code ControllerActionTransformerSpec} uses: a + * {@code GrailsAwareClassLoader} whose only URL-driven injector is the controller action + * transformer, with {@code shouldInject} forced on because sources compiled from a String have no + * {@code grails-app/controllers} URL to recognise. The {@code @Artefact('Controller')} annotation in + * each source still drives the normal annotation-driven injection, which is what applies the + * {@code Controller} trait. + */ +class ControllerFixture { + + static GroovyClassLoader createTransformingClassLoader() { + ControllerActionTransformer transformer = new ControllerActionTransformer() { + @Override + boolean shouldInject(URL url) { + true + } + } + transformer.compilationUnit = new CompilationUnit() + new GrailsAwareClassLoader().tap { + classInjectors = [transformer] as ClassInjector[] + } + } + + static AttributeCountingRequest createCountingRequest(ServletContext servletContext, String method, String requestUri) { + new AttributeCountingRequest(servletContext, method, requestUri) + } +} + +/** + * A mock request that counts attribute operations, used once outside the measured region to report + * how much request-attribute bookkeeping a generated controller action actually performs. + * + * This exists so a benchmark can state what it measures rather than assert it: the count printed + * during setup is the difference the generated code makes, independent of the timing numbers. + */ +class AttributeCountingRequest extends MockHttpServletRequest { + + private int getAttributeCount + + private int setAttributeCount + + private int removeAttributeCount + + AttributeCountingRequest(ServletContext servletContext, String method, String requestUri) { + super(servletContext, method, requestUri) + } + + @Override + Object getAttribute(String name) { + this.getAttributeCount++ + super.getAttribute(name) + } + + @Override + void setAttribute(String name, Object value) { + this.setAttributeCount++ + super.setAttribute(name, value) + } + + @Override + void removeAttribute(String name) { + this.removeAttributeCount++ + super.removeAttribute(name) + } + + void resetCounts() { + this.getAttributeCount = 0 + this.setAttributeCount = 0 + this.removeAttributeCount = 0 + } + + String describeCounts() { + "getAttribute=${this.getAttributeCount} setAttribute=${this.setAttributeCount} removeAttribute=${this.removeAttributeCount}" + } +} diff --git a/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerMappingsFixture.groovy b/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerMappingsFixture.groovy new file mode 100644 index 00000000000..2ccf2c127d3 --- /dev/null +++ b/grails-benchmarks/src/main/groovy/org/apache/grails/benchmarks/controllers/ControllerMappingsFixture.groovy @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.grails.benchmarks.controllers + +import org.springframework.context.ApplicationContext + +import grails.core.GrailsApplication +import grails.web.mapping.UrlMapping +import org.grails.web.mapping.DefaultUrlMappingEvaluator +import org.grails.web.mapping.DefaultUrlMappingsHolder +import org.grails.web.mapping.mvc.GrailsControllerUrlMappings + +/** + * Mapping sets for {@code ControllerMappingCollectionBenchmark}, wrapped in the + * {@code GrailsControllerUrlMappings} that {@code UrlMappingsHandlerMapping} consults per request. + */ +class ControllerMappingsFixture { + + /** + * A mapping set sized and shaped like a mid-size application's {@code UrlMappings.groovy}: a + * handful of static URLs, five REST resource blocks, some multi-token dynamic URLs, a couple of + * method-scoped mappings, the catch-all default mapping, and the error mappings. + */ + static GrailsControllerUrlMappings createApplicationMappings(GrailsApplication grailsApplication, + ApplicationContext applicationContext) { + // These MUST be double-quoted GStrings, exactly as a real UrlMappings.groovy is written. + // The DSL captures variables by letting its delegate resolve $category and friends; with + // single quotes they stay literal text, every mapping becomes a fixed path, and match() + // silently returns null so the benchmark measures failed lookups. + createMappings(grailsApplication, applicationContext) { + '/'(controller: 'application', action: 'index') + '/login'(controller: 'auth', action: 'login') + '/logout'(controller: 'auth', action: 'logout') + '/health'(controller: 'health', action: 'index') + + '/api/books'(resources: 'book') + '/api/authors'(resources: 'author') + '/api/publishers'(resources: 'publisher') + '/api/orders'(resources: 'order') + '/api/customers'(resources: 'customer') + + "/store/$category/$subcategory?"(controller: 'store', action: 'browse') + "/blog/$year/$month?/$day?/$slug?"(controller: 'blog', action: 'show') + "/report/$id/download"(controller: 'report', action: 'download') + + get '/search'(controller: 'search', action: 'index') + post '/feedback'(controller: 'feedback', action: 'save') + + "/$controller/$action?/$id?(.$format)?"() + + '500'(view: '/error') + '404'(view: '/notFound') + } + } + + /** + * A mapping set in which several patterns deliberately overlap on the same URI, so that + * {@code matchAll} returns a multi-element candidate array. The application set above produces + * one or two candidates for a typical URI; this one shows how the per-candidate work in + * {@code collectControllerMappings} scales, which a two-candidate measurement alone cannot. + */ + static GrailsControllerUrlMappings createOverlappingMappings(GrailsApplication grailsApplication, + ApplicationContext applicationContext) { + createMappings(grailsApplication, applicationContext) { + '/api/books'(resources: 'book') + + "/api/books/$id"(controller: 'book', action: 'show') + "/api/$section/$id"(controller: 'book', action: 'show') + "/$controller/$action?/$id?(.$format)?"() + } + } + + private static GrailsControllerUrlMappings createMappings(GrailsApplication grailsApplication, + ApplicationContext applicationContext, Closure> mappings) { + DefaultUrlMappingEvaluator evaluator = new DefaultUrlMappingEvaluator(applicationContext) + Listnamespace property from the class when the artefact is created, so the value is taken from
+ * the artefact registry rather than read from the class again.
+ *
+ * The class passed in is the class of the controller issuing the redirect, which is not necessarily + * the controller currently executing - one controller may redirect on behalf of another - so the registry is + * looked up by that class and never by the executing controller.
+ * + * @param controllerClass The class of the controller issuing the redirect + * @return The declared namespace, or null if the class declares none + */ + private Object resolveNamespace(Class> controllerClass) { + GrailsApplication application = getGrailsApplication() + if (application != null) { + GrailsClass controllerArtefact = application.getArtefact(ControllerArtefactHandler.TYPE, controllerClass.getName()) + if (controllerArtefact instanceof GrailsControllerClass) { + return ((GrailsControllerClass) controllerArtefact).getNamespace() + } + } + // A controller that was never registered as an artefact - one constructed directly, as a unit test may do - + // has no GrailsControllerClass to read, so fall back to the class itself. + GrailsClassUtils.getStaticFieldValue(controllerClass, GrailsControllerClass.NAMESPACE_PROPERTY) + } + /** * Used the synchronizer token pattern to avoid duplicate form submissions * @@ -303,7 +330,7 @@ trait Controller implements ResponseRenderer, ResponseRedirector, RequestForward * @param request The servlet request */ private boolean consumeToken(GrailsWebRequest webRequest) { - final request = webRequest.getCurrentRequest() + final request = webRequest.getRequest() SynchronizerTokensHolder tokensHolderInSession = (SynchronizerTokensHolder) request.getSession(false)?.getAttribute(SynchronizerTokensHolder.HOLDER) if (!tokensHolderInSession) return false diff --git a/grails-controllers/src/main/groovy/grails/artefact/controller/support/RequestForwarder.groovy b/grails-controllers/src/main/groovy/grails/artefact/controller/support/RequestForwarder.groovy index 1dcbde36b61..5e502ce3298 100644 --- a/grails-controllers/src/main/groovy/grails/artefact/controller/support/RequestForwarder.groovy +++ b/grails-controllers/src/main/groovy/grails/artefact/controller/support/RequestForwarder.groovy @@ -110,7 +110,7 @@ trait RequestForwarder implements WebAttributes { Map model = params.model instanceof Map ? (Map) params.model : Collections.EMPTY_MAP - HttpServletRequest request = webRequest.currentRequest + HttpServletRequest request = webRequest.request HttpServletResponse response = webRequest.currentResponse for (Map.EntryallowedMethods property.
+ *
+ * @param controllerClass the controller class
+ * @return the map literal, or null if the controller does not declare a non empty map literal
+ */
+ private MapExpression getAllowedMethodsMapExpression(final ClassNode controllerClass) {
+ final FieldNode allowedMethodsField = controllerClass.getField(DefaultGrailsControllerClass.ALLOWED_HTTP_METHODS_PROPERTY);
+ if (allowedMethodsField == null) {
+ return null;
+ }
+ final Expression initialAllowedMethodsExpression = allowedMethodsField.getInitialExpression();
+ if (!(initialAllowedMethodsExpression instanceof MapExpression)) {
+ return null;
+ }
+ final MapExpression allowedMethodsMapExpression = (MapExpression) initialAllowedMethodsExpression;
+ return allowedMethodsMapExpression.getMapEntryExpressions().isEmpty() ? null : allowedMethodsMapExpression;
+ }
+
+ /**
+ * @param allowedMethodsMapExpression the map literal assigned to the controller's allowedMethods property
+ * @param methodName the name of an action
+ * @return true if the action is a key in the allowedMethods map and is therefore restricted to specific request methods
+ */
+ private boolean isActionRestricted(final MapExpression allowedMethodsMapExpression, final String methodName) {
+ for (MapEntryExpression allowedMethodsMapEntryExpression : allowedMethodsMapExpression.getMapEntryExpressions()) {
+ final Expression allowedMethodsMapEntryKeyExpression = allowedMethodsMapEntryExpression.getKeyExpression();
+ if (allowedMethodsMapEntryKeyExpression instanceof ConstantExpression) {
+ final ConstantExpression allowedMethodsMapKeyConstantExpression = (ConstantExpression) allowedMethodsMapEntryKeyExpression;
+ if (methodName.equals(allowedMethodsMapKeyConstantExpression.getValue())) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
/**
* This will wrap the method body in a try catch block which does something
* like this:
@@ -637,14 +662,23 @@ protected void wrapMethodBodyWithExceptionHandling(final ClassNode controllerCla
final CatchStatement catchStatement = new CatchStatement(new Parameter(new ClassNode(Exception.class), caughtExceptionArgumentName), catchBlockCode);
final Statement methodBody = methodNode.getCode();
+ final BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(controllerClassNode, methodNode.getName());
+
BlockStatement tryBlock = new BlockStatement();
- BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(controllerClassNode, methodNode.getName());
- tryBlock.addStatement(codeToHandleAllowedMethods);
+ if (!codeToHandleAllowedMethods.isEmpty()) {
+ tryBlock.addStatement(codeToHandleAllowedMethods);
+ }
tryBlock.addStatement(methodBody);
final TryCatchStatement tryCatchStatement = new TryCatchStatement(tryBlock, new EmptyStatement());
tryCatchStatement.addCatch(catchStatement);
+ if (codeToHandleAllowedMethods.isEmpty()) {
+ // Nothing wrote the ALLOWED_METHODS_HANDLED request attribute, so there is nothing to clean up.
+ methodNode.setCode(tryCatchStatement);
+ return;
+ }
+
final ArgumentListExpression argumentListExpression = new ArgumentListExpression();
argumentListExpression.addExpression(new ConstantExpression(ALLOWED_METHODS_HANDLED_ATTRIBUTE_NAME));
diff --git a/grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/api/ControllersDomainBindingApi.java b/grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/api/ControllersDomainBindingApi.java
index 9aad4693e15..aa884611eb5 100644
--- a/grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/api/ControllersDomainBindingApi.java
+++ b/grails-controllers/src/main/groovy/org/grails/plugins/web/controllers/api/ControllersDomainBindingApi.java
@@ -47,7 +47,8 @@ public class ControllersDomainBindingApi {
* @param instance The target instance
*/
public static void initialize(Object instance) {
- autowire(instance);
+ GrailsApplication application = findApplication();
+ autowire(instance, getDomainClass(instance, application));
}
/**
@@ -57,7 +58,8 @@ public static void initialize(Object instance) {
* @param namedArgs The named arguments
*/
public static void initialize(Object instance, Map namedArgs) {
- PersistentEntity dc = getDomainClass(instance);
+ GrailsApplication application = findApplication();
+ PersistentEntity dc = getDomainClass(instance, application);
if (dc == null) {
DataBindingUtils.bindObjectToInstance(instance, namedArgs);
}
@@ -65,49 +67,44 @@ public static void initialize(Object instance, Map namedArgs) {
DataBindingUtils.bindObjectToDomainInstance(dc, instance, namedArgs);
DataBindingUtils.assignBidirectionalAssociations(instance, namedArgs, dc);
}
- autowire(instance);
+ autowire(instance, dc);
}
- private static PersistentEntity getDomainClass(Object instance) {
- PersistentEntity domainClass = null;
- if (!Environment.isInitializing()) {
- final GrailsApplication grailsApplication = Holders.findApplication();
- if (grailsApplication != null) {
- try {
- domainClass = grailsApplication.getMappingContext().getPersistentEntity(instance.getClass().getName());
- } catch (GrailsConfigurationException e) {
- //no-op
- }
- }
- }
-
- return domainClass;
+ /**
+ * @return The current application, or null while the environment is still initializing or before an
+ * application has been bound
+ */
+ private static GrailsApplication findApplication() {
+ return Environment.isInitializing() ? null : Holders.findApplication();
}
- private static void autowire(Object instance) {
- if (!Environment.isInitializing()) {
-
- GrailsApplication application = Holders.findApplication();
- if (application != null) {
-
- try {
- PersistentEntity domainClass = application.getMappingContext().getPersistentEntity(instance.getClass().getName());
- if (domainClass != null) {
+ private static PersistentEntity getDomainClass(Object instance, GrailsApplication application) {
+ if (application == null) {
+ return null;
+ }
+ try {
+ return application.getMappingContext().getPersistentEntity(instance.getClass().getName());
+ } catch (GrailsConfigurationException e) {
+ // ignore, Mapping Context not initialized yet
+ return null;
+ }
+ }
- if (domainClass.getMapping().getMappedForm().isAutowire()) {
- final ApplicationContext applicationContext = Holders.findApplicationContext();
- if (applicationContext != null) {
- applicationContext
- .getAutowireCapableBeanFactory()
- .autowireBeanProperties(instance, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, false);
- }
- }
- }
- } catch (GrailsConfigurationException e) {
- // ignore, Mapping Context not initialized yet
+ private static void autowire(Object instance, PersistentEntity domainClass) {
+ if (domainClass == null) {
+ return;
+ }
+ try {
+ if (domainClass.getMapping().getMappedForm().isAutowire()) {
+ final ApplicationContext applicationContext = Holders.findApplicationContext();
+ if (applicationContext != null) {
+ applicationContext
+ .getAutowireCapableBeanFactory()
+ .autowireBeanProperties(instance, AutowireCapableBeanFactory.AUTOWIRE_BY_NAME, false);
}
-
}
+ } catch (GrailsConfigurationException e) {
+ // ignore, Mapping Context not initialized yet
}
}
}
diff --git a/grails-controllers/src/test/groovy/grails/artefact/ControllerRedirectSpec.groovy b/grails-controllers/src/test/groovy/grails/artefact/ControllerRedirectSpec.groovy
new file mode 100644
index 00000000000..073518e14fa
--- /dev/null
+++ b/grails-controllers/src/test/groovy/grails/artefact/ControllerRedirectSpec.groovy
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package grails.artefact
+
+import org.springframework.mock.web.MockHttpServletRequest
+import org.springframework.mock.web.MockHttpServletResponse
+import org.springframework.mock.web.MockServletContext
+import org.springframework.web.context.WebApplicationContext
+import org.springframework.web.context.request.RequestContextHolder
+import spock.lang.Specification
+
+import grails.core.DefaultGrailsApplication
+import grails.core.GrailsApplication
+import grails.core.GrailsControllerClass
+import grails.util.GrailsWebMockUtil
+import grails.web.mapping.LinkGenerator
+import grails.web.mapping.mvc.RedirectEventListener
+import org.grails.core.artefact.ControllerArtefactHandler
+import org.grails.web.servlet.mvc.ParameterCreationListener
+import org.grails.web.util.GrailsApplicationAttributes
+
+class ControllerRedirectSpec extends Specification {
+
+ MockServletContext servletContext = new MockServletContext()
+
+ List