Rejoiner - Relay support thoughts

[DianaSuvorova]

I have tried to spin up a relay client with rejoiner-based GraphQL server on the back end and here are my observations:

1. relay client requires graphql.schema to be available locally. So GrapQL server needs to expose a way to get a schema. This is as simple as using schema printer and exposing it via /schema endpoint.
   My need to update an example to demo complete integration.
   private static final String schema = new SchemaPrinter().print(SCHEMA);

2. I feel a bit uneasy about the fact that ID fields annotations belong to actual source .proto files.
   I actually have not tested it and not sure if it works properly given that Relay support is experimental.
   It could use some more centralized config. For example assume all fields with name id are actually an GraphQL ID type and provide a way to turn it off and/or provide a different name for an ID field. For example new SchemaProviderModule(uid). Since there is a way to update the types on case-by-case basis, I feel this might have a better separation of concerns than to actually bake relay-related artifacts into .proto files.

3. Similarly I feel that relay support is awesome in general, but should be optional. Maybe it can be an an option to the SchemaProviderModule(relay: true).

4. I haven't tested @RelayNode signature, but I like the idea.

5. I haven't checked if mutations and connections work as per spec but planning to get to it soon.

[siderakis]

Overall Relay support is not in a good state. The main functionality I wanted to add was to be able to easily annotate Proto message which would do two things. 1) Tell Rejoiner which field to use when populating a value for the ID. 2) Annotate methods with the RelayNode annotation so that Rejoiner can fetch data.

1. That makes sense.
2. There are a bunch of approaches we could take, annotating protos, annotating Java code, external config file (maybe YAML). Most of Rejoiner was based on Java annotations/code, I'd also like to support proto annotations and this was just a place to experiment (since Relay support itself is experimental). I'm open to supporting all of those options, since I see pros and cons depending on the situation.
3. Good idea. Also, if nothing is annotated with @RelayId or @RelayNode I'd hope the schema wouldn't mention Relay at all.
4. Thanks :-)
5. There might be a great opportunity to map Google API design recommendation for pagination (https://aip.dev/158) onto Relay pagination.

[CelsoSantos]

There is something that is not clear to me...
I'm using graphql-java-kickstart which implements Relay (for a number of reasons this existed already in the project I'm working on).

Can we use/take advantage of this Relay (and if so, how..) or to fully take advantage of Rejoiner we would have to use Rejoiner's implementation of Relay?

[DianaSuvorova]

@CelsoSantos this is a good question... so relay is really just bunch of additional requirements/assumption about your schema on top of standard GraphQL spec. Btw, I actually don't know what is going to happen if you try to use relay client with the server that does not comply 100% with the requirements (if you find out -let me know).

So I think graphql-java-kickstart provides some utils to add otherwise very repetitive schema annotation so is rejoiner. I don't think you can use rejoiner together with graphql-java-kickstart, it comes with its own graphql server implementation, in examples they use jetty

I have started this branch for TodoMVC example for relay and rejoiner. This seemed to work, but I have never done it for real use case though... Keep me posted if you go that route...

[CelsoSantos]

According to the spec, one needs to annotate the proto definition with [(google.api.graphql.relay_options).id = true]; right?

@DianaSuvorova I took a look at your code. I see the @RelayNode annotation but your protos don't have that annotation. Is that on purpose?

On another note, building protos with Maven using that annotation is producing an error due to being unable to use/find relay.proto:

[INFO]     Processing (java): author.proto
protoc-jar: executing: [/tmp/protocjar14842319688386487255/bin/protoc.exe, -I/workspace/proto, -I/tmp/protocjar14618580364098423728/include, -I/workspace/proto/my-company/ac/author/v1, --java_out=/workspace/proto/target/generated-sources, /workspace/proto/my-company/ac/author/v1/author.proto]
relay.proto: File not found.
my-company/ac/author/v1/author.proto:10:1: Import "relay.proto" was not found or had errors.
my-company/ac/author/v1/author.proto:9:1: warning: Import google/protobuf/empty.proto is unused.
[ERROR] /workspace/proto/evooq/ac/author/v1/author.proto [0:0]: relay.proto: File not found.
[ERROR] /workspace/proto/evooq/ac/author/v1/author.proto [10:1]:  Import "relay.proto" was not found or had errors.
[ERROR] /workspace/proto/evooq/ac/author/v1/author.proto [9:1]:  warning: Import google/protobuf/empty.proto is unused.
[ERROR] /workspace/proto/evooq/ac/author/v1/author.proto [0:0]: 

Do I need to replicate/implement relay.proto locally?

[DianaSuvorova]

@CelsoSantos

I'd be surprised you need to reimplement relay.proto I think it is project's compilation issue.

I see the @RelayNode annotation but your protos don't have that annotation. Is that on purpose?

Yeah, I probably has stopped at this point. Cause I thought it is too far down the stack. Ie I think Relay support gets too invasive if you need to built it in proto defs.

I am probably not very helpful... but would you mind sharing why you decided to go for Relay instead of other GraphQL client (ie apollo) Since I went similar route and while I very much like Relay's approach, guidelines and structure I have decided in favor of Apollo since it was much easier to get up and running and it does not dictate additional requirements on the server part..

would be curious to hear your thoughts