X Tutup
Skip to content

Deprecate FieldsContainer on code registry #3211

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.

Sign up for GitHub

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

Merged
bbakerman merged 1 commit into from
May 19, 2023
Merged

Deprecate FieldsContainer on code registry #3211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions src/main/java/graphql/schema/GraphQLCodeRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package graphql.schema;

import graphql.Assert;
import graphql.DeprecatedAt;
import graphql.Internal;
import graphql.PublicApi;
import graphql.schema.visibility.GraphqlFieldVisibility;
Expand Down Expand Up @@ -56,11 +57,29 @@ public GraphqlFieldVisibility getFieldVisibility() {
* @param fieldDefinition the field definition
*
* @return the DataFetcher associated with this field. All fields have data fetchers
*
* @see #getDataFetcher(GraphQLObjectType, GraphQLFieldDefinition)
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
*/
@Deprecated
@DeprecatedAt("2023-05-13")
public DataFetcher<?> getDataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition) {
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
}

/**
* Returns a data fetcher associated with a field within an object type
*
* @param parentType the container type
* @param fieldDefinition the field definition
*
* @return the DataFetcher associated with this field. All fields have data fetchers
*/
public DataFetcher<?> getDataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition) {
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
}

/**
* Returns a data fetcher associated with a field located at specified coordinates.
*
Expand Down Expand Up @@ -242,11 +261,29 @@ private Builder markChanged(boolean condition) {
* @param fieldDefinition the field definition
*
* @return the DataFetcher associated with this field. All fields have data fetchers
*
* @see #getDataFetcher(GraphQLObjectType, GraphQLFieldDefinition)
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
*/
@Deprecated
@DeprecatedAt("2023-05-13")
public DataFetcher<?> getDataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition) {
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
}

/**
* Returns a data fetcher associated with a field within an object type
*
* @param parentType the container type
* @param fieldDefinition the field definition
*
* @return the DataFetcher associated with this field. All fields have data fetchers
*/
public DataFetcher<?> getDataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition) {
return getDataFetcherImpl(FieldCoordinates.coordinates(parentType, fieldDefinition), fieldDefinition, dataFetcherMap, systemDataFetcherMap, defaultDataFetcherFactory);
}

/**
* Returns a data fetcher associated with a field located at specified coordinates.
*
Expand Down Expand Up @@ -331,11 +368,31 @@ public Builder dataFetcher(FieldCoordinates coordinates, DataFetcher<?> dataFetc
* @param dataFetcher the data fetcher code for that field
*
* @return this builder
*
* @see #dataFetcher(GraphQLObjectType, GraphQLFieldDefinition, DataFetcher)
* @deprecated This is confusing because {@link GraphQLInterfaceType}s cant have data fetchers. At runtime only a {@link GraphQLObjectType}
* can be used to fetch a field. This method allows the mapping to be made, but it is never useful if an interface is passed in.
*/
@Deprecated
@DeprecatedAt("2023-05-13")
public Builder dataFetcher(GraphQLFieldsContainer parentType, GraphQLFieldDefinition fieldDefinition, DataFetcher<?> dataFetcher) {
return dataFetcher(FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName()), dataFetcher);
}


/**
* Sets the data fetcher for a specific field inside an object type
*
* @param parentType the object type
* @param fieldDefinition the field definition
* @param dataFetcher the data fetcher code for that field
*
* @return this builder
*/
public Builder dataFetcher(GraphQLObjectType parentType, GraphQLFieldDefinition fieldDefinition, DataFetcher<?> dataFetcher) {
return dataFetcher(FieldCoordinates.coordinates(parentType.getName(), fieldDefinition.getName()), dataFetcher);
}

/**
* Called to place system data fetchers (eg Introspection fields) into the mix
*
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/idl/SchemaDirectiveWiringEnvironmentImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public GraphQLFieldDefinition getFieldDefinition() {
public DataFetcher<?> getFieldDataFetcher() {
assertNotNull(fieldDefinition, () -> "An output field must be in context to call this method");
assertNotNull(fieldsContainer, () -> "An output field container must be in context to call this method");
return codeRegistry.getDataFetcher(fieldsContainer, fieldDefinition);
return codeRegistry.getDataFetcher(FieldCoordinates.coordinates(fieldsContainer, fieldDefinition), fieldDefinition);
}

@Override
Expand Down
9 changes: 7 additions & 2 deletions src/test/groovy/graphql/analysis/values/ValueTraverserTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import graphql.schema.GraphQLInputObjectType
import graphql.schema.GraphQLInputSchemaElement
import graphql.schema.GraphQLList
import graphql.schema.GraphQLNamedSchemaElement
import graphql.schema.GraphQLObjectType
import graphql.schema.GraphQLScalarType
import graphql.schema.idl.SchemaDirectiveWiring
import graphql.schema.idl.SchemaDirectiveWiringEnvironment
Expand Down Expand Up @@ -861,8 +862,12 @@ type Profile {
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> env) {
GraphQLFieldsContainer fieldsContainer = env.getFieldsContainer()
GraphQLFieldDefinition fieldDefinition = env.getFieldDefinition()
if (! fieldsContainer instanceof GraphQLObjectType) {
return fieldDefinition
}
GraphQLObjectType containingObjectType = env.getFieldsContainer() as GraphQLObjectType

final DataFetcher<?> originalDF = env.getCodeRegistry().getDataFetcher(fieldsContainer, fieldDefinition)
final DataFetcher<?> originalDF = env.getCodeRegistry().getDataFetcher(containingObjectType, fieldDefinition)
final DataFetcher<?> newDF = { DataFetchingEnvironment originalEnv ->
ValueVisitor visitor = new ValueVisitor() {
@Override
Expand All @@ -884,7 +889,7 @@ type Profile {
return originalDF.get(newEnv);
}

env.getCodeRegistry().dataFetcher(fieldsContainer, fieldDefinition, newDF)
env.getCodeRegistry().dataFetcher(containingObjectType, fieldDefinition, newDF)

return fieldDefinition
}
Expand Down
16 changes: 13 additions & 3 deletions src/test/groovy/graphql/schema/idl/SchemaGeneratorDirectiveHelperTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,14 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
@Override
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> directiveEnv) {
GraphQLFieldDefinition field = directiveEnv.getElement()
def container = directiveEnv.fieldsContainer
if (!container instanceof GraphQLObjectType) {
return field
}
//
// we use the non shortcut path to the data fetcher here so prove it still works
def fetcher = directiveEnv.getCodeRegistry().getDataFetcher(directiveEnv.fieldsContainer, field)

def fetcher = directiveEnv.getCodeRegistry().getDataFetcher(container as GraphQLObjectType, field)
def newFetcher = wrapDataFetcher(fetcher, { dfEnv, value ->
def directiveName = directiveEnv.appliedDirective.name
if (directiveName == "uppercase") {
Expand Down Expand Up @@ -484,10 +489,15 @@ class SchemaGeneratorDirectiveHelperTest extends Specification {
@Override
GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> environment) {
GraphQLFieldDefinition element = environment.getElement()
return wrapField(environment.fieldsContainer, element, environment.getBuildContext(), environment.getCodeRegistry())

def container = environment.fieldsContainer
if (! container instanceof GraphQLObjectType) {
return element
}
return wrapField(container as GraphQLObjectType, element, environment.getBuildContext(), environment.getCodeRegistry())
}

private GraphQLFieldDefinition wrapField(GraphQLFieldsContainer parentType, GraphQLFieldDefinition field, Map<String, Object> contextMap, GraphQLCodeRegistry.Builder codeRegistry) {
private GraphQLFieldDefinition wrapField(GraphQLObjectType parentType, GraphQLFieldDefinition field, Map<String, Object> contextMap, GraphQLCodeRegistry.Builder codeRegistry) {
def originalFetcher = codeRegistry.getDataFetcher(parentType, field)

String key = mkFieldKey(parentType.getName(), field.getName())
Expand Down
X Tutup