### 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
### 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
<!-- 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
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
### 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
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
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
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
#### 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
- consistent qualified imports
- less convoluted initialization of pro logging HTTP manager
- pass pro HTTP manager directly instead of via Has
- remove some dead healthcheck code
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3639
GitOrigin-RevId: dfa7b9c62d1842a07a8514cdb77f1ed86064fb06
spec: https://github.com/hasura/graphql-engine-mono/pull/2278
Briefly:
- extend metadata so that allowlist entries get a new scope field
- update `add_collection_to_allowlist` to accept this new scope field,
and adds `update_scope_of_collection_in_allowlist` to change the scope
- scope can be global or role-based; a collection is available for every
role if it is global, and available to every listed role if it is role-based
- graphql-engine-oss is aware of role-based allowlist metadata; collections
with non-global scope are treated as if they weren't in the allowlist
To run the tests:
- `cabal run graphql-engine-tests -- unit --match Allowlist`
- py-tests against pro:
- launch `graphql-engine-pro` with `HASURA_GRAPHQL_ADMIN_SECRET` and `HASURA_GRAPHQL_ENABLE_ALLOWLIST`
- `pytest test_allowlist_queries.py --hge-urls=... --pg-urls=... --hge-key=... --test-allowlist-queries --pro-tests`
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/2477
Co-authored-by: Anon Ray <616387+ecthiender@users.noreply.github.com>
Co-authored-by: Robert <132113+robx@users.noreply.github.com>
GitOrigin-RevId: 01f8026fbe59d8701e2de30986511a452fce1a99
## Description
This PR is a subset of #3069, that does roughly that #3031 was aiming to do: add the schema cache building phase for relationships from remote servers. This PR does not change any of the code that *uses* remote relationships, meaning we ignore the added schema cache information. It also contains dependency-tracking code, which was originally missing from #3031; in turn, this pulls some of the metadata API as well, since we identify remote relationships by how they were created.
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3540
GitOrigin-RevId: ed962b6d07fd4adbf0a71e0d79736a4e8b422fea
_(This PR is on top of #3352.)_
## Description
This PR overhauls our documentation CI steps to push all generated server documentation to the `gh-pages` branch of the OSS repo. The goal of this PR is to arrive in the situation where `https://hasura.github.io/graphql-engine/server/` is automatically populated to contain the following:
- all the markdown files from `server/documentation`, copied verbatim, no transformation applied
- all the notes, collected from the code by the `extract-notes.sh` script, in `server/notes`
- the generated haddock documentation for each major release or branch in `server/haddock`.
To do so, this PR does the following:
- it includes the script to extract notes from #3352,
- it rewrites the documentation checking CI step, to generate the notes and publish the resulting "server/documentation" folder,
- it includes a new CI step to deploy the documentation to the `gh-pages` branch
Of note:
- we will generate a different haddock folder for each main branch and release; in practice, that means the _main_, _stable_, _alpha_, _beta_ branches, and every build tagged with a version number
- the step that builds the haddock documentation checks that ALL projects in the repo build, including pro, but the deploy only deploys the graphql-engine documentation, as it pushes it to a publicly-accessible place
## Required work
**DO NOT MERGE THIS PR IT IS NOT READY**. Some work needs to go into this PR before it is ready.
First of all: the `gh-pages` branch of the OSS repo does NOT yet contain the documentation scaffolding that this new process assumes. At the bare minimum, it should be a orphan branch that contains a top-level README.md file, and a _server_ folder. An example of the bare minimum required can be previewed [on my fork](https://nicuveo.github.io/graphql-engine/server/).
The content of the `server/documentation` folder needs to be adjusted to reflect this; at the very least, a `README.md` file needs to be added to do the indexing (again, see the placeholder [on my fork](https://nicuveo.github.io/graphql-engine/server/) for an example).
This way of publishing documentation must be validated against [proposed changes to the documentation](https://github.com/hasura/graphql-engine-mono/pull/3294). @marionschleifer what do you think?
~~The buildkite code in this branch is currently untested, and I am not sure how to test it.~~
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3380
GitOrigin-RevId: b24f6759c64ae29886c1f1b481b172febc512032
### Description
The GraphQL spec has to conflicting requirements:
1. an object must contain at least one field: the schema may not contain empty objects
2. the _query_root_ must always be present
Given _1_, the schema generation code removes from the schema all fields that would result in empty objects, such as a table for which a user does not have select permissions. But, as a result, our code also potentially removes _query_root_ if it is empty, breaking _2_.
This PR introduces a dummy "placeholder" field in the query root if it's empty, to ensure we never remove it from the schema.
### Remaining work
- [x] changelog entry
- [x] tests
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/148
GitOrigin-RevId: bfd6bfcc2f3de92900b6ba566012f093399ca037
### Description
This PR is the result of a discussion in #3363. Namely, we would like to remove all uses of `unsafeMkName`, or at the very least document every single one of them, to avoid similar issues. To do so, this PR does the following:
- it adds a hlint suggestion not to use that function:
- suggestions don't mark the PR as failed, but will be shown at review time
- it is possible to disable that hint with `{- HLINT ignore myFunction "unsafe" -}`
- wherever possible, it removes uses of `unsafeMkName` in favour of `mkName`
- it adds a comment with a tracking issue for the two remaining uses:
- #3478
- #3479
### Remaining work
- discuss whether this hint should make the linter step fail, since the linter step isn't required to merge anyway, and there is a way to disable the hint wherever we think the use of that function is acceptable
- check that none of those uses were load-bearing and result in errors now
PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3480
GitOrigin-RevId: 0a7e3e9d1a48185764c04ab61e34b58273af347c