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.

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

feat!: make json object a scalar #56

Closed

Conversation

xsoufiane
Copy link

@xsoufiane xsoufiane commented Jan 19, 2021

please review 🙏

Motivation

This PR is part of the series that aims to make federation possible in Sangria, and it comes as a response to the federation issue discussed here.

Changes

This PR implements the new contract described in the testkit PR, by ensuring that json objects are GraphQL scalars.

Consequences

  • By accepting these changes, any sangria application using the circe marshaller will be able to accept json objects (in variables) as scalars. In particular, let's assume the following schema :
    scalar _Any
    type Query {
      _entities(representations: [_Any!]!): [_Entity]!
    } 
    
    with these changes, we can execute these kind of queries (see how a json object is passed as a scalar):
    {
      "query": "query($representations: [_Any!]!) {...}"
      "variables": {
        "_representations": [
          {
            "__typename": "Product",
            "upc": "B00005N5PF"
          },
          ...
        ]
      }
    }
    
  • As for the risks, they are the same as discussed [here]((feat(MarshallingBehaviour)!: allow map to be a scalar sangria-marshalling-testkit#22). Basically, extra care must be taking on the application side to ensure that the scalar json objects are not misused.

Notes for reviewer

At the current moment, tests fail because the PR still requires a new release of the testkit, incorporating the changes proposed here. Once, the new release has been published, another commit should be done updating the build with the new version.

@xsoufiane
Copy link
Author

me and @yanns found a way to make federation possible for Sangria without having to change the marshallers and the root sangria lib, and without breaking the existing security level for everyone:) the implementation is done here

@xsoufiane xsoufiane closed this Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants