Commit Graph

12 Commits

Author SHA1 Message Date
Daniel Chambers
0624a7553d Aggregate relationships GraphQL schema and execution (#756)
## Description
This PR implements the GraphQL schema and execution for aggregate
relationships.

In the `schema` crate, the new `model_aggregate_relationship_field`
function handles generating schema for ModelAggregateTarget
relationships. It mostly delegates the meat of its implementation to
reused logic; some refactoring has occurred to make this possible.
This involved changes in `select_many`, `select_aggregate` and
`model_arguments`. The creation of the model arguments field argument
now exists in `model_arguments` and is reused by `select_many` and
`select_aggregate`. The creation of all aggregate field arguments is now
in `select_aggregate::generate_select_aggregate_arguments`, and is then
reused when generating the aggregate relationship field. That field is
annotated with the new `RelationshipToModelAggregate` annotation.

In the `execute` crate, the logic around generating an the aggregate
selection IR was moved from `select_aggregate` into `model_selection`.
This was so it can be reused by the logic in `relationship` that now
uses it to generate an aggregate selection when encountering an
`RelationshipToModelAggregate` field.
Inside `relationship` some rearranging was done so that
`build_local_model_relationship` and `build_remote_relationship` could
work with either a normal model selection IR or the new aggregate
selection IR. The necessitated moving the creation of that IR outside
those functions into the caller, so the different callers can create
different IR (normal vs aggregate IR). This also reduced code
duplication.

New tests have been added to `engine` that cover aggregate relationships
and also remote joined aggregate relationships.

This PR also corrects two bugs in metadata resolve revealed by new
testing:
* The filter input field name in `GraphqlConfig` must be specified if
using an aggregate relationship
* The filter input type name defined on a `Model` must be specified if
that model is the target of an aggregate relationship. Conversely, the
filter input type name can be specified if the `Model` itself doesn't
define an aggregate, but is still involved in a aggregate relationship
(this previously produced an error).

This PR completes the feature, but it is still hidden behind the
experimental flag. There will be a follow up PR to remove that and
expose the functionality by default.

JIRA: [V3ENGINE-160](https://hasurahq.atlassian.net/browse/V3ENGINE-160)

[V3ENGINE-160]:
https://hasurahq.atlassian.net/browse/V3ENGINE-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: d499371906f7af71a4017c7c3ae75b7693cd3fa7
2024-06-26 09:10:33 +00:00
Samir Talwar
63ac02bc07 Take snapshots of passing and failing metadata resolution. (#763)
### What

In order to more easily monitor and review changes to metadata
resolution, this introduces snapshot testing for both successful and
failing calls to `resolve`. I used [Insta](https://insta.rs/) for this.

### How

For tests of the failure case, we already had a text file with the
expected error, so I have turned those files into snapshot files. I
wrote a small script to move the files rather than deleting and
recreating them so I could guarantee that the contents have not changed.
(Unfortunately, Git's diff doesn't always recognise the move as a move
because Insta has added a header.)

For tests of the successful case, I added a line to snapshot the
metadata rather than discarding it.

I also rewrote the tests to use `insta::glob` so we could get rid of
`test_each`.

V3_GIT_ORIGIN_REV_ID: 41bef4cf77bddb8d20d7c101df52ae149e8b0476
2024-06-26 08:15:29 +00:00
Daniel Harvey
cedd4e56c6 Use new boolean expressions in arguments (#753)
<!-- Thank you for submitting this PR! :) -->

## Description

Adding a test for this and fixing a few missing parts. Mostly threading
`boolean_expression_types` everywhere and adding them to our type
lookups. Behind a feature flag so this is a functional no-op.

V3_GIT_ORIGIN_REV_ID: 5fd6d5b9e06f0216e770b0715c59c0479881017f
2024-06-25 14:35:07 +00:00
Daniel Chambers
b1ee3ae1b8 Metadata resolve for aggregates over relationships (#731)
## Description
This PR continues on from #725 and adds the metadata resolve logic to
validate the usage of aggregates applied to relationships. The metadata
resolve logic is gated behind a new `enable_aggregate_relationships`
flag and disabled by default.

The new resolve logic lives in
`crates/metadata-resolve/src/stages/relationships/mod.rs`, however, most
of the actual logic that validates the usage of the aggregate expression
with the relationship has been reused from the model aggregate resolve
code. Subsequently, that code
(`crates/metadata-resolve/src/stages/models/aggregation.rs`) was
refactored to return a distinct error type
`ModelAggregateExpressionError` that can be composed with the
`RelationshipError` type, so the same errors can be returned with the
additional context of the relationship they were found in.

New metadata resolve error tests have been added in
`crates/metadata-resolve/tests/failing/aggregate_expression_in_relationship/*`.
Also, two new passing metadata resolve tests have been added to cover
the happy case of root field and relationship aggregate expressions:
`crates/metadata-resolve/tests/passing/aggregate_expressions/*`

JIRA: [V3ENGINE-160](https://hasurahq.atlassian.net/browse/V3ENGINE-160)

[V3ENGINE-160]:
https://hasurahq.atlassian.net/browse/V3ENGINE-160?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: 8cd7040cc9277940641d5ac239d7b34f8c1462c5
2024-06-21 01:08:54 +00:00
Daniel Harvey
e14908e083 Use file globs in metadata-resolve tests (#728)
<!-- Thank you for submitting this PR! :) -->

## Description

Because we globbed for folders, we had to put a test in the root of each
folder which is messy. If we glob for files instead, like the `failing`
tests do, we can have a better time. Functional no-op.

V3_GIT_ORIGIN_REV_ID: 381f5a44d9ae6100198bd28a286493bc3a2ba98e
2024-06-18 12:19:07 +00:00
Daniel Harvey
8dae5ae742 Add example boolean expression type metadata for an Elasticsearch range (#699)
<!-- Thank you for submitting this PR! :) -->

## Description

This PR adds a test with example metadata for how to map to something
like an Elasticsearch [Range
query](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-range-query.html).
Functional no-op.

V3_GIT_ORIGIN_REV_ID: 727e1706aab75957214b8cb13876eb23b843ba29
2024-06-13 08:37:32 +00:00
Daniel Harvey
d34c419929 Allow nested metadata-resolve tests (#697)
<!-- Thank you for submitting this PR! :) -->

## Description

We have a set of tests in `metadata-resolve` that uses globs to find
files in either `passing` or `failing` folders and check them. This
allows those files to live in nested folders for tidyness. Functional
no-op.

V3_GIT_ORIGIN_REV_ID: 22d42e986676a110f455c59c3df9e6946c5d996c
2024-06-11 14:11:10 +00:00
Daniel Harvey
75ced29d11 Resolve nested object boolean expressions (#680)
<!-- Thank you for submitting this PR! :) -->

## Description

This adds the ability to describe nested object boolean expressions,
which become `fieldPath` items in the generated
`ndc_models::ComparisonTarget::Column` items. This allows us to describe
filtering a `User` based on some element in their nested `address` field
(like `postcode`, for example).

Like the other `BooleanExpressionType` work, this remains behind a
feature flag so should make no user-facing changes.

It is also missing a whole heap of metadata resolve checks, going to
follow with these after doing the happy path to unblock other work.

V3_GIT_ORIGIN_REV_ID: c89e2942a651d349fca97706affcf40d91afeefb
2024-06-11 12:57:21 +00:00
Daniel Harvey
5fd6b6c03a Resolve boolean_expression_type metadata (#630)
<!-- Thank you for submitting this PR! :) -->

## Description

This somewhat resolves the `BooleanExpressionType` metadata type.

There is a lot that is missing, but everything is behind a feature so I
want to merge what is here before moving onto getting it working with
the later parts of the pipeline. We'll start with a lot of duplication
and then work to remove as much as we can.

Functional no-op for users, the feature flag is not exposed to the
actual binary.

How to review this PR:

- Check that any changes to existing code are cosmetic and naming-only
to ensure this is a no-op
- Look through the new resolve step and compare it to the
[RFC](https://github.com/hasura/v3-engine/blob/main/rfcs/open-dd-expression-type-changes.md)
and see if it is doing roughly what you might expect.
- Don't get too bogged down in missing things / style stuff, there are
definitely missing things and bad style.

V3_GIT_ORIGIN_REV_ID: 24e3b3e72e62d0094db3b461cb8c6d359982755d
2024-06-03 13:35:49 +00:00
Daniel Chambers
a132759dac Fixed arbitrary ordering of directions in GraphqlConfig causing error (#625)
## Description
If the user specified their orderings in their GraphqlConfig in a
different order, they would be presented with an error.

```yaml
enumTypeNames:
  - directions: # Reversed ordering of this list
      - Desc
      - Asc
    typeName: OrderBy
```

> ERR Code=opendds-validation Message="invalid metadata: error building
schema: unable to build schema: metadata is not consistent: invalid
directions: Desc,Asc defined in orderByInput of GraphqlConfig ,
currenlty there is no support for partial directions. Please specify a
type which has both 'asc' and 'desc' directions"
7:56PM ERR Supergraph Build failed.

This has been fixed.

## Changelog

### Product

_(Select all products this will be available in)_

- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->

### Type

<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->

_(Select only one. In case of multiple, choose the most appropriate)_

- [ ] highlight
- [ ] enhancement
- [x] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->

### Changelog entry

<!--
  - Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
  - For Highlights start with a H4 heading (#### <entry title>)
  - Get the changelog entry reviewed by your team
-->

Fixed arbitrary ordering of directions in GraphqlConfig causing invalid
error

<!-- changelog-entry : end : DO NOT REMOVE -->

<!-- changelog : end : DO NOT REMOVE -->

V3_GIT_ORIGIN_REV_ID: b27a0dd3b5c665f54ea58a40af8b2b1bfb0d2434
2024-05-28 05:43:48 +00:00
Tom Harding
e4c5d7a752 Add the SubgraphIdentifier type (#613)
## Description

Parallel to #611, in service of removing supergraph config, we'd like to
map all current supergraph config into a generated subgraph. To ensure
that we don't end up with an unfortunate naming collision, I've decided
to refine the definition of a subgraph identifier to exclude any string
starting with `__internal`, which we can then have as our "internal
namespace". This change introduces the new type, and updates the
"unknown_namespace" subgraph to come under this umbrella.

This change should be a no-op to any user who hasn't used a subgraph
name prefixed with `__internal`.

<!--
  Questions to consider answering:
  1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
  3. What is the conceptual design behind this PR?
  4. How can this PR be tested/verified?
  5. Does the PR have limitations?
  6. Does the PR introduce breaking changes?
-->

## Changelog

User-defined subgraphs may no longer be given names that start with
`__internal`, as this will be reserved for internally generated
subgraphs.

- Add a changelog entry (in the "Changelog entry" section below) if the
changes
  in this PR have any user-facing impact. See
[changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
  `no-changelog-required` label to the PR.

### Product

_(Select all products this will be available in)_

- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->

### Type

<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->

_(Select only one. In case of multiple, choose the most appropriate)_

- [ ] highlight
- [ ] enhancement
- [ ] bugfix
- [x] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->

### Changelog entry

<!--
  - Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
  - For Highlights start with a H4 heading (#### <entry title>)
  - Get the changelog entry reviewed by your team
-->

User-defined subgraphs may no longer have names that start with
`__internal`.

<!-- changelog-entry : end : DO NOT REMOVE -->

<!-- changelog : end : DO NOT REMOVE -->

V3_GIT_ORIGIN_REV_ID: a6df1bba7b1f2cda4593654c15d168f25b7134b2
2024-05-27 10:06:42 +00:00
Tom Harding
da62dfb2b0 Allow supergraph config to appear in subgraphs (#611)
## Description

As per [APG-113](https://hasurahq.atlassian.net/browse/APG-113), we
would like to remove the separate concept of supergraphs. This is the
first step: we allow the supergraph config to appear in subgraphs. The
next step will be to remove the non-subgraph version of it.

This PR updates the subgraph enum type to allow for supergraph config
objects, but does nothing else. This means it should be a no-op for
current users, but will be forward-compatible with the eventual goal of
the epic. We also throw in a makeshift test framework that we can
probably replace with Insta at a later date.

<!--
  Questions to consider answering:
  1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
  3. What is the conceptual design behind this PR?
  4. How can this PR be tested/verified?
  5. Does the PR have limitations?
  6. Does the PR introduce breaking changes?
-->

## Changelog

- Add a changelog entry (in the "Changelog entry" section below) if the
changes
  in this PR have any user-facing impact. See
[changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
  `no-changelog-required` label to the PR.

### Product

_(Select all products this will be available in)_

- [x] community-edition
- [x] cloud
<!-- product : end : DO NOT REMOVE -->

### Type

<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->

_(Select only one. In case of multiple, choose the most appropriate)_

- [ ] highlight
- [ ] enhancement
- [ ] bugfix
- [x] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->

### Changelog entry

<!--
  - Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
  - For Highlights start with a H4 heading (#### <entry title>)
  - Get the changelog entry reviewed by your team
-->

Supergraph configuration (`CompatibilityConfig`, `AuthConfig`, and
`GraphqlConfig`) may now be defined in any subgraph. Note that
supergraph configuration may still only be defined once for any given
supergraph.

<!-- changelog-entry : end : DO NOT REMOVE -->

<!-- changelog : end : DO NOT REMOVE -->

[APG-113]:
https://hasurahq.atlassian.net/browse/APG-113?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

V3_GIT_ORIGIN_REV_ID: a1b9911c4abb332fe37914efd1c3afbbd554a5d0
2024-05-24 08:59:55 +00:00