From dd237fe9d3c435fec51d4da09fb0153456d492ea Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:21:20 +1100 Subject: [PATCH 1/9] Make String parseValue coercing consistent with JS implementation --- .../graphql/scalar/GraphqlStringCoercing.java | 14 +++++-- src/main/resources/i18n/Scalars.properties | 2 + src/main/resources/i18n/Scalars_de.properties | 2 + .../groovy/graphql/ScalarsStringTest.groovy | 38 +++++++++++++++++-- 4 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/main/java/graphql/scalar/GraphqlStringCoercing.java b/src/main/java/graphql/scalar/GraphqlStringCoercing.java index 541d157fd9..d794edfe14 100644 --- a/src/main/java/graphql/scalar/GraphqlStringCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlStringCoercing.java @@ -24,11 +24,19 @@ @Internal public class GraphqlStringCoercing implements Coercing { - private String toStringImpl(Object input) { return String.valueOf(input); } + private String parseValueImpl(@NotNull Object input, Locale locale) { + if (!(input instanceof String)) { + throw new CoercingParseValueException( + i18nMsg(locale, "String.unexpectedRawValueType", typeName(input)) + ); + } + return (String) input; + } + private String parseLiteralImpl(@NotNull Object input, Locale locale) { if (!(input instanceof StringValue)) { throw new CoercingParseLiteralException( @@ -56,12 +64,12 @@ public String serialize(@NotNull Object dataFetcherResult) { @Override @Deprecated public String parseValue(@NotNull Object input) { - return toStringImpl(input); + return parseValueImpl(input, Locale.getDefault()); } @Override public String parseValue(@NotNull Object input, @NotNull GraphQLContext graphQLContext, @NotNull Locale locale) throws CoercingParseValueException { - return toStringImpl(input); + return parseValueImpl(input, locale); } @Override diff --git a/src/main/resources/i18n/Scalars.properties b/src/main/resources/i18n/Scalars.properties index 0897fe58eb..4dbc68d683 100644 --- a/src/main/resources/i18n/Scalars.properties +++ b/src/main/resources/i18n/Scalars.properties @@ -27,3 +27,5 @@ Float.unexpectedAstType=Expected an AST type of ''IntValue'' or ''FloatValue'' b # Boolean.notBoolean=Expected a value that can be converted to type ''Boolean'' but it was a ''{0}'' Boolean.unexpectedAstType=Expected an AST type of ''BooleanValue'' but it was a ''{0}'' +# +String.unexpectedRawValueType=Expected a String input, but it was a ''{0}'' diff --git a/src/main/resources/i18n/Scalars_de.properties b/src/main/resources/i18n/Scalars_de.properties index de929f4770..ce045d8404 100644 --- a/src/main/resources/i18n/Scalars_de.properties +++ b/src/main/resources/i18n/Scalars_de.properties @@ -30,3 +30,5 @@ Float.unexpectedAstType=Erwartet wurde ein AST type von ''IntValue'' oder ''Floa # Boolean.notBoolean=Erwartet wurde ein Wert, der in den Typ ''Boolean'' konvertiert werden kann, aber es war ein ''{0}'' Boolean.unexpectedAstType=Erwartet wurde ein AST type ''BooleanValue'', aber es war ein ''{0}'' +# +String.unexpectedRawValueType=Erwartet wurde eine String-Eingabe, aber es war ein ''{0}'' diff --git a/src/test/groovy/graphql/ScalarsStringTest.groovy b/src/test/groovy/graphql/ScalarsStringTest.groovy index e19e02796a..d4b729af66 100644 --- a/src/test/groovy/graphql/ScalarsStringTest.groovy +++ b/src/test/groovy/graphql/ScalarsStringTest.groovy @@ -4,6 +4,7 @@ import graphql.execution.CoercedVariables import graphql.language.BooleanValue import graphql.language.StringValue import graphql.schema.CoercingParseLiteralException +import graphql.schema.CoercingParseValueException import spock.lang.Shared import spock.lang.Specification import spock.lang.Unroll @@ -54,7 +55,6 @@ class ScalarsStringTest extends Specification { def "String serialize #value into #result (#result.class)"() { expect: Scalars.GraphQLString.getCoercing().serialize(value, GraphQLContext.default, Locale.default) == result - Scalars.GraphQLString.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result where: value | result @@ -64,10 +64,9 @@ class ScalarsStringTest extends Specification { } @Unroll - def "String serialize #value into #result (#result.class) with deprecated methods"() { + def "String serialize #value into #result (#result.class) with deprecated method"() { expect: Scalars.GraphQLString.getCoercing().serialize(value) == result // Retain deprecated method for test coverage - Scalars.GraphQLString.getCoercing().parseValue(value) == result // Retain deprecated method for test coverage where: value | result @@ -76,4 +75,37 @@ class ScalarsStringTest extends Specification { customObject | "foo" } + @Unroll + def "String parseValue #value into #result"() { + expect: + Scalars.GraphQLString.getCoercing().parseValue("123ab", GraphQLContext.default, Locale.default) == "123ab" + } + + @Unroll + def "String parseValue #value into #result with deprecated method"() { + expect: + Scalars.GraphQLString.getCoercing().parseValue("123ab") == "123ab" // Retain deprecated method for test coverage + } + + @Unroll + def "String parseValue throws exception for non-String values"() { + when: + Scalars.GraphQLString.getCoercing().parseValue(literal, GraphQLContext.default, Locale.default) + then: + def ex = thrown(CoercingParseValueException) + + where: + literal | _ + 123 | _ + true | _ + customObject | _ + } + + def "String parseValue English exception message"() { + when: + Scalars.GraphQLString.getCoercing().parseValue(9001, GraphQLContext.default, Locale.ENGLISH) + then: + def ex = thrown(CoercingParseValueException) + ex.message == "Expected a String input, but it was a 'Integer'" + } } From a05f6c419439ff180605dbfd11c59a49a99448a5 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 15:50:27 +1100 Subject: [PATCH 2/9] Fix typos --- src/main/java/graphql/scalar/GraphqlStringCoercing.java | 2 +- src/test/groovy/graphql/ScalarsStringTest.groovy | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main/java/graphql/scalar/GraphqlStringCoercing.java b/src/main/java/graphql/scalar/GraphqlStringCoercing.java index d794edfe14..b330f254ae 100644 --- a/src/main/java/graphql/scalar/GraphqlStringCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlStringCoercing.java @@ -28,7 +28,7 @@ private String toStringImpl(Object input) { return String.valueOf(input); } - private String parseValueImpl(@NotNull Object input, Locale locale) { + private String parseValueImpl(@NotNull Object input, @NotNull Locale locale) { if (!(input instanceof String)) { throw new CoercingParseValueException( i18nMsg(locale, "String.unexpectedRawValueType", typeName(input)) diff --git a/src/test/groovy/graphql/ScalarsStringTest.groovy b/src/test/groovy/graphql/ScalarsStringTest.groovy index d4b729af66..8a725eb122 100644 --- a/src/test/groovy/graphql/ScalarsStringTest.groovy +++ b/src/test/groovy/graphql/ScalarsStringTest.groovy @@ -75,14 +75,12 @@ class ScalarsStringTest extends Specification { customObject | "foo" } - @Unroll - def "String parseValue #value into #result"() { + def "String parseValue value into result"() { expect: Scalars.GraphQLString.getCoercing().parseValue("123ab", GraphQLContext.default, Locale.default) == "123ab" } - @Unroll - def "String parseValue #value into #result with deprecated method"() { + def "String parseValue value into result with deprecated method"() { expect: Scalars.GraphQLString.getCoercing().parseValue("123ab") == "123ab" // Retain deprecated method for test coverage } From 28cc1f23ce81c6f3bcfa9fbcdffb02db47fe29fe Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:00:11 +1100 Subject: [PATCH 3/9] Fix incorrect bintray redirect --- build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle b/build.gradle index 609d59347a..45091769f6 100644 --- a/build.gradle +++ b/build.gradle @@ -70,6 +70,7 @@ gradle.buildFinished { buildResult -> repositories { mavenCentral() + gradlePluginPortal() mavenLocal() } From 63887824cde9d3bda6fbb1145bf3b8f1bc6cc06d Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:03:55 +1100 Subject: [PATCH 4/9] Add pluginManagement --- build.gradle | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 45091769f6..1f0bdd227a 100644 --- a/build.gradle +++ b/build.gradle @@ -70,10 +70,16 @@ gradle.buildFinished { buildResult -> repositories { mavenCentral() - gradlePluginPortal() mavenLocal() } +pluginManagement { + repositories { + mavenCentral() + gradlePluginPortal() + } +} + jar { from "LICENSE.md" from "src/main/antlr/Graphql.g4" From 7515f3a45739d4c5299d9f80692258d398c7e1bb Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:09:45 +1100 Subject: [PATCH 5/9] Amend settings.gradle --- build.gradle | 7 ------- settings.gradle | 6 ++++++ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/build.gradle b/build.gradle index 1f0bdd227a..609d59347a 100644 --- a/build.gradle +++ b/build.gradle @@ -73,13 +73,6 @@ repositories { mavenLocal() } -pluginManagement { - repositories { - mavenCentral() - gradlePluginPortal() - } -} - jar { from "LICENSE.md" from "src/main/antlr/Graphql.g4" diff --git a/settings.gradle b/settings.gradle index 72827aa7ad..3caa89fd59 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,2 +1,8 @@ rootProject.name = 'graphql-java' +pluginManagement { + repositories { + gradlePluginPortal() + mavenCentral() + } +} From d4c128229b68709544155b1efffd20f95515f0cc Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:13:01 +1100 Subject: [PATCH 6/9] Reorder settings --- settings.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/settings.gradle b/settings.gradle index 3caa89fd59..00db1d12d7 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,8 +1,8 @@ -rootProject.name = 'graphql-java' - pluginManagement { repositories { gradlePluginPortal() mavenCentral() } } + +rootProject.name = 'graphql-java' From 9ddffb445f7b2148af342e5574339c4ce16ad9cf Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:15:07 +1100 Subject: [PATCH 7/9] Update jmh plugin --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 609d59347a..acfea22bc0 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,7 @@ plugins { id "biz.aQute.bnd.builder" version "6.3.1" id "io.github.gradle-nexus.publish-plugin" version "1.1.0" id "groovy" - id "me.champeau.jmh" version "0.6.6" + id "me.champeau.jmh" version "0.6.8" } def getDevelopmentVersion() { From d1743132ff15d9ee61326b5a2b8671dfb9dff2f4 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 16:41:26 +1100 Subject: [PATCH 8/9] Prevent metadata redirection for jmh plugin --- build.gradle | 2 +- settings.gradle | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index acfea22bc0..609d59347a 100644 --- a/build.gradle +++ b/build.gradle @@ -10,7 +10,7 @@ plugins { id "biz.aQute.bnd.builder" version "6.3.1" id "io.github.gradle-nexus.publish-plugin" version "1.1.0" id "groovy" - id "me.champeau.jmh" version "0.6.8" + id "me.champeau.jmh" version "0.6.6" } def getDevelopmentVersion() { diff --git a/settings.gradle b/settings.gradle index 00db1d12d7..17dc6978a0 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,7 +1,14 @@ pluginManagement { repositories { - gradlePluginPortal() mavenCentral() + maven { + url 'https://plugins.gradle.org/m2' + metadataSources { + ignoreGradleMetadataRedirection() + mavenPom() + artifact() + } + } } } From f31ceb4dc7615a2e28e14911c8171a877ff92a9b Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sat, 26 Nov 2022 17:00:14 +1100 Subject: [PATCH 9/9] Add comment explaining metadata redirection problem --- settings.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/settings.gradle b/settings.gradle index 17dc6978a0..160b054c14 100644 --- a/settings.gradle +++ b/settings.gradle @@ -4,6 +4,7 @@ pluginManagement { maven { url 'https://plugins.gradle.org/m2' metadataSources { + // Avoid redirection to defunct JCenter when Gradle module metadata is not published by a plugin (e.g. JMH plugin) ignoreGradleMetadataRedirection() mavenPom() artifact()