Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check references via idRef in UnreferencedShapes #2105

Merged
merged 6 commits into from
Jan 24, 2024

Conversation

milesziemer
Copy link
Contributor

Applications of traits with @idRef members don't create relationships between shapes, so removing unreferenced shapes from the following:

service FooService {
    ...
    operations: [GetFoo]
}

operation GetFoo {
    output := {
        foo: Foo
    }
}

@trait
@idRef(failWhenMissing: true, selector: "*")
string myIdRef

@myIdRef(Referenced)
structure Foo {}

stucture Referenced {}

will remove Referenced from the model. This commit changes UnreferencedShapes so that Referenced is considered connected to FooService because it is referenced by an @idRef on a shape connected to the service.

Determining which shapes are connected via id ref is done by the new IdRefShapeReferences class, which uses the following process:

  1. Get all trait definition shapes.
  2. Find all paths from those trait definition shapes to shapes with the idRef trait. Each path found says that applications of the trait definition may have a shape reference.
  3. For each path, get all applications of the trait on connected shapes and search the node value using the path to the shape that had idRef.
  4. For each search result, exclude all connected shapes from being unreferenced.

Other changes:

  • A few test cases that expected to receive an UnreferencedShape event were updated
  • An asShapeId method was added to StringNode

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Applications of traits with `@idRef` members don't create relationships
between shapes, so removing unreferenced shapes from the following:
```
service FooService {
    ...
    operations: [GetFoo]
}

operation GetFoo {
    output := {
        foo: Foo
    }
}

@trait
@idref(failWhenMissing: true, selector: "*")
string myIdRef

@myIdRef(Referenced)
structure Foo {}

stucture Referenced {}
```
will remove `Referenced` from the model. This commit changes
UnreferencedShapes so that `Referenced` is considered connected
to `FooService` because it is referenced by an `@idRef` on a shape
connected to the service.

Determining which shapes are connected via id ref is done by the
new IdRefShapeReferences class, which uses the following process:
1. Get all trait definition shapes.
2. Find all paths from those trait definition shapes to shapes with
the idRef trait. Each path found says that applications of the trait
definition may have a shape reference.
3. For each path, get all applications of the trait on connected shapes
and search the node value using the path to the shape that had idRef.
4. For each search result, exclude all connected shapes from being
unreferenced.

Other changes:
- A few test cases that expected to receive an UnreferencedShape
event were updated
- An `asShapeId` method was added to StringNode
@milesziemer milesziemer marked this pull request as ready for review January 19, 2024 17:39
@milesziemer milesziemer requested a review from a team as a code owner January 19, 2024 17:39
Previous impl wouldn't consider shapes referenced through multiple
levels of idRef as connected, since idRef'd shapes could themselves
be connected to other shapes that have idRef references. Refactoring
to a custom NeighborProvider means the shape walker just traverses
those relationships in a single pass.
@milesziemer milesziemer force-pushed the check-idref-in-unref-shapes branch from a335b4f to 505285e Compare January 22, 2024 20:20
@milesziemer milesziemer force-pushed the check-idref-in-unref-shapes branch 4 times, most recently from 0ec93bb to 8fbb1d3 Compare January 23, 2024 22:19
Also makes most NodeQuery queries static instances, and
moves some stuff around in tests.
@milesziemer milesziemer force-pushed the check-idref-in-unref-shapes branch from 8fbb1d3 to 67b47d3 Compare January 23, 2024 22:20
Changes NodeQuery to use a single instance of a Queue for keeping
track of and aggregating query results to avoid extra allocations
within queries and when joining the results.
@milesziemer milesziemer force-pushed the check-idref-in-unref-shapes branch from cd46c50 to e957978 Compare January 23, 2024 23:40
@milesziemer milesziemer merged commit f406fbf into smithy-lang:main Jan 24, 2024
10 checks passed
hpmellema pushed a commit to hpmellema/smithy that referenced this pull request Jan 25, 2024
Applications of traits with `@idRef` members don't create relationships
between shapes, so removing unreferenced shapes from the following:
```
service FooService {
    ...
    operations: [GetFoo]
}

operation GetFoo {
    output := {
        foo: Foo
    }
}

@trait
@idref(failWhenMissing: true, selector: "*")
string myIdRef

@myIdRef(Referenced)
structure Foo {}

stucture Referenced {}
```
will remove `Referenced` from the model. This commit changes
UnreferencedShapes so that `Referenced` is considered connected
to `FooService` because it is referenced by an `@idRef` on a shape
connected to the service.

This commit adds a new kind of shape relationship, ID_REF, from
a shape to any shapes referenced by `@idRef`'s within the shape's
trait values. This relationship is created by doing the following:
1. Get all trait definition shapes.
2. Find all paths from those trait definition shapes to shapes with
the idRef trait. Each path found says that applications of the trait
definition may have a shape reference.
3. For each path, get all applications of the trait and search their
node values using the path to the shape that had idRef.
4. Check the node value - if it is a shape id, create the ID_REF
relationship between the shape that originally had the trait applied,
and the found shape id.
NeighborProvider was updated to allow wrapping an existing provider
in a new one with ID_REF relationships, and UnreferencedShapes to
use the wrapped provider. UnreferencedShapes will now traverse the
ID_REF relationships, so any idRef'd shapes will be considered
connected.

Other changes:
- A few test cases that expected to receive an UnreferencedShape
event were updated
- An `asShapeId` method was added to StringNode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants