If a type's name is overridden, then TypeReference behaves oddly. about graphiti HOT 8 CLOSED

@paulofaria, is this an easy fix? I'd be happy to work on it if you can point me in the right direction.

from graphiti.

Hey, @cshadek. This is indeed a bug. I'll work on it on the next release. I don't have any indication of when that might happen, though. Please send a PR if you want this fixed sooner rather than later.

from graphiti.

I came across this issue myself today.
I dig into the code and I couldn't find a solution without touching the underlying GraphQL package too.
So I created a fork each. The Graphiti fork depends on my GraphQL fork (referenced in the Package.swift file.

If we want to keep the GraphQL package untouched, I think we need some more heavy refactoring:
In FileProvider.swift the GraphQLTypeReference is created with a reflection (Reflection.name(for: type.wrappedType)), not with the typename itself. But as we pass only the Type.Type into that function, I don't have an idea to access that alias property.

Any additional insights or better ways are welcome.

from graphiti.

Oof, yeah this is a tough one. @alexsteinerde, you're right - the crux of the issue is TypeProvider.swift L53, where we map a Graphiti TypeReference to a GraphQL GraphQLTypeReference using only the name of the type itself (as opposed to the "mapped" name as specified in a type declaration)... This code is executed incrementally as the Graphiti schema is built, so unfortunately we cannot inspect the TypeProvider to find the declared name, because it might not have been declared yet! Ugh.

The GraphQL package does a type-reference reduction operation here after the entire schema has been declared. But by this time, we've long since lost the separation between the Swift name and the declared name.

I see 2 potential approaches:

1. Change Graphiti.Schema to have its own type-reference resolution process, prior to creating GraphQLType objects.
2. Change GraphQL.GraphQLTypeReference to have 2 string fields (maybe swiftName and graphQLName or something), in which either are used to match it up to another type during the GraphQL type reduction process.

from graphiti.

I can confirm that this is also an issue when using Optional types (not only TypeReferences).

For example, the following code fails on the optional LocationObject? because it was aliased to Location in GraphQL:

func testOptionalOfNamedType() throws {
    struct LocationObject: Codable {
        let id: String
        let name: String
    }

    struct User: Codable {
        let id: String
        let location: LocationObject?
    }

    struct TestResolver {
        func user(context _: NoContext, arguments _: NoArguments) -> User {
            return User(
                id: "user1",
                location: LocationObject(
                    id: "location1",
                    name: "Earth"
                )
            )
        }
    }

    let testSchema = try Schema<TestResolver, NoContext> {
        Type(User.self) {
            Field("id", at: \.id)
            Field("location", at: \.location)
        }
        Type(LocationObject.self, as: "Location") {
            Field("id", at: \.id)
            Field("name", at: \.name)
        }
        Query {
            Field("user", at: TestResolver.user)
        }
    }
    let api = TestAPI<TestResolver, NoContext>(
        resolver: TestResolver(),
        schema: testSchema
    )

    let group = MultiThreadedEventLoopGroup(numberOfThreads: System.coreCount)
    defer { try? group.syncShutdownGracefully() }

    XCTAssertEqual(
        try api.execute(
            request: """
            query {
              user {
                location {
                  name
                }
              }
            }
            """,
            context: NoContext(),
            on: group
        ).wait(),
        GraphQLResult(data: [
            "user": [
                "location": [
                    "name": "Earth",
                ],
            ],
        ])
    )
}

It returns:

XCTAssertEqual failed: ("{"data":{"user":{"location":null}},"errors":[{"message":"Cannot complete value of unexpected type \"LocationObject\".","path":[]}]}") is not equal to ("{"data":{"user":{"location":{"name":"Earth"}}}}")

from graphiti.

Thanks for confirming the issue and detecting a related one.

1. Change Graphiti.Schema to have its own type-reference resolution process, prior to creating GraphQLType objects.

I don't know how much work it would be to write a second resolution process. But from an outsiders perspective I would question if this wouldn't be code/work duplication.

2. Change GraphQL.GraphQLTypeReference to have 2 string fields (maybe swiftName and graphQLName or something), in which either are used to match it up to another type during the GraphQL type reduction process.

That's what I did in my forks. Except that I did use a different naming.
This seems to be the only solution I have in mind at the moment. But personally it feels a bit off to make changes to the GraphQL package because of an implementation detail issue in a higher package.

from graphiti.

@alexsteinerde I took a look at your forks - thanks for referencing them! I'm agree that we should probably not require GraphQL changes in order to support the Graphiti feature of aliasing types.

I pushed an alternative fix that uses approach 1, and created an MR here: #90
I'd love it if you could review! Thanks again!

from graphiti.

Thanks @NeedleInAJayStack! Looks good for me.
I tested it in a local project and it works fine.

from graphiti.