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

4th
May 2017
(Those two sites are what I use to quickly double check syntax etc…)
Vi-ka
@Vi-dot
May 04 2017 08:52

@apottere @bbakerman Still doesn't work with that :

POST /api/graphql HTTP/1.1
Content-Type: application/json
{
    "query": "mutation nameDefinedNoWhere($mention: mentionIn!) { insertMention(mention: $mention) {id doi raw} }",
    "variables": {
        "mention": {
            "doi": "10.3847/0004-637X/827/2/92",
            "raw": "test"
        }
    },
    "operationName": "insertMention"
}

Neither with stringify variables data

POST /api/graphql HTTP/1.1
Content-Type: application/json
{
    "query": "mutation nameDefinedNoWhere($mention: mentionIn!) { insertMention(mention: $mention) {id doi raw} }",
    "variables": "{\"mention\": {\"doi\": \"10.3847/0004-637X/827/2/92\", \"raw\": \"test\"}}",
    "operationName": "insertMention"
}

I get a 500 error without any message

Vi-ka
@Vi-dot
May 04 2017 13:01
It worked but only with insertMention instead of thisNameDoesntReallyMatter o_O
Andrew Potter
@apottere
May 04 2017 14:09
that name doesn’t really matter since it’s the only operation, but if you send an operationName then they have to match
you don’t have to send an operationName if there’s only one operation
Bojan Tomić
@kaqqao
May 04 2017 14:30
@andimarek @bbakerman I was thinking of working on malicious query protection​... I need this for my project, so it makes sense to do in graphql-java itself. There's already a couple of issues open suggesting different approaches, so I was wondering if you already some thoughts or plans. If I were to submit a PR, what approach would you rather have me take? Complexity analysis (static or dynamic), timeouts, depth limiting or something else?
Charles Kasey Reed
@kaseyreed
May 04 2017 16:16

Heya, I've been playing around with the IDL schema support on the latest branch of this library. I'm trying to split up my schema file. Type extensions seem like a convenient way to do this. However, once I declare a type extension for the second time the library throws a TypeRedefinitionError without trying to actually extend the type.

For example in query.graphqls:

type Query {
     aField: String!
}

extend type Query {
   bField: String!
}

In second-file.graphqls:

extend type Query {
  cField: String!
}

I would expect the Schema Compiler to be able to resolve the Query type with these extensions, but the library doesn't seem to support this. Is this something that is desired? Already implemented? Possible? If y'all think it is, I can file an issue / PR, but would need some guidance on how to proceed.

This would make splitting up large schemas very convenient.

Andreas Marek
@andimarek
May 04 2017 16:17
@kaseyreed Is this a official IDL feature? (type extensions)
Charles Kasey Reed
@kaseyreed
May 04 2017 16:21
That's a great question. It's not very well documented :/
It's part of apollographql/graphql-tools though: https://github.com/apollographql/graphql-tools/pull/90/files
And it seems that initial support was added to this implementation, but falls short after extending more than once
Andreas Marek
@andimarek
May 04 2017 16:38
@kaseyreed ok ... then we will probably not support this. IDL itself is not completely specified, so we will try to align ourself wit the PR about IDLs and the js reference implementation... @bbakerman your thoughts?
Charles Kasey Reed
@kaseyreed
May 04 2017 16:41
Andreas Marek
@andimarek
May 04 2017 16:48
@kaseyreed thanks
Andrew Potter
@apottere
May 04 2017 17:09
fwiw, it’s not in the graphql.org schema section: http://graphql.org/learn/schema/
Brad Baker
@bbakerman
May 04 2017 22:59
yeah none of the IDL is in the schema - however it is in the graphqljs grammar and in the graphql-java grammer
and its implied via the spec through examples
expressed in IDL
as for TypeExtensions - they are also in the grammar but it seems I have not done enough work in supporting them properly
its more an admission of work than a deliberate decision
sounds challenging though - I will see what I can do
Charles Kasey Reed
@kaseyreed
May 04 2017 23:03
@bbakerman Awesome! I'd love to see this feature. Was actually attempting to prototype it, but got caught up in some other work. Might get back to it this weekend.
Brad Baker
@bbakerman
May 04 2017 23:04
Type redefinition is there to protect against
type Query {
   a
}

type Query {
   b
}
but in this case its not smart enough to know it should not do it for
extends
Charles Kasey Reed
@kaseyreed
May 04 2017 23:06
Yeah... Interestingly, the schema compiler allows you to extend a type once... It's the second extends that fails.
Brad Baker
@bbakerman
May 04 2017 23:06
yeah I am going to say it doesnt have tests for that case
I really didnt spend efforts in getting that use case in place
and you have found it wanting
Charles Kasey Reed
@kaseyreed
May 04 2017 23:07
Yeah... I think the use-case is for large schemas
Brad Baker
@bbakerman
May 04 2017 23:07
right
you can compose multiple files with multiple types
but this is extends (like in class X extends Y) which is a different thing
Charles Kasey Reed
@kaseyreed
May 04 2017 23:08
Yup, but the issue is your main Query type has to have all the fields for the app. The extends syntax allows you to separate those concerns.
Yeah :thumbsup:
Brad Baker
@bbakerman
May 04 2017 23:08
right I get it - just didnt built it out enough and test it enough
thanks for reporting
Charles Kasey Reed
@kaseyreed
May 04 2017 23:09
No worries. I can file a formal issue on the repo if you want?
Brad Baker
@bbakerman
May 04 2017 23:09
please
Charles Kasey Reed
@kaseyreed
May 04 2017 23:09
:thumbsup:
Brad Baker
@bbakerman
May 04 2017 23:12
@kaqqao - regarding malicious queries - one option would be to put a Instrumentation in play since it is given the document before execution
Bojan Tomić
@kaqqao
May 04 2017 23:13
Ping, for the question above (it sank quickly). In short, are there any plans/thoughts for malicious query protection? I'm about to start working on it in my project and might as well contribute directly to graphql-java.
Brad Baker
@bbakerman
May 04 2017 23:13
you could then throws exceptions if you decide the parsed query is too complex
Bojan Tomić
@kaqqao
May 04 2017 23:14
Ah, cool, you did see the question :)
Brad Baker
@bbakerman
May 04 2017 23:14
I started a go at it a while ago but its tricky - since is it pure malicious (too deep) or could be expensive (data fetchers express a cost back to the system)
Sangria uses the latter
(in fact both really)
Charles Kasey Reed
@kaseyreed
May 04 2017 23:15
It would also be interesting to explore Static / Persisted Query support: https://dev-blog.apollodata.com/5-benefits-of-static-graphql-queries-b7fa90b0b69a
Bojan Tomić
@kaqqao
May 04 2017 23:15
I was going to replicate the Sangria impl as it can be done without touching the execution strategies​. But it's limited to static analysis
Brad Baker
@bbakerman
May 04 2017 23:16
nothing stops persisted support I think and its not even a base libary things
since the input to persisted queries is a “number” not a query
eg: run query 1234 - ok yup that i query x {….}
*is
persistent query support should be outside the library I think (unless some one can express why it needs to be or needs some base support code)
Charles Kasey Reed
@kaseyreed
May 04 2017 23:20
Yeah... I guess it's a matter of tooling rather than library support...
@bbakerman graphql-java/graphql-java#406