trustification / trustify

Apache License 2.0
8 stars 15 forks source link

Add GraphQL API to trustify server #360

Closed gildub closed 3 weeks ago

gildub commented 1 month ago

Initial integration of GraphQL service using async-graphql providing the following :

I changed advisory entity field advisory.issuer_id to advisory.organization_id. The reason is because I initially thought graphql schema mapping was automatically looking up the related entity (organization) for a foreign key. That's the way some GraphQL frameworks work. This doesn't seem relevant with async-graphql, meanwhile I kept the change because it makes more sense. In case we keep issuer_id as foreign key then it would make sense to name the associated entity "Issuer".

carlosthe19916 commented 1 month ago

@gildub just pasting my question/comment from our last meeting (where you did a demo)

It seems what is exposed are the Model/Entities directly. Does it mean the DB must be designed in a way in which the Model/Entity is 100% ready for client consumption? Currently the REST approach is exposing DTOs not the Model/Entity directly

And by Model/Entity I meant the Database tables. This should be something to consider and be evaluated

gildub commented 1 month ago

@carlosthe19916 ,

We can expose only what we need to.

For example,

Beyond that we can define structs to create new GraphQL objects which we populate with whatever data that don't have to be directly corresponding to an Entity table. For instance the data could be partly the result of some entity data and/or the combination of several other sources.

gildub commented 1 month ago

@bobmcwhirter, I assumed there are no used case for using distinct Actix-web servers for each Rest and Graphql APIs. To be revisited if needed. The other assumption is the GraphQL endpoint, the /graphql is to get started but it's ultimately a team decision to be taken.

As we have /api for the REST API, do we want to change that to not confuse it with GraphQL's API ? Or maybe that's not a subject and /api is by default the REST one. Also how to name the GraphQL endpoint ?

bobmcwhirter commented 1 month ago

Let's share the same actix http server. No reason to add more open ports.

And I'd be fine with /graphql/v1 (or without version if you think that's better) in the URL space.

gildub commented 4 weeks ago

@bobmcwhirter, versioning vs evolution is the question we need to answer.

GraphQL can use both but has a tendency to use evolution.

With evolution :

The evolution approach allows to not use versioning but has cons.

One solution to overcome those issues is to use semantic versioning at the object and/or field levels. This is described here https://blog.logrocket.com/versioning-fields-graphql/

But at the end it means a developer must actively introspect the schema to check if the schema was upgraded and if fields re are deprecated.

There are solution as well with the help of extensions, also supported by async-graphql, to advertise deprecation.

So by using evolution and combining semantic versioning we can get best of both world. And /graphql/v1 is not needed. What do you think ?