From 6b6b1d0ddf3575ed79a86cfc29ae7996004216da Mon Sep 17 00:00:00 2001 From: Paul King Date: Tue, 18 Aug 2026 13:34:48 +1000 Subject: [PATCH] GROOVY-12267: Remove the dead declared-method check from JMX operation export buildOperationMapFrom guarded operation export with (declaredMethods.contains(name) && !OPS_EXCEPTION_LIST.contains(name)) || (!OPS_EXCEPTION_LIST.contains(name)) which reduces to its right operand, so the declaredMethods lookup above it was computed and never used and the comment claiming it avoided methods from parents described something the code did not do. Drop the dead operand and the unused lookup, and state the actual contract in the javadoc: the default export spans the whole inheritance chain, and a caller wanting a narrower set names the operations explicitly in a descriptor. Behaviour is unchanged. Making the declared-method check live instead was considered and rejected: JmxBuilder's own embedded-descriptor fixtures declare their operations on a base class, and 'operations: "*"' routes to this method and has to mean all of them, so restricting to declared methods breaks four existing tests and gives '*' a meaning it should not have. Adds two tests pinning behaviour that was previously untested: inherited operations are exported, and getMetaClass/setMetaClass are not, the latter because they resolve to the metaClass MetaProperty and are dropped by the getter/setter filter. --- .../jmx/builder/JmxMetaMapBuilder.groovy | 10 +++++--- .../jmx/builder/JmxMetaMapBuilderTest.groovy | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/subprojects/groovy-jmx/src/main/groovy/groovy/jmx/builder/JmxMetaMapBuilder.groovy b/subprojects/groovy-jmx/src/main/groovy/groovy/jmx/builder/JmxMetaMapBuilder.groovy index b341310d7b7..a11c2ba8ca0 100644 --- a/subprojects/groovy-jmx/src/main/groovy/groovy/jmx/builder/JmxMetaMapBuilder.groovy +++ b/subprojects/groovy-jmx/src/main/groovy/groovy/jmx/builder/JmxMetaMapBuilder.groovy @@ -389,6 +389,11 @@ class JmxMetaMapBuilder { * **************************************/ /** * Returns a meta map of operations from given object. + *

+ * Every public method reachable on the object is exported, inherited ones included, less + * the names in {@code OPS_EXCEPTION_LIST}. To export a narrower set, name the wanted + * operations explicitly in a descriptor rather than relying on this default. + * * @param object to profile * @return The meta map generated. */ @@ -396,11 +401,8 @@ class JmxMetaMapBuilder { def methods = object.metaClass.methods def ops = [:] - def declaredMethods = object.class.declaredMethods*.name - methods.each { method -> - // avoid picking up extra methods from parents - if ((declaredMethods.contains(method.name) && !OPS_EXCEPTION_LIST.contains(method.name)) || (!OPS_EXCEPTION_LIST.contains(method.name))) { + if (!OPS_EXCEPTION_LIST.contains(method.name)) { String mName = method.name MetaProperty prop = (mName.length() > 3 && (mName.startsWith("get") || mName.startsWith("set")) || diff --git a/subprojects/groovy-jmx/src/test/groovy/groovy/jmx/builder/JmxMetaMapBuilderTest.groovy b/subprojects/groovy-jmx/src/test/groovy/groovy/jmx/builder/JmxMetaMapBuilderTest.groovy index b44c0c3b7d0..97b78e53769 100644 --- a/subprojects/groovy-jmx/src/test/groovy/groovy/jmx/builder/JmxMetaMapBuilderTest.groovy +++ b/subprojects/groovy-jmx/src/test/groovy/groovy/jmx/builder/JmxMetaMapBuilderTest.groovy @@ -340,6 +340,31 @@ class JmxMetaMapBuilderTest { assert map."set".name == "set" } + @Test + void testMetaClassAccessorsAreNotExportedAsOperations() { + // Not exported because getMetaClass/setMetaClass resolve to the metaClass MetaProperty + // and are dropped by the getter/setter filter, not because they are named in + // OPS_EXCEPTION_LIST. Pinned so that a change to that filter cannot quietly publish + // them as remotely invokable operations. + [new MockManagedObject(), new MockManagedGroovyObject()].each { object -> + def map = JmxMetaMapBuilder.buildOperationMapFrom(object) + assert !map."getMetaClass" + assert !map."setMetaClass" + } + } + + @Test + void testInheritedOperationsRemainExported() { + // The default export deliberately spans the whole inheritance chain: JmxBuilder's own + // embedded-descriptor fixtures declare their operations on a base class, and + // 'operations: "*"' routes here and must mean all of them. + def map = JmxMetaMapBuilder.buildOperationMapFrom(new EmbeddedAllOps()) + + assert map."doNothing" + assert map."doTwoThings" + assert map."doThreeThings" + } + @Test void testBuildOperationFromDescriptorMap() { def object = new MockManagedObject()