Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LegacyCoercing support #3218

Merged
merged 3 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package graphql.execution.values.legacycoercing;

import graphql.GraphQLContext;
import graphql.Scalars;
import graphql.execution.values.InputInterceptor;
import graphql.scalar.CoercingUtil;
import graphql.schema.GraphQLInputType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.math.BigDecimal;
import java.util.Locale;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;

import static graphql.Assert.assertNotNull;
import static graphql.scalar.CoercingUtil.isNumberIsh;

public class LegacyCoercingInputInterceptor implements InputInterceptor {

/**
* This will ONLY observe legacy values and invoke the callback when it gets one. you can use this to enumerate how many
* legacy values are hitting you graphql implementation
*
* @param observerCallback a callback allowing you to observe a legacy scalar value
*
* @return an InputInterceptor that only observes values
*/
public static LegacyCoercingInputInterceptor observesValues(BiConsumer<Object, GraphQLInputType> observerCallback) {
return new LegacyCoercingInputInterceptor(((input, graphQLInputType) -> {
observerCallback.accept(input, graphQLInputType);
return input;
}));
}

/**
* This will change legacy values as it encounters them to something acceptable to the more strict coercion rules.
*
* @return an InputInterceptor that migrates values to a more strict value
*/
public static LegacyCoercingInputInterceptor migratesValues() {
return migratesValues((input, type) -> {
});
}

/**
* This will change legacy values as it encounters them to something acceptable to the more strict coercion rules.
* The observer callback will be invoked if it detects a legacy value that it will change.
*
* @param observerCallback a callback allowing you to observe a legacy scalar value before it is migrated
*
* @return an InputInterceptor that both observes values and migrates them to a more strict value
*/
public static LegacyCoercingInputInterceptor migratesValues(BiConsumer<Object, GraphQLInputType> observerCallback) {
return new LegacyCoercingInputInterceptor(((input, graphQLInputType) -> {
observerCallback.accept(input, graphQLInputType);
if (Scalars.GraphQLBoolean.equals(graphQLInputType)) {
return coerceLegacyBooleanValue(input);
}
if (Scalars.GraphQLFloat.equals(graphQLInputType)) {
return coerceLegacyFloatValue(input);
}
if (Scalars.GraphQLInt.equals(graphQLInputType)) {
return coerceLegacyIntValue(input);
}
if (Scalars.GraphQLString.equals(graphQLInputType)) {
return coerceLegacyStringValue(input);
}
return input;
}));
}

private final BiFunction<Object, GraphQLInputType, Object> behavior;

private LegacyCoercingInputInterceptor(BiFunction<Object, GraphQLInputType, Object> behavior) {
this.behavior = assertNotNull(behavior);
}

@Override
public Object intercept(@Nullable Object input, @NotNull GraphQLInputType graphQLType, @NotNull GraphQLContext graphqlContext, @NotNull Locale locale) {
if (isLegacyValue(input, graphQLType)) {
// we ONLY apply the new behavior IF it's an old acceptable legacy value.
// so for compliant values - we change nothing and invoke no behaviour
// and for values that would not coerce anyway, we also invoke no behavior
return behavior.apply(input, graphQLType);
}
return input;
}

@SuppressWarnings("RedundantIfStatement")
static boolean isLegacyValue(Object input, GraphQLInputType graphQLType) {
if (Scalars.GraphQLBoolean.equals(graphQLType)) {
return isLegacyBooleanValue(input);
} else if (Scalars.GraphQLFloat.equals(graphQLType)) {
return isLegacyFloatValue(input);
} else if (Scalars.GraphQLInt.equals(graphQLType)) {
return isLegacyIntValue(input);
} else if (Scalars.GraphQLString.equals(graphQLType)) {
return isLegacyStringValue(input);
} else {
return false;
}
}

static boolean isLegacyBooleanValue(Object input) {
return input instanceof String || CoercingUtil.isNumberIsh(input);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR should be merged as is. One teeny nit in case someone is reading this PR in the future

On naming, technically this method is "is maybe legacy boolean", because only the strings "true" and "false" are acceptable legacy string values.

Retaining this function is perfectly fine as the usual boolean coercion that happens after this interceptor will catch strings that are not either "true" or "false"

}

static boolean isLegacyFloatValue(Object input) {
return input instanceof String;
}

static boolean isLegacyIntValue(Object input) {
return input instanceof String;
}

static boolean isLegacyStringValue(Object input) {
return !(input instanceof String);
}

static Object coerceLegacyBooleanValue(Object input) {
if (input instanceof String) {
String lStr = ((String) input).toLowerCase();
if (lStr.equals("true")) {
return true;
}
if (lStr.equals("false")) {
return false;
}
return input;
} else if (isNumberIsh(input)) {
BigDecimal value;
try {
value = new BigDecimal(input.toString());
} catch (NumberFormatException e) {
// this should never happen because String is handled above
return input;
}
return value.compareTo(BigDecimal.ZERO) != 0;
}
// unchanged
return input;
}

static Object coerceLegacyFloatValue(Object input) {
if (isNumberIsh(input)) {
BigDecimal value;
try {
value = new BigDecimal(input.toString());
} catch (NumberFormatException e) {
return input;
}
return value.doubleValue();
}
return input;
}

static Object coerceLegacyIntValue(Object input) {
if (isNumberIsh(input)) {
BigDecimal value;
try {
value = new BigDecimal(input.toString());
} catch (NumberFormatException e) {
return input;
}
try {
return value.intValueExact();
} catch (ArithmeticException e) {
return input;
}
}
return input;
}


static Object coerceLegacyStringValue(Object input) {
return String.valueOf(input);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,201 @@
package graphql.execution.values.legacycoercing

import graphql.GraphQLContext
import graphql.schema.GraphQLInputType
import spock.lang.Specification

import java.util.function.BiConsumer

import static graphql.Scalars.GraphQLBoolean
import static graphql.Scalars.GraphQLFloat
import static graphql.Scalars.GraphQLInt
import static graphql.Scalars.GraphQLString

class LegacyCoercingInputInterceptorTest extends Specification {

def "can detect legacy boolean values"() {
when:
def isLegacyValue = LegacyCoercingInputInterceptor.isLegacyValue(input, inputType)
then:
isLegacyValue == expected

where:
input | inputType | expected
"true" | GraphQLBoolean | true
"false" | GraphQLBoolean | true
"TRUE" | GraphQLBoolean | true
"FALSE" | GraphQLBoolean | true
"junk" | GraphQLBoolean | true
// not acceptable to the old
true | GraphQLBoolean | false
false | GraphQLBoolean | false
["rubbish"] | GraphQLBoolean | false
}

def "can change legacy boolean values"() {
def interceptor = LegacyCoercingInputInterceptor.migratesValues()
when:
def value = interceptor.intercept(input, inputType, GraphQLContext.getDefault(), Locale.getDefault())
then:
value == expected

where:
input | inputType | expected
"true" | GraphQLBoolean | true
"false" | GraphQLBoolean | false
"TRUE" | GraphQLBoolean | true
"FALSE" | GraphQLBoolean | false

// left alone
"junk" | GraphQLBoolean | "junk"
true | GraphQLBoolean | true
false | GraphQLBoolean | false
["rubbish"] | GraphQLBoolean | ["rubbish"]
}

def "can detect legacy float values"() {
when:
def isLegacyValue = LegacyCoercingInputInterceptor.isLegacyValue(input, inputType)
then:
isLegacyValue == expected

where:
input | inputType | expected
"1.0" | GraphQLFloat | true
"1" | GraphQLFloat | true
"junk" | GraphQLFloat | true
// not acceptable to the old
666.0F | GraphQLFloat | false
666 | GraphQLFloat | false
["rubbish"] | GraphQLFloat | false
}

def "can change legacy float values"() {
def interceptor = LegacyCoercingInputInterceptor.migratesValues()
when:
def value = interceptor.intercept(input, inputType, GraphQLContext.getDefault(), Locale.getDefault())
then:
value == expected

where:
input | inputType | expected
"1.0" | GraphQLFloat | 1.0F
"1" | GraphQLFloat | 1.0F

// left alone
"junk" | GraphQLFloat | "junk"
666.0F | GraphQLFloat | 666.0F
666 | GraphQLFloat | 666
["rubbish"] | GraphQLFloat | ["rubbish"]
}

def "can detect legacy int values"() {
when:
def isLegacyValue = LegacyCoercingInputInterceptor.isLegacyValue(input, inputType)
then:
isLegacyValue == expected

where:
input | inputType | expected
"1.0" | GraphQLInt | true
"1" | GraphQLInt | true
"junk" | GraphQLInt | true
// not acceptable to the old
666.0F | GraphQLInt | false
666 | GraphQLInt | false
["rubbish"] | GraphQLInt | false
}

def "can change legacy int values"() {
def interceptor = LegacyCoercingInputInterceptor.migratesValues()
when:
def value = interceptor.intercept(input, inputType, GraphQLContext.getDefault(), Locale.getDefault())
then:
value == expected

where:
input | inputType | expected
"1.0" | GraphQLInt | 1
"1" | GraphQLInt | 1

// left alone
"junk" | GraphQLInt | "junk"
666.0F | GraphQLInt | 666.0F
666 | GraphQLInt | 666
["rubbish"] | GraphQLInt | ["rubbish"]
}

def "can detect legacy String values"() {
when:
def isLegacyValue = LegacyCoercingInputInterceptor.isLegacyValue(input, inputType)
then:
isLegacyValue == expected

where:
input | inputType | expected
666.0F | GraphQLString | true
666 | GraphQLString | true
["rubbish"] | GraphQLString | true

// strings that are strings dont need to change
"xyz" | GraphQLString | false
"abc" | GraphQLString | false
"junk" | GraphQLString | false

}

def "can change legacy String values"() {
def interceptor = LegacyCoercingInputInterceptor.migratesValues()
when:
def value = interceptor.intercept(input, inputType, GraphQLContext.getDefault(), Locale.getDefault())
then:
value == expected
where:
// its just String.valueOf()
input | inputType | expected
"xyz" | GraphQLString | "xyz"
"abc" | GraphQLString | "abc"
"junk" | GraphQLString | "junk"
666.0F | GraphQLString | "666.0"
666 | GraphQLString | "666"
["rubbish"] | GraphQLString | "[rubbish]"
}

def "can observe values "() {
def lastValue = null
def lastType = null

def callback = new BiConsumer<Object, GraphQLInputType>() {
@Override
void accept(Object o, GraphQLInputType graphQLInputType) {
lastValue = o
lastType = graphQLInputType
}
}
def interceptor = LegacyCoercingInputInterceptor.observesValues(callback)
when:
lastValue = null
lastType = null
def value = interceptor.intercept(input, inputType, GraphQLContext.getDefault(), Locale.getDefault())

then:
// nothing changes - it observes only
value == input
lastValue == expectedLastValue
lastType == expectedLastType

where:
input | inputType | expectedLastValue | expectedLastType
"true" | GraphQLBoolean | "true" | GraphQLBoolean
"1.0" | GraphQLFloat | "1.0" | GraphQLFloat
"1" | GraphQLInt | "1" | GraphQLInt
1 | GraphQLString | 1 | GraphQLString

// no observation if its not needed
true | GraphQLBoolean | null | null
1.0F | GraphQLFloat | null | null
1 | GraphQLInt | null | null
"x" | GraphQLString | null | null

}
}