## Description
This PR moves all the tests that were added to
`crates/engine/tests/validate_metadata_artifacts/aggregate_expressions`
to the metadata-resolve `metadata_golden_tests`.
All the `error.txt` files got renamed to `expected_error.txt` and the
`metadata is not consistent:` bit on the front of the error got trimmed
off.
I had to change the way that `test_failing_metadata` works so that
instead of looking for directories under `failing/` it looks for
`metadata.json` files under `failing/`. This is because not every
directory has a test in it, some directories are just used for grouping
(eg `failing/aggregate_expressions/`). This doesn't change what tests
get run. The only side effect is that every test name has `_metadata`
suffixed to it (the filename). 😢 `test-each` doesn't appear to offer a
way to disable this behaviour.
V3_GIT_ORIGIN_REV_ID: dece7cd3bb4623c51050d12471d2c0990fd69bfd
Rules:
* The schema ID is set by the `id` property, falling back to `rename`.
* The schema ID is always prefixed by a URL base.
* The schema name is set by the `rename` property, falling back to `id`.
* The schema title is set by the `title` property, falling back to
`rename`, then `id`.
* If the name or title are automatically created from the ID, remove
characters that might choke a code generator, such as ' ' or '/'.
V3_GIT_ORIGIN_REV_ID: fac46a0849c2f36642daeca2971fdce7d253256a
This is Part 3 in a stacked PR set that delivers aggregate root field
support.
* Part 1: OpenDD: https://github.com/hasura/v3-engine/pull/683
* Part 2: Metadata Resolve: https://github.com/hasura/v3-engine/pull/684
JIRA: [V3ENGINE-159](https://hasurahq.atlassian.net/browse/V3ENGINE-159)
## Description
This PR implements the GraphQL API for aggregate root fields. The
GraphQL schema matches the design in the [Aggregate and Grouping
RFC](https://github.com/hasura/v3-engine/blob/main/rfcs/aggregations.md#aggregations-walkthrough).
### Schema Generation
The main new part of the GraphQL schema generation can be found in
`crates/schema/src/aggregates.rs`. This is where we generate the new
aggregate selection types. However, the root field generation can be
found in `crates/schema/src/query_root/select_aggregate.rs`.
The new `filter_input` type generation lives in
`crates/schema/src/model_filter_input.rs`. As this type effectively
encapsulates the existing field arguments used on the Select Many root
field, the code to generate them has moved into `model_filter_input.rs`
and `select_many.rs` simply reuses the functionality from there (without
actually using the filter input type!).
### IR
The main aggregates IR generation for the aggregate root field happens
in `crates/execute/src/ir/query_root/select_aggregate.rs`. It reads all
the input arguments to the root field and then kicks the selection logic
over to `model_aggregate_selection_ir` from
`crates/execute/src/ir/model_selection.rs`.
`crates/execute/src/ir/model_selection.rs` has received some refactoring
to facilitate that new `model_aggregate_selection_ir` function; it
mostly shares functionality with the existing `model_selection_ir`,
except instead of creating fields IR, it creates aggregates IR instead.
The actual reading of the aggregate selection happens in
`crates/execute/src/ir/aggregates.rs`.
The aggregates selection IR captures the nested JSON structure of the
aggregate selection, because NDC does not return aggregates in the same
nested JSON structure as the GraphQL request. NDC takes a flat list of
aggregate operations to run. This captured nested JSON structure is used
during response rewriting to convert NDC's flat list into the nested
structure that matches the GraphQL request.
The aggregate selection IR is placed onto `ModelSelection` alongside the
existing fields IR. Since both fields and aggregates can be put into the
one NDC request (even though they are not right now), this made sense.
They both translate onto one NDC `Query`. This necessitated making the
field selection optional on the `ModelSelection`
(`ModelSelection.selection`), since aggregate requests currently don't
use them.
### Planning
`crates/execute/src/plan/model_selection.rs` takes care of mapping the
aggregates into the NDC request from the generated IR.
There has been a new `ProcessResponseAs` variant added in
`crates/execute/src/plan.rs` to capture how to read and reshape an NDC
aggregates response. This is handled in
`crates/execute/src/process_response.rs` where the captured JSON
structure in the IR is used to restore NDC's flat aggregates list into
the required nested JSON output structure.
### Testing
The Custom Connector has been updated with functionality to allow
aggregates over nested object fields
(`crates/custom-connector/src/query.rs`).
New execution and introspection tests have been added to
`crates/engine/tests/execute/aggregates/` to test aggregates against
Postgres and the Custom Connector.
[V3ENGINE-159]:
https://hasurahq.atlassian.net/browse/V3ENGINE-159?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
V3_GIT_ORIGIN_REV_ID: ff47f13eaca70d10de21e102a6667110f8f8af40
This is Part 2 in a stacked PR set that delivers aggregate root field
support.
* Part 1: OpenDD: https://github.com/hasura/v3-engine/pull/683
* Part 3: GraphQL API: https://github.com/hasura/v3-engine/pull/685
JIRA: [V3ENGINE-159](https://hasurahq.atlassian.net/browse/V3ENGINE-159)
## Description
This PR implements the metadata resolve phase of the engine and adds
support for resolving `AggregateExpression`s and validates their use
when linked to a `Model`.
The bulk of the changes can be found in:
* `crates/metadata-resolve/src/stages/aggregates/*` - This is where the
`AggregateExpression`s are resolved
* `crates/metadata-resolve/src/stages/models/mod.rs` - This is where we
validate the `AggregateExpression` specified for use in the model is
actually compatible with the model and its data connector
The `ndc-spec` version used has been lifted to the latest version that
adds support for aggregates over nested objects
(https://github.com/hasura/ndc-spec/pull/144). This necessitated changes
in the Custom Connector, but actual functionality to implement
aggregation over nested objects is implemented in Part 3.
There are also some changes in
`crates/metadata-resolve/src/types/subgraph.rs` where the `Display`
trait for the various `Qualified<T>`, `QualifiedTypeReference`, etc
types has been reworked so that they print more cleanly, with the
subgraph being put outside the type syntax, and array types getting
formatted correctly. For example, previous an array of Varchars would
have printed as `Varchar (in subgraph default)!`, now it properly
formats as `[Varchar!]! (in subgraph default)`.
This was necessary to make useful error messages using these types.
A tonne of tests have been added in
`crates/engine/tests/validate_metadata_artifacts/aggregate_expressions`
to test every error condition of the metadata resolve process.
[V3ENGINE-159]:
https://hasurahq.atlassian.net/browse/V3ENGINE-159?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
V3_GIT_ORIGIN_REV_ID: ffd859127a3f1560707f06ef01906c9d1b183d31
This is a no-op refactor of IR related types. Introducing a new concrete
`IR` enum with `Mutation` and `Query` variants, where each contains a
map of root fields.
V3_GIT_ORIGIN_REV_ID: 76a197f5cf1efb34a8493c2b3356bea2bc2feb3c
<!-- Thank you for submitting this PR! :) -->
## Description
<!--
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?
-->
This PR reverts the changes to `FieldMapping` and introduces the
`argument_mapping` in `ColumnMapping` instead.
V3_GIT_ORIGIN_REV_ID: badca2416b1a1b82d1a596aa60bf7a39b3b6152a
<!-- Thank you for submitting this PR! :) -->
## Description
It turns out that every usage of `Builder::allow_all_namespaced`
concerned itself with either introspection nodes or (in the GDS case)
`None`.
Combined with the fact that both the SDL and GDS implementations of
`SchemaContext::introspection_namespace_node` were trivial, the
`NamespacedNodeInfo` contained in
`Namespaced::AllowAll(S::NamespacedNodeInfo)` is always functionally
meaningless.
This PR removes the `NamespacedNodeInfo` from `Namespaced::AllowAll` as
well as `SchemaContext::introspection_namespace_node`, thus simplifying
the implementation.
<!--
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)_
- [ ] community-edition
- [ ] 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
- [ ] 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
-->
_Replace with changelog entry_
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 1525905488203ff07182aa700c9fff26512743ab
<!-- Thank you for submitting this PR! :) -->
## Description
<!--
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?
-->
JIRA: https://hasurahq.atlassian.net/browse/V3ENGINE-148
This is part 1 of supporting field arguments in the engine.
Part 2 is here: https://github.com/hasura/v3-engine/pull/695
It adds the OpenDD types according to the RFC and resolves them.
V3_GIT_ORIGIN_REV_ID: f7736c968489e35c9133f2c6e276d05baf84d4be
<!-- Thank you for submitting this PR! :) -->
## Description
The level of nesting in this file makes it hard to follow, this breaks
the deepest nesting into a function. Functional no-op.
V3_GIT_ORIGIN_REV_ID: 03777bdae0c772754f5ca370dcc3d9577abc9c80
If a function doesn't return a value, terminate with a semicolon.
I also moved `implicit_hasher` and `return_self_not_must_use` to the
"definitely keep disabling this" list, and installed
[Bacon](https://dystroy.org/bacon/) in the Nix shell to make it easier
to run Clippy.
V3_GIT_ORIGIN_REV_ID: ffb17b42d982518aec433a1676dba0a0dd0ad95d
<!-- Thank you for submitting this PR! :) -->
## Description
Should have been removed in https://github.com/hasura/v3-engine/pull/680
V3_GIT_ORIGIN_REV_ID: 027d4bb88c32d400cf260677e2fef3802e479af4
<!-- 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
It turns out that code generators based on JSON schemas don't always
play nicely with characters such as `' '` or `'/'`. To work around this,
when we use the schema name as a title, we replace any non-alphanumeric
character or underscore with an underscore (`'_'`).
V3_GIT_ORIGIN_REV_ID: 6af9a92c84c59294ff28bae27d7421627cde6a80
<!-- 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
<!-- Thank you for submitting this PR! :) -->
## Description
When we generate a JSON schema via `schemars`, we end up with duplicate
types in the schema, which get names like `ValueExpression2`, `Role2`,
and so on. This isn't ideal, and seems to arise for two reasons:
1. The type is polymorphic, and is monomorphised in two ways, and thus
the types can't be unified.
2. The type is monomorphic, but is used inside and outside of its home
module.
The first problem was fixed previously by splitting polymorphic types,
but the second has proven to be a bit more work. This PR finally solves
the problem by introducing a new library, `jsonschema-tidying`:
* First, we search the definitions within the JSON schema for any whose
names end in a number, such as `ValueExpression2` or `MetadataV2`.
* Then, we look for types whose names match everything up to the final
numeric digits, and discard any types for whom we can't find a match (so
we keep `ValueExpression2` because `ValueExpression` exists, but discard
`MetadataV2` because `MetadataV` does not).
* Next, we remove the duplicate definition from the definitions map,
potentially breaking links in both the schema _and_ the rest of the
definitions map.
* Finally, we traverse the entirety of the tree looking for any
references to the duplicate entry, and replace them with references to
the original entry.
This PR has no direct user-facing change, however it _will_ have an
effect on the docs generation code, which will hopefully result in
tidier documentation.
<!--
Questions to consider answering:
1. What user-facing changes are being made?
4. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
5. What is the conceptual design behind this PR?
6. How can this PR be tested/verified?
7. Does the PR have limitations?
8. 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)_
- [ ] community-edition
- [ ] 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
- [x] 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
-->
_Replace with changelog entry_
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: fe73acf7d9df0b9867852e673e53cb086e3725d3
<!-- Thank you for submitting this PR! :) -->
## Description
With the introduction of `NamespacedGetter` as the means deciding how to
interpret what it means to extract "namespaced" data from a schema it is
superfluous to thread along the "namespace" that lookups will be based
on.
This PR removes those namespace arguments.
As a nice side effect this removes the cases where we used to have to
supply a dummy role value when generating a role-agnostic schema.
<!--
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)_
- [ ] community-edition
- [ ] 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
- [ ] 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
-->
_Replace with changelog entry_
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 0aca2b3838f1b0d944b3c41ddd54d20dc74503a4
<!-- Thank you for submitting this PR! :) -->
## Description
We want to test `where` clauses over nested selects, so first let's get
a working test for selecting everything. Adds new test tables and
updates them in the test metadata etc. Functional no-op.
V3_GIT_ORIGIN_REV_ID: 8ebc6b27379b930eaf2220061c2ec58cd1650167
Add a test case for input type infinite recursion bug (https://github.com/hasura/v3-engine/pull/676). I confirmed the
test failure when the fix was removed.
V3_GIT_ORIGIN_REV_ID: b9d0a9b2a65328aa52f5ec057dcba6470c4e3956
## Description
This PR implements argument presets for `DataConnectorLink`, which can
be used to forward request headers as function/procedure arguments to
NDC. This PR implements the execution part.
**Note**: response header forwarding is not implemented yet.
V3_GIT_ORIGIN_REV_ID: ff69129aee3e3052367ca42acdec3922cbc2cb0c
<!-- Thank you for submitting this PR! :) -->
## Description
We'd like not to tie our boolean expressions to one particular data
connector, we were doing this in relationships and had no need to.
Functional no-op.
V3_GIT_ORIGIN_REV_ID: 0af16ad92117a45ac1f424b49d9f13184787f63f
## Description
This PR implements argument presets and response headers for
`DataConnectorLink`, which can be used to forward request headers as
function/procedure arguments to NDC, and also forward response headers
from NDC back to the client. This PR implements only the OpenDD changes
and metadata resolving part.
V3_GIT_ORIGIN_REV_ID: 2b7f20af7c2f192c0103553bccb000114dcba916
<!-- Thank you for submitting this PR! :) -->
## Description
This PR is a refactoring which is a functional no-op.
<!--
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)_
- [ ] community-edition
- [ ] 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
- [ ] 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
-->
_Replace with changelog entry_
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 625475ef23ef7fd2f72921e951fa1a957807516a
Previously we required an operator called `_eq` to be called `_eq` in
all data connectors that used it.
Now we can give them nice names, and map each one to a different
operator name per data connector.
V3_GIT_ORIGIN_REV_ID: 1d2f678e25d932e5ea45d34999cf5709ef0038b4
<!-- Thank you for submitting this PR! :) -->
## Description
Very occasionally these tests fail with the wrong duplicated graphql
name. Not sure why, I assume some unpredictable HashMap ordering
somewhere. In lieu of changing all HashMaps to BTreeMaps in engine
(which isn't immediately possible because of missing `Ord` instances for
`serde_json::Value`), let's make only one set of failing items in these
tests so the same one goes wrong each time.
Functional no-op.
V3_GIT_ORIGIN_REV_ID: 6dff998ae66aedbcca05dd05dc6fcb12e6d704c3
## Description
JIRA: https://hasurahq.atlassian.net/browse/V3ENGINE-180
Fix an edge case where the function
`build_annotations_from_input_object_type_permissions` produce infinite
recursions
when an input object type contains fields with self-referential types.
Note: This PR doesn't contain any tests. They'll be added in a separate
PR ([JIRA](https://hasurahq.atlassian.net/browse/V3ENGINE-188)). This
fix needs to be done quickly and merged as other teams (graphql
connector) are blocked by this.
V3_GIT_ORIGIN_REV_ID: 4cc09cb96871804d4daf83d7e49a6f4529c1943d
The original Aggregates RFC is missing some descriptions fields in its
specification of the Open DD metadata, which means that those particular
items currently don't get a description in GraphQL.
This PR adds the missing descriptions to the RFC document.
V3_GIT_ORIGIN_REV_ID: a93d5d92c427fe95f85ed3889660d2c22bc08f5a
<!-- Thank you for submitting this PR! :) -->
## Description
This makes the absolute happiest path `BooleanExpressionType` work.
Caveats:
- We ignore operator mappings, so we assume that the OpenDD operator
name (ie, `_eq`) is the same as the data connector's name for it.
- We have not tried to implement `comparisonRelationships` in any way
yet.
- Have not tried nested objects yet.
All functionality here is behind a feature flag so has no user-facing
impact.
V3_GIT_ORIGIN_REV_ID: 5edb706c6d8b03b9fa59433ce24f05f37a69729a
## Description
There is a bug in NDC validation of command where it returns early
without validating. This is because we perform the NDC validation right
after resolving the command source, but haven't attached the "resolved
command source" to the command yet. As a result the command's source is
`None`, and the validation does an early return.
This PR fixes it by taking the command source directly. If there is no
source to resolve, we don't have to perform any NDC validation of the
command.
Note: this is not the case with models. Model's source resolving
attaches the "resolved source", then performs the validation.
## 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
- [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
-->
Fix skipping of NDC validation of commands due to wrong assumption.
<!-- changelog-entry : end : DO NOT REMOVE -->
<!-- changelog : end : DO NOT REMOVE -->
V3_GIT_ORIGIN_REV_ID: 5b9f4c922ee83394f1b6fe840cd934b6e138e2e9
<!-- Thank you for submitting this PR! :) -->
## Description
We were using `String` for these, and making some assumptions about NDCs
and OpenDD operator names being the same. This uses newtypes and
documents where these assumptions are being made. Functional no-op.
V3_GIT_ORIGIN_REV_ID: 195e55db6fadf48c956684130e23f647eaa17fb1
Calling `.to_owned()` on a reference, `.to_vec()` on a vector reference,
etc. are just synonyms for `.clone()` which are less explicit about
cloning. Let's be explicit.
This also removes some unnecessary clones.
V3_GIT_ORIGIN_REV_ID: 1bc00c4106f0346303d73e4268c89030c0ce93fc