-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
GraphQL Configuration Options #5782
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
GraphQL Configuration Options #5782
Conversation
Not yet tested - essentially an RFC
@omairvaiyani I liked a lot the features you designed. I just wanted to discuss further what is the best way to setup them. I didn't review before because I actually spent the whole day digesting the setup you suggested and trying to think the pros/cons. I think we need to address 3 things:
So, talking about the first item, I am wondering if it would be better to store the GraphQL options in the database's Schema collection. The advantage of this approach: we could then allow the developer to setup the GraphQL API through the Parse Dashboard. What do you think about it? |
@davimacedo I think it's perfectly sensible to stagger the feature flexibility, although internally we may wish to maintain the code paths for future use? I'll address the potential steps as you've outline above, and then touch on the idea of storing the configuration at the database level below. Step 1
The way I've structured the PR, the fields and operations are altered using the custom resolver in Step 2. Currently, only the inclusion/exclusion of classes are provided in a static manner. The PR can be adjusted to instead provide the class-specific configurations as static data as well, though obviously this would significantly increase the Parse Server config file/json maintained by developers. Step 2
If we are to make this a separate step, we may have to make Step 1: "Allow developers to include or exclude classes.", and use this step to introduce field and operational type customisation. I am not against this idea, though the manner in which we expose the config (in-memory vs. database) may change the ideal delineation of these steps. Step 3
This step is definitely the one to keep last and I would rely heavily on your experience having done this in the past. I've personally not dealt with schema merging before - my idea was to build on the nested nature of the Parse GraphQL schema and allow custom types and operations to live under the user's own namespaces, where Storing the config in the database I'll hold off making any further changes to the PR until you've had a chance to digest this, appreciate it's a long one! |
So let's go with steps 1 and 2 now and we can address step 3 later. My idea of step 2 is more in-depth. I was thinking to allow the customization of any type (not only the ones generated from classes) like File, for example. But it can be a step 4. I loved the idea of |
@davimacedo I've almost finished building something akin to |
refactor and add graphql router, controller and config cache
Codecov Report
@@ Coverage Diff @@
## master #5782 +/- ##
==========================================
- Coverage 93.74% 93.74% -0.01%
==========================================
Files 148 150 +2
Lines 10301 10682 +381
==========================================
+ Hits 9657 10014 +357
- Misses 644 668 +24
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It actually looks pretty good to me. Great job!!!
I've just added minor comments to discuss about. Besides I think it is only a matter of creating more test cases and we are ready to merge. Try to keep at least the same average of test coverage that the project currently has.
This code has been moved into a separate feature branch.
@davimacedo @douglasmuraoka I believe the first 3 steps are now complete and ready for unit-tests. I could do with some help here as I'm running into some errors with the jasmine suite. I have not changed any of the settings but am getting this error when I run
|
In order to run the tests, you need to have a MongoDB running in mongodb://localhost:27017 The easier way is through mongodb-runner.
|
Let me know if you need any help. |
This will be followed up with a backwards compatability fix for the `ClassFields` issue to avoid breakages for our users
Also includes some minor code refactoring
This PR empowers the Parse GraphQL API with custom user-defined schema. The developers can now write their own types, queries, and mutations, which will merged with the ones that are automatically generated. The new types are resolved by the application's cloud code functions. Therefore, regarding #5777, this PR closes the cloud functions needs and also addresses the graphql customization topic. In my view, I think that this PR, together with #5782 and #5818, when merged, closes the issue. How it works: 1. When initializing ParseGraphQLServer, now the developer can pass a custom schema that will be merged to the auto-generated one: ``` parseGraphQLServer = new ParseGraphQLServer(parseServer, { graphQLPath: '/graphql', graphQLCustomTypeDefs: gql` extend type Query { custom: Custom @namespace } type Custom { hello: String @resolve hello2: String @resolve(to: "hello") userEcho(user: _UserFields!): _UserClass! @resolve } `, }); ``` Note: - This PR includes a @namespace directive that can be used to the top level field of the nested queries and mutations (it basically just returns an empty object); - This PR includes a @resolve directive that can be used to notify the Parse GraphQL Server to resolve that field using a cloud code function. The `to` argument specifies the function name. If the `to` argument is not passed, the Parse GraphQL Server will look for a function with the same name of the field; - This PR allows creating custom types using the auto-generated ones as in `userEcho(user: _UserFields!): _UserClass! @resolve`; - This PR allows to extend the auto-generated types, as in `extend type Query { ... }`. 2. Once the schema was set, you just need to write regular cloud code functions: ``` Parse.Cloud.define('hello', async () => { return 'Hello world!'; }); Parse.Cloud.define('userEcho', async req => { return req.params.user; }); ``` 3. Now you are ready to play with your new custom api: ``` query { custom { hello hello2 userEcho(user: { username: "somefolk" }) { username } } } ``` should return ``` { "data": { "custom": { "hello": "Hello world!", "hello2": "Hello world!", "userEcho": { "username": "somefolk" } } } } ```
Ignore this comment - problem fixed! @davimacedo So i set up https://postgresapp.com/ on OSX and ran:
But receiving this error in all the tests:
I do want to crack this so I can be helpful in future PRs given that postgres is clearly a core feature in parse-server, but for this PR, I'm conscious that its going to become more difficult to manage the merge conflicts the longer behind it gets. Could you sub in for me here and work out the failing tests? I'll continue reading up on setting up postgres with parse in the mean time for future PRs! |
fixes failing postgres tests
Graphql config merge master
@davimacedo I believe this PR is now complete, and needs reviewing / ensuring that the changes are in sync with the other GraphQL PRs. I have pulled in the latest master and adjusted some of the merge conflicts in the latest commit on this PR. |
… into graphql-config
@omairvaiyani I've just pushed a new commit to your branch merging the latest version of master and fixing some tests due to the change of ClassFields to ClassCreateFields. The PR overall looks good to me. But, since it is a lot of code, I'd love to also hear from @douglasmuraoka mainly regarding the changes in the GraphQL API and from @dplewis mainly regarding the new controller and cache that the PR is introducing to the Parse Server. |
The caching looks good to me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found some minor issues, added some comments to address them.
Awesome job, btw, @omairvaiyani 🚀 :)
@davimacedo GraphQL API LGTM 👍
@douglasmuraoka thanks! hope the changes are sufficient |
Sure it is 👍, thanks! :) |
@omairvaiyani Good job! Would you also be willed to write something about it in the GraphQL guide? |
@davimacedo Thanks! And Yup I'm working on the docs now |
This PR empowers the Parse GraphQL API with custom user-defined schema. The developers can now write their own types, queries, and mutations, which will merged with the ones that are automatically generated. The new types are resolved by the application's cloud code functions. Therefore, regarding parse-community#5777, this PR closes the cloud functions needs and also addresses the graphql customization topic. In my view, I think that this PR, together with parse-community#5782 and parse-community#5818, when merged, closes the issue. How it works: 1. When initializing ParseGraphQLServer, now the developer can pass a custom schema that will be merged to the auto-generated one: ``` parseGraphQLServer = new ParseGraphQLServer(parseServer, { graphQLPath: '/graphql', graphQLCustomTypeDefs: gql` extend type Query { custom: Custom @namespace } type Custom { hello: String @resolve hello2: String @resolve(to: "hello") userEcho(user: _UserFields!): _UserClass! @resolve } `, }); ``` Note: - This PR includes a @namespace directive that can be used to the top level field of the nested queries and mutations (it basically just returns an empty object); - This PR includes a @resolve directive that can be used to notify the Parse GraphQL Server to resolve that field using a cloud code function. The `to` argument specifies the function name. If the `to` argument is not passed, the Parse GraphQL Server will look for a function with the same name of the field; - This PR allows creating custom types using the auto-generated ones as in `userEcho(user: _UserFields!): _UserClass! @resolve`; - This PR allows to extend the auto-generated types, as in `extend type Query { ... }`. 2. Once the schema was set, you just need to write regular cloud code functions: ``` Parse.Cloud.define('hello', async () => { return 'Hello world!'; }); Parse.Cloud.define('userEcho', async req => { return req.params.user; }); ``` 3. Now you are ready to play with your new custom api: ``` query { custom { hello hello2 userEcho(user: { username: "somefolk" }) { username } } } ``` should return ``` { "data": { "custom": { "hello": "Hello world!", "hello2": "Hello world!", "userEcho": { "username": "somefolk" } } } } ```
* add parse-graph-ql configuration for class schema customisation Not yet tested - essentially an RFC * refactor and add graphql router, controller and config cache * fix(GraphQLController): add missing check isEnabled * chore(GraphQLController): remove awaits from cache put * chore(GraphQLController): remove check for if its enabled * refactor(GraphQLController): only use cache if mounted * chore(GraphQLController): group all validation errors and throw at once * chore(GraphQLSchema): move transformations into controller validation * refactor(GraphQL): improve ctrl validation and fix schema usage of config * refactor(GraphQLSchema): remove code related to additional schema This code has been moved into a separate feature branch. * fix(GraphQLSchema): fix incorrect default return type for class configs * refactor(GraphQLSchema): update staleness check code to account for config * fix(GraphQLServer): fix regressed tests due to internal schema changes This will be followed up with a backwards compatability fix for the `ClassFields` issue to avoid breakages for our users * refactor: rename to ParseGraphQLController for consistency * fix(ParseGraphQLCtrl): numerous fixes for validity checking Also includes some minor code refactoring * chore(GraphQL): minor syntax cleanup * fix(SchemaController): add _GraphQLConfig to volatile classes * refactor(ParseGraphQLServer): return update config value in setGraphQLConfig * testing(ParseGraphQL): add test cases for new graphQLConfig * fix(GraphQLController): fix issue where config with multiple items was not being mapped to the db * fix(postgres): add _GraphQLConfig default schema on load fixes failing postgres tests * GraphQL @mock directive (parse-community#5836) * Add mock directive * Include tests for @mock directive * Fix existing tests due to the change from ClassFields to ClassCreateFields * fix(parseClassMutations): safer type transformation based on input type * fix(parseClassMutations): only define necessary input fields * fix(GraphQL): fix incorrect import paths
Progress
Completed:
"_GraphQLConfig"
ParseGraphQLServer#setGraphQLConfig
{ enabledForClasses: ["_User", "City", "Car"] }
{ disabledForClasses: ["SecurityToken"] }
{ classConfigs: [{ className: "_User", query: { find: false }, mutation: { destroy: false }, type: { inputFields: ["name", "city", "utm"], outputFields: ["name", "city"] } }] }
Work left:
parseGraphQLConfig
JSON before it is stored in the databaseNot yet tested - this is essentially a functional, but work-in-progress RFC.
Further the discussion here, I have implemented a set of config options that briefly speaking, allow:
Please see the exact configuration in /src/Options/index.js
Internally, the only non-backwards compatible change is due to the introduction of input types,
CreateFields
andUpdateFields
, in favour of the singularInputFields
. This will require certain test-case refactoring, but this, nor any other change should impact the end-user. Those who skip the configuration entirely will still get the exact same results from their pre-existing queries.The one config option that is yet to be implemented is
additionalSchema
, which I've described as a function that resolves an entirely separateGraphQLSchema
that could be appended, stitched or federated into the auto-generated schema. I'm still figuring out the use-case and feasibility of this feature.I welcome comments for improving this PR before I begin working on unit tests!