borinquenkid commented on code in PR #16586: URL: https://github.com/apache/grails-core/pull/16586#discussion_r4365070501
########## grails-core/src/main/groovy/org/grails/core/DefaultGrailsControllerClass.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 org.grails.core + +import java.lang.invoke.MethodHandle +import java.lang.invoke.MethodHandles +import java.lang.invoke.MethodType +import java.lang.reflect.Method +import java.lang.reflect.Modifier + +import groovy.transform.CompileStatic +import groovy.transform.stc.POJO +import org.springframework.util.ReflectionUtils + +import grails.config.Settings +import grails.core.GrailsApplication +import grails.core.GrailsControllerClass +import grails.util.Environment +import grails.web.Action +import grails.web.UrlConverter + +/** + * Evaluates the conventions contained within controllers to perform auto-configuration. + * + * @author Graeme Rocher + * @author Steven Devijver + * @since 0.1 + */ +@SuppressWarnings('rawtypes') +@CompileStatic +@POJO +class DefaultGrailsControllerClass extends AbstractInjectableGrailsClass implements GrailsControllerClass { + + public static final String CONTROLLER = 'Controller' + + private static final String DEFAULT_CLOSURE_PROPERTY = 'defaultAction' + public static final String ALLOWED_HTTP_METHODS_PROPERTY = 'allowedMethods' + public static final Object[] EMPTY_ARGS = new Object[0] + public static final String SCOPE = 'scope' + public static final String SCOPE_SINGLETON = 'singleton' + private String scope + private Map<String, ActionInvoker> actions = new HashMap<>() + private String defaultActionName + private String namespace + protected Map<String, String> actionUriToViewName = new HashMap<>() + + DefaultGrailsControllerClass(Class<?> clazz) { + super(clazz, CONTROLLER) + namespace = getStaticPropertyValue(NAMESPACE_PROPERTY, String) + defaultActionName = getStaticPropertyValue(DEFAULT_CLOSURE_PROPERTY, String) + if (defaultActionName == null) { + defaultActionName = INDEX_ACTION + } + methodStrategy(actions) + this.scope = getStaticPropertyValue(SCOPE, String) + } + + void initialize() { + // no-op + } + + @Override + void setGrailsApplication(GrailsApplication grailsApplication) { + this.@grailsApplication = grailsApplication + if (this.scope == null) { + this.scope = grailsApplication.getConfig().getProperty(Settings.CONTROLLERS_DEFAULT_SCOPE, SCOPE_SINGLETON) + } + } + + @Override + Set<String> getActions() { + return actions.keySet() + } + + String getNamespace() { + return namespace + } + + @Override + String getScope() { + return scope + } + + @Override + boolean isSingleton() { + return SCOPE_SINGLETON.equalsIgnoreCase(getScope()) + } + + @Override + String getDefaultAction() { + return this.defaultActionName + } + + private void methodStrategy(Map<String, ActionInvoker> methodNames) { + + Class superClass = getClazz() + MethodHandles.Lookup lookup = MethodHandles.lookup() + while (superClass != Object && superClass != GroovyObject) { + for (Method method in superClass.getMethods()) { + if (Modifier.isPublic(method.getModifiers()) && method.getAnnotation(Action) != null) { + String methodName = method.getName() + if (Environment.isDevelopmentMode()) { + methodNames.put(methodName, new ReflectionInvoker(method)) + } + else { + MethodHandle mh + try { + mh = lookup.findVirtual(superClass, methodName, MethodType.methodType(method.getReturnType())) + methodNames.put(methodName, new MethodHandleInvoker(mh)) + } catch (NoSuchMethodException | IllegalAccessException e) { + methodNames.put(methodName, new ReflectionInvoker(method)) + } + } + + } + } + superClass = superClass.getSuperclass() + } + + if (!isActionMethod(defaultActionName) && methodNames.size() == 1 && !isReadableProperty('scaffold')) { + defaultActionName = methodNames.keySet().iterator().next() + } + } + + @Override + boolean mapsToURI(String uri) { + if (uri.startsWith('/')) { + String[] tokens = uri.substring(1).split('\\/') + if (tokens.length > 0) { + String controllerName = tokens[0] + if (getLogicalPropertyName().equals(controllerName)) { + if (tokens.length > 1) { + String actionName = tokens[1] + if (actions.containsKey(actionName) || defaultActionName.equals(actionName)) { + return true + } + } + else { + return true + } + } + } + } + return false + } + + /** + * Register a new {@link grails.web.UrlConverter} with the controller + * + * @param urlConverter The {@link grails.web.UrlConverter} to register + */ + @Override + void registerUrlConverter(UrlConverter urlConverter) { + for (String actionName in new ArrayList<>(actions.keySet())) { + actionUriToViewName.put(urlConverter.toUrlElement(actionName), actionName) + actions.put(urlConverter.toUrlElement(actionName), actions.remove(actionName)) + } + defaultActionName = urlConverter.toUrlElement(defaultActionName) + } + + /** + * Invokes the controller action for the given name on the given controller instance + * + * @param controller The controller instance + * @param action The action name + * @return The result of the action + * @throws Throwable + */ + @Override + Object invoke(Object controller, String action) throws Throwable { + if (action == null) action = this.defaultActionName + ActionInvoker handle = actions.get(action) + if (handle == null) throw new IllegalArgumentException('Invalid action name: ' + action) + return handle.invoke(controller) + } + + String actionUriToViewName(String actionUri) { + String actionName = actionUriToViewName.get(actionUri) + + return actionName != null ? actionName : actionUri + } + + private interface ActionInvoker { + Object invoke(Object controller) throws Throwable + } + + private class ReflectionInvoker implements ActionInvoker { + private final Method method + + ReflectionInvoker(Method method) { + this.method = method + ReflectionUtils.makeAccessible(method) + } + + @Override + Object invoke(Object controller) throws Throwable { + return method.invoke(controller) + } + } + + private class MethodHandleInvoker implements ActionInvoker { + private final MethodHandle handle + + MethodHandleInvoker(MethodHandle handle) { + this.handle = handle + } + + @Override + Object invoke(Object controller) throws Throwable { + return handle.invokeWithArguments(controller) Review Comment: Fixed in 477cbb0040 (pushed by jdaugherty): each action handle is adapted once with `asType` and `asSpreader` to the `(Object[])Object` shape Groovy emits for `invokeExact`, so dispatch is an exact direct invocation again and `invokeWithArguments` is gone. ########## grails-core/src/main/groovy/grails/util/Holders.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.util + +import java.lang.reflect.InvocationTargetException + +import groovy.transform.CompileStatic +import org.apache.commons.logging.Log +import org.apache.commons.logging.LogFactory +import org.springframework.context.ApplicationContext +import org.springframework.context.Lifecycle +import org.springframework.util.Assert + +import grails.config.Config +import grails.core.GrailsApplication +import grails.plugins.GrailsPluginManager +import org.grails.core.io.support.GrailsFactoriesLoader +import org.grails.core.support.GrailsApplicationDiscoveryStrategy + +/** + * Allows looking up key classes in a static context + * + * + * @author Burt Beckwith + * @author Graeme Rocher + * + * @since 2.0 + */ +@CompileStatic +class Holders { + + private static final Log LOG = LogFactory.getLog(Holders) + private static Holder<GrailsPluginManager> pluginManagers = new Holder<>('PluginManager') + private static Holder<Boolean> pluginManagersInCreation = new Holder<>('PluginManagers in creation') + private static Holder<Config> configs = new Holder<>('config') + private static Holder<Map<?, ?>> flatConfigs = new Holder<>('flat config') + + private static List<GrailsApplicationDiscoveryStrategy> applicationDiscoveryStrategies = GrailsFactoriesLoader.loadFactories(GrailsApplicationDiscoveryStrategy, Holders.getClassLoader()) + private static Holder servletContexts + + static { + + createServletContextsHolder() + } + + private static GrailsApplication applicationSingleton // TODO remove + + private Holders() { + // static only + } + + static void addApplicationDiscoveryStrategy(GrailsApplicationDiscoveryStrategy strategy) { + applicationDiscoveryStrategies.add(strategy) + } + + static void clear() { + pluginManagers.set(null) + pluginManagersInCreation.set(null) + configs.set(null) + flatConfigs.set(null) + if (servletContexts != null) { + servletContexts.set(null) + } + applicationDiscoveryStrategies.clear() + applicationSingleton = null + } + + static void setServletContext(final Object servletContext) { + servletContexts.set(servletContext) + } + + static Object getServletContext() { + return get(servletContexts, 'servletContext') + } + + static ApplicationContext getApplicationContext() { + for (GrailsApplicationDiscoveryStrategy strategy in applicationDiscoveryStrategies) { + ApplicationContext applicationContext = strategy.findApplicationContext() + if (applicationContext != null) { + boolean running = ((Lifecycle) applicationContext).isRunning() + if (running) { + return applicationContext + } + } + } + throw new IllegalStateException('Could not find ApplicationContext, configure Grails correctly first') + } + + /** + * + * @return The ApplicationContext or null if it doesn't exist + */ + static ApplicationContext findApplicationContext() { + for (GrailsApplicationDiscoveryStrategy strategy in applicationDiscoveryStrategies) { + ApplicationContext applicationContext = strategy.findApplicationContext() + if (applicationContext != null) { + return applicationContext + } + } + return null + } + + /** + * + * @return The ApplicationContext or null if it doesn't exist + */ + static GrailsApplication findApplication() { + for (GrailsApplicationDiscoveryStrategy strategy in applicationDiscoveryStrategies) { + GrailsApplication grailsApplication = strategy.findGrailsApplication() + if (grailsApplication != null) { + return grailsApplication + } + } + return applicationSingleton + } + + static GrailsApplication getGrailsApplication() { + GrailsApplication grailsApplication = findApplication() + Assert.notNull(grailsApplication, 'GrailsApplication not found') + return grailsApplication + } + + static void setGrailsApplication(GrailsApplication application) { + applicationSingleton = application + } + + static void setConfig(Config config) { + configs.set(config) + + // reset flat config + flatConfigs.set(config == null ? null : config) + } + + static Config getConfig() { + return get(configs, 'config') + } + + static Map<?, ?> getFlatConfig() { + Map<?, ?> flatConfig = get(flatConfigs, 'flatConfig') + return flatConfig == null ? Collections.emptyMap() : flatConfig + } + + static void setPluginManagerInCreation(boolean inCreation) { + pluginManagersInCreation.set(inCreation) + } + + static void setPluginManager(GrailsPluginManager pluginManager) { + if (pluginManager != null) { + pluginManagersInCreation.set(false) + } + pluginManagers.set(pluginManager) + } + + static GrailsPluginManager getPluginManager() { + return getPluginManager(false) + } + + static GrailsPluginManager getPluginManager(boolean mappedOnly) { + while (true) { + Boolean inCreation = get(pluginManagersInCreation, 'PluginManager in creation', mappedOnly) + if (inCreation == null) { + inCreation = false + } + if (!inCreation) { + break + } + + try { + Thread.sleep(100) + } + catch (InterruptedException e) { + break + } + } + + return get(pluginManagers, 'PluginManager', mappedOnly) + } + + static GrailsPluginManager currentPluginManager() { + GrailsPluginManager current = getPluginManager() + Assert.notNull(current, 'No PluginManager set') + return current + } + + static void reset() { + setPluginManager(null) + setGrailsApplication(null) + setServletContext(null) Review Comment: Fixed in 477cbb0040: `reset()` guards `setServletContext(null)` with `servletContexts != null` like `clear()`, so it runs to the end outside a servlet environment. `HoldersSpec` asserts that instead of the NPE. ########## grails-core/src/main/groovy/org/grails/config/CompositeConfig.groovy: ########## @@ -0,0 +1,269 @@ +/* + * 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.grails.config + +import groovy.transform.CompileStatic +import groovy.transform.stc.POJO + +import grails.config.Config +import grails.util.GrailsStringUtils +import org.grails.core.exceptions.GrailsConfigurationException + +/** + * A {@link Config} composed of other Configs + * + * @author Graeme Rocher + * @since 3.0 + */ +@CompileStatic +@POJO +class CompositeConfig implements Config { + + protected Deque<Config> configs = new ArrayDeque<>() + + /** + * Adds a config at the highest level of precedence + * + * @param config + */ + void addFirst(Config config) { + configs.addFirst(config) + } + + /** + * Adds a config at the lowest level of precedence + * + * @param config + */ + void addLast(Config config) { + configs.addLast(config) + } + + @Override + @Deprecated + Map<String, Object> flatten() { + Map<String, Object> flattened = new LinkedHashMap<>() + for (Config c in configs) { + flattened.putAll(c.flatten()) + } + return flattened + } + + @Override + Properties toProperties() { + Properties properties = new Properties() + for (Config c in configs) { + properties.putAll(c.toProperties()) + } + return properties + } + + @Override + Config merge(Map<String, Object> toMerge) { + throw new UnsupportedOperationException('Config cannot be modified') + } + + @Override + def <T> T getProperty(String key, Class<T> targetType, T defaultValue, List<T> allowedValues) { + T v = getProperty(key, targetType, defaultValue) + if (!allowedValues.contains(v)) { + throw new GrailsConfigurationException('Invalid configuration value [\$value] for key [\${key}]. Possible values \$allowedValues') Review Comment: Fixed in 477cbb0040: the messages in `CompositeConfig` and `NavigableMapConfig` (and the one in `ArtefactTypeAstTransformation`) are GStrings now and print the key, the value and the allowed values. New cases pin the texts. ########## grails-core/src/test/groovy/grails/util/HoldersSpec.groovy: ########## @@ -0,0 +1,130 @@ +/* + * 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.util + +import grails.config.Config +import grails.core.GrailsApplication +import grails.plugins.GrailsPluginManager +import org.grails.core.support.GrailsApplicationDiscoveryStrategy +import spock.lang.Specification + +class HoldersSpec extends Specification { + + void cleanup() { + // clear(), unlike reset(), guards its servlet-context write and is therefore safe to call + // outside a servlet environment (grails-core has no grails.web.context.WebRequestServletHolder + // on its classpath, so Holders.servletContexts is never populated here) + Holders.clear() Review Comment: Fixed in 477cbb0040: `HoldersSpec.cleanup()` re-registers the strategies `GrailsFactoriesLoader` supplies after `clear()`, so specs that follow in the same fork see the chain they would have seen without it. ########## grails-core/src/test/groovy/org/grails/spring/context/annotation/ClosureClassIgnoringComponentScanBeanDefinitionParserSpec.groovy: ########## @@ -0,0 +1,103 @@ +/* + * 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.grails.spring.context.annotation + +import java.lang.reflect.Field + +import org.springframework.beans.factory.support.DefaultListableBeanFactory +import org.springframework.beans.factory.xml.XmlBeanDefinitionReader +import org.springframework.beans.factory.xml.XmlReaderContext +import org.springframework.context.annotation.ClassPathBeanDefinitionScanner +import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider +import org.springframework.core.io.ByteArrayResource +import org.springframework.core.type.filter.TypeFilter +import spock.lang.Specification + +import grails.plugins.GrailsPluginManager + +/** + * {@link #configureScanner} and its resource-pattern $-exclusion logic are exercised + * end-to-end, using real Spring collaborators, via {@link GrailsComponentScanPostProcessorSpec}, + * whose {@code resourcePatternResolver()} builds the identical anonymous {@code AntPathMatcher}. + * Real {@code ParserContext}/{@code Element} XML-parsing collaborators are heavy to construct + * here without also driving a full namespace-handler parse, so this spec focuses on + * {@link #createScanner}, the plugin-manager type-filter wiring specific to this class. + */ +class ClosureClassIgnoringComponentScanBeanDefinitionParserSpec extends Specification { + + ClosureClassIgnoringComponentScanBeanDefinitionParser parser = new ClosureClassIgnoringComponentScanBeanDefinitionParser() + + private static XmlReaderContext readerContextFor(DefaultListableBeanFactory registry) { + new XmlBeanDefinitionReader(registry).createReaderContext(new ByteArrayResource(new byte[0])) + } + + private static List<TypeFilter> includeFiltersOf(ClassPathBeanDefinitionScanner scanner) { + Field field = ClassPathScanningCandidateComponentProvider.getDeclaredField('includeFilters') Review Comment: Fixed in 477cbb0040: both specs drive Spring through `XmlBeanDefinitionReader` parsing a `<grails:component-scan>` element, so no private field is read and `createScanner` is not called. The schema is not packaged in the jar, so they parse without validation; that is #16635. ########## grails-core/src/main/groovy/org/grails/plugins/AbstractGrailsPlugin.groovy: ########## @@ -0,0 +1,260 @@ +/* + * 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.grails.plugins + +import groovy.transform.CompileStatic +import groovy.transform.stc.POJO +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.beans.BeansException +import org.springframework.context.ApplicationContext +import org.springframework.core.env.ConfigurableEnvironment +import org.springframework.core.env.PropertySource +import org.springframework.util.Assert + +import grails.config.Config +import grails.core.GrailsApplication +import grails.plugins.GrailsPlugin +import grails.plugins.GrailsPluginManager +import grails.util.GrailsNameUtils +import org.apache.grails.core.plugins.PluginUtils +import org.grails.core.AbstractGrailsClass +import org.grails.plugins.support.WatchPattern + +/** + * Abstract implementation that provides some default behaviours + * + * @author Graeme Rocher + */ +@CompileStatic +abstract class AbstractGrailsPlugin extends GroovyObjectSupport implements GrailsPlugin { + + private static final Logger LOG = LoggerFactory.getLogger(AbstractGrailsPlugin) + + public static final String PLUGIN_YML = PluginUtils.PLUGIN_YML_CONFIG + public static final String PLUGIN_YML_PATH = PluginUtils.PLUGIN_YML_CONFIG_PATH + public static final String PLUGIN_GROOVY = PluginUtils.PLUGIN_GROOVY_CONFIG + public static final String PLUGIN_GROOVY_PATH = PluginUtils.PLUGIN_GROOVY_CONFIG_PATH + + protected GrailsApplication grailsApplication + protected boolean isBase = false + protected String version = '1.0' + protected Map<String, Object> dependencies = new HashMap<>() + protected String[] dependencyNames = [] as String[] + protected Class<?> pluginClass + protected ApplicationContext applicationContext + protected GrailsPluginManager manager + protected String[] evictionList = [] as String[] + protected Config config + + /** + * Wrapper Grails class for plugins. + * + * @author Graeme Rocher + */ + @POJO + @CompileStatic + static class GrailsPluginClass extends AbstractGrailsClass { Review Comment: Not intended, and fixed in 477cbb0040 and 05370e969f: `GrailsPluginClass` is a non-static, package-private inner class again, and `DefaultGrailsPlugin` passes the enclosing instance explicitly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
