These are chat archives for graphql-java/graphql-java

5th
Jun 2017
Brad Baker
@bbakerman
Jun 05 2017 00:01
I will believ Java 9 when Java 9 actually gets released
Bojan Tomić
@kaqqao
Jun 05 2017 01:55
@bbakerman Yup. I get the feeling :) The good thing is Rx, Spring and, I guess, others did release compliant implementations, so we have compatible reactive streams even if Java 9 never gets anywhere...
Brad Baker
@bbakerman
Jun 05 2017 01:56
in regards to subscriptions - I think in a raw sense you have the things you need to get it going - that is your data fetcher returns an object that represents a subscription to that “sub query type” and you would need further cooperation between server and client on how they are going to keep that subscription alive
I am not sure how much more the base engine should give in terms of trying to manage things for you… since it gets very “implementation specific” very quickly
Bojan Tomić
@kaqqao
Jun 05 2017 01:59
Yeah, that was my worry...
But the only way I can imagine implementing subscriptions right now is rather clumsy... Something like:
Bojan Tomić
@kaqqao
Jun 05 2017 02:06
1- Client fires a subscription query
2- The server somehow (probably in an instrumentation?) saves the actual text of the selection set part of the query
3- The server fetches the current state and synchronously sends the response
4- A mutation later fires and triggers a change
5- The server re-executes the previously saved query and pushes the new result to the client (this is in broad strokes, ignoring many gory details)
The saving of the query text to later be able to rerun it is the iffy part... By the time the fetcher runs you lost the textual query and there currently is no way to execute an already parsed one
So a way to just give a SelectionSet to the execute method would surely help
Bojan Tomić
@kaqqao
Jun 05 2017 02:16
There seems to be a lot implemented in the ref. impl. I'll try to understand what's in there as of now.
Brad Baker
@bbakerman
Jun 05 2017 02:17
@kaqqao - it wont be instrumentation - it will be the data fetcher for “subscription type"
subscription NewMessages {
  newMessage(roomId: 123) {
    sender
    text
  }
}
e.g. the data fetcher that makes “newMessage” type in this case
the data fetcher is given the selection set of fields
DataFetchingFieldSelectionSet
the subscription is set up then between client and data fetcher during the initial “subscription” query
I think a 4) muatition later occurs and 5) server re-executes a sub query is the tricky part indeed - I am not surethe base engine would be able to do that
since it cant know what mutations cause new “events” to be streamed and its not going to be managing “saved queries” easily
Bojan Tomić
@kaqqao
Jun 05 2017 02:21
@bbakerman Ok, yes, but how do you get from the saved DataFetchingFieldSelectionSet to ExecutionResult once a relevant event occurs?
True, the library can't reasonably manage the saved queries nor the events... This is entirely application specific
So what I meant is #335 seems kind of necessary here (to be able to execute a query without its original text form)
Bojan Tomić
@kaqqao
Jun 05 2017 02:36
Or I guess one can do weird stuff with the AstPrinter...
Andreas Marek
@andimarek
Jun 05 2017 02:37
@kaqqao yes, there will be something like that ... but I am really not sure regarding the overall design for subscriptions
so of course there will be some things in graphql-java"to support subscription, but maybe there will be rather a second module to manager the state for example
Brad Baker
@bbakerman
Jun 05 2017 02:40
an example might be the apollo graphql server - they have 3 distinct modules for subscriptions
Andreas Marek
@andimarek
Jun 05 2017 02:42
@bbakerman do you have a link for that?
Bojan Tomić
@kaqqao
Jun 05 2017 02:42
Anyone knows what's inside the ref. impl? I didn't know it has anything subscription related until moments ago, but I now see some tests... Exploring...
Andreas Marek
@andimarek
Jun 05 2017 02:44
@kaqqao yes, they basically implemented the algorithm described in the spec
Andreas Marek
@andimarek
Jun 05 2017 02:53
but it is very low level stuff: it basically deals with executing the selectionSet from the subscription if an event fires ... but maybe this is exactly what you are looking for @kaqqao
Bojan Tomić
@kaqqao
Jun 05 2017 02:55
@andimarek Yup, sounds like it :)
Brad Baker
@bbakerman
Jun 05 2017 03:11
still means you have to wire in all the subscription management and upstream persistence etc.. yourself right
Andreas Marek
@andimarek
Jun 05 2017 07:15
@bbakerman @kaqqao I looked more closely at how the ref impl does implement the spec: They don't handle any state or event system. They basically just provide a way to execute a subscription statement. They make use of the fact, that they always supported async values. A example can be found here: https://github.com/graphql/graphql-js/blob/master/src/subscription/__tests__/subscribe-test.js#L80
This test creates a simply subscription system by having a EventEmitter: sendImportantEmail is called by a mutation (or some other part of the system), it then causes a new async value to be produced by data.importantEmail(). And this value is the root Value of the subscription.
Andreas Marek
@andimarek
Jun 05 2017 07:21
This only works because the default resolve fn can handle Promises
Andreas Marek
@andimarek
Jun 05 2017 07:40
scratch that ... it is not true: the resolved value for the subscription "input" is expected to be a AsyncIterator (in the example data. importantEmail() )
and this AsyncIterator result is then mapped over the general execute function to produce an actual value: https://github.com/graphql/graphql-js/blob/master/src/subscription/subscribe.js#L125
Jason Shi
@shishuwu
Jun 05 2017 09:09
For IDL, "[]" mean: it is a List, right?
Bojan Tomić
@kaqqao
Jun 05 2017 09:26
@shishuwu Yup, correct
Brad Baker
@bbakerman
Jun 05 2017 09:32
and ! means non null
so [Foo!]! is a non null list of non null Foos
Bojan Tomić
@kaqqao
Jun 05 2017 09:36
@andimarek That sounds like an interesting implementation! I wonder why was the AsyncIterator necessary. Sounds as if each new event could have been produced by mapping execute over the result directly. Anyway, do you think the approach is worth copying (in general)?
Andreas Marek
@andimarek
Jun 05 2017 09:42
@kaqqao well the AsyncIterator is a elegant (soon to be standard) solution for having a stream of event
of course you could map each result directly over execute, but then subscribe would return a one-time subscription
and every time it is finished you would need to call subscribe again or have some other way to handle the next event
so a stream of events seems to be the fitting abstraction
Bojan Tomić
@kaqqao
Jun 05 2017 09:47
@andimarek Didn't know about AsyncIterator. Thought it's something that they had invented. And I agree, it does sound fitting.
Bojan Tomić
@kaqqao
Jun 05 2017 09:56
Sounds a lot like an Observable, right? Or I need a coffee...
Brad Baker
@bbakerman
Jun 05 2017 09:56
right
a stream of values with the added “it is done” indicator
Andreas Marek
@andimarek
Jun 05 2017 09:58
I think async iteration is more simple than observable, but I don't follow this whole topic in JS very closely currently
Bojan Tomić
@kaqqao
Jun 05 2017 10:03
Yeah, sounds like a somewhat specialized case of observable... It's hard to believe the time I was able to read JavaScript was just a little while ago. Feels like ages.
Daniel Ocampo
@danielocampo2
Jun 05 2017 22:50
hey @andimarek just created the PR for the issue I mentioned earlier
this is kind of a show stopper for us in my organisation, so if there's a change of having this merge and released as a fix it'd great. I know I am asking for a lot but we really need it. Thanks to anyone that can review it!
Brad Baker
@bbakerman
Jun 05 2017 22:59
Loooking now
the graphql-annotations project is in stasis at the moment. I am trying to find time to fix it up and geta release out
Daniel Ocampo
@danielocampo2
Jun 05 2017 23:09
@bbakerman thanks!