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

[groovy] branch GROOVY-5169 updated (8811fcd -> 2963713)

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

emilles pushed a change to branch GROOVY-5169
in repository https://gitbox.apache.org/repos/asf/groovy.git.


 discard 8811fcd  GROOVY-5169 (pt.2): JSON output: include field-based properties
 discard 924efcd  GROOVY-7682, GROOVY-8371: JSON output: exclude static properties
     new 784b118  GROOVY-7682, GROOVY-8371: JSON output: exclude static properties
     new 2963713  GROOVY-5169 (pt.2): JSON output: include field-based properties

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (8811fcd)
            \
             N -- N -- N   refs/heads/GROOVY-5169 (2963713)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 subprojects/groovy-json/build.gradle | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY-5169
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 2963713c605267e5dcbc049143b62afac088bc72
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"}'
         '''
     }
 

[groovy] 01/02: GROOVY-7682, GROOVY-8371: JSON output: exclude static properties

Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

emilles pushed a commit to branch GROOVY-5169
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 784b1184718db9355b16ded597ab4b880ecda0d4
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jan 4 14:58:29 2022 -0600

    GROOVY-7682, GROOVY-8371: JSON output: exclude static properties
---
 subprojects/groovy-json/build.gradle               | 14 +++--
 .../java/groovy/json/DefaultJsonGenerator.java     |  2 +
 .../test/groovy/groovy/json/JsonOutputTest.groovy  | 72 +++++++++++++++++++++-
 3 files changed, 82 insertions(+), 6 deletions(-)

diff --git a/subprojects/groovy-json/build.gradle b/subprojects/groovy-json/build.gradle
index d9fcb6b..7190c9c 100644
--- a/subprojects/groovy-json/build.gradle
+++ b/subprojects/groovy-json/build.gradle
@@ -21,13 +21,19 @@ plugins {
 }
 
 dependencies {
-    api rootProject  // JsonBuilder extends GroovyObjectSupport...
+    api rootProject // JsonBuilder extends GroovyObjectSupport...
+
     testImplementation project(':groovy-test')
     testImplementation project(':groovy-dateutil')
     testImplementation 'net.javacrumbs.json-unit:json-unit:2.28.0'
-    testRuntimeOnly "org.slf4j:slf4j-api:${versions.slf4j}"
-    testRuntimeOnly project(':groovy-ant') // for JavadocAssertionTests
-    testRuntimeOnly 'com.google.code.gson:gson:2.8.9' // json-unit requires gson, jackson1, or jackson2
+
+    testRuntimeOnly project(':'), {
+        capabilities {
+            requireCapability 'org.apache.groovy:groovy-grapes'
+        }
+    }
+    testRuntimeOnly project(':groovy-ant')
+    testRuntimeOnly 'com.google.code.gson:gson:2.8.9' // json-unit requires gson, jackson1 or jackson2
 }
 
 plugins.withId('eclipse') {
diff --git a/subprojects/groovy-json/src/main/java/groovy/json/DefaultJsonGenerator.java b/subprojects/groovy-json/src/main/java/groovy/json/DefaultJsonGenerator.java
index 360ff72..cb9a8b8 100644
--- a/subprojects/groovy-json/src/main/java/groovy/json/DefaultJsonGenerator.java
+++ b/subprojects/groovy-json/src/main/java/groovy/json/DefaultJsonGenerator.java
@@ -55,6 +55,7 @@ import static groovy.json.JsonOutput.EMPTY_STRING_CHARS;
 import static groovy.json.JsonOutput.OPEN_BRACE;
 import static groovy.json.JsonOutput.OPEN_BRACKET;
 import static java.lang.reflect.Modifier.isPublic;
+import static java.lang.reflect.Modifier.isStatic;
 
 /**
  * A JsonGenerator that can be configured with various {@link JsonGenerator.Options}.
@@ -242,6 +243,7 @@ public class DefaultJsonGenerator implements JsonGenerator {
 
         for (MetaProperty mp : metaProperties) {
             if (!isPublic(mp.getModifiers())) continue; // GROOVY-5169
+            if ( isStatic(mp.getModifiers())) continue; // GROOVY-7682
 
             // skip write-only property: see File
             if (mp instanceof MetaBeanProperty) {
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 2f053a5..41c3ceb 100644
--- a/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy
+++ b/subprojects/groovy-json/src/test/groovy/groovy/json/JsonOutputTest.groovy
@@ -325,7 +325,7 @@ final class JsonOutputTest {
                 ] as JsonStreet[])
         ])
 
-        assert JsonOutput.prettyPrint(JsonOutput.toJson(city)) == '''\
+        assert JsonOutput.prettyPrint(toJson(city)) == '''\
             {
                 "name": "Paris",
                 "districts": [
@@ -448,7 +448,7 @@ final class JsonOutputTest {
         assert toJson({'\u0002' 0}) == '{"\\u0002":0}'
     }
 
-    @Test
+    @Test // GROOVY-7519
     void testFile() {
         def file  = File.createTempFile('test', 'file-json')
         file.deleteOnExit()
@@ -473,6 +473,74 @@ final class JsonOutputTest {
             assert json == '{"bar":"bar"}'
         '''
     }
+
+    @Test // GROOVY-7682
+    void testStackOverflowError1() {
+        assertScript '''
+            @Grab('joda-time:joda-time:2.10.10')
+            import org.joda.time.DateTime
+            import org.joda.time.format.DateTimeFormat
+            import org.joda.time.format.DateTimeFormatter
+
+            DateTimeFormatter formatter = DateTimeFormat.forPattern('yyyy-MM-dd HH:mm:ss.SSS z')
+            DateTime dt = formatter.parseDateTime('2015-11-20 13:37:21.123 EST')
+            String json = groovy.json.JsonOutput.toJson(dt)
+            net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals("""{
+                "afterNow":false,
+                "beforeNow":true,
+                "centuryOfEra":20,
+                "dayOfMonth":20,
+                "dayOfWeek":5,
+                "dayOfYear":324,
+                "equalNow":false,
+                "era":1,
+                "hourOfDay":13,
+                "millisOfDay":49041123,
+                "millisOfSecond":123,
+                "minuteOfDay":817,
+                "minuteOfHour":37,
+                "monthOfYear":11,
+                "secondOfDay":49041,
+                "secondOfMinute":21,
+                "weekOfWeekyear":47,
+                "weekyear":2015,
+                "year":2015,
+                "yearOfCentury":15,
+                "yearOfEra":2015,
+                "zone":{
+                    "ID":"America/New_York",
+                    "fixed":false,
+                    "uncachedZone":{
+                        "ID":"America/New_York",
+                        "cachable":true,
+                        "fixed":false
+                    }
+                }
+            }""", json)
+        '''
+    }
+
+    @Test // GROOVY-8371
+    void testStackOverflowError2() {
+        assertScript '''
+            final  date = java.time.LocalDate.of(1970, 1, 1)
+            String json = groovy.json.JsonOutput.toJson(date)
+            net.javacrumbs.jsonunit.JsonAssert.assertJsonEquals("""{
+                "chronology":{
+                    "calendarType":"iso8601",
+                    "id":"ISO"
+                },
+                "dayOfMonth":1,
+                "dayOfWeek":"THURSDAY",
+                "dayOfYear":1,
+                "era":"CE",
+                "leapYear":false,
+                "month":"JANUARY",
+                "monthValue":1,
+                "year":1970
+            }""", json)
+        '''
+    }
 }
 
 //------------------------------------------------------------------------------