In 1.1.0, passing custom scalars or GraphQLEnum to mocks fails

[vincentisambart]

Summary

In Apollo iOS 1.1.0, when you pass enum or custom scalar values to create test mocks, you get a JSON decoding error:

ApolloTestSupport/TestMock.swift:93: Fatal error: 'try!' expression unexpectedly raised an error: Apollo.GraphQLExecutionError(path: topFrameContent.align, underlying: ApolloAPI.JSONDecodingError.couldNotConvert(value: AnyHashable(ApolloAPI.GraphQLEnum<Api4.BannerAlign>.case(Api4.BannerAlign.center)), to: Swift.String))

For custom scalars, I could work around this problem by modifying their init(_jsonValue value: JSONValue) throws to allow value being of their own type(if let myCustomScalar = value as? MyCustomScalar), but I cannot do that to ApolloAPI.GraphQLEnum.

At first thought it was me incorrectly use the Apollo iOS API, but the convenience init is expecting concrete values of the custom scalars and GraphQLEnum, so I looks like a bug to me.

Version

1.1.0

Steps to reproduce the behavior

You need a type that has fields with enums and/or custom scalars.

enum Alignment {
  LEFT
  CENTER
  RIGHT
} 
type Banner {
  alignment: Alignment
}

And in Swift the following test code will work on Apollo iOS 1.0 but will end up with a JSONDecodingError in 1.1.

let mock = Mock<Banner>(alignment: GraphQLEnum(Banner.center))

Sorry I don't have the time to make a full reproduction project so the code above was not tested, but should fail seeing the behavior on our project.

Logs

No response

Anything else?

No response

[AnthonyMDev]

Thanks so much for the report! I'll look into this and get a fix out!

[AnthonyMDev]

I've got a unit test reproducing this now and I'm working on a fix!

[vincentisambart]

Thank you for the quick fix!

[alexjameslittle]

We are still having this issue on the latest version 1.1.2.

couldNotConvert(value: AnyHashable(ApolloAPI.GraphQLEnum<ProfileFriendStatus>.case(ProfileFriendStatus.none)), to: Swift.String)

[RussellToon]

And I still seem to have this issue in 1.1.3

[calvincestari]

@alexjameslittle, @RussellToon - it would help if either of you are able to provide a sample application that demonstrates the issue you're still experiencing.

[alexjameslittle]

I'm not sure I have the project available to provide an example anymore, however we added this based on some inspiration from this repo and it solved our issues. We now no longer have these issues

@testable import Apollo
@testable import ApolloAPI
@testable import ApolloTestSupport

public extension RootSelectionSet {
    static func from(query: Mock<Query>) throws -> Self {
        try .init(data: query._selectionSetMockData)
    }

    static func from(subscription: Mock<Subscription>) throws -> Self {
        try .init(data: subscription._selectionSetMockData)
    }

    static func from(mutation: Mock<Mutation>) throws -> Self {
        try .init(data: mutation._selectionSetMockData)
    }
}

public extension RootSelectionSet {
    init(
        data: JSONObject,
        variables: GraphQLOperation.Variables? = nil
    ) throws {
        let accumulator = TestMockSelectionSetMapper<Self>()
        let executor = GraphQLExecutor { object, info in
            object[info.responseKeyForField]
        }
        executor.shouldComputeCachePath = false

        self = try executor.execute(
            selectionSet: Self.self,
            on: data,
            variables: variables,
            accumulator: accumulator
        )
    }
}

[AnthonyMDev]

Thanks for the code example! I'm not sure what problem you are having that this code resolves for you, since that is basically what we are doing already in the function we have for this.

FWIW, the GraphQLExecutor was changed significantly in 1.2.0. Curious to know if upgrading to 1.2.0 resolves these crashes for you. (Though 1.2 currently has a few other known bugs we're working on resolving that may prevent you from upgrading at the moment in some situations)

[alexjameslittle]

I just managed to recreate it again on our current version 1.1.2. I haven't had the chance to test on any newer version yet as it's a large company project.

This is the root selection set extension that gets called in our test if we don't use the extension i linked:

extension RootSelectionSet {

  /// Initializes a `SelectionSet` with a raw JSON response object.
  ///
  /// The process of converting a JSON response into `SelectionSetData` is done by using a
  /// `GraphQLExecutor` with a`GraphQLSelectionSetMapper` to parse, validate, and transform
  /// the JSON response data into the format expected by `SelectionSet`.
  ///
  /// - Parameters:
  ///   - data: A dictionary representing a JSON response object for a GraphQL object.
  ///   - variables: [Optional] The operation variables that would be used to obtain
  ///                the given JSON response data.
  public init(
    data: JSONObject,
    variables: GraphQLOperation.Variables? = nil
  ) throws {
    let accumulator = GraphQLSelectionSetMapper<Self>(
      handleMissingValues: .allowForOptionalFields
    )
    let executor = GraphQLExecutor { object, info in
      return object[info.responseKeyForField]
    }
    executor.shouldComputeCachePath = false

    self = try executor.execute(
      selectionSet: Self.self,
      on: data,
      variables: variables,
      accumulator: accumulator
    )
  }

}

The obvious difference being ours uses the Apollo TestMockSelectionSetMapper

[AnthonyMDev]

What does your call site look like in your test? The from(_ mock:withVariables:) function is present in 1.1.1 and on.

[alexjameslittle]

Now that you mention it, it looks like it's some convenience extensions of ours that have confused things and directly initialised the Data instead of using from(_ mock:withVariables:).

This was our own implementation issue, sorry!

[AnthonyMDev]

Ah gotcha, that makes sense! In order to do that with the mocks, you must have been accessing the _selectionSetMockData property. Underscore prefixed properties are meant for internal use only for this very reason. :) You can use them if you want to, but they allow you to make more bugs for yourself. If you try to avoid using our internal functions, you wouldn't be able to do this, and would be forced to use the correct function.

Glad you resolved this issue!