Entity validation using schema fragments with external entity types - #116
Open
john-h-kastner-aws wants to merge 1 commit into
Open
Entity validation using schema fragments with external entity types#116john-h-kastner-aws wants to merge 1 commit into
john-h-kastner-aws wants to merge 1 commit into
Conversation
Signed-off-by: jkastner <jkastner@amazon.com>
|
|
||
| Following the same basic design, there are some alternative syntax options to consider. | ||
|
|
||
| * We could use a different keyword: `extern`, shorter, also used by C and Rust; `import`, perhaps wrongly gives the impression that we're loading in an entity definition from somewhere; `use`, similar to `import`. |
Member
There was a problem hiding this comment.
I like the use of external or extern rather than use / import, which to me implies that we have access to the definition of the imported entity. extern makes it clear this is just a symbol that is declared externally.
|
|
||
| When combining multiple fragments, we will attempt to link external entity type declarations with concrete definitions. Entity types are linked when their names match precisely (accounting for any containing namespace), but may never be linked with a builtin or common type definition even if the names are the same. An entity type may be declared as external any number of times, but it may only have one concrete definition across all schema fragments. If there is a definition, then that entity type becomes concrete in the resulting fragment, and is treated exactly the same as any other entity type. When there is no concrete definition the entity type remains external in the resulting fragment, but we do not report an error. | ||
|
|
||
| If every external entity type in a fragment has a corresponding definition, then the fragment is fully concrete. We can construct a complete `Schema` from it and use it exactly as if there were no external entity type declarations. In this situation, external declarations act as a helpful tool for schema authors to explicitly specify the dependencies between different schema fragments, but could be omitted entirely. |
Member
There was a problem hiding this comment.
nit: it says fully concrete but later refers to complete, I prefer complete to talk about a complete or closed schema with no unresolved external references.
| ## Alternatives | ||
|
|
||
| * All undefined type names are assumed to be entity types. This isn't actually an alternative, but it's worth looking at why we can't do this. Schema fragments already have a meaning in which an undefined name may legitimately refer to a common type or action defined in another fragment. Assuming that every unresolved reference is an entity type would silently reinterpret those references and change the meaning of existing fragments, so a fragment could no longer be composed with the definitions it was actually written against. We might also frame this as a soundness issue — an entity that validated against a fragment could fail to validate against the complete schema. | ||
| * Add syntax for tagging the kind of type reference when it's used. E.g., `other: B::<entity>`. If `B` is defined as an entity, then it resolves to `B`, otherwise `B` is assumed to be external. This could also be useful for resolving ambiguity when a schema defines `B` as a common type and an entity type. I don't like this option because it doesn't explicitly say that `B` is external, just that it's an entity. We also wouldn't be able to detect simple typos like `Usr` instead of `User` before building the complete schema. |
Member
There was a problem hiding this comment.
I didn't understand that part, is <entity> a tag that would indicate what kind of type B is?
|
|
||
| To validate a `User` entity we would check that all elements of `my_actions` are one of the declared instances of `Shared::Action`. | ||
|
|
||
| We could not validate a request which applies to an external action. |
Member
There was a problem hiding this comment.
Beyond this limitation, are there other reasons we should not consider an extension to actions?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rendered
Checklist
text/SUMMARY.mdin numerical order