Commit Graph

15 Commits

Author SHA1 Message Date
Antoine Leblanc
42e5205eb5 server: reduce schema contexts to the bare minimum
### Description

This monster of a PR took way too long. As the title suggests, it reduces the schema context carried in the readers to the very strict minimum. In practice, that means that to build a source, we only require:
  - the global `SchemaContext`
  - the global `SchemaOptions` (soon to be renamed `SchemaSourceOptions`)
  - that source's `SourceInfo`

Furthermore, _we no longer carry "default" customization options throughout the schema_. All customization information is extracted from the `SourceInfo`, when required. This prevents an entire category of bugs we had previously encountered, such as parts of the code using uninitialized / unupdated customization info.

In turn, this meant that we could remove the explicit threading of the `SourceInfo` throughout the schema, since it is now always available through the reader context.

Finally, this meant making a few adjustments to relay and actions as well, such as the introduction of a new separate "context" for actions, and a change to how we create some of the action-specific postgres scalar parsers.

I'll highlight with review comments the areas of interest.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6709
GitOrigin-RevId: ea80fddcb24e2513779dd04b0b700a55f0028dd1
2022-11-17 10:35:54 +00:00
Samir Talwar
342391f39d Upgrade Ormolu to v0.5.
This upgrades the version of Ormolu required by the HGE repository to v0.5.0.1, and reformats all code accordingly.

Ormolu v0.5 reformats code that uses infix operators. This is mostly useful, adding newlines and indentation to make it clear which operators are applied first, but in some cases, it's unpleasant. To make this easier on the eyes, I had to do the following:

* Add a few fixity declarations (search for `infix`)
* Add parentheses to make precedence clear, allowing Ormolu to keep everything on one line
* Rename `relevantEq` to `(==~)` in #6651 and set it to `infix 4`
* Add a few _.ormolu_ files (thanks to @hallettj for helping me get started), mostly for Autodocodec operators that don't have explicit fixity declarations

In general, I think these changes are quite reasonable. They mostly affect indentation.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6675
GitOrigin-RevId: cd47d87f1d089fb0bc9dcbbe7798dbceedcd7d83
2022-11-02 20:55:13 +00:00
Samir Talwar
96a768e526 Upgrade HLint to v3.4.1.
This upgrades CI and anyone using Nix to HLint v3.4.1.

If you're not using Nix, this doesn't actually _do_ anything on your
local machine; it's just a suggestion.

It also applies a bunch of simple HLint refactors, using
`make lint-hs-fix`.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6324
GitOrigin-RevId: de8267e4909d6dcd3f83543188517f3aaeebc5f3
2022-10-12 19:14:56 +00:00
Daniel Harvey
e2ced4011d server: import local Postgres modules as Postgres
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5962
GitOrigin-RevId: 862862c34b6c633c94ee8ae1f075afca2799fd2b
2022-09-21 11:35:50 +00:00
Antoine Leblanc
512a4dbb92 Use a different reader context for sources and remote schemas with new SchemaT monad.
### Description

This PR changes all the schema code to operate in a specific `SchemaT` monad, rather than in an arbitrary `m` monad. `SchemaT` is intended to be used opaquely with `runSourceSchema` and `runRemoteSchema`. The main goal of this is to allow a different reader context per part of the schema: this PR also minimizes the contexts. This means that we no longer require `SchemaOptions` when building remote schemas' schema, and this PR therefore removes a lot of dummy / placeholder values accordingly.

### Performance and stacking

This PR has been through several iterations. #5339 was the original version, that accomplished the same thing by stacking readers on top of the stack at every remote relationship boundary. This raised performance concerns, and @0x777 confirmed with an ad-hoc test that in some extreme cases we could see up to a 10% performance impact. This version, while more verbose, allows us to unstack / re-stack the readers, and avoid that problem. #5517 adds a new benchmark set to be able to automatically measure this on every PR.

### Remaining work

- [x] a comment (or perhaps even a Note?) should be added to `SchemaT`
- [x] we probably want for #5517 to be merged first so that we can confirm the lack of performance penalty

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5458
GitOrigin-RevId: e06b83d90da475f745b838f1fd8f8b4d9d3f4b10
2022-09-06 16:49:23 +00:00
Philip Lykke Carlsen
4431fb5ea9 Add schema implementation for Aggregation Predicates
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5259
GitOrigin-RevId: f53e310951fa4eb7570006d8c616398a98078632
2022-08-22 15:59:21 +00:00
paritosh-08
9d23a10f33 server: fix behaviour of custom table name for graphql-default naming convention
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5290
GitOrigin-RevId: bc398989d82a0e78bfcf87d5aa81bcd6a709c67f
2022-08-17 12:47:49 +00:00
Antoine Leblanc
666f4cacb7 Only require MkRootFieldName in top-level functions (that make root fields)
### Description

By definition, root fields are at the root of the schema: only functions that craft root fields need to know about how to customize the name of root fields. However, the presence of `Has MkRootFieldName` in `MonadBuildSchemaBase` meant that the entirety of the schema building code was implicitly aware of / capable of altering root field names.

This PR removes this constraint, and moves it to the functions that do craft root fields. This has several upsides:
- it makes it more explicit where root fields are being crafted
- it prevents functions that should not use this from mistakenly applying it to non-root fields
- it simplifies the shared schema context

### Future work

- can we maybe pass this as an argument, instead of making it a required part of the context?
- ~~AFAICT, we only ever use `mempty` for it: is this actually dead code that we should actually just remove altogether?~~

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5235
GitOrigin-RevId: 4268751f3ab87ae8e03b6fe9e1efa1b096200027
2022-08-03 19:09:50 +00:00
Antoine Leblanc
0a69db81c9 Move RoleName into SchemaContext.
### Description

I am not 100% sure about this PR; while I think the code is better this way, I'm willing to be convinced otherwise.

In short, this PR moves the `RoleName` field into the `SchemaContext`, instead of being a nebulous `Has RoleName` constraint on the reader monad. The major upside of this is that it makes it an explicit named field, rather than something that must be given as part of a tuple of arguments when calling `runReader`.

However, the downside is that it breaks the helper permissions functions of `Schema.Table`, which relied on `Has RoleName r`. This PR makes the choice of passing the role name explicitly to all of those functions, which in turn means first explicitly fetching the role name in a lot of places. It makes it more explicit when a schema building block relies on the role name, but is a bit verbose...

### Alternatives

Some alternatives worth considering:
- attempting something like `Has context r, Has RoleName context`, which would allow them to be independent from the context but still fetch the role name from the reader, but might require type annotations to not be ambiguous
- keeping the permission functions the same, with `Has RoleName r`, and introducing a bunch of newtypes instead of using tuples to explicitly implement all the required `Has` instances
- changing the permission functions to `Has SchemaContext r`, since they are functions used only to build the schema, and therefore may be allowed to be tied to the context.

What do y'all think?

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5073
GitOrigin-RevId: 8fd09fafb54905a4d115ef30842d35da0c3db5d2
2022-07-29 15:38:44 +00:00
Samir Talwar
f01c7f4ee6 server: Remove QErr and Code dependencies from the schema parsers.
In the process of decoupling the schema parsers from the GraphQL Engine, we need to remove dependencies on `Hasura.Base.Error`.

First of all, we have avoided using `QErr` in schema parsers code, instead returning a more appropriate data type which can be converted to a `Hasura.Base.Error.QErr` later.

Secondly, we create a new `ParseErrorCode` type to represent parse failure types, which are then converted to a `Hasura.Base.Error.Code` later.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5181
GitOrigin-RevId: 8655e26adb1e7d5e3d552c77a8a403f987b53467
2022-07-27 12:26:44 +00:00
paritosh-08
d66abcefab server: transform enum with upper for graphql-default
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5032
GitOrigin-RevId: 206a673f4bd59700436f6160feeb7df415f637ad
2022-07-19 06:56:45 +00:00
Tom Harding
2e7e6fd98a Move SchemaOptions to its own module, remove magic bools
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5048
GitOrigin-RevId: f666a10c6af5feda9d761d3ffee5c77695361fdf
2022-07-14 17:59:01 +00:00
Samir Talwar
c980af1b8f Move MkTypename and NamingCase into their own modules.
This moves `MkTypename` and `NamingCase` into their own modules, with the intent of reducing the scope of the schema parsers code, and trying to reduce imports of large modules when small ones will do.

PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4978
GitOrigin-RevId: 19541257fe010035390f6183a4eaa37bae0d3ca1
2022-07-12 14:01:28 +00:00
Philip Lykke Carlsen
824697a1e8 Break up class BackendSchema in two
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4901
GitOrigin-RevId: fdef33b69626d1694dc1938bb76e001284dd1b50
2022-06-30 15:23:35 +00:00
Philip Lykke Carlsen
a409e79167 Move postgres specific schema into the Postgres namespace
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4895
GitOrigin-RevId: 62772d966bf2b4a4dcb26c1dd0bd4f30749b7c91
2022-06-29 16:05:35 +00:00