Skip to content

SchemaMappingInspector field resolution should check public fields to match PropertyFetchingImpl #1101

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

Closed
meostyles opened this issue Dec 16, 2024 · 0 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@meostyles
Copy link

meostyles commented Dec 16, 2024

To generate the SchemaReport, the SchemaMappingGenerator uses BeanUtils#getPropertyDescriptor, which internally only caches those fields with public getters in the propertyDescriptors collection

@Nullable
PropertyDescriptor getPropertyDescriptor(String name) {
    PropertyDescriptor pd = (PropertyDescriptor)this.propertyDescriptors.get(name);
    if (pd == null && StringUtils.hasLength(name)) {
        pd = (PropertyDescriptor)this.propertyDescriptors.get(StringUtils.uncapitalize(name));
        if (pd == null) {
            pd = (PropertyDescriptor)this.propertyDescriptors.get(StringUtils.capitalize(name));
        }
    }

    return pd;
}

This does not match the PropertyFetchingImpl, which is the default responsible for resolving the field and fetching the data from it

private Object getPropertyViaFieldAccess(CacheKey cacheKey, Object object, String propertyName) throws FastNoSuchMethodException {
    Class<?> aClass = object.getClass();
    try {
        Field field = aClass.getField(propertyName);
        FIELD_CACHE.putIfAbsent(cacheKey, field);
        return field.get(object);
    } catch (NoSuchFieldException e) {
        if (!USE_SET_ACCESSIBLE.get()) {
            throw new FastNoSuchMethodException(cacheKey.toString());
        }
        // if not public fields then try via setAccessible
        try {
            Field field = aClass.getDeclaredField(propertyName);
            field.setAccessible(true);
            FIELD_CACHE.putIfAbsent(cacheKey, field);
            return field.get(object);
        } catch (SecurityException | NoSuchFieldException ignored2) {
            throw new FastNoSuchMethodException(cacheKey.toString());
        } catch (IllegalAccessException e1) {
            throw new GraphQLException(e);
        }
    } catch (IllegalAccessException e) {
        throw new GraphQLException(e);
    }
}

How does this affect us?

We have a number of fields that are public final fields, but do not have public getters. Spring for GraphQL resolves the data, but the schema report warns that the fields are unmapped, which is untrue. We hook into the schema report and throw an exception if there are any unmapped fields by using GraphQlSourceBuilderCustomizer#inspectSchemaMappings. It is important for us to ensure that changes to our data model do not silently break our GraphQL API without us realising.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 16, 2024
@rstoyanchev rstoyanchev self-assigned this Jan 30, 2025
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 30, 2025
@rstoyanchev rstoyanchev added this to the 1.4.0-M1 milestone Jan 30, 2025
@rstoyanchev rstoyanchev changed the title SchemaMappingInspector field resolution should match PropertyFetchingImpl SchemaMappingInspector field resolution should check fields to match PropertyFetchingImpl Jan 30, 2025
@rstoyanchev rstoyanchev changed the title SchemaMappingInspector field resolution should check fields to match PropertyFetchingImpl SchemaMappingInspector field resolution should check public fields to match PropertyFetchingImpl Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants