Handle getters of java.util.Record. Upgrade gson to safe version.

[cgenrich]

No description provided.

[kaqqao]

SPQR is still on java 8 (as graphql-java is on Java 8). In the next release, they're upgrading to Java 11, and SPQR will follow.
Since records aren't in java 11 either, this should really be made into a separate Module. Would you be interested in doing that?

[cgenrich]

SPQR is still on java 8 (as graphql-java is on Java 8). In the next release, they're upgrading to Java 11, and SPQR will follow. Since records aren't in java 11 either, this should really be made into a separate Module. Would you be interested in doing that?

This PR will not require a move from Java 8. It will however allow users of these libraries to use higher versions of Java. I've been using this patch with Java 17. I don't see a reason for a new module as this change is fully backwards compatible and requires no changes to the build configuration.

[kaqqao]

Oh, I see... You don't actually check if the class is a record. That would cause backwards incompatible changes to people's schemas (new fields suddenly exposed without changes in the configuration). So ideally, this would be turned into its own ResolverBuilder (akin to the current BeanResolverBuilder), that can then be automatically enabled for actual Record types, and op-in for everything else. If you would, I'd kindly ask you to create the new ResolverBuilder and I'll worry about the rest. Would you be up for it?
If not, I'll take care of it myself.

[cgenrich]

Oh, I see... You don't actually check if the class is a record. That would cause backwards incompatible changes to people's schemas (new fields suddenly exposed without changes in the configuration). So ideally, this would be turned into its own ResolverBuilder (akin to the current BeanResolverBuilder), that can then be automatically enabled for actual Record types, and op-in for everything else. If you would, I'd kindly ask you to create the new ResolverBuilder and I'll worry about the rest. Would you be up for it? If not, I'll take care of it myself.

I'll take a look and let you know. Thank you for considering the change.

[kaqqao]

Thank you for contributing!

[cgenrich]

Oh, I see... You don't actually check if the class is a record. That would cause backwards incompatible changes to people's schemas (new fields suddenly exposed without changes in the configuration). So ideally, this would be turned into its own ResolverBuilder (akin to the current BeanResolverBuilder), that can then be automatically enabled for actual Record types, and op-in for everything else. If you would, I'd kindly ask you to create the new ResolverBuilder and I'll worry about the rest. Would you be up for it? If not, I'll take care of it myself.

@kaqqao I started working on this but am having trouble replicating the issue of a method suddenly appearing in the schema. Could you recommend a Unit test? I created two ResolverBuilders and reverted changes to ClassUtils but want to ensure I haven't missed anything:

package io.leangen.graphql.metadata.strategy.query;

import java.lang.reflect.Method;

/**
 * Checks for record like behavior of getter methods.
 *
 */
public interface RecordLikeGetter {
    default boolean isGetter(Method method) {
        try {
            return method.getParameterCount() == 0
                    && method.getDeclaringClass().getDeclaredField(method.getName())
                    .getType().equals(method.getReturnType());
        } catch (NoSuchFieldException | SecurityException e) {
            return false;
        }
    }
}

package io.leangen.graphql.metadata.strategy.query;

import java.lang.reflect.Method;

import io.leangen.graphql.util.ClassUtils;

/**
 * Support classes that behave like Java 14 records.
 *
 */
public class RecordLikeResolverBuilder extends PublicResolverBuilder implements RecordLikeGetter {
    public RecordLikeResolverBuilder(String... basePackages) {
        super(basePackages);
        operationInfoGenerator = new PropertyOperationInfoGenerator();
    }

    @Override
    protected boolean isQuery(Method method, ResolverBuilderParams params) {
        return super.isQuery(method, params) && isGetter(method);
    }

    @Override
    protected boolean isMutation(Method method, ResolverBuilderParams params) {
        return super.isMutation(method, params) && ClassUtils.isSetter(method);
    }
}

package io.leangen.graphql.metadata.strategy.query;

import java.lang.reflect.Method;

import io.leangen.graphql.util.ClassUtils;

public class BeanRecordResolverBuilder extends BeanResolverBuilder implements RecordLikeGetter {
    public BeanRecordResolverBuilder(String... basePackages) {
        super(basePackages);
    }

    @Override
    protected boolean isQuery(Method method, ResolverBuilderParams params) {
        return super.isQuery(method, params) && (ClassUtils.isGetter(method) || isGetter(method));
    }

    @Override
    protected boolean isMutation(Method method, ResolverBuilderParams params) {
        return super.isMutation(method, params) && ClassUtils.isSetter(method);
    }
}

[kaqqao]

I've just added 2 new ResolverBuilders:

• RecordResolverBuilder, which supports records only, and is registered by default
• RecordLikeResolverBuilder, which work on any record-like type, but must be registered explicitly

See #453 for more details.

I'm really sorry for not merging your PR, but I think dedicated builders are the way to go here, as that ensures full backwards compatibility.