sbglasius commented on code in PR #16296: URL: https://github.com/apache/grails-core/pull/16296#discussion_r3965391603
########## grails-common/src/main/groovy/org/apache/grails/common/reflect/ReflectionUtils.java: ########## @@ -0,0 +1,196 @@ +/* + * 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.common.reflect; + +import java.lang.reflect.Field; +import java.lang.reflect.InaccessibleObjectException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.jspecify.annotations.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.springframework.util.ClassUtils; + +/** + * Reads the properties of a bean whose class is not {@code public}. + * + * <p>A class that is not public -- anonymous, local, or package-private -- cannot have its read + * methods invoked from another package even when the methods themselves are public, so reflection + * over such a bean needs help. Groovy 4 stamped {@code ACC_PUBLIC} on anonymous inner classes and + * Groovy 5 does not, which is why beans of that shape reach the framework at all. + * + * <p>This compatibility handling is deliberately visible: {@link #warnOnNonPublicClass} reports the + * class once so an application can be corrected, and so the handling can be withdrawn if the + * compiler stops producing non-public classes for this shape. + * + * @since 8.0.0 + */ +public final class ReflectionUtils { + + private static final Logger LOG = LoggerFactory.getLogger(ReflectionUtils.class); + + /** Best-effort upper bound, so that generated class names cannot grow the set without limit. */ + private static final int MAX_WARNED_CLASSES = 1024; + + /** + * Keyed by class name rather than by {@link Class}, so that reporting a class never retains its + * class loader. A reload therefore does not report the same class again, which is intended: a + * reload loop would otherwise repeat every warning. + */ + private static final Set<String> WARNED_NON_PUBLIC_CLASSES = ConcurrentHashMap.newKeySet(); + + /** + * Packages whose classes an application cannot declare public itself, so reporting them would + * only be noise. {@code java.util.KeyValueHolder}, handed out by {@code Map.entry}, is the one + * that turns up in practice. + * + * <p>The {@code grails} packages are deliberately absent: a framework class reaching this code + * is worth seeing, and the tests declare their fixtures under {@code org.grails}. + */ + private static final String[] NON_APPLICATION_PACKAGES = { + "java.", "javax.", "jakarta.", "groovy.", "org.apache.groovy.", "org.codehaus.groovy.", + "org.springframework." + }; + + private ReflectionUtils() { + } + + /** + * Resolves a read method that can actually be invoked on {@code target}. + * + * <p>Where the property is declared by an interface, the interface method is returned: it is + * declared by an accessible type, and virtual dispatch still reaches the implementation. Where + * it is not -- a non-public class with no interface declaring the getter -- a private copy of + * the method is widened, so that the accessibility flag cannot leak into a {@code Method} + * instance shared through a descriptor cache. + * + * @param readMethod the property's read method, typically from a {@code PropertyDescriptor} + * @param targetClass the class being read, used to resolve the interface method + * @param target the instance being read, or {@code null} for a static read method + * @return a method that may be invoked on {@code target} + */ + public static Method resolveInvokableReadMethod(Method readMethod, Class<?> targetClass, @Nullable Object target) + throws NoSuchMethodException { + Method invokable = ClassUtils.getInterfaceMethodIfPossible(readMethod, targetClass); Review Comment: Done in `60598510cf`. I probed it against spring-core 7.0.9 with the built fixtures first, and it behaves as you said: | fixture | `getInterfaceMethodIfPossible` | `getPubliclyAccessibleMethodIfPossible` | |---|---|---| | anonymous impl of public interface | `PublicThing.getName` ✅ | `PublicThing.getName` ✅ | | package-private covariant override of a public superclass | `CovariantThing.getValue` ❌ | `PublicCovariantBase.getValue` ✅ | | package-private, no interface | ❌ | ❌ | So the covariant shape no longer calls `setAccessible` at all, and `invoke` still dispatches to the override. Widening is now confined to a getter with no public type anywhere in its hierarchy, which is the only shape that is genuinely non-standard. -- 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]
