You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2022/01/14 10:56:29 UTC

[groovy] 03/04: GROOVY-5169 (pt.2): JSON output: include field-based properties

This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 480a72df1858a2e31842cd1f157bbf0a081b7336
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jan 5 09:56:03 2022 -0600

    GROOVY-5169 (pt.2): JSON output: include field-based properties
---
 src/main/java/groovy/lang/MetaClassImpl.java       |  4 +--
 src/test/groovy/ClosureTest.groovy                 |  9 +++--
 src/test/groovy/Property2Test.groovy               | 14 +++-----
 .../antlr4/util/ASTComparatorCategory.groovy       | 26 +++++---------
 .../console/ui/ScriptToTreeNodeAdapter.groovy      | 42 ++++++++++------------
 .../groovy/jmx/builder/JmxMetaMapBuilder.groovy    | 38 ++++++++++----------
 .../test/groovy/groovy/json/JsonOutputTest.groovy  |  4 +--
 7 files changed, 62 insertions(+), 75 deletions(-)

diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 45ca41c..3710648 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -2313,11 +2313,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
         // simply return the values of the metaproperty map as a List
         List<MetaProperty> ret = new ArrayList<>(propertyMap.size());
         for (MetaProperty mp : propertyMap.values()) {
-            if (mp instanceof CachedField) continue;
-
             if (mp instanceof MetaBeanProperty) {
                 MetaBeanProperty mbp = (MetaBeanProperty) mp;
-                // filter out DGM beans
+                // filter out extrinsic properties (DGM, ...)
                 boolean getter = true, setter = true;
                 MetaMethod getterMetaMethod = mbp.getGetter();
                 if (getterMetaMethod == null || getterMetaMethod instanceof GeneratedMetaMethod || getterMetaMethod instanceof NewInstanceMetaMethod) {
diff --git a/src/test/groovy/ClosureTest.groovy b/src/test/groovy/ClosureTest.groovy
index 19e0a15..8ca0195 100644
--- a/src/test/groovy/ClosureTest.groovy
+++ b/src/test/groovy/ClosureTest.groovy
@@ -22,6 +22,8 @@ import groovy.test.GroovyTestCase
 import org.codehaus.groovy.control.MultipleCompilationErrorsException
 
 import static groovy.lang.Closure.IDENTITY
+import static java.lang.reflect.Modifier.isPublic
+import static java.lang.reflect.Modifier.isStatic
 
 final class ClosureTest extends GroovyTestCase {
 
@@ -204,8 +206,11 @@ final class ClosureTest extends GroovyTestCase {
         Closure.metaClass = null
 
         Closure.metaClass.properties.each { property ->
-            def closure = { println it }
-            closure."$property.name" == closure."${MetaProperty.getGetterName(property.name, property.type)}"()
+            int modifiers = property.modifiers
+            if (isPublic(modifiers) && !isStatic(modifiers)) {
+                Closure closure = { -> }
+                closure."$property.name" == closure."${MetaProperty.getGetterName(property.name, property.type)}"()
+            }
         }
     }
 
diff --git a/src/test/groovy/Property2Test.groovy b/src/test/groovy/Property2Test.groovy
index 152657e..5ebe2aa 100644
--- a/src/test/groovy/Property2Test.groovy
+++ b/src/test/groovy/Property2Test.groovy
@@ -23,7 +23,7 @@ import groovy.test.GroovyTestCase
 /**
  * Tests the use of getMetaPropertyValues() and getProperties() for Beans and Expandos.
  */
-class Property2Test extends GroovyTestCase {
+final class Property2Test extends GroovyTestCase {
 
     void testGetPropertiesBeanCheckingValues() {
         def foo = new Foo()
@@ -31,19 +31,16 @@ class Property2Test extends GroovyTestCase {
         // these are the properties and their values that should be there
         def props = ['name': 'James', 'count': 1, 'location': 'London', 'blah': 9]
         foo.properties.each { name, value ->
-            // GROOVY-996 - We should see protected properties, but not  private ones.
-            assert name != "invisible"
+            // GROOVY-996, GROOVY-10438: should see protected properties, but not private ones
+            if (name == 'invisible') return
 
-            def pvalue = props[name]
+            def pvalue = props.remove(name)
             if (pvalue != null)
                 assert pvalue == value
-
-            // remove this one from the map
-            props.remove(name)
         }
 
         // make sure there are none left over
-        assert props.size() == 0
+        assert props.isEmpty()
     }
 
     void testMetaPropertyValuesFromObject() {
@@ -71,4 +68,3 @@ class Property2Test extends GroovyTestCase {
         assert props.size() == 0
     }
 }
-
diff --git a/src/test/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy b/src/test/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
index 25671ea..129da6a 100644
--- a/src/test/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
+++ b/src/test/org/apache/groovy/parser/antlr4/util/ASTComparatorCategory.groovy
@@ -219,14 +219,14 @@ class ASTComparatorCategory {
     static reflexiveEquals(a, b, ignore = []) {
         if (a.getClass() != b.getClass()) {
             log.warning(" !!!! DIFFERENCE WAS FOUND! ${a.getClass()} != ${b.getClass()}")
-            return false;
+            return false
         }
 
         def objects = [a, b]
         Boolean res = this.objects[objects]
         if (res != null) {
             log.info("Skipping [$a, $b] comparison as they are ${ res ? "" : "un" }equal.")
-            return res;
+            return res
         }
         else if (this.objects.containsKey(objects)) {
             log.info("Skipping as they are processed at higher levels.")
@@ -239,29 +239,21 @@ class ASTComparatorCategory {
             return true
 
         def difference = a.metaClass.properties.find { MetaProperty mp  ->
-            MetaBeanProperty p = (MetaBeanProperty) mp
-            if (!p.getter)
+            if (mp !instanceof MetaBeanProperty || !mp.getter)
                 return false
 
-            def name = p.name
+            def name = mp.name
             lastName = "$name :::: ${ a.getClass() } ${ a.hashCode() }"
 
-
             for (Map.Entry<Class, List<String>> me : COLLECTION_PROPERTY_CONFIGURATION) {
-                if (!(me.key.isCase(a) && me.key.isCase(b))) {
-                    continue;
-                }
-
-                String propName = me.value[0];
-
-                if (name != propName) {
-                    continue;
+                if (name != me.value[0] || !(me.key.isCase(a) && me.key.isCase(b))) {
+                    continue
                 }
 
-                def aValue = a."${propName}"; // FIXME when the propName is "classes", a classNode will be added to moduleNode.classes
-                def bValue = b."${propName}";
+                def aValue = a."${name}" // FIXME when the name is "classes", a classNode will be added to moduleNode.classes
+                def bValue = b."${name}"
 
-                String orderName = me.value[1];
+                String orderName = me.value[1]
 
                 return new LinkedList(aValue?.getClass()?.isArray() ? Arrays.asList(aValue) : (aValue ?: [])).sort {c1, c2 -> c1."${orderName}" <=> c2."${orderName}"} !=
                         new LinkedList(bValue?.getClass()?.isArray() ? Arrays.asList(bValue) : (bValue ?: [])).sort {c1, c2 -> c1."${orderName}" <=> c2."${orderName}"}
diff --git a/subprojects/groovy-console/src/main/groovy/groovy/console/ui/ScriptToTreeNodeAdapter.groovy b/subprojects/groovy-console/src/main/groovy/groovy/console/ui/ScriptToTreeNodeAdapter.groovy
index f825de2..426590f 100644
--- a/subprojects/groovy-console/src/main/groovy/groovy/console/ui/ScriptToTreeNodeAdapter.groovy
+++ b/subprojects/groovy-console/src/main/groovy/groovy/console/ui/ScriptToTreeNodeAdapter.groovy
@@ -215,30 +215,26 @@ class ScriptToTreeNodeAdapter {
      * Creates the property table for the node so that the properties view can display nicely.
      */
     private List<List<String>> getPropertyTable(node) {
-        node.metaClass.properties?.
-                findAll { it.getter }?.
-                collect {
-                    def name = it.name.toString()
-                    def value
-                    try {
-                        // multiple assignment statements cannot be cast to VariableExpression so
-                        // instead reference the value through the leftExpression property, which is the same
-                        if (node instanceof DeclarationExpression &&
-                                (name == 'variableExpression' || name == 'tupleExpression')) {
-                            value = toString(node.leftExpression)
-                        } else {
-                            value = toString(it.getProperty(node))
-                        }
-                    } catch (GroovyBugError reflectionArtefact) {
-                        // compiler throws error if it thinks a field is being accessed
-                        // before it is set under certain conditions. It wasn't designed
-                        // to be walked reflectively like this.
-                        value = null
+        node.metaClass.properties.findResults { mp ->
+            if (mp instanceof MetaBeanProperty && mp.getter) {
+                String name = mp.name, type = mp.type.simpleName
+                Object value = null
+                try {
+                    // multiple assignment statements cannot be cast to VariableExpression so
+                    // instead reference the value through the leftExpression property, which is the same
+                    if (node instanceof DeclarationExpression && (name == 'variableExpression' || name == 'tupleExpression')) {
+                        value = toString(node.leftExpression)
+                    } else {
+                        value = toString(mp.getProperty(node))
                     }
-                    def type = it.type.simpleName.toString()
-                    [name, value, type]
-                }?.
-                sort { it[0] }
+                } catch (GroovyBugError ignore) {
+                    // compiler throws error if it thinks a field is being accessed
+                    // before it is set under certain conditions. It wasn't designed
+                    // to be walked reflectively like this.
+                }
+                [name, value, type]
+            }
+        }.sort { list -> list[0] } // sort by name
     }
 
     // GROOVY-8339: to avoid illegal access to a non-visible implementation class - can be removed if a more general solution is found
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 7a77889..4c8d8c8 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
@@ -19,7 +19,9 @@
 package groovy.jmx.builder
 
 import javax.management.ObjectName
+
 import java.lang.reflect.Constructor
+import java.lang.reflect.Modifier
 
 /**
  * The JmxMetaMapBuilder class is used to collect meta data passed in JmxBuilder nodes.  Once collected,
@@ -27,8 +29,8 @@ import java.lang.reflect.Constructor
  */
 class JmxMetaMapBuilder {
 
-    private static final ATTRIB_EXCEPTION_LIST = ["class", "descriptor", "jmx", "metaClass"]
-    private static final OPS_EXCEPTION_LIST = [
+    private static final Set<String> ATTRIB_EXCEPTION_LIST = ["class", "descriptor", "jmx", "metaClass"]
+    private static final Set<String> OPS_EXCEPTION_LIST = [
             "clone",
             "equals",
             "finalize",
@@ -186,22 +188,20 @@ class JmxMetaMapBuilder {
      * @return the meta map for the attribute
      */
     static Map buildAttributeMapFrom(def object) {
-        def properties = object.metaClass.getProperties()
-
         def attribs = [:]
-        properties.each { MetaProperty prop ->
-            if (!ATTRIB_EXCEPTION_LIST.contains(prop.name)) {
-                def attrib = [:]
-                def getterPrefix = (prop.type.name == "java.lang.Boolean" || prop.type.name == "boolean") ? "is" : "get"
-                def name = JmxBuilderTools.capitalize(prop.name)
-                attrib.name = name
-                attrib.displayName = "Property ${prop.name}".toString()
-                attrib.readable = true
-                attrib.getMethod = getterPrefix + name
-                attrib.writable = false
-                attrib.type = prop.type.name
-                attrib.property = prop
-                attribs.put(name, attrib)
+        object.metaClass.properties.each { MetaProperty prop ->
+            if (!ATTRIB_EXCEPTION_LIST.contains(prop.name)
+                    && !Modifier.isPrivate(prop.modifiers)) { // GROOVY-10438
+                String name = JmxBuilderTools.capitalize(prop.name)
+                attribs.put(name, [
+                   name: name,
+                   type: prop.type.name,
+                   displayName: "Property ${prop.name}".toString(),
+                   getMethod: (prop.type.name == 'boolean' ? 'is' : 'get') + name,
+                   property: prop,
+                   readable: true,
+                   writable: false
+                ])
             }
         }
         return attribs
@@ -393,10 +393,10 @@ class JmxMetaMapBuilder {
      * @return The meta map generated.
      */
     static Map buildOperationMapFrom(def object) {
-        def methods = object.metaClass.getMethods()
+        def methods = object.metaClass.methods
         def ops = [:]
 
-        def declaredMethods = object.getClass().getDeclaredMethods()*.name
+        def declaredMethods = object.class.declaredMethods*.name
 
         methods.each { method ->
             // avoid picking up extra methods from parents
diff --git a/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy b/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy
index 41c3ceb..b7cabee 100644
--- a/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy
+++ b/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy
@@ -463,14 +463,14 @@ final class JsonOutputTest {
     void testPropertyModifiers1() {
         assertScript '''
             class Pogo {
-                public    String foo = 'foo' //TODO
+                public    String foo = 'foo'
                 public    String getBar() { 'bar' }
                 protected String getBaz() { 'baz' }
                 private   String getBoo() { 'boo' }
             }
 
             String json = groovy.json.JsonOutput.toJson(new Pogo())
-            assert json == '{"bar":"bar"}'
+            assert json == '{"foo":"foo","bar":"bar"}'
         '''
     }