Only apply the schema definition prefix when explicitly specified - #715
Open
josephschorr wants to merge 1 commit into
Open
Only apply the schema definition prefix when explicitly specified#715josephschorr wants to merge 1 commit into
josephschorr wants to merge 1 commit into
Conversation
Schema definition prefixes were auto-detected: `schema write` and `import` read the existing schema from the server, extracted the common prefix from its definitions, and applied that prefix to the schema being written. This existed to support Serverless, which is gone, and it means writing an unprefixed schema to a permissions system with a prefixed one silently rewrites the definitions. Remove `determinePrefixForSchema` so the prefix is applied only when `--schema-definition-prefix` is passed. The flag continues to work exactly as before when specified. `schema copy` is unaffected in behavior: the prefix it inferred came from the source schema, whose definitions already carry it, and `compiler.ObjectTypePrefix` does not re-prefix an already-prefixed definition. It now skips a redundant compile. `schema write` and `import` each drop a ReadSchema round-trip. `commands.ReadSchema` had no remaining callers and is removed.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
Schema definition prefixes are currently auto-detected.
zed schema writeandzed importread the existing schema from the server, extract the common prefix from its definitions, and apply that prefix to the schema being written. This existed to support Serverless, which is gone, and it has a bad failure mode: writing an unprefixed schema to a permissions system that has a prefixed one silently rewrites your definitions.This removes
determinePrefixForSchema, so the prefix is applied only when--schema-definition-prefixis passed. The flag itself is unchanged and continues to work exactly as before when specified — prefixing is now opt-in rather than the default.Per-command impact
schema write— the actual behavior change. An unprefixed schema now writes through as authored. Also drops aReadSchemaround-trip.import— same, and likewise one fewer round-trip.schema copy— no behavior change. The prefix it inferred came from the source schema, whose definitions already carry it, andcompiler.ObjectTypePrefixdoes not re-prefix an already-prefixed definition. It just skips a redundant compile now.commands.ReadSchemahad no remaining callers once the inference was gone, so it is removed as well.Tests
Added a
TestSchemaWritecase asserting that an existing prefixed schema on the server does not cause the written schema to be prefixed; it fails against the previous behavior. TheReadSchemamock still serves the prefixed schema so that reintroducing inference fails the assertion rather than silently passing.TestDeterminePrefixForSchemais removed along with the function.TestRewriteSchemaand the import prefix tests are untouched and still pass, covering the explicit-prefix paths.Help text on all three commands notes that no prefix is added unless specified, and
docs/zed.mdis regenerated.