-
Notifications
You must be signed in to change notification settings - Fork 348
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
Apollo Uploads don't work correctly #518
Comments
File upload is not really part of the GraphQL spec so that was not something we were looking at. That being said, it should be possible to support it through the schema generator hooks and custom data fetcher. Can you provide sample project with the test setup? |
I currently don't have time to create a full example project but I manage to fix the issue with a custom data fetcher. Since class ShortCircuitObjectMapper : ObjectMapper() {
@Suppress("UNCHECKED_CAST")
override fun <T : Any?> convertValue(fromValue: Any?, toValueType: Class<T>?): T {
if (fromValue?.javaClass == toValueType) {
return fromValue as T
}
return super.convertValue(fromValue, toValueType)
}
} @Component
class DataFetcherFactoryProvider(@Autowired private val hooks: SchemaGeneratorHooks) : KotlinDataFetcherFactoryProvider(hooks) {
private val shortCircuitObjectMapper = ShortCircuitObjectMapper()
override fun functionDataFetcherFactory(target: Any?, kFunction: KFunction<*>): DataFetcherFactory<Any> =
DataFetcherFactories.useDataFetcher(
FunctionDataFetcher(
target = target,
fn = kFunction,
objectMapper = shortCircuitObjectMapper,
executionPredicate = hooks.dataFetcherExecutionPredicate))
} The JavaDoc of
But that is misleading. The short-circuit mechanism I implemented in the fix has been removed, see FasterXML/jackson-databind#2220 The caller is now responsible for checking if conversion is actually needed. I would suggest to inlcude such a check in if (argument.javaClass == klazz) {
return argument
} |
Another caveat: if the declared parameter is of type |
@erikhofer with #525 default data fetcher will correctly use proper jackson object mapper from the context. |
Resolved in #525 |
@erikhofer Do you have an example of the implementation you have done to manage the sending of files using this Kotlin library? |
@gonzalt03 It's not a small example but you can look at the project here https://github.com/codefreak/codefreak/tree/master/src/main/kotlin/org/codefreak/codefreak/graphql |
I had problems with ObjectMapper trying to serialize ApplicationPart (uploaded file). This is how I did it ( I intercept the import com.coxautodev.graphql.tools.SchemaParserOptions;
import com.fasterxml.jackson.databind.JavaType;
import com.fasterxml.jackson.databind.ObjectMapper;
import graphql.schema.*;
import graphql.servlet.core.ApolloScalars;
import graphql.servlet.core.DefaultObjectMapperConfigurer;
import org.apache.catalina.core.ApplicationPart;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
@Configuration
public class GraphQLConfig {
@Bean
public GraphQLScalarType uploadScalar() {
return ApolloScalars.Upload;
}
@Bean
public SchemaParserOptions schemaParserOptions() {
var defaultObjectMapperConfigurer = new DefaultObjectMapperConfigurer();
return SchemaParserOptions.newOptions()
.objectMapperProvider(fieldDefinition -> {
var mapper = new ObjectMapper() {
@Override
protected Object _convert(Object fromValue, JavaType toValueType) throws IllegalArgumentException {
if (toValueType.getRawClass().equals(ApplicationPart.class)) {
return fromValue;
}
if (toValueType.hasContentType()
&& toValueType.getContentType().getRawClass().equals(ApplicationPart.class)) {
return fromValue;
}
return super._convert(fromValue, toValueType);
}
};
defaultObjectMapperConfigurer.configure(mapper);
return mapper;
})
.build();
}
} This is what I have in the graphql Mutation scalar Upload
type Mutation {
saveOrder(attachments: [Upload!]!, order: OrderInput!): Order
} In the service: import com.coxautodev.graphql.tools.GraphQLMutationResolver;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
@Service
public class OrderService implements GraphQLMutationResolver {
@Transactional
public Order saveOrder(List<ApplicationPart> attachments, @NotNull Order order) {
//
}
} my build.gradle: dependencies {
implementation("com.graphql-java-kickstart:graphql-spring-boot-starter:5.10.0")
} |
Library Version
1.4.2
Describe the bug
Im using graphql-kotlin to build the schema for graphql-java (because I can't switch to WebFlux at the moment). I'd like to use Apollo uploads but they don't quite work. The schema is created correctly but the datafetcher throws at runtime.
Stack trace
The object passed to
FunctionDataFetcher.convertParameterValue()
already is of typeApplicationPart
. It is passed to the jackson object mapper anyway. The object mapper then fails to deserialize the object.graphql-kotlin/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/execution/FunctionDataFetcher.kt
Lines 86 to 93 in bd530a8
Code changed in master but has the same behavior:
graphql-kotlin/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/execution/FunctionDataFetcher.kt
Lines 74 to 80 in 2259b9d
To Reproduce
Provide
graphql.servlet.core.ApolloScalars.Upload
as a ScalarType with Java typeorg.apache.catalina.core.ApplicationPart
viaSchemaGeneratorHooks
. The correct Java type should bejavax.servlet.http.Part
but interfaces cannot be used as parameters. (Side question: why are interfaces forbidden for scalars?)Create a mutation function that takes an
ApplicationPart
as a parameter.Call the mutation, see https://github.com/jaydenseric/apollo-upload-examples
Expected behavior
The
Part
object should not be deserialized but passed to the function directly. Maybe skip mapping if actual and desired type are the same?The text was updated successfully, but these errors were encountered: