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

Building a GraphQL response can fail when the JVM is in the Turkish Locale and a field starts with an i #3385

Closed
David-Whitehouse-i2 opened this issue Nov 30, 2023 · 5 comments
Milestone

Comments

@David-Whitehouse-i2
Copy link

David-Whitehouse-i2 commented Nov 30, 2023

In our product we were asked to support Turkish on our application server, but graphql-java fails to build a response for any private field with accessors that starts with an i that is inherited from a base class, when in the Turkish locale.

The client received lots of error containing messages such as:

The field at path '/blah/id' was declared as a non null type, but the code involved in retrieving data has wrongly returned a null value.  The graphql specification requires that the parent field be set to null, or if that is non nullable that it bubble up null to its parent and so on. The non-nullable type is 'BaseClass' within parent type 'ParentClass'"

I wrote a new test extending the test class PropertyDataFetcherTest.groovy. You should be able to just append the following to the bottom of that class.

    class BaseObject {
        private String id

        String getId() {
            return id
        }

        void setId(String value) {
            id = value;
        }
    }

    class OtherObject extends BaseObject {}

    def "Can access private property from base class that starts with i in Turkish"() {
        given:
        Locale oldLocale = Locale.getDefault()
        Locale.setDefault(new Locale("tr", "TR"))

        def environment = env(new OtherObject(id: "aValue"))
        def fetcher = PropertyDataFetcher.fetching("id")

        when:
        String propValue = fetcher.get(environment)

        then:
        propValue == 'aValue'

        cleanup:
        Locale.setDefault(oldLocale)
    }

The weird class inheritance was to simulate the equivalent java structure we had and when debugged it appears to follow the same failure path we see in PropertyFetchingImpl::getPropertyViaFieldAccess.

Possible fix

One offending line of code looks to be at PropertyFetchingImpl.java#L216

This code normally converts our property id to the method name getId but when in the Turkish locale it becomes getİd. This obviously fails, and graphql-java's further attempts to figure out a way of accessing this field also fail in our case.

I've tested replacing that line of code with the following (plus the Locale import):

String getterName = prefix + propertyName.substring(0, 1).toUpperCase(Locale.ROOT) + propertyName.substring(1);

and that works in our setup. Locale.ROOT is the suggested fix from the java docs here.

@dondonz
Copy link
Member

dondonz commented Dec 1, 2023

Thanks for reporting this and writing a test. As another data point, which version of GraphQL Java are you running?

@David-Whitehouse-i2
Copy link
Author

Thanks for the quick response! We're currently on 19.6. We're looking into upgrading to latest (21.3), but need to figure out compatibility with graphql-java-kickstart, whose latest is compiled against 19.x. Our next release is 6 months away though.

@dondonz
Copy link
Member

dondonz commented Dec 4, 2023

Perhaps you've already thought about this, have you considered using Spring for GraphQL as an alternative to kickstart? It's the official Spring integration and it's actively maintained, they regularly update to the latest version of GraphQL Java. It would mean you always have the latest fixes.

I'm not sure how the kickstart project is going with releases.

@David-Whitehouse-i2
Copy link
Author

Using Spring is excellent advice, and my aim is to migrate to it as soon as possible. Unfortunately our service is just part of a monolithic app server, so structural changes like that are tricky.

@dondonz dondonz added this to the 2024 April: breaking changes milestone Jan 3, 2024
@dondonz
Copy link
Member

dondonz commented Feb 11, 2024

Fixed by #3388

@dondonz dondonz closed this as completed Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants