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.

Sign up for GitHub

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

Jump to bottom

Update ExecutionContextBuilder.java #127

Merged
merged 1 commit into from May 8, 2016
Merged

Update ExecutionContextBuilder.java #127

merged 1 commit into from May 8, 2016

Conversation

dminkovsky
Copy link
Contributor

From 2.2

if a GraphQL query document contains multiple operations, each operation must be named. When submitting a query document with multiple operations to a GraphQL service, the name of the desired operation to be executed must also be provided.

@dminkovsky
Update ExecutionContextBuilder.java
b5e0092 
From [2.2](http://facebook.github.io/graphql/#sec-Language.Query-Document)

> if a GraphQL query document contains multiple operations, each operation must be named. When submitting a query document with multiple operations to a GraphQL service, the name of the desired operation to be executed must also be provided.
@dminkovsky
Copy link
Contributor Author

dminkovsky commented Apr 28, 2016 •
edited

I mean I think this is a bug. I convinced myself that this wasn't a bug some time ago, but now looking at it again, I don't see how.

@dminkovsky
Copy link
Contributor Author

I will add a test

@yrashk
Copy link
Contributor

yrashk commented May 6, 2016

From the looks of it, it seems that this patch is indeed addressing an implementation bug. Just wondering what was the reasoning that you used to convince ourself this wasn't a bug before, if you can remember? Just want to make sure we're not missing something else here.

@danielkwinsor
Copy link
Contributor

Yes I can tell that this fixes a bug, although in a different way than I have a test for. I'd like to see a validator check that adds this to results.errors, rather than throw an exception here. We're now escaping outside of the ExecutionContext engine upon receiving bad input.

By allowing this, we'd introduce a (second) source of thrown, uncaught exceptions but fixing thrown uncaught exceptions upon valid input like http://facebook.github.io/graphql/#sec-Operation-Name-Uniqueness

@danielkwinsor danielkwinsor merged commit 7ee775a into graphql-java:master May 8, 2016
@dminkovsky dminkovsky mentioned this pull request Aug 31, 2016
Closed
@dminkovsky dminkovsky deleted the patch-3 branch September 13, 2016 02:56
dminkovsky added a commit to dminkovsky/graphql-java that referenced this pull request Dec 18, 2016
@dminkovsky
GH graphql-java#196: Add tests for graphql-java#127
730cd53
dminkovsky added a commit that referenced this pull request Dec 18, 2016
@dminkovsky
Merge pull request #207 from dminkovsky/add-127-test
ed7fbb2 
Add tests for #127
GrigoryPtashko added a commit to GrigoryPtashko/graphql-java that referenced this pull request Dec 22, 2016
@GrigoryPtashko
Merge remote-tracking branch 'upstream/master'
e6939a1 
* upstream/master:
  Fix graphql-java#208: Cannot use custom execution strategies on mutations
  Parsing: ensure that keywords can be used as names
  Fixed grammar: type conditions in inline fragments are optional
  Fixed directive introspection
  GH graphql-java#196: Add tests for graphql-java#127
  Added 'query' and 'mutation' as keywords in the grammar
  graphql-java#263 - make execution context thread safe by being mostly immutable and otherwise thread safe
  Fix graphql-java#244: Support for integer-valued ID fields
  Ensured type references are replaced regardless of type definition order Added a simple schema validation mechanism and a rule for recursive input types Added a test to ensure schema validation detects invalid recursive input types Added a test to ensure no dangling references exist Added a test to ensure input type references can not be used in place of output types
  Fixed DataFetcher example and typo in the readme
  Finish the GraphQL Schema parsing so it supports SchemaDefinition, DirectiveDefinition, and TypeExtensionDefinition type declarations.
  Initial take on adding GraphQL Schema parsing.
  change to fix open-jdk image instead java to fix build
  update to latest gradle version 3.2
  ValuesResolver fixes for input object literals and variable values
  graphql-java#172 Allow TypeReference as InputType
  Reverted import to be in line with upstream
  Directive argument is no longer validated as field argument
GrigoryPtashko added a commit to GrigoryPtashko/graphql-java that referenced this pull request Apr 15, 2017
@GrigoryPtashko
Merge remote-tracking branch 'upstream/master' into bg
f6cafdb 
* upstream/master:
  Fiddle with language
  Clarify supported clients in README.md
  Increasing test coverage for FragmentsOnCompositeType
  Increasing test coverage for FieldsOnCorrectType.java
  Increasing test coverage for ArgumentsOfCorrectType.java
  Validate name
  Missing <I,O> on Coercing
  No starts in imports
  Spec validation for lone anonymouse operation
  Fix graphql-java#317: Coercing<T> should be Coercing<I, O> ?
  105 - lenient numerical scalars
  graphql-java#308 - corrected readme
  graphql-java#308 - missing readme updates
  graphql-java#308 - males for  a more fluent description of the schema
  Add jacoco plugin. It generates test coverage.
  import problem
  update related projects
  update related projects
  update links
  update related projects list, update link to contributors
  made DataFetchingEnvironment an interface.  I appreciate that some one else did a PR on this but it had merge conflicts and I could not easily get the branch in play so it was easier to replicate the approach.
  graphql-java#269 - add an instrumentation callback for all the data retrieval
  Update README.md
  Add link to gitter.im chat room in README
  Fix README example error reported in graphql-java#295
  Allows a ExecutionProvider to be provided by the caller
  Now allows you to specify a new "instrumentation" implementation when building the top level GraphQL level
  Renamed based on PR feedback
  fixed up tests.  Still needs the new builder pattern
  new lines and formatting
  feedback about naming
  graphql-java#273 - updated readme and made compilable examples
  graphql-java#273 - merged master and now accounts for mutation strategy.  Also updated the readme
  graphql-java#261 -  PR feedback
  Issue 281: Spec allows for 'extensions' map in result but graphql-java does not.
  Updated README For v2.3.0
  graphql-java#273 - updated readme to reflect generics
  graphql-java#273 - use generics where possible
  graphql-java#278 - consistent parameter ordering
  graphql-java#278 - merged in upstream master with tweaks
  graphql-java#278 - generate a unique id for a graphql query
  Fix graphql-java#208: Cannot use custom execution strategies on mutations
  Parsing: ensure that keywords can be used as names
  273 - use builder pattern in GraphQL top level object
  Fixed grammar: type conditions in inline fragments are optional
  Fixed directive introspection
  GH graphql-java#196: Add tests for graphql-java#127
  Added 'query' and 'mutation' as keywords in the grammar
  graphql-java#253 - oops - included code I used to help generate the hashcode / equals
  graphql-java#253 - hash code and equals on error helper
  graphql-java#263 - make execution context thread safe by being mostly immutable and otherwise thread safe
  graphql-java#269 - this adds parameter objects to Instrumentation SPI and hence makes it less brittle for future changes.  Hmm missed test commit
  graphql-java#269 - this adds parameter objects to Instrumentation SPI and hence makes it less brittle for future changes
  graphql-java#269 - this adds instrumentation to the execution of the graphql query
  use getter
  edge as well
  allow customized prefix
  relay interfaces
  Fix graphql-java#244: Support for integer-valued ID fields
  Ensured type references are replaced regardless of type definition order Added a simple schema validation mechanism and a rule for recursive input types Added a test to ensure schema validation detects invalid recursive input types Added a test to ensure no dangling references exist Added a test to ensure input type references can not be used in place of output types
  Problem: passing input object as a resolved class doesn't work
  Fixed DataFetcher example and typo in the readme
  Finish the GraphQL Schema parsing so it supports SchemaDefinition, DirectiveDefinition, and TypeExtensionDefinition type declarations.
  Initial take on adding GraphQL Schema parsing.
  change to fix open-jdk image instead java to fix build
  update to latest gradle version 3.2
  ValuesResolver fixes for input object literals and variable values
  graphql-java#172 Allow TypeReference as InputType
  Reverted import to be in line with upstream
  Directive argument is no longer validated as field argument
  PropertyDataFetcher supports AutoValue style classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dminkovsky @yrashk @danielkwinsor