Skip to content
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

Generate both interface and type from abstract class #220

Closed
StarF666 opened this issue May 9, 2019 · 6 comments · Fixed by #221
Closed

Generate both interface and type from abstract class #220

StarF666 opened this issue May 9, 2019 · 6 comments · Fixed by #221
Labels
type: enhancement New feature or request

Comments

@StarF666
Copy link

StarF666 commented May 9, 2019

I'm currently still playing around with graphql-kotlin. Biggest "issue" currently for me is that I'm using abstract classes in my model to implement base classes for some entity types (using hibernate to access database). When using the generator this generates a schema where I cannot query for a base type and then use the "...on SomeType" syntax to handle the subclasses. I would have to write interfaces mirroring the abstract classes properties which would mean repeating my code.

I thought about changing the generator so it will generate a type AND interface from an abstract kotlin class. But I was not able to dig deep enough into the generators code to archieve this. It is easy to generate an interface instead of a type but not two (interface and type).

I don't know if this would break any rules or conventions of GraphQL. I really like what I discovered so far but GraphQL seems to be a bit limited with inheritance. I could manually write the interfaces but that would mean double source of truth.

Maybe I'm missing something but would the solution of generating both interface and type be a feasible solution?

@StarF666 StarF666 added the type: enhancement New feature or request label May 9, 2019
@StarF666 StarF666 changed the title Generate interface an type from abstract class Generate both interface and type from abstract class May 9, 2019
@gscheibel
Copy link
Contributor

Hello @StarF666,

Do you have a snippet of code showing showing the Kotlin sources and the expected GraphQL schema output?
it would help us a lot.

Thanks

@StarF666
Copy link
Author

StarF666 commented May 9, 2019

Here is a sample model with the same structure like I'm using for my persistence layer (without the JPA annotations obviously):

abstract class Vehicle(
        open val name: String
)

class Car(
        override val name: String,
        val doors: Int
) : Vehicle(name)

class Bicycle(
        override val name: String,
        val gears: Int
) : Vehicle(name)

And this is what would be needed as a GraphQL schema:

interface VehicleInterface {
  name: String!
}

type Vehicle implements VehicleInterface {
  name: String!
}

type Car implements VehicleInterface {
  doors: Int!
  name: String!
}

type Bicycle implements VehicleInterface {
  gears: Int!
  name: String!
}

type Query {
  getVehicle(type: String!): [VehicleInterface!]!
}

Currently it only creates the "type vehicle" but no interface so I can query for a vehicle but cannot use a query like this:

query {
  getVehicle(type: "any") {
    name,
    ...on Bicycle {gears}
    ...on Car {doors}
  }
}

I hope this clarifies it a bit. I know it seems a bit odd to generate an interface although there isn't one on the kotlin side but GraphQL does not support direct type inheritance (as far as I know). Thanks for taking your time.

@dariuszkuc
Copy link
Collaborator

dariuszkuc commented May 13, 2019

Fix released in 0.3.1

@StarF666
Copy link
Author

StarF666 commented May 15, 2019

Thanks for your work. I tested the new version against my model and found another issue. Here is an updated test model which causes the following error:

Caused by: graphql.AssertException: All types within a GraphQL schema must have unique names. No two provided types may have the same name. No provided type may have a name which conflicts with any built in types (including Scalar and Introspection types). You have redefined the type 'Bicycle' from being a 'GraphQLObjectType' to a 'GraphQLObjectType' at graphql.schema.GraphQLTypeCollectingVisitor.assertTypeUniqueness(GraphQLTypeCollectingVisitor.java:105) ~[graphql-java-11.0.jar:na]

Test model:

abstract class BaseTestEntity(
        val id: Int = -1
)


class Feature(val name: String) : BaseTestEntity()


abstract class Vehicle(
        open var name: String = "Mustang",
        open var features: List<Feature> = listOf()
) : BaseTestEntity()


class Bicycle(
        override var name: String,
        var gears: Int,
        override var features: List<Feature>
) : Vehicle(name, features)


class Car(
        override var name: String,
        var doors: Int,
        override var features: List<Feature>
) : Vehicle()

I added a class "Feature" which each Vehicle has a list of and whcih also inherits from BaseTestEntity. This inheritance causes the trouble. I'm not really sure, what the generated schema should look like or if I'm hitting some boundary here.

P.S.: I get the same error when I change the abstract classes to interfaces.

@smyrick
Copy link
Contributor

smyrick commented May 15, 2019

GraphQL cant have an interface implement another interface today, so this is is not an error but "expected" graphql/graphql-spec#373

I agree that it is limiting now in Kotlin code but if you think in terms of the resulting schema and how you would query this it makes it very complicated.

query MultiInterface {
  getAllEntities {
    id
    ... on Vehicle {
      name
      # double nested fragment!
      ... on Car {
       doors
      }
    }
  }
}

@StarF666
Copy link
Author

Thanks again for your effort. I will watch GraphQL spec and see if/how they will extend their inhereritance system. I understand the problem with circular references from your previous comment. Currently my problem is with said base classes abstracting technical database concerns. So to make my model finally work, I used custom hook "isValidSuperclass" to exclude my base classes from schema generation:

override fun isValidSuperclass(kClass: KClass<*>): Boolean {
    return when (kClass.qualifiedName) {
        "com.acme.somepackage.BaseEntity" -> false
        "com.acme.somepackage.AuditedEntity" -> false
        else -> true
    }
}

dariuszkuc added a commit to dariuszkuc/graphql-kotlin that referenced this issue Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

4 participants