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
I didn't track why these were left behind. Presumably GHC 9.2 has an improved redundant constraint checker, so that explains a few. Otherwise, perhaps code got refactored along the way.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6256
GitOrigin-RevId: b6275edf3e867f8e33bdec533ce9932381d36bbb
- Remove a few unnecessary helper functions
- Delete kind annotations
- Bring GHC warnings and language extensions more in line with those of the `graphql-engine` library
- Constrain unconstrained dependency on `hasql-pool`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6251
GitOrigin-RevId: 10c2530f007f70cf1464cec36566ee2264589881
- Remove `onJust` in favor of the more general `for_`
- Remove `withJust` which was used only once
- Remove `hashNub` in favor of `Ord`-based `uniques`
- Simplify some of the implementations in `Hasura.Prelude`
- Add `hlint` hint from `maybe True` to `all`, and `maybe False` to `any`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6173
GitOrigin-RevId: 2c6ebbe2d04f60071d2a53a2d43c6d62dbc4b84e
This PR is the result of running the following commands:
```bash
$ git grep -l '".* : "' -- '*.hs' | xargs sed -i -E 's/(".*) : "/\1: "/'
$ scripts/dev.sh test --integration --accept
```
Also manually fixed a few tests and docs
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6148
GitOrigin-RevId: cf8b87605d41d9ce86613a41ac5fd18691f5a641
If we don't do this, we might end up applying metadata with a stale schema cache.
Following the principle of least surprise, replacing the metadata should probably compute inconsistencies with regards to the actual state of the database.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/6026
GitOrigin-RevId: ff7469d7d9857c8a9f517d5d0b6f1ecf463621b3
Result of executing the following commands:
```shell
# replace "as Q" imports with "as PG" (in retrospect this didn't need a regex)
git grep -lE 'as Q($|[^a-zA-Z])' -- '*.hs' | xargs sed -i -E 's/as Q($|[^a-zA-Z])/as PG\1/'
# replace " Q." with " PG."
git grep -lE ' Q\.' -- '*.hs' | xargs sed -i 's/ Q\./ PG./g'
# replace "(Q." with "(PG."
git grep -lE '\(Q\.' -- '*.hs' | xargs sed -i 's/(Q\./(PG./g'
# ditto, but for [, |, { and !
git grep -lE '\[Q\.' -- '*.hs' | xargs sed -i 's/\[Q\./\[PG./g'
git grep -l '|Q\.' -- '*.hs' | xargs sed -i 's/|Q\./|PG./g'
git grep -l '{Q\.' -- '*.hs' | xargs sed -i 's/{Q\./{PG./g'
git grep -l '!Q\.' -- '*.hs' | xargs sed -i 's/!Q\./!PG./g'
```
(Doing the `grep -l` before the `sed`, instead of `sed` on the entire codebase, reduces the number of `mtime` updates, and so reduces how many times a file gets recompiled while checking intermediate results.)
Finally, I manually removed a broken and unused `Arbitrary` instance in `Hasura.RQL.Network`. (It used an `import Test.QuickCheck.Arbitrary as Q` statement, which was erroneously caught by the first find-replace command.)
After this PR, `Q` is no longer used as an import qualifier. That was not the goal of this PR, but perhaps it's a useful fact for future efforts.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5933
GitOrigin-RevId: 8c84c59d57789111d40f5d3322c5a885dcfbf40e
The intent is to generalize `columnParser` to the point where it is the same across all backends, and then remove the interface in favor of a single implementation.
This extracts out `enumParser` and `possiblyNullable` as the two main areas that differ across backends. We may split `possiblyNullable` further so that we can extract some of that logic out into a common function too.
With these changes, the various `columnParser` implementations become semantically equivalent. They still do different things, and so reconciling them will require further changes.
Co-Authored-By: Antoine Leblanc <antoine@hasura.io>
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5841
GitOrigin-RevId: eec1770931eed5d72da70c97d7d0f00e33fa15d2
### Description
This PR attempts to fix several issues with source customization as it relates to remote relationships. There were several issues regarding casing: at the relationship border, we didn't properly set the target source's case, we didn't have access to the list of supported features to decide whether the feature was allowed or not, and we didn't have access to the global default.
However, all of that information is available when we build the schema cache, as we do resolve the case of some elements such as function names: we can therefore resolve source information at the same time, and simplify both the root of the schema and the remote relationship border.
To do this, this PR introduces a new type, `ResolvedSourceCustomization`, to be used in the Schema Cache, as opposed to the metadata's `SourceCustomization`, following a pattern established by a lot of other types.
### Remaining work and open questions
One major point of confusion: it seems to me that we didn't set the case at all across remote relationships, which would suggest we would use the case of the LHS source across the subset of the RHS one that is accessible through the remote relationship, which would in turn "corrupt" the parser cache and might result in the wrong case being used for that source later on. Is that assesment correct, and was I right to fix it?
Another one is that we seem not to be using the local case of the RHS to name the field in an object relationship; unless I'm mistaken we only use it for array relationships? Is that intentional?
This PR is also missing tests that would show-case the difference, and a changelog entry. To my knowledge, all the tests of this feature are in the python test suite; this could be the opportunity to move them to the hspec suite, but this might be a considerable amount of work?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5619
GitOrigin-RevId: 51a81b713a74575e82d9f96b51633f158ce3a47b
This abstracts `CircularT`'s test cases to work against "any" memoizer, and then runs them against `MemoizeT` as well.
Surprisingly (or not), this works without issue; `MemoizeT` passes all tests with a couple of extra instances.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5780
GitOrigin-RevId: 461880caf9220dc3f52d622a22e8b8bcd594e404
### 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
This includes TH.Lift instances.
I am motivated to make this change because `unordered-containers` is set to either v0.2.17.0 or v0.2.19.1 in nixpkgs-unstable.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5620
GitOrigin-RevId: 7fd3024fdbf6a948adbdf5f4187d47d5da9acbda
This PR expands the OpenAPI specification generated for metadata to include separate definitions for `SourceMetadata` for each native database type, and for DataConnector.
For the most part the changes add `HasCodec` implementations, and don't modify existing code otherwise.
The generated OpenAPI spec can be used to generate TypeScript definitions that distinguish different source metadata types based on the value of the `kind` properly. There is a problem: because the specified `kind` value for a data connector source is any string, when TypeScript gets a source with a `kind` value of, say, `"postgres"`, it cannot unambiguously determine whether the source is postgres, or a data connector. For example,
```ts
function consumeSourceMetadata(source: SourceMetadata) {
if (source.kind === "postgres" || source.kind === "pg") {
// At this point TypeScript infers that `source` is either an instance
// of `PostgresSourceMetadata`, or `DataconnectorSourceMetadata`. It
// can't narrow further.
source
}
if (source.kind === "something else") {
// TypeScript infers that this `source` must be an instance of
// `DataconnectorSourceMetadata` because `source.kind` does not match
// any of the other options.
source
}
}
```
The simplest way I can think of to fix this would be to add a boolean property to the `SourceMetadata` type along the lines of `isNative` or `isDataConnector`. This could be a field that only exists in serialized data, like the metadata version field. The combination of one of the native database names for `kind`, and a true value for `isNative` would be enough for TypeScript to unambiguously distinguish the source kinds.
But note that in the current state TypeScript is able to reference the short `"pg"` name correctly!
~~Tests are not passing yet due to some discrepancies in DTO serialization vs existing Metadata serialization. I'm working on that.~~
The placeholders that I used for table and function metadata are not compatible with the ordered JSON serialization in use. I think the best solution is to write compatible codecs for those types in another PR. For now I have disabled some DTO tests for this PR.
Here are the generated [OpenAPI spec](https://github.com/hasura/graphql-engine-mono/files/9397333/openapi.tar.gz) based on these changes, and the generated [TypeScript client code](https://github.com/hasura/graphql-engine-mono/files/9397339/client-typescript.tar.gz) based on that spec.
Ticket: [MM-66](https://hasurahq.atlassian.net/browse/MM-66)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5582
GitOrigin-RevId: e1446191c6c832879db04f129daa397a3be03f62
This does not yet enable Aggregation Predicates to users, but enables building the execution backend and tests of the schema.
This is a prerequisite for:
* #5174
* #5261
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5607
GitOrigin-RevId: e07beb01949724545131629c111d41a7ec4636f2
This makes it easier to refactor `BackendSchema`, because if the type of a type class method is changed, it's easier to update the corresponding dummy implementations.
Partially addresses hasura/graphql-engine-mono#2971, in the sense that this aids refactors.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5443
GitOrigin-RevId: 65e169d01415a04e7c419a628cf32e743448543d
The module `Hasura.SQL.AnyBackend` was introduced (in #751) to centralize the logic for case-switching behavior that depends on the particular flavor of relational DB backend (Postgres vs MSSQL vs BigQuery vs MySQL vs DataConnectors). This allows us to write a bunch of code in a backend-agnostic way, even if runtime behavior does depend on the chosen backend. At the same time, it allows us to write backend-specific code without having to care (too much) about the existence of other backends.
In #851 this module was rewritten to use Template Haskell.
I've heard that one of the reasons for the use of TH was that this would make it easier to keep backends out of the compilation product entirely. This would allow customers, especially on OSS, to benefit from simpler software licensing.
However:
1. This conditional compilation never materialized.
2. It's not clear whether writing this particular module based on TH would be sufficient for conditional compilation. And in any case, it can be done using CPP pragmas as well.
3. The TH code is extraordinarily complex. Since its introduction, it has been documented extraordinarily well, but it's still very difficult to maintain and/or refactor, due to its non-idiomatic nature.
4. Hasura's company objectives are now Cloud-oriented, so that software licensing issues work differently, and in particular, do not depend on what's part of the compilation product.
So this PR reverts on #851 by spelling out the code generated by TH. This is a net-negative diff size. IOW we used to generate less code than the size of the code doing the generating. This makes the code readable and maintainable.
The generated code has been modified in one way, which I'll now describe.
In the scenario that support for a new backend is introduced, a constructor is added to the `BackendType` type. This would then cause `liftTag` to be partial, thus raising a compiler warning. Resolving this requires adding corresponding constructors to the `BackendTag` and `AnyBackend` types. This would then require amending **almost** all other methods.
The exceptions are `composeAnyBackend` and `unpackAnyBackend`. These methods test whether two values are compatible, i.e. belong to the same backend. Both have a default case that in one way or another ignores the input values. Using TH here ensures that all values that belong together are caught. But after spelling out the TH, the presence of the default case means that no compiler warning is thrown for a missing match of matching values. So in the default case, we now do an explicit check for equality. If there _is_ an equality, that means that there is a missing `case`. So this is reported as an `error` (which is very crude, but it should be).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5333
GitOrigin-RevId: 5aaf0a93394bd740aa7371526d3175c8142b3541
### Description
This PR moves some strictness annotations to a concrete use site, rather than putting `seq` in an helper function.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5436
GitOrigin-RevId: 1f279e05333ab80167ad2e18d09b8792eddc52c3
### Description
This PR is a first step towards having a dedicated reader context per schema block. It adds the required Reader instance, and switches from a `SchemaT ReaderT` stack to a `ReaderT SchemaT` stack. Furthermore, it cleans up / harmonizes some of the top-level schema building functions.
Sources and remotes are now built each within their own run of `runReaderT`: for now, the reader context is the same in both cases, meaning no special care is required at the boundary of remote relationships.
Actions are explicitly run with the source context for now; we could envision creating a third and distinct context for them.
This PR is expected to be a no-op.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5300
GitOrigin-RevId: a014e5b3504eb4ef740c820d305d6d2695f622f7
It's about time.
To do this I had to check a few more boxes.
* I copied the flags from `graphql-engine.cabal` to the libraries in `server/lib`.
* I moved `Cacheable` instances of schema parser types beside the typeclass declaration.
* I removed imports of `Hasura.Prelude` from the tests, and rewrote them accordingly.
* I copied the `TestMonad` parse monad into `server/src-test/Hasura/GraphQL/Schema/RemoteTest.hs`, which was using it. I think this could be done with the real thing, but I tried replacing it with constraints and it messed with my head somewhat.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5311
GitOrigin-RevId: ebebcc50a16f2d517b7f730fe72410827ca3e86c
## Description ✍️
This PR fixes the config status update when the `Service configured successfully` message is written before the server is actually spawned. Now the status is updated only when the server is spawned successfully. To be specific, this change posts the status closer to where we log `starting API server`.
### Related Issues ✍
#2751
### Solution and Design ✍
We update the status inside `runHGEServer` function. This helps in adding the message only when the server is started. If any exception is thrown before the server is spawned, only that message is written to `config_status` table instead of the `Service configured successfully` message.
## Affected components ✍️
- ✅ Server
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5179
Co-authored-by: Naveen Naidu <30195193+Naveenaidu@users.noreply.github.com>
Co-authored-by: Anon Ray <616387+ecthiender@users.noreply.github.com>
GitOrigin-RevId: 7860008403aa0645583e26915f620b66a5bbc531
Followup to hasura/graphql-engine-mono#4713.
The `memoizeOn` method, part of `MonadSchema`, originally had the following type:
```haskell
memoizeOn
:: (HasCallStack, Ord a, Typeable a, Typeable b, Typeable k)
=> TH.Name
-> a
-> m (Parser k n b)
-> m (Parser k n b)
```
The reason for operating on `Parser`s specifically was that the `MonadSchema` effect would additionally initialize certain `Unique` values, which appear (nested in) the type of `Parser`.
hasura/graphql-engine-mono#518 changed the type of `memoizeOn`, to additionally allow memoizing `FieldParser`s. These also contained a `Unique` value, which was similarly initialized by the `MonadSchema` effect. The new type of `memoizeOn` was as follows:
```haskell
memoizeOn
:: forall p d a b
. (HasCallStack, HasDefinition (p n b) d, Ord a, Typeable p, Typeable a, Typeable b)
=> TH.Name
-> a
-> m (p n b)
-> m (p n b)
```
Note the type `p n b` of the value being memoized: by choosing `p` to be either `Parser k` or `FieldParser`, both can be memoized. Also note the new `HasDefinition (p n b) d` constraint, which provided a `Lens` for accessing the `Unique` value to be initialized.
A quick simplification is that the `HasCallStack` constraint has never been used by any code. This was realized in hasura/graphql-engine-mono#4713, by removing that constraint.
hasura/graphql-engine-mono#2980 removed the `Unique` value from our GraphQL-related types entirely, as their original purpose was never truly realized. One part of removing `Unique` consisted of dropping the `HasDefinition (p n b) d` constraint from `memoizeOn`.
What I didn't realize at the time was that this meant that the type of `memoizeOn` could be generalized and simplified much further. This PR finally implements that generalization. The new type is as follows:
```haskell
memoizeOn ::
forall a p.
(Ord a, Typeable a, Typeable p) =>
TH.Name ->
a ->
m p ->
m p
```
This change has a couple of consequences.
1. While constructing the schema, we often output `Maybe (Parser ...)`, to model that the existence of certain pieces of GraphQL schema sometimes depends on the permissions that a certain role has. The previous versions of `memoizeOn` were not able to handle this, as the only thing they could memoize was fully-defined (if not yet fully-evaluated) `(Field)Parser`s. This much more general API _would_ allow memoizing `Maybe (Parser ...)`s. However, we probably have to be continue being cautious with this: if we blindly memoize all `Maybe (Parser ...)`s, the resulting code may never be able to decide whether the value is `Just` or `Nothing` - i.e. it never commits to the existence-or-not of a GraphQL schema fragment. This would manifest as a non-well-founded knot tying, and this would get reported as an error by the implementation of `memoizeOn`.
tl;dr: This generalization _technically_ allows for memoizing `Maybe` values, but we probably still want to avoid doing so.
For this reason, the PR adds a specialized version of `memoizeOn` to `Hasura.GraphQL.Schema.Parser`.
2. There is no longer any need to connect the `MonadSchema` knot-tying effect with the `MonadParse` effect. In fact, after this PR, the `memoizeOn` method is completely GraphQL-agnostic, and so we implement hasura/graphql-engine-mono#4726, separating `memoizeOn` from `MonadParse` entirely - `memoizeOn` can be defined and implemented as a general Haskell typeclass method.
Since `MonadSchema` has been made into a single-type-parameter type class, it has been renamed to something more general, namely `MonadMemoize`. Its only task is to memoize arbitrary `Typeable p` objects under a combined key consisting of a `TH.Name` and a `Typeable a`.
Also for this reason, the new `MonadMemoize` has been moved to the more general `Control.Monad.Memoize`.
3. After this change, it's somewhat clearer what `memoizeOn` does: it memoizes an arbitrary value of a `Typeable` type. The only thing that needs to be understood in its implementation is how the manual blackholing works. There is no more semantic interaction with _any_ GraphQL code.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4725
Co-authored-by: Daniel Harvey <4729125+danieljharvey@users.noreply.github.com>
GitOrigin-RevId: 089fa2e82c2ce29da76850e994eabb1e261f9c92
### 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
For some reason these functions exist in `Backends.Postgres.SQL.Value`.
We don't want to depend on that module here.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5292
GitOrigin-RevId: a09bd3cdb0caf08938bce0728a8d281344c1d4ce
Moves code from `Hasura.RQL.Types.Metadata` that is specific to serialization into a new module, `Hasura.RQL.Types.Metadata.Serialization`.
I'm breaking up #5184 into smaller PRs. This is the third and final PR in that effort. This PR is stacked on #5210 and #5211.
The tracking issue is https://hasurahq.atlassian.net/browse/MM-35
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5212
GitOrigin-RevId: 6cde6d52173590fafe0969a06f2a3411db4fbc78
Introduces a new function, `metadataToDTO`, that converts a `Metadata` value to a `MetadataV3` DTO value. This is the next step in the alternative serialization path for metadata that comes with a generated OpenAPI specification.
This PR carves up the existing `metadataToOrdJSON` function so that helpers previously embedded in the `where` block of that function can also be used in the implementation of `metadataToDTO`. If I did everything correctly `metadataToOrdJSON` should behave exactly as before.
In a followup PR I will move the extracted helpers to a new submodule, `Hasura.RQL.Types.Metadata.Serialization`, since they add up to several hundred lines of code.
I'm breaking up #5184 into smaller PRs, and this is the second PR in that effort. This PR is stacked on #5210.
The tracking issue is https://hasurahq.atlassian.net/browse/MM-35
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5211
GitOrigin-RevId: 2596ed5312d7b1232c47ae1d08a51d8ead11fcb8
A following PR moves serialization-related code out `Hasura.RQL.Types.Metadata` into a specialized submodule. To avoid circular dependencies a number of other definitions also need to be moved into their own submodule. This PR does that extra moving first so that we can keep each PR as small, and as easy to review as possible.
There are a lot of changed lines; but it's all moving code from one module to another.
I'm breaking up #5184 into smaller PRs, and this is the first PR in that effort.
The tracking issue is https://hasurahq.atlassian.net/browse/MM-35
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5210
GitOrigin-RevId: 6fb6e29a967ab5ad4724006c8e0addd2d63a3946
### 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
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
Updates to the latest version of autodocodec and uses the new features, in particular `discriminatedUnionCodec`.
This allows us to remove the `ValueWrapper*` types and `sumTypeCodec`. Sum types are now encoded as discriminated unions.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5155
GitOrigin-RevId: 20bfdc12b28d35db354c4a149b9175fab0b2b7d2
This is now the sole in-universe dependency of the schema parsers. As
such, we need to extract it as a library before we can extract the
schema parsers as a library.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5202
GitOrigin-RevId: fbe571855768e56dc8b8e259b8efe900de3ecc54
Makes the init specs shorter by using `shouldBe` instead of `shouldSatisfy` wherever possible. This also makes test failures more expressive.
It also simplifies boolean logic in most places, following HLint warnings. These changes brought to you by `hlint --refactor`, which is basically magic.
I have left some redundancy in the boolean logic for clarity, along with the appropriate HLint suppressions.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5087
GitOrigin-RevId: 52bf3626be2615e6a32a0fc0e8be19cca31ee4ad
This introduces an `ErrorMessage` newtype which wraps `Text` in a manner which is designed to be easy to construct, and difficult to deconstruct.
It provides functionality similar to `Data.Text.Extended`, but designed _only_ for error messages. Error messages are constructed through `fromString`, concatenation, or the `toErrorValue` function, which is designed to be overridden for all meaningful domain types that might show up in an error message. Notably, there are not and should never be instances of `ToErrorValue` for `String`, `Text`, `Int`, etc. This is so that we correctly represent the value in a way that is specific to its type. For example, all `Name` values (from the _graphql-parser-hs_ library) are single-quoted now; no exceptions.
I have mostly had to add `instance ToErrorValue` for various backend types (and also add newtypes where necessary). Some of these are not strictly necessary for this changeset, as I had bigger aspirations when I started. These aspirations have been tempered by trying and failing twice.
As such, in this changeset, I have started by introducing this type to the `parseError` and `parseErrorWith` functions. In the future, I would like to extend this to the `QErr` record and the various `throwError` functions, but this is a much larger task and should probably be done in stages.
For now, `toErrorMessage` and `fromErrorMessage` are provided for conversion to and from `Text`, but the intent is to stop exporting these once all error messages are converted to the new type.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5018
GitOrigin-RevId: 84b37e238992e4312255a87ca44f41af65e2d89a
This removes the one remaining instance of `unsafeMkName` in production
code, uses `G.name` where possible in tests, and ignores instances where
it's not possible (such as `instance Arbitrary G.Name`).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5074
GitOrigin-RevId: d8049edf1f1bc2ef25f34874ef5bd5a5934bd33d
### Description
A trivial PR, extracted out of #4936, that removes remote schema permissions from the schema context, as they are only ever used at the top level: whether or not we need to use remote schema permissions is not something that impacts _how_ we build the schema, but whether some parts of the schema should be built at all, and therefore doesn't need to be accessible throughout the build process.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/5050
GitOrigin-RevId: 734673370393d5640ad753222982baf2698f6d8f
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
Earlier, if the `select` root field had a custom root field set, the same custom root field was then used for the streaming subscription root field as well. This leads to duplicate root fields being generated in the `subscription_root`.
This PR fixes that. It provides a way to customize the streaming subscription root field and not use the `select` root field's custom root field name for the streaming subscription root field.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4967
Co-authored-by: Anon Ray <616387+ecthiender@users.noreply.github.com>
GitOrigin-RevId: 54e74ce97561b0e5cfdfc60d1ca340aaebecf7d4
This reduces the usage of "utils" modules in the parsers code, especially those that are simply re-exported from elsewhere, to facilitate extracting the parsers code into its own library.
It mostly inlines the imports that are re-exported from `Hasura.Prelude` and `Data.Parser.JSONPath`. It also removes references to `Data.*.Extended` modules. When necessary, it re-implements the functionality (which is typically trivial).
It does not tackle all external dependencies. I observed the following that will take more work:
- `Data.GADT.Compare.Extended`
- `Data.Text.Extended`
- `Hasura.Base.Error`
- `Hasura.RQL.Types.Common`
- `Hasura.Server.Utils`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4964
GitOrigin-RevId: 54ad3c1b7a31f13e34340ebe9fcc36d0ad57b8bd
This improves `parseJSONPath` and `encodeJSONPath` to encode special characters appropriately by delegating to Aeson.
This also makes a couple of improvements to `encodeJSONPath`.
1. The function is moved from `Hasura.Base.Error` to `Data.Parser.JSONPath`. This still doesn't seem too appropriate but it is somewhat better. I am basing this on the fact that its test cases already lived in `Data.Parser.JSONPathSpec`.
2. It now returns `Text`, not `String`.
4. It quotes strings with double quotes (`"`) rather than single quotes (`'`), just like JSON.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4935
GitOrigin-RevId: bf44353cd740500245f2e38907a7d6263ae0291c
This reflects the two different usages, which should not be conflated.
We also propagate the type a little more, to avoid `Text`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4931
GitOrigin-RevId: 16278f14aa4c2cb5667ea54bbb6b25e6d362835c
We only use these `Show` instances in error messages (where we call
`show` explicitly anyway) and test cases (in which Hspec requires `Show
a` for any `a` in an assertion).
This removes the instance in favor of a custom `showQErr` function
(which serializes the error to JSON). It is then used in certain error
message production which previously called `show` on a `QErr`.
There are two places where we serialize a QErr and then construct a new
QErr from the resulting string. Instead, we modify the existing QErr to
add extra information.
An orphan `Show QErr` instance is retained for tests so that we can have
nice test failure messages.
This is preparation for future changes in which the error message within
`QErr` will not be exposed directly, and therefore will not have a
`Show` instance. That said, it feels like a sensible kind of cleanup
anyway.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4897
GitOrigin-RevId: 8f79f7a356f0aea571156f39aefac242bf751f3a
The definition of the Parse monad (which implements MonadParse) can be simplified from using two monad transformers to a single monad. We can simplify from:
```haskell
newtype Parse a = Parse
{ unParse :: ReaderT JSONPath (Except ParseError) a }
```
to
```haskell
newtype Parse a = Parse
{ unParse :: Except ParseError a }
```
In other words, we don't actually need a Reader monad at all.
The technique is that rather than _always_ keeping track of the `JSONPath` while traversing the query, instead simply wait until an error occurs, and if it does, we adjust its `JSONPath` while we're unrolling the stack, using `withExceptT`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4807
Co-authored-by: Daniel Harvey <4729125+danieljharvey@users.noreply.github.com>
GitOrigin-RevId: 94de2c97dc65cb0bd918050cf5e99ac62168b331
### Description
This PR rewrites OpenAPI to be more idiomatic. Some noteworthy changes:
- we accumulate all required information during the Analyze phase, to avoid having to do a single lookup in the schema cache during the OpenAPI generation phase (we now only need the schema cache as input to run the analysis)
- we no longer build intermediary endpoint information and aggregate it, we directly build the the `PathItem` for each endpoint; additionally, that means we no longer have to assume that different methods have the same metadata
- we no longer have to first declare types, then craft references: we do everything in one step
- we now properly deal with nullability by treating "typeName" and "typeName!" as different
- we add a bunch of additional fields in the generated "schema", such as title
- we do now support enum values in both input and output positions
- checking whether the request body is required is now performed on the fly rather than by introspecting the generated schema
- the methods in the file are sorted by topic
### Controversial point
However, this PR creates some additional complexity, that we might not want to keep. The main complexity is _knot-tying_: to avoid lookups when generating the OpenAPI, it builds an actual graph of input types, which means that we need something similar to (but simpler than) `MonadSchema`, to avoid infinite recursions when analyzing the input types of a query. To do this, this PR introduces `CircularT`, a lesser `SchemaT` that aims at avoiding ever having to reinvent this particular wheel ever again.
### Remaining work
- [x] fix existing tests (they are all failing due to some of the schema changes)
- [ ] add tests to cover the new features:
- [x] tests for `CircularT`
- [ ] tests for enums in output schemas
- [x] extract / document `CircularT` if we wish to keep it
- [x] add more comments to `OpenAPI`
- [x] have a second look at `buildVariableSchema`
- [x] fix all missing diagnostics in `Analyze`
- [x] add a Changelog entry?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4654
Co-authored-by: David Overton <7734777+dmoverton@users.noreply.github.com>
GitOrigin-RevId: f4a9191f22dfcc1dccefd6a52f5c586b6ad17172
…fix #5426"
This reverts commit f85742318167d1e51f463c45fcd00f26269c2555.
## Description ✍️
With this commit there is the possiblity that you could get conflicting
type definitions with remote schemas. Reverting for now as we determine
a solution. At which point we will add this back in.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4879
Co-authored-by: Gil Mizrahi <8547573+soupi@users.noreply.github.com>
GitOrigin-RevId: 932b4a9226717c826d4bde7e375695354cee8c0c
This came about as I tried to add an instance over catalog versions and
found they were just simple integers most of the time (and in one case,
a float).
I think this change also clarifies how catalog versions work.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4864
GitOrigin-RevId: a6b7db86de564b71a8c2b602bee6a456b8e20d63
The code that builds the GraphQL schema, and `buildGQLContext` in particular, is partial: not every value of `(ServerConfigCtx, GraphQLQueryType, SourceCache, HashMap RemoteSchemaName (RemoteSchemaCtx, MetadataObject), ActionCache, AnnotatedCustomTypes)` results in a valid GraphQL schema. When it fails, we want to be able to return better error messages than we currently do.
The key thing that is missing is a way to trace back GraphQL type information to their origin from the Hasura metadata. Currently, we have a number of correctness checks of our GraphQL schema. But these correctness checks only have access to pure GraphQL type information, and hence can only report errors in terms of that. Possibly the worst is the "conflicting definitions" error, which, in practice, can only be debugged by Hasura engineers. This is terrible DX for customers.
This PR allows us to print better error messages, by adding a field to the `Definition` type that traces the GraphQL type to its origin in the metadata. So the idea is simple: just add `MetadataObjId`, or `Maybe` that, or some other sum type of that, to `Definition`.
However, we want to avoid having to import a `Hasura.RQL` module from `Hasura.GraphQL.Parser`. So we instead define this additional field of `Definition` through a new type parameter, which is threaded through in `Hasura.GraphQL.Parser`. We then define type synonyms in `Hasura.GraphQL.Schema.Parser` that fill in this type parameter, so that it is not visible for the majority of the codebase.
The idea of associating metadata information to `Definition`s really comes to fruition when combined with hasura/graphql-engine-mono#4517. Their combination would allow us to use the API of fatal errors (just like the current `MonadError QErr`) to report _inconsistencies_ in the metadata. Such inconsistencies are then _automatically_ ignored. So no ad-hoc decisions need to be made on how to cut out inconsistent metadata from the GraphQL schema. This will allow us to report much better errors, as well as improve the likelihood of a successful HGE startup.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4770
Co-authored-by: Samir Talwar <47582+SamirTalwar@users.noreply.github.com>
GitOrigin-RevId: 728402b0cae83ae8e83463a826ceeb609001acae
This implements an initial set of DTO types that represent serialized metadata. These new types come with codecs using autodocodec which are used to derive both JSON serialization, and OpenAPI documentation. This ensures that we can automatically generate API documentation that is guaranteed to match JSON produced by the server.
For the moment the new types are not used for anything except to generate an early version of an OpenAPI document. Because this is early work the DTO types for each metadata format version list top-level properties only with placeholders for the types of each top-level property. This early iteration demonstrates using a sum type in Haskell that maps to a tagged union in OpenAPI (using the `version` field value as a tag).
This work is experimental and incomplete! Please do not incorporate the generated OpenAPI documentation into essential workflows at this time.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4801
GitOrigin-RevId: d2f110a6237b73520cdba24667333ef14e8cdd3d
### Description
This PR removes the need for the `SourceCache` when building the schema for the actions. To do so, it changes the way we represent custom types in the source cache. Instead of trying to reuse the same `ObjectTypeDefinition` and `TypeRelationship`. we now have separate `AnnotatedObjectType` and `AnnotatedRelationship`. When building them, at schema cache building time, we persist all the relevant source information, so that it's all available at schema building time.
This PR makes no attempt at re-using `RemoteRelationship` primitives, to avoid having to change the way async action queries are executed, and to avoid having to make complicated changes to how we parse and represent those relationships.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4813
GitOrigin-RevId: 3cc65c5a043c8d3da5f7214eed40c558c4349327
Pretty much all quasi-quoted names in the server code base have ended up in `Hasura.GraphQL.Parser.Constants`. I'm now finding this unpleasant for two reasons:
1. I would like to factor out the parser code into its own Cabal package, and I don't want to have to expose all these names.
2. Most of them really have nothing to do with the parsers.
In order to remedy this, I have:
1. moved the names used by parser code to `Hasura.GraphQL.Parser.DirectiveName`, as they're all related to directives;
2. moved `Hasura.GraphQL.Parser.Constants` to `Hasura.Name`, changing the qualified import name from `G` to `Name`;
3. moved names only used in tests to the appropriate test case;
4. removed unused items from `Hasura.Name`; and
5. grouped related names.
Most of the changes are simply changing `G` to `Name`, which I find much more meaningful.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4777
GitOrigin-RevId: a77aa0aee137b2b5e6faec94495d3a9fbfa1348b
## Description ✍️
The `--help` header was out of date. This PR updates it to match the description on github.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4831
GitOrigin-RevId: 449f71b2901318132e45445632b1a8af358c0afb
### Description
This small clean-up PR makes one further step towards backend-agnostic actions: it makes all the code parsing custom types backend agnostic. Surprisingly, this could be done *without* the need to finish generalizing the column parser. The remaining sore point is async queries, that still target Postgres explicitly.
In theory, this is enough to start allowing non-Postgres scalars in custom types. In practice, however:
- no other backend exposes scalars in a way that would allow users to do that as of this PR;
- we currently have no strategy to avoid / detect scalar collisions across backends.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4691
GitOrigin-RevId: bfe63fb131e306663d4406697ce23c02736566c5
>
## Description ✍️
->
Fleshes out the Capabilities types used by Data Connector agents.
### Related Issues ✍
->
https://hasurahq.atlassian.net/browse/GDW-85
### Solution and Design ✍
>
Capabilities are divided into sections for queries, mutation, subscriptions, filtering and relationships:
```haskell
data Capabilities = Capabilities
{ cQueries :: Maybe QueryCapabilities,
cMutations :: Maybe MutationCapabilities,
cSubscriptions :: Maybe SubscriptionCapabilities,
cFiltering :: Maybe FilteringCapabilities,
cRelationships :: Maybe RelationshipCapabilities
}
```
Each section is optional. If the section is absent it means the the related capabilities are not supported by the agent.
The types for each section can contain additional details about exactly what is supported. E.g.
```haskell
data QueryCapabilities = QueryCapabilities
{ qcSupportsPrimaryKeys :: Bool
}
```
These are currently mostly empty, but will be filled in later.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4671
GitOrigin-RevId: 3d48570266bfce5e177a543a0ed6f63a7b450f0b
When a user changes request options in a custom action through the
console that triggers a request to the server to test webhook transform
options, and to show a preview of the result. If the action uses an
environment variable in its webhook URL, and there is no mock value for
that variable in the action's sample context then the user will see an
error. This change expands the error message to explain what caused the
error, and how to fix it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4556
GitOrigin-RevId: 75b19bae17aac982c2bdfbd4417bd55923889f2f
## Description
Following on from #4572, this removes more dead code as identified by Weeder. Comments and thoughts similarly welcome!
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4587
GitOrigin-RevId: 73aa6a5a2833ee41d29b71fcd0a72ed19822ca73
(Work here originally done by awjchen, rebased and fixed up for merge by
jberryman)
This is part of a merge train towards GHC 9.2 compatibility. The main
issue is the use of the new abstract `KeyMap` in 2.0. See:
https://hackage.haskell.org/package/aeson-2.0.3.0/changelog
Alex's original work is here:
#4305
BEHAVIOR CHANGE NOTE: This change causes a different arbitrary ordering
of serialized Json, for example during metadata export. CLI users care
about this in particular, and so we need to call it out as a _behavior
change_ as we did in v2.5.0. The good news though is that after this
change ordering should be more stable (alphabetical key order).
See: https://hasurahq.slack.com/archives/C01M20G1YRW/p1654012632634389
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4611
Co-authored-by: awjchen <13142944+awjchen@users.noreply.github.com>
GitOrigin-RevId: 700265162c782739b2bb88300ee3cda3819b2e87
### Description
This PR is a first step in a series of cleanups of action relationships. This first step does not contain any behavioral change, and it simply reorganizes / prunes / rearranges / documents the code. Mainly:
- it divides some files in RQL.Types between metadata types, schema cache types, execution types;
- it renames some types for consistency;
- it minimizes exports and prunes unnecessary types;
- it moves some types in places where they make more sense;
- it replaces uses of `DMap BackendTag` with `BackendMap`.
Most of the "movement" within files re-organizes declarations in a "top-down" fashion, by moving all TH splices to the end of the file, which avoids order or declarations mattering.
### Optional list types
One main type change this PR makes is a replacement of variant list types in `CustomTypes.hs`; we had `Maybe [a]`, or sometimes `Maybe (NonEmpty a)`. This PR harmonizes all of them to `[a]`, as most of the code would use them as such, by doing `fromMaybe []` or `maybe [] toList`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4613
GitOrigin-RevId: bc624e10df587eba862ff27a5e8021b32d0d78a2
## Motivation
This PR rewrites most of Relay to achieve the following:
- ~~fix a bug in which the same node id could refer to two different tables in the schema~~
- remove one of the few remaining uses of the source cache in the schema building code
In doing so, it also:
- simplifies the `BackendSchema` class by removing `node` from it,
- makes it much easier for other backends to support Relay,
- documents, re-organizes, and clarifies the code.
## Description
This PR introduces a new `NodeId` version ~~, and adapts the Postgres code to always generate this V2 version~~. This new id contains the source name, in addition to the table name, in order to disambiguate similar table names across different sources (which is now possible with source customization). In doing so, it now explicitly handles that case for V1 node ids, and returns an explicit error message instead of running the risk of _silently returning the wrong information_.
Furthermore, it adapts `nodeField` to support multiple backends; most of the code was trivial to generalize, and as a result it lowers the cost of entry for other backends, that now only need to support `AFNodeId` in their translation layer.
Finally, it removes one more cycle in the schema building code, by using the same trick we used for remote relationships instead of using the memoization trick of #4576.
## Remaining work
- ~~[ ]write a Changelog entry~~
- ~~[x] adapt all tests that were asserting on an old node id~~
## Future work
This PR was adapted from its original form to avoid a breaking change: while it introduces a Node ID V2, we keep generating V1 IDs and the parser rejects V2 IDs. It will be easy to make the switch at a later data in a subsequent PR.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4593
GitOrigin-RevId: 88e5cb91e8b0646900547fa8c7c0e1463de267a1
>
## Description ✍️
- Creates a new `/capabilities` endpoint for the GDC agent API
- Removes capabilities from the `/schema` endpoint
- Removes the `/config-schema` endpoint and includes the `ConfigSchemaResponse` within the `CapabilitiesResponse`
### Related Issues ✍
->
https://hasurahq.atlassian.net/browse/GDW-85
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4478
GitOrigin-RevId: 426662ee9e751343d94207d439a5025df65d2de7
## Description
This PR adds a config file for [`weeder`](https://github.com/ocharles/weeder) to the `-mono` repository. `weeder` checks for dead code by building a call graph from the given entry points (currently every module named `Main` with a `main` function) and then marking every function _not_ in that call graph as dead code.
To avoid very large PRs, I'm going to tackle this in a series. This first PR adds the basic configuration, plus removes as many weeds as it took for me to realise this was going to become a very big PR. The PRs after this will largely be removing dead code, until the final PR that will add Weeder to the CI pipeline.
### Related Issues
This closes#2973.
## Affected components
- Server
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4572
GitOrigin-RevId: ac8eaa9473e5ac1f16babcb35388694392d0d7dc
This is a first step towards clarifying the role of `UnpreparedValue` as part of the IR. It certainly does not belong in the parser framework.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4588
GitOrigin-RevId: d1582a0b266729b79e00d31057178a4099168e6d
### Description
When generalizing the code, back in late 2020, we over-eagerly generalized parts of the code that are specific to RQL's DML. This was in part due to the fact that, at the time, the DML types were all mixed alongside other types in `RQL.Types`. As a result, a lot of `RQL.DML.Internal` was generic over the backend type, instead of being specialized to `'Postgres 'Vanilla`.
A consequence of this is that, before this PR, `DML.Internal` ended up having a dependency on non-Postgres backends, due to the use of `annBoolExp`, which requires a `BackendMetadata` instance. Since the code was written in a generic manner, `DML.Internal` in turn depended on having the metadata instances in scope... This PR changes that to, instead, explicitly import the Postgres instance.
(Note that this module didn't import `RQL.Types.Metadata.Instances`, but depends on a module that imports it, and **orphan instances are transitively imported**, as evidenced by the need for that explicit import in #4568.)
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4573
GitOrigin-RevId: 7b82b5d7c23c03654518a1816802d400f37c3c64
### Description
The main goal of this PR is, as stated, to remove the circular dependency in the schema building code. This cycle arises from the existence of remote relationships: when we build the schema for a source A, a remote relationship might force us to jump to the schema of a source B, or some remote schema. As a result, we end up having to do a dispatch from a "leaf" of the schema, similar to the one done at the root. In turn, this forces us to carry along in the schema a lot of information required for that dispatch, AND it forces us to import the instances in scope, creating an import loop.
As discussed in #4489, this PR implements the "dependency injection" solution: we pass to the schema a function to call to do the dispatch, and to get a generated field for a remote relationship. That way, this function can be chosen at the root level, and the leaves need not be aware of the overall context.
This PR grew a bit bigger than that, however; in an attempt to try and remove the `SourceCache` from the schema altogether, it changed a lot of functions across the schema building code, to thread along the `SourceInfo b` of the source being built. This avoids having to do cache lookups within a given source. A few cases remain, such as relay, that we might try to tackle in a subsequent PR.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4557
GitOrigin-RevId: 9388e48372877520a72a9fd1677005df9f7b2d72
### Description
There were several functions in `GraphQL.Schema.Common` that were unrelated to the schema building process, and were about metadata manipulation or dependency computation. Having those functions in the schema part of the code forces several places in the code to depend on the schema code, despite being completely unrelated.
This PR moves those functions where they make sense: alongside similar functions in `RQL.Types.*`, and rewrites `getRemoteDependencies` for clarity (it was using the term "indirect dependency" in a way that was inconsistent with the rest of the code).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4568
GitOrigin-RevId: 948a18cebbb337a8bb6367c1f2d2ef5628209d96
### Description
Several places in the code used `a /= []`, which is inelegant. To my surprise, hlint did not warn about this, despite the fact that it forces an `Eq` instance on the elements. This PR replaces all occurrences of that pattern with `not (null a)` and adds a lint warning for it.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4569
GitOrigin-RevId: 6471e75ade9e71e5d583a0dac7815c01870c696b
By generalizing the instances, they can be written as attached instance derivations, rather than standalone ones.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4518
GitOrigin-RevId: 7a387911cf6ad46fe6acd36648275d6c2c68ffe3
A very minor cleanup (came out of documenting the architecture of actions). Does what's mentioned in the title.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4451
GitOrigin-RevId: d480ff438256df468df65b43d15f92a30b14b997
Previously, these were represented with a HashMap, but supposedly that map can never be empty. Now, it uses NEHashMap, which carries the non-empty invariant behind a smart constructor.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4481
GitOrigin-RevId: 93ad9aaa9354f25a1ba10e8207ae19614e1e439e
## Description
As identified in hasura/graphql-engine#8096, the format string we used for timestamps was incorrect; we were using `%F`, which expands to `%Y-%m-%d`; but that meant that the year was not padded to four digits: `0001` would be represented simply as `1`. However, Postgres inteprets that `1` as `2001`, probably due to interpretation rules about two-digit years (in `25/12/01`, `01` is indeed `2001`).
```
# create table timestamp_test ( test timestamptz );
CREATE TABLE
# insert into timestamp_test values ('1-01-01T00:00:57Z');
INSERT 0 1
# select * from timestamp_test;
test
------------------------
2001-01-01 00:00:57+00
(1 row)
```
To fix this, this PR changes the format string to use `%0Y`, which always pads the year number with zeroes.
## Remaining work
- [x] write Changelog entry
- [ ] copy timestamp tests from the python suite into the hspec tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3536
GitOrigin-RevId: fa144111358339fd4a35b32d888c1d2c5b418ea6
### Description
As part of the cache building process, we create / update / migrate the catalog that each DB uses as a place to store event trigger information. The function that decides how this should be done was doing an explicit `case ... of` on the backend tag, instead of delegating to one of the backend classes. The downsides of this is that:
- it adds a "friction point" where the backend matters in the core of the engine, which is otherwise written to be almost entirely backend-agnostic
- it creates imports from deep in the engine to the `Backends`, which we try to restrict to a very small set of clearly identified files (the `Instances` files)
- it is currently implemented using a "catch all" default case, which might not always be correct for new backends
This PR makes the catalog updating process a part of `BackendMetadata`, and cleans the corresponding schema cache code.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4457
GitOrigin-RevId: 592f0eaa97a7c38f4e6d4400e1d2353aab12c97e
## Description
As the name suggests, `DML.Internal` contains internal implementation details of RQL's DML. However, a lot of unrelated parts of the codebase still use some of the code it contains. This PR fixes this, and removes all imports of `RQL.DML.Internal` from outside of `RQL.DML`. Most of the time, this involves moving a function out of `DML.Internal` to an underlying module (see `getRolePermInfo`) or moving a function _back_ into it (see `checkRetCols`).
This PR also clarifies a bit the situation with `withTyAnn` and `withTypeAnn` by renaming the former into `withScalarTypeAnn` and moving them together. Worth noting: there might be a bug lurking in that function, as it doesn't seem to use the proper type annotations for some extension types!
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4380
GitOrigin-RevId: c8ae5b4e8378fefc0bcccf778d97813df727d3cb
## Description
This PR removes `RQL.Types`, which was now only re-exporting a bunch of unrelated modules.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4363
GitOrigin-RevId: 894f29a19bff70b3dad8abc5d9858434d5065417
## Description
This small PR moves all functions in `RQL.Types.hs` to better locations. Most `askX` functions are moved alongside the `unsafe` functions they use. Several other functions are moved closer to their call site. `MetadataM` is moved alongside `Metadata`. This PR also documents the `ask` functions.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4355
GitOrigin-RevId: 0498a7e8f98e7a94af911dd375cad84ace7ddffa
### Description
`HasSystemDefined` is defined in `RQL.Types`, but only used in one place, `LegacyCatalog`, to avoid passing a boolean around. It is easily replaced by an ad-hoc `ReaderT`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4337
GitOrigin-RevId: 649d758bb2b18b39533429dda5ab71afde62fb53
### Description
Small PR that moves code out of `RQL.Types.hs`. Specifically, it moves `HasServerConfigCtx` to where `ServerConfigCtx` is defined. This removes code from `RQL.Types`, makes the dependency on `Server.Types` more explicit, and will make some further cleanups easier.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4336
GitOrigin-RevId: 95bb3467d741763892c4e68a38760497157ba1aa
With the current implementation, only the first call to `waitForShutdown` on a given
`ShutdownLatch` will return, while others will block (typically indefinitely). That's not
how one would expect a shutdown latch to work.
This isn't currently a concrete issue because we only wait once on each `ShutdownLatch`.
But in the context of #4154 we'll probably end up wanting to wait for shutdown from
multiple threads.
This adds a number of tests to verify the current behaviour, and adds a test for multiple
`waitForShutdown` calls that fails prior to the functional change.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4162
GitOrigin-RevId: 9a108858d11390b847404f30bc7b93c06fc3f966
In hasura/graphql-engine@7547786b2b, we made the logic that does a specific correctness check on the GraphQL schema runnable _post_-startup. However, in order to error out as early as possible, we explicitly do it on startup anyway.
Unfortunately a few instances of that logic were forgotten: until now we only executed it for non-relay authenticated GraphQL schemas.
This re-implements the "conflicting types" check for both the Relay schema, and for the unauthenticated schemas.
One key point where it does _not_ re-implement it is in `buildFullestDBSchema`, which is a very sketchy method that builds a GraphQL schema that is _only_ used internally in order to mark parts of the schema as inconsistent. There are two reasons I do not add the check here:
- This logic should get replaced entirely - see hasura/graphql-engine-mono#3882.
- Having conflicting types in `buildFullestDBSchema` does not imply there is actually a user-facing issue. The only "conflicting types" which are problematic are the ones which result in illegal _user-facing_ GraphQL schemas. `buildFullestDBSchema` is _never_ user-facing.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3885
GitOrigin-RevId: 49517dcbf51bcd97a3890514ec5e47499f770f32
`encJFromAssocList` is broken since it does not appropriately escape the keys of the JSON dictionaries. Presumably it should.
This is a hotfix for a JSON encoding issue. Longer-term solutions are being discussed in hasura/graphql-engine-mono#3736.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4139
GitOrigin-RevId: 27afff319d268a4ec017c8eafd694cc17c2fd7f0
UPDATE: After testing in CI it turns out that the compile time Improvement is better than expected: even though we always have to recompile the OSS lib (due to Version.hs), downstream packages like Pro and multi-tenant can still benefit from some caching and avoid full recompilation. In the best case this takes us from 22 minutes to 13 minutes total.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/4104
GitOrigin-RevId: 76cbfc157064b33856e30f4c2b2ab2366f9c6089
### Motivation
While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us.
However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch.
While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short:
- lack of documentation
- no clear distinction between public / internal components
- "unidiomatic" Haskell code (such as using `Either Result Error`)
While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part.
### Description
This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more.
This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set:
```
⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz'
```
### Note
This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
## Description
Some of the documentation/organizational changes I was putting into the suggestions for #3624 were a bit too convoluted for GitHub's suggestion interface, so I'm putting them here instead.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3910
Co-authored-by: Solomon <24038+solomon-b@users.noreply.github.com>
GitOrigin-RevId: 06e0cb08bd18e7f8b21452df0697cfd80bc56fde
### Motivation
#2338 introduced a way to validate REST queries against the metadata after a change, to properly report any inconsistency that would emerge from a change in the underlying structure of our schema. However, the way this was done was quite complex and error-prone. Namely: we would use the generated schema parsers to statically execute an introspection query, similar to the one we use for remote schemas, then parse the resulting bytestring as it were coming from a remote schema.
This led to several issues: the code was using remote schema primitives, and was associated with remote schema code, despite being unrelated, which led to absurd situations like creating fake `Variable`s whose type was also their name. A lot of the code had to deal with the fact that we might fail to re-parse our own schema. Additionally, some of it was dead code, that for some reason GHC did not warn about? But more fundamentally, this architecture decision creates a dependency between unrelated pieces of the engine: modifying the internal processing of root fields or the introspection of remote schemas now risks impacting the unrelated `OpenAPI` feature.
### Description
This PR decouples that process from the remote schema introspection logic and from the execution engine by making `Analyse` and `OpenAPI` work on the generic `G.SchemaIntrospection` instead. To accomplish this, it:
- adds `GraphQL.Parser.Schema.Convert`, to convert from our "live" schema back to a flat `SchemaIntrospection`
- persists in the schema cache the `admin` introspection generated when building the schema, and uses it both for validation and for generating the `OpenAPI`.
### Known issues and limitations
This adds a bit of memory pressure to the engine, as we persist the entire schema in the schema cache. This might be acceptable in the short-term, but we have several potential ideas going forward should this be a problem:
- cache the result of `Analyze`: when it becomes possible to build the `OpenAPI` purely with the result of `Analyze` without any additional schema information, then we could cache that instead, reducing the footprint
- caching the `OpenAPI`: if it doesn't need to change every time the endpoint is queried, then it should be possible to cache the entire `OpenAPI` object instead of the schema
- cache a copy of the `FieldParsers` used to generate the schema: as those are persisted through the GraphQL `Context`, and are the only input required to generate the `Schema`, making them accessible in the schema cache would allow us to have the exact same feature with no additional memory cost, at the price of a slightly slower and more complicated process (need to rebuild the `Schema` every time we query the OpenAPI endpoint)
- cache nothing at all, and rebuild the admin schema from scratch every time.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3962
Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com>
GitOrigin-RevId: a8b9808170b231fdf6787983b4a9ed286cde27e0
### Description
This is it! This PR enables the Metadata API for remote relationships from remote schemas, adds tests, ~~adds documentation~~, adds an entry to the Changelog. This is the release PR that enables the feature.
### Checklist
- [ ] Tests:
- [x] RS-to-Postgres (high level)
- [x] RS-to-RS (high level)
- [x] From RS specifically (testing for edge cases)
- [x] Metadata API tests
- [ ] Unit testing the actual engine?
- [x] Changelog entry
- [ ] Documentation?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3974
Co-authored-by: Vamshi Surabhi <6562944+0x777@users.noreply.github.com>
Co-authored-by: Vishnu Bharathi <4211715+scriptnull@users.noreply.github.com>
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: c9aebf12e6eebef8d264ea831a327b968d4be9d2
### Description
This very small PR fixes an error introduced in #3811, when changing the collision detection code: we were properly doing collision detection for remote schemas for the unauthenticated context, and also removing remote relationships... but then we were not using the result to build the schema.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3986
GitOrigin-RevId: 26a5553bf82574f2764fd594b0616dfea95a4757
- remove an unused return value
- untangle database query logic slightly
- rename printErrExit functions, and use them more consistently
- simplify the top-level exception handling
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3900
GitOrigin-RevId: a6727c6f899aed00e6a04bd822727341fd51acc4
### Description
This PR extends the `RemoteSchema` parsers to also include remote relationships. This include a significant refactoring of the top level schema building blocks, since remote schemas can no longer be built in isolation: they have to be built within the same run of `MonadSchema` as the sources. It is originally taken from the changes in #3069 and was slightly adapted.
I highly recommend turning OFF whitespace in the Github UI for `Schema.hs`, since I've adjusted the indentation of two large functions.
### Warning
Given the lack of a feature flag, this PR technically **enables the feature**. While the metadata API is not plugged in, a savvy user could use `replace_metadata` to set a metadata that contains remote joins from remote schemas, and they would be enabled. Is this acceptable?
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3811
GitOrigin-RevId: a5b00f865cdb8890b0fc02b139c2ebd48929f138
### Description
This small PR improves the representation of an endpoint method from `Text` to an enum of the supported methods. Additionally, it cleans some of the instances defined on surrounding types (such as Postgres-specific instances on Endpoint types).
Due to a name conflict, this makes `RQL.Types.Endpoint` impossible to re-export from `RQL.Types`, which in turn forces several other modules to import it explicitly, which I think is fine since we want to ultimately get rid of `RQL.Types`.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3965
GitOrigin-RevId: 33869007d0d818ddf486fb61d1f6099f9dad7570
<!-- Thank you for ss in the Title above ^ -->
## Description ✍️
<!-- Please fill this se-->
<!-- Describe the changes from a user's perspective -->
Add some documentation on `ActionFieldG` type.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3945
GitOrigin-RevId: d9543ed7a8fe3ccfe9f5267c3a2ac71fb040f4db
### Description
This PR cleans `processRemoteJoins` by splitting the code, introducing comments, and applied the same strategies than #3810 did. Most importantly, it introduces a new module `RemoteJoin.Source`, made to be very similar to `RemoteJoin.RemoteSchema`, that exposes the required tooling to make a join call to a source, which decluters `Join`. Furthermore, this PR uses the same "dependency injection" to make the core of `Join` free from IO: this opens the door to testing the join engine in the unit tests.
None of the functions were modified when moved from their old module to the new one, but there's no way to easily see this in a diff.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3894
GitOrigin-RevId: 1e7c43006f092326e061f9ba12674e207b628bef
### Description
This PR moves Hasura-specific schema functions from `Hasura.GraphQL.Parser.Class` into `Hasura.GraphQL.Schema.Common`. It also removes the two corresponding monad aliases, and consequently harmonizes several parts of the code to use the same common constraint.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3947
GitOrigin-RevId: 40985a7d86da97a311bd480f9a57cc18c350c2a8
## Description
We go through the module `Hasura.Backends.MSSQL.FromIr` and split it into separate self-contained units, which we document.
Note that this PR has a slightly opinionated follow-up PR #3909 .
### Related Issues
Fix#3666
### Solution and Design
The module `FromIr` has given rise to:
* `FromIr.Expression`
* `FromIr.Query`
* `FromIr.Delete`
* `FromIr.Insert`
* `FromIr.Update`
* `FromIr.SelectIntoTempTable`
And `Execute.MutationResponse` has become `FromIr.MutationResponse` (after some slight adaptation of types).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3908
GitOrigin-RevId: 364acf1bcdf74f2e19464c31cdded12bd8e9aa59
### Description
This PR improves the `Collect` module by re-ordering the functions to make clear what is public API and what is internal implementation. Furthermore, it makes use of `traverseOf` and `traverseFields` to reduce duplication. To do so, it also introduces a few more lenses in the rest of the codebase, and uses this opportunity to harmonize some structures that were not following our naming convention.
While the diff is massive, a lot of it is just code moving around; the file is now divided into separate sections:
- entry points: IR types for which we want to run the collection
- internal monadic structure
- internal traversals: functions that do nothing but drill down further
- actual transformations: the three cases where we do actually have work to do: selection sets on which we do want to insert join columns, extract remote relationships... those functions are left unchanged by this PR
- internal helpers
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3863
GitOrigin-RevId: f7cbecfae9eed9737b62acfa5848bfcf9d4651f6
### Description
#3810 was merged with comments still open; this small PR does a few minute clean-ups to address some remaining nits.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3941
GitOrigin-RevId: 3d15eb399828123640a73247b848bc4ddff02c38
### Description
This very small PR introduces `unionWithM`, to allow hashmap union that might fail, and uses it to transform an `error` into a `throw500`. It also reorders `HashMap.Strict.Extended` to group all "union" functions together.
There is, however, a broader question of whether we should encourage the proliferation of such functions. If so, we might also want to consider:
- `mapWithKeyM`, to remove the `unsafeMkName` of `RemoteJoin.Collect`
- `forWithKey`, as a flipped version of `traverseWithKey`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3917
GitOrigin-RevId: a488d5bf04a73878b9e42f27ede36199bb4c920a
### Description
This PR adds the ability to perform remote joins from remote schemas in the engine. To do so, we alter the definition of an `ExecutionStep` targeting a remote schema: the `ExecStepRemote` constructor now expects a `Maybe RemoteJoins`. This new argument is used when processing the execution step, in the transport layer (either `Transport.HTTP` or `Transport.WebSocket`).
For this `Maybe RemoteJoins` to be extracted from a parsed query, this PR also extends the `Execute.RemoteJoin.Collect` module, to implement "collection" from a selection set. Not only do those new functions extract the remote joins, but they also apply all necessary transformations to the selection sets (such as inserting the necessary "phantom" fields used as join keys).
Finally in `Execute.RemoteJoin.Join`, we make two changes. First, we now always look for nested remote joins, regardless of whether the join we just performed went to a source or a remote schema; and second we adapt our join tree logic according to the special cases that were added to deal with remote server edge cases.
Additionally, this PR refactors / cleans / documents `Execute.RemoteJoin.RemoteServer`. This is not required as part of this change and could be moved to a separate PR if needed (a similar cleanup of `Join` is done independently in #3894). It also introduces a draft of a new documentation page for this project, that will be refined in the release PR that ships the feature (either #3069 or a copy of it).
While this PR extends the engine, it doesn't plug such relationships in the schema, meaning that, as of this PR, the new code paths in `Join` are technically unreachable. Adding the corresponding schema code and, ultimately, enabling the metadata API will be done in subsequent PRs.
### Keeping track of concrete type names
The main change this PR makes to the existing `Join` code is to handle a new reserved field we sometimes use when targeting remote servers: the `__hasura_internal_typename` field. In short, a GraphQL selection set can sometimes "branch" based on the concrete "runtime type" of the object on which the selection happens:
```graphql
query {
author(id: 53478) {
... on Writer {
name
articles {
title
}
}
... on Artist {
name
articles {
title
}
}
}
}
```
If both of those `articles` are remote joins, we need to be able, when we get the answer, to differentiate between the two different cases. We do this by asking for `__typename`, to be able to decide if we're in the `Writer` or the `Artist` branch of the query.
To avoid further processing / customization of results, we only insert this `__hasura_internal_typename: __typename` field in the query in the case of unions of interfaces AND if we have the guarantee that we will processing the request as part of the remote joins "folding": that is, if there's any remote join in this branch in the tree. Otherwise, we don't insert the field, and we leave that part of the response untouched.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3810
GitOrigin-RevId: 89aaf16274d68e26ad3730b80c2d2fdc2896b96c
…rmance
It makes sense to try to utilize multiple threads for metadata
operations since we expect them to come one at a time (and likely at
lower load periods anyway).
As noted, although we build roles in parallel now, the admin role is
still a bottleneck. For replace_metadata on huge_schema, on my machine
I get:
BEFORE: 22.7 sec
AFTER: 13.5 sec
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3911
GitOrigin-RevId: 4d4ee6ac8b5506603e70e4fc666a3aacc054d493
### Description
Several libraries define `catMaybes` as `mapMaybe id`. We had it defined in `Data.HashMap.Strict.Extended` already. This small PR also defines it in `Extended` modules for other containers and replaces every occurrence of `mapMaybe id` accordingly.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3884
GitOrigin-RevId: d222a2ca2f4eb9b725b20450a62a626d3886dbf4
In hasura/graphql-engine#5144, we noticed that having remote relationships in the schema is problematic for Relay. In particular, we don't support remote schemas in Relay at all, and because of this, remote relationships were also broken.
The fix was easy: when we're building the schema for Relay, whenever we encounter a remote relationship in our configuration, we simply skip building that field. The implementation was easy: (see hasura/graphql-engine#5145)
```diff
- SFRemoteRelationship info -> pure $ mkRemoteRelationshipFld info
+ SFRemoteRelationship info ->
+ -- https://github.com/hasura/graphql-engine/issues/5144
+ if isRelay then [] else pure $ mkRemoteRelationshipFld info
```
A test case was added in that PR to prevent us from accidentally re-including remote relationships in the Relay schema. (However, it now looks like that test case does not function correctly.)
The above code was later refactored in #3037, making use of the `MaybeT` effect. However, this effect was not used correctly, so that the result of the check was ignored.
This fixes the code to use the `MaybeT` effect correctly.
CC @0x777 @rakeshkky
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3868
GitOrigin-RevId: e528303e01eacf60173cba1eec1898986cf12359
### Description
There were several places in the codebase where we would either implement a generic container, or express the need for one. This PR extracts / creates all relevant containers, and adapts the relevant parts of the code to make use of said new generic containers. More specifically, it introduces the following modules:
- `Data.Set.Extended`, for new functions on `Data.Set`
- `Data.HashMap.Strict.Multi`, for hash maps that accept multiple values
- `Data.HashMap.Strict.NonEmpty`, for hash maps that can never be constructed as empty
- `Data.Trie`, for a generic implementation of a prefix tree
This PR makes use of those new containers in the following parts of the code:
- `Hasura.GraphQL.Execute.RemoteJoin.Types`
- `Hasura.RQL.Types.Endpoint*`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3828
GitOrigin-RevId: e6c1b971bcb3f5ab66bc91d0fa4d0e9df7a0c6c6
The only purpose was enabling the developer API by default. I don't
think that justifies a flag and CPP usage.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3820
GitOrigin-RevId: 058c9a7b03e5e164ef88e35c42f50bae3c42b5b6
### Description
This PR is one further step towards remote joins from remote schemas. It introduces a custom partial AST to represent queries to remote schemas in the IR: we now need to augment what used to be a straightforward GraphQL AST with additional information for remote join fields.
This PR does the minimal amount of work to adjust the rest of the code accordingly, using `Void` in all places that expect a type representing remote relationships.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3794
GitOrigin-RevId: 33fc317731aace71f82ad158a1951ea93350d6cc
No logic in this PR, just tidying things up (renaming the backend from `Experimental` to `DataWrapper`).
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3779
GitOrigin-RevId: f11acf563ccd8b9f16bc23c5e92da392aa4cfb2c
## Description
This PR is in reference to #2449 (support IP blacklisting for multitenant)
*RFC Update: Add support for IPv6 blocking*
### Solution and Design
Using [http-client-restricted](https://hackage.haskell.org/package/http-client-restricted) package, we're creating the HTTP manager with restricting capabilities. The IPs can be supplied from the CLI arguments as `--ipv4BlocklistCidrs cidr1, cidr2...` or `--disableDefaultIPv4Blocklist` for a default IP list. The new manager will block all requests to the provided CIDRs.
We are extracting the error message string to show the end-user that given IP is blocked from being set as a webhook. There are 2 ways to extract the error message "connection to IP address is blocked". Given below are the responses from event trigger to a blocked IP for these implementations:
- 6d74fde316f61e246c861befcca5059d33972fa7 - We return the error message string as a HTTPErr(HOther) from `Hasura/Eventing/HTTP.hs`.
```
{
"data": {
"message": "blocked connection to private IP address "
},
"version": "2",
"type": "client_error"
}
```
- 88e17456345cbb449a5ecd4877c84c9f319dbc25 - We case match on HTTPExceptionContent for InternaException in `Hasura/HTTP.hs` and extract the error message string from it. (this is implemented as it handles all the cases where pro engine makes webhook requests)
```
{
"data": {
"message": {
"type": "http_exception",
"message": "blocked connection to private IP address ",
"request": {
"secure": false,
"path": "/webhook",
"responseTimeout": "ResponseTimeoutMicro 60000000",
"queryString": "",
"method": "POST",
"requestHeaders": {
"Content-Type": "application/json",
"X-B3-ParentSpanId": "5ae6573edb2a6b36",
"X-B3-TraceId": "29ea7bd6de6ebb8f",
"X-B3-SpanId": "303137d9f1d4f341",
"User-Agent": "hasura-graphql-engine/cerebushttp-ip-blacklist-a793a0e41-dirty"
},
"host": "139.59.90.109",
"port": 8000
}
}
},
"version": "2",
"type": "client_error"
}
```
### Steps to test and verify
The restricted IPs can be used as webhooks in event triggers, and hasura will return an error message in reponse.
### Limitations, known bugs & workarounds
- The `http-client-restricted` has a needlessly complex interface, and puts effort into implementing proxy support which we don't want, so we've inlined a stripped down version.
- Performance constraint: As the blocking is checked for each request, if a long list of blocked CIDRs is supplied, iterating through all of them is not what we would prefer. Using trie is suggested to overcome this. (Added to RFC)
- Calls to Lux endpoints are inconsistent: We use either the http manager from the ProServeCtx which is unrestricted, or the http manager from the ServeCtx which is restricted (the latter through the instances for MonadMetadataApiAuthorization and UserAuthentication). (The failure scenario here would be: cloud sets PRO_ENDPOINT to something that resolves to an internal address, and then restricted requests to those endpoints fail, causing auth to fail on user requests. This is about HTTP requests to lux auth endpoints.)
## Changelog
- ✅ `CHANGELOG.md` is updated with user-facing content relevant to this PR.
## Affected components
- ✅ Server
- ✅ Tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3186
Co-authored-by: Robert <132113+robx@users.noreply.github.com>
GitOrigin-RevId: 5bd2de2d028bc416b02c99e996c7bebce56fb1e7
Numbers from CI for the new (currently noisy) `replace_metadata` adhoc benchmark:
chinook: 0.19s -> 0.16
huge_schema: 36.98s -> 29.89
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3685
GitOrigin-RevId: be79b666858b03e8407c0d89765e9aac0af8d40a
It was only used for one purpose. This makes the sketchy manager handling in schema cache init a bit more visible.
Should help make the change in #2449 more robust.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3677
GitOrigin-RevId: e34b990bafb4893663ae195d5bf329130056f1ff
I discovered and removed instances of Boolean Blindness about whether json numbers should be stringified or not.
Although quite far-reaching, this is a completely mechanical change and should have no observable impact outside the server code.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3763
GitOrigin-RevId: c588891afd8a6923a135c736f6581a43a2eddbc7
TL;DR
---
We go from this:
```haskell
(|
withRecordInconsistency
( (|
modifyErrA
( do
(info, dependencies) <- liftEitherA -< buildRelInfo relDef
recordDependencies -< (metadataObject, schemaObject, dependencies)
returnA -< info
)
|) (addTableContext @b table . addRelationshipContext)
)
|) metadataObject
```
to this:
```haskell
withRecordInconsistencyM metadataObject $ do
modifyErr (addTableContext @b table . addRelationshipContext) $ do
(info, dependencies) <- liftEither $ buildRelInfo relDef
recordDependenciesM metadataObject schemaObject dependencies
return info
```
Background
---
We use Haskell's `Arrows` language extension to gain some syntactic sugar when working with `Arrow`s. `Arrow`s are a programming abstraction comparable to `Monad`s.
Unfortunately the syntactic sugar provided by this language extension is not very sweet.
This PR shows how we can sometimes avoid using `Arrow`s altogether, without loss of functionality or correctness. It is a demo of a technique that can be used to cut down the amount of `Arrows`-based code in our codebase by about half.
Approach
---
Although _in general_ not every `Monad` is an `Arrow`, specific `Arrow` instantiations are exactly as powerful as their `Monad` equivalents. Otherwise they wouldn't be very equivalent, would they?
Just like `liftEither` interprets the `Either e` monad into an arbitrary monad implementing `MonadError e`, we add `interpA` which interprets certain concrete monads such as `Writer w` into specific arrows, e.g. ones satisfying `ArrowWriter w`. This means that the part of the code that only uses such interpretable effects can be written _monadically_, and then used in _arrow_ constructions down the line.
This approach cannot be used for arrow effects which do not have a monadic equivalent. In our codebase, the only instance of this is `ArrowCache m`, implemented by the `Rule m` arrow. So code written with `ArrowCache m` in the context cannot be rewritten monadically using this technique.
See also
---
- #1827
- #2210
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3543
Co-authored-by: jkachmar <8461423+jkachmar@users.noreply.github.com>
GitOrigin-RevId: eb79619c95f7a571bce99bc144ce42ee65d08505
In order to respond to GraphQL queries that make use of the introspection fields `__type` or `__schema`, we need two things:
- an overview of the relevant GraphQL type information, stored in a `Schema` object, and
- to have included the `__type` and `__schema` fields in the `query_root` that we generate.
It used to be necessary to do the above items in that order, since the `__type` and `__schema` fields (i.e. the respective `FieldParser`s) were generated _from_ a `Schema` object.
Thanks to recent refactorings in `Hasura.GraphQL.Schema.Introspect` (see hasura/graphql-engine-mono#2835 or hasura/graphql-engine@5760d9289c), the introspection fields _themselves_ are now `Schema`-agnostic, and simply return a function that takes a `Schema` object after parsing. For instance, the type of `schema`, corresponding to the `__schema` field, has literally changed as follows:
```diff
-schema :: MonadParse n => Schema -> FieldParser n ( J.Value)
+schema :: MonadParse n => FieldParser n (Schema -> J.Value)
```
This means that the introspection fields can be included in the GraphQL schema *before* we have generated a `Schema` object. In particular, rather than the current architecture of generating `Schema` at startup time for every role, we can instead generate `Schema` ad-hoc at query parsing time, only for those queries that make use of the introspection fields. This avoids us storing a `Schema` for every role for the lifetime of the server.
However: this introduces a functional change, as the code that generates the `Schema` object, and in particular the `accumulateTypeDefinitions` method, also does certain correctness checks, to prevent exposing a spec-incompliant GraphQL schema. If these correctness checks are being done at parsing time rather than startup time, then we catch certain errors only later on. For this reason, this PR adds an explicit run of this type accumulation at startup time. For efficiency reasons, and since this correctness check is not essential for correct operation of HGE, this is done for the admin role only.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3231
GitOrigin-RevId: 23701c548b785929b28667025436b6ce60bfe1cd
There are three minor cleanups here:
- The first argument to the `setMetadataInCatalog` method is always `Just`. It is thus important to avoid `Maybe`, because this means that a crucial piece of code (saving metadata) is completely untested.
- Rather than spelling them out, we can derive the `Semigroup`/`Monoid` instances for `MetadataModifier` through the `Endo` type.
- I've renamed the name of the getter of the `MetadataModifier` newtype to **r**unMetadataModifier. Using record puns, this allows us to write:
```diff
- putMetadata $ unMetadataModifier metadataModifier metadata
+ putMetadata $ runMetadataModifier metadata
```
which is nicer to read.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3703
GitOrigin-RevId: fd36b3c5202017f5afc943c01dfdd7c82c099bdd
#### TODO
- [x] fix `hashable >= 1.3.1` serialization ordering issue [^1]
- `test_graphql_mutations.py::TestGraphQLMutateEnums` was failing
- [x] fix `unordered-containers` serialization ordering issue [^2]
- `test_graphql_queries.py` was failing on Citus
- [ ] verify that no new failures have been introduced
- [ ] open issues to fix the above
- identify test cases that "leak" implementation details by depending on `hashable` instance ordering
- bump `hashable >= 1.3.1` and update test cases with new ordering OR modify them so that ordering is stable
- bump `unordered-containers >= 0.2.15.0` and update test cases with new ordering OR modify them so that ordering is stable
- one of the test cases was failing on string equality comparison for a generated Citus query
- we probably don't want to _actually_ do this unless there are _very specific_ guarantees we want to make about generated query structure
---
Just what it says on the tin.
https://github.com/hasura/graphql-engine-mono/pull/3538 updated the freeze file a few weeks ago, but it looks like the index state hadn't been updated since December so a lot of stuff that had newer versions didn't get updated.
---
EDIT: I should add, the motivation for doing this in the first place is that `hspec > 2.8.4` now supports specifying filtering spec trees based on patterns provided by the `HSPEC_MATCH` environment variable.
For example, one could have a script that executes the following:
```
HSPEC_MATCH="PostgreSQL" \
ghcid \
--command \
'cabal repl graphql-engine:test:tests-hspec \
--repl-option -O0 \
--repl-option -fobject-code' \
--test "main"
```
...which will loop on typechecking the `tests-hspec` component, and then as soon as it passes (i.e. no warnings or errors) will run _only_ the `PostgreSQL` sub-components.
[^1]: `hashable >= 1.3.1.0` [updated its default salts](https://github.com/haskell-unordered-containers/hashable/pull/196), which [broke serialization ordering](https://github.com/haskell/aeson/issues/837)
[^2]: `unordered-containers >= 0.2.16.0` [introduced changes to some of its internal functions](https://hackage.haskell.org/package/unordered-containers-0.2.16.0/changelog) which seem like they could have affected serialization stability
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3672
GitOrigin-RevId: bbd1d48c73db4021913f0b5345b7315a8d6525d3
We build the GraphQL schema by combining building blocks such as `tableSelectionSet` and `columnParser`. These building blocks individually build `{InputFields,Field,}Parser` objects. Those object specify the valid GraphQL schema.
Since the GraphQL schema is role-dependent, at some point we need to know what fragment of the GraphQL schema a specific role is allowed to access, and this is stored in `{Sel,Upd,Ins,Del}PermInfo` objects.
We have passed around these permission objects as function arguments to the schema building blocks since we first started dealing with permissions during the PDV refactor - see hasura/graphql-engine@5168b99e46 in hasura/graphql-engine#4111. This means that, for instance, `tableSelectionSet` has as its type:
```haskell
tableSelectionSet ::
forall b r m n.
MonadBuildSchema b r m n =>
SourceName ->
TableInfo b ->
SelPermInfo b ->
m (Parser 'Output n (AnnotatedFields b))
```
There are three reasons to change this.
1. We often pass a `Maybe (xPermInfo b)` instead of a proper `xPermInfo b`, and it's not clear what the intended semantics of this is. Some potential improvements on the data types involved are discussed in issue hasura/graphql-engine-mono#3125.
2. In most cases we also already pass a `TableInfo b`, and together with the `MonadRole` that is usually also in scope, this means that we could look up the required permissions regardless: so passing the permissions explicitly undermines the "single source of truth" principle. Breaking this principle also makes the code more difficult to read.
3. We are working towards role-based parsers (see hasura/graphql-engine-mono#2711), where the `{InputFields,Field,}Parser` objects are constructed in a role-invariant way, so that we have a single object that can be used for all roles. In particular, this means that the schema building blocks _need_ to be constructed in a role-invariant way. While this PR doesn't accomplish that, it does reduce the amount of role-specific arguments being passed, thus fixing hasura/graphql-engine-mono#3068.
Concretely, this PR simply drops the `xPermInfo b` argument from almost all schema building blocks. Instead these objects are looked up from the `TableInfo b` as-needed. The resulting code is considerably simpler and shorter.
One way to interpret this change is as follows. Before this PR, we figured out permissions at the top-level in `Hasura.GraphQL.Schema`, passing down the obtained `xPermInfo` objects as required. After this PR, we have a bottom-up approach where the schema building blocks themselves decide whether they want to be included for a particular role.
So this moves some permission logic out of `Hasura.GraphQL.Schema`, which is very complex.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3608
GitOrigin-RevId: 51a744f34ec7d57bc8077667ae7f9cb9c4f6c962
