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

Document limitations of idiomatic Kotlin for practical use in graphql-kotlin #284

Closed
lennartj opened this issue Jul 30, 2019 · 9 comments
Closed

Comments

@lennartj
Copy link

lennartj commented Jul 30, 2019

Describe limitations of idiomatic Kotlin

If I understand things correctly, graphql-kotlin aims to convert idiomatic Kotlin types to GraphQL schema definitions and related documents, without forcing developers to create lots of boilerplate code in the process. However, most API:s created using idiomatic Kotlin, contains a slew of collection types which are - by default - not supported in GraphQL itself.

To me, this implies a stronger need for documenting the examples which are - by default - not supported by graphql-kotlin, and I could not find any such examples on the graphql-kotlin wiki. I would suggest adding such documentation; let me add 2 examples which seems addressed only to some extent in the otherwise well-done spring-boot example:

Recommendation / pattern for non-List collections

Sets and Lists have different server-side semantics. Conversion between the two is a simple matter - but puts extra requirements on the data within the collection (equality, comparability etc.). It would therefore be helpful to document (on the wiki, preferably) how to cope with a REST service generated from data types containing SortedSets, TreeMaps and the like.

If the strategy requires creating loads of new types or custom type definitions to properly generate a GraphQL schema from the present Kotlin code, it is less feasible to use the same codebase for a transport model in the REST and GraphQL cases - and that means the value of graphql-kotlin is somewhat reduced compared to generate the GraphQL schema by hand.

In issue #201, adding Sets (or other, idiomatic Kotlin types) to the GraphQL-kotlin was denied. However, the challenge in creating a single transport model in Kotlin (used not only in the GraphQL channel, but also in - say - Messaging, Caching etc.) requires use of types not directly supported in GraphQL. Hence, the usability of the library would benefit from documenting these cases.

Recommendation / pattern for inheritance and fragments

GraphQL basically uses composition / fragments to avoid repeating common datastructures. Java/Kotlin uses both composition and inheritance. How could we instruct the SchemaGenerator to synthesize fragments for certain types?

@smyrick
Copy link
Contributor

smyrick commented Jul 31, 2019

@lennartj Thank you for your comments. One of the hardest parts of software is documentation, however I think we are lacking in some areas as you pointed out.

The wiki could use a real upgrade as we have added more features. It was brought up at one point if we wanted to have a separate place for documentation outside of the wiki, but I think the format works well enough for now, as long as we link properly to other pages. We will work to update these pages with examples.

For now I am going to update the Lists section: https://github.com/ExpediaDotCom/graphql-kotlin/wiki/Lists

@lennartj
Copy link
Author

lennartj commented Jul 31, 2019

Hello @smyrick (and all others),

First ...

Thanks for the work you have done/are putting into creating a reflective approach to get a simple integration between Kotlin and GraphQL.

Secondly ...

I am still puzzled on the most important of the use cases, namely how to avoid making a separate transport model, only suitable for GraphQL use. The example mock transport model below is idiomatic Kotlin, and works well when exposed over REST or JMS to various types of native Java client (using - say - JSON transport format). Its semantics are Kotlin-clear in terms of mutability, repeatability etc. for both the allergies and the formerMemberships properties (including its accessors).

Example: Idiomatic Kotlin Transport Entity Patterns

However, the Sets do not work within the GraphQL specification. Hence, we need to morph this transport model to both produce a desired JSON transport format and enable generation of a well-formed GraphQL schema definition. Hence, for example, the Set pattern could be something like the following

open class XXX (

        // The actual parameter is suited for Jackson/JSON serialization and deserialization
        @GraphQLIgnore
        val aSet: SortedSet<SomeComparableType>? = null) {

    // Adaption of a Set to GraphQL schema/transport mechanics
    @JacksonIgnore
    @GraphQLDescription("Optional set of former Memberships within this organisation.")
    fun getASet(): List<SomeComparableType>? = when (aSet == null) {
        true -> null
        else -> aSet.toList()
    }
}

Above is one (rather ignorant, admittedly) possible pattern suggestion to make a single transport class which contains a Set expose that collection as a List over GraphQL. The problem is of course that the pattern above will incur a code bloat in close to every transport class in the system... which is bad (tm). Granted, this type of type conversion could be done in a standardized Aspect (activated by a slick annotation, maybe 😉) rather than hard-coded into every relevant place within the transport model, currently such annotations do not exist within the graphql-kotlin and you have also said that such constructs should not be supported.

Hence ... I believe that the impedance mismatch between Kotlin constructs available when developing a standard JMS/REST/WebSocket API and the Kotlin constructs available when developing a GraphQL API is currently rather big. Documentation and examples of these cases is therefore really important - especially if you estimate that Java/Kotlin programmers are more used to REST than GraphQL at present.

Trivial Example Transport class

open class TransportTypeMembership @JvmOverloads constructor(

        id: Long = UNKNOWN,

        legacyID: String? = UNKNOWN_LEGACY_ID,

        importedAt: LocalDateTime? = null,

        val formerMemberships: MutableSet<HistoricDateRange>? = null,

        val allergies: MutableSet<Allergy> = TreeSet()

) : AbstractImportedEntity(id, legacyID, importedAt), Serializable, Comparable<TransportMembership> { 
        // ... and several member properties / functions follow here ... 
}

@dariuszkuc
Copy link
Collaborator

Great questions. graphql-kotlin does not aim to generate valid GraphQL schema out of arbitrary Kotlin code. The goal is to simplify generation of the GraphQL schema by automatically generating it from the code but we are still bound to the restrictions of the GraphQL specification. We try to make this process as flexible as possible by utilizing various hooks.

By default, we are not supporting arbitrary data structures like maps and sets as they are not supported by the GraphQL spec which itself is tied to a JSON representation. GraphQL is statically typed and relies on the strong types so returning arbitrary dynamic maps of objects is at odds with the basic principles of the GraphQL. It is definitely possible to automatically convert map elements to a list of some key/value pairs wrapped in some type but it would also result in some very generic types (that would also have to be unique across different fields returning different maps) that don’t have much meaning behind them. User generated types can have much more meaning behind them and as such are preferable when defining the schemas. You can see this issue has been discussed for quite some time - GraphQL spec #101

Similarly, Set could be easily serialized to a JSON list but problems arise when you try to accept this GraphQL list and convert it to a set. As you mentioned, Set and List have different properties - one is unordered collection (*except for specific implementations) with unique elements vs ordered collection that allows duplicates. From client perspective there is no way to inform them that given GraphQL list should have unique elements - so what should be done on the server side? Should we remove duplicates or fail the request? That is why we decided to no support sets out of the box and let the application owners define their own behavior if needed.

I am still puzzled on the most important of the use cases, namely how to avoid making a separate transport model, only suitable for GraphQL use. The example mock transport model below is idiomatic Kotlin, and works well when exposed over REST or JMS to various types of native Java client (using - say - JSON transport format). Its semantics are Kotlin-clear in terms of mutability, repeatability etc. for both the allergies and the formerMemberships properties (including its accessors).

GraphQL was created to address shortcomings of the REST approach for building front-end (namely mobile) APIs. While backend services can definitely call GraphQL endpoints it might not always be an optimal approach. When developing new GraphQL services it is very tempting to port existing REST APIs and expose them as part of the graph but it will generally result in suboptimal schemas. I'm pretty sure there would be some use cases when reusing same objects across different APIs would be beneficial but I would not treat GraphQL as a drop-in replacement for REST/JMS.

@lennartj
Copy link
Author

lennartj commented Aug 1, 2019

I wrote:

I am still puzzled on the most impodhQL use. The example mock transport model below is idiomatic Kotlin, and works well when exposed over REST or JMS to various types of native Java client (using - say - JSON transport format). Its semantics are Kotlin-clear in terms of mutability, repeatability etc. for both the allergies and the formerMemberships properties (including its accessors).

@dkuc84 responded:

GraphQL was created to address shortcomings of the REST approach for building front-end (namely mobile) APIs. While backend services can definitely call GraphQL endpoints it might not always be an optimal approach. When developing new GraphQL services it is very tempting to port existing REST APIs and expose them as part of the graph but it will generally result in suboptimal schemas. I'm pretty sure there would be some use cases when reusing same objects across different APIs would be beneficial but I would not treat GraphQL as a drop-in replacement for REST/JMS.

I am not implying replacing existing JMS/Async or REST/Sync protocols with GraphQL. I am implying that the transport model should be the same. The transport model is the Maven Project which defines the entities which are serialized to - and deserialized from - XML or JSON. For a software component x, there would typically be a set of maven projects on the form:

  1. x-transport-model: Defines the entities published/used externally using idiomatic Kotlin or Java. Typically contains Jackson annotations or equivalent.
  2. x-impl-rest: Adopts the x-transport-model to REST and implements the resources using JavaEE, SpringBoot other underlying tech. Typically contains JaxRS annotations or equivalent.
  3. x-impl-messaging: Adopts the x-transport-model to asynchronous messaging such as JMS using Artemis, SoniqMQ or other underlying tech.

... and I would - of course - want to add another adaptation project:

  1. x-impl-graphql: Adopts the x-transport-model to GraphQL, generating the GraphQL schema using the classes/entities defined within the x-transport-model project. This is where I would like to include a dependency on graphql-kotlin and also place the schema generation hooks, Coercing types etc. Moreover, the entities are imported from the common x-transport-model which defines the external API types.

So ... what is the problem?

You currently need to re-implement the x-transport-model only for use within the x-impl-graphqlproject since the static type definitions within the x-transport-model (such as SortedSet, SortedMap etc.) are not supported by graphql-kotlin.

This defies the purpose of having a common transport model for software components, and forces development teams to manually keep 2 models in sync (i.e. the x-transport-model and its adaptation within x-graphql-impl) ... which is what the GraphQL-Kotlin project seems to want to avoid when creating a library which generates GraphQL schema code-first.

Another approach is to implement AspectJ aspects and pointcuts as required to transform non-supported (by graphql-kotlin) data structures implemented within the transport model to GrapQL-Kotlin-supported data structures. This must be done anyways .. but there needs to be mechanics in GraphQL-Kotlin to support it.

Fair?

@dariuszkuc
Copy link
Collaborator

Thanks for the explanation. While you definitely could define some Kotlin data classes that define your shared data model used across REST/JMS/GraphQL/ service (either by restricting them to GraphQL supported data types or customizing schema generation process by using the hooks) I personally don't think it would be the best option.

With traditional transport models like REST or JMS you are always bound to return complete object back. GraphQL is a query language for your APIs that allows you to selectively return only the fields that you want but it is also a lot more. Consider a simple example when we would want to return some date time value on an object - how do you represent it on the POJOs? With REST you would generally serialize it to some single field (e.g. long timestamp or some pre-formatted string) and provide single representation of that object back to the user. With GraphQL you can easily give your clients multiple options on how they want to retrieve it, e.g.

class MyWidget(val id: String, private val dateTime: ZonedDateTime) {
    fun defaultDateTime() = DateTimeFormatter.ISO_LOCAL_DATE_TIME.format(dateTime)
    fun formattedDateTime(format: String) = DateTimeFormatter.ofPattern(format).format(dateTime)
    fun timestamp() = dateTime.toInstant().toEpochMilli()
}

From GraphQL perspective all of those functions are just fields (some taking arguments) on the object that can be selected by the client. This is a very simple example how you could provide additional functionality over REST but you could also easily extend it to use those functions to call some other service/database to return additional objects, etc. If you define shared data classes (e.g. x-transport-model) then you are restricting both your GraphQL schemas (as you will be restricting the shape of your schema and also won't be providing those extra functionality) and also REST/JMS implementations as you might want to only use the supported GraphQL types. That is why I wrote that directly porting existing REST API data to a GraphQL schema will generally result in suboptimal schemas. In my opinion schemas should be driven by the "product" (e.g. your clients such as mobile apps) and return data in the format that is optimized for their consumption.


Side note - aspects and pointcuts don't work well with coroutines (which we highly recommend to use for you queries) due to the continuations.

@smyrick
Copy link
Contributor

smyrick commented Aug 1, 2019

@lennartj I would have to second @dkuc84 points. GraphQL is not meant to be just a drop in replacement of your REST API where you can select fields. If you still need all the same data in the same format as your existing REST models, then there is no need to have another GraphQL layer on top of that, you can just call your existing REST API.

If instead you rethink the data you are sending clients and how you can have resolvers deeply nested in the schema, so that data is not fetched if client does not request it, you can start to design a schema, rather than an API. This mindset requires that you break away from REST conventions and adopt a GraphQL way of modelling, which is different in code as well.

If you want to see some more examples, I recommend you watch a conference talk I gave at Apollo Day San Francisco about how we use a federated schema at Expedia Group

https://youtu.be/MuD3TAP0D9Y

@lennartj
Copy link
Author

lennartj commented Aug 1, 2019

I think that the main point here is to avoid having to spend time to keep 2 models (or more) in sync, folks. To avoid creating a separate GraphQL transport model, and treat it as just another transport and discovery protocol.

From an enterprise maintainability and usability perspective, I would be considerably more interested in GraphQL's ability to empower client-side development automatically provide API introspection in the development tools (i.e. exploratory development) and let clients select and fetch exactly the relevant data from the service (i.e. increase development velocity and reduce network resource consumption).

This is especially valid since the enterprise domain model (frequently stashed in entities/persistent storage) which normally is used to populate any transport model object's properties contains all available properties which carry business meaning. These properties must normally be copied to a transport model object and converted to any of the available channels ... in an effective, understandable, visible and usable way.

... and so I see GraphQL as transport mechanism for a (currently) limited set of clients, namely web clients. Instead of supplying and maintaining a multitude of specialized methods in a REST API, I would see GraphQL as a means to reduce service-side bloat/cost by moving capabilities to the client application. I don't see GraphQL as a specification for how to develop and maintain a separate domain or transport model. The example from @dkuc84 above with the 3 formats and methods is just as simple to develop in a REST service - but the point is that the common transport model would define these properties and GraphQL would assist in introspecting and combining them in better ways than - say - REST.

@dariuszkuc
Copy link
Collaborator

While sharing common domain model between traditional services (i.e. REST webservices, sharing DB models, etc) and clients is very useful, in general you would not be sharing your GraphQL data model between your GraphQL server and your apps/clients. With GraphQL services each client generates their own data models based on their queries (using some tooling like Apollo Android, etc) that will only expose the fields they use.

My example with exposing additional representation of the fields was to show a fundamental difference between GraphQL and REST services - you can definitely return that extra data in REST service but now your response would always contain duplicate data represented in a number of different ways. As a result while developing REST services we generally try to avoid that and attempt to normalize the data. With GraphQL there is no penalty for adding those additional representations and it actually encourages denormalization of your data to provide your clients exactly the data they need. Those additional fields are only computed if clients request them and adding new functionality/fields to existing graph does not impact your existing clients.

You can definitely use GraphQL to expose your enterprise domain model data but (in my opinion) if you approach development of your GraphQL APIs from this perspective (bottom up) you will be severely limiting yourself. GraphQL is a query language for your APIs.

@smyrick
Copy link
Contributor

smyrick commented Sep 5, 2019

We are going to close this issue for the reasons discussed above. We will keep this in-mind when discussing future implementations. If you would like to request another specific feature please do not hesitate to create another issue.

@smyrick smyrick closed this as completed Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants