-
Notifications
You must be signed in to change notification settings - Fork 17
fix(yml): keep folded block scalars stable across encode round trips #244
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
4a6a06e
fix(overlay): keep folded block scalars stable across repeated applies
AshGodfrey 15da3d1
test(overlay): cover the nil node guard in stabilizeFoldedScalars
AshGodfrey 8f14af9
fix(yml): stabilize folded scalars at every encode boundary
AshGodfrey 37c7fb9
fix(overlay): keep a nil overlay serializable
AshGodfrey c3cbd0a
fix(oq): stabilize folded scalars in YAML query output
AshGodfrey 77720c4
test: address review feedback on folded scalar coverage
AshGodfrey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| package openapi_test | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/speakeasy-api/openapi/openapi" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // The trailing table row is indented one space further than the rows above it, which | ||
| // makes yaml.v3 emit an extra line break before it on every encode. Marshalling has to | ||
| // stay a fixed point regardless of whether an overlay was involved. | ||
| const foldedScalarDocument = `openapi: 3.1.0 | ||
| info: | ||
| title: Test | ||
| version: 1.0.0 | ||
| description: >- | ||
| ### Widgets | ||
|
|
||
| | Name | Kind | | ||
| | ---- | ---- | | ||
| | acme | ` + "`petstore`" + ` | | ||
| paths: {} | ||
| ` | ||
|
|
||
| func TestMarshal_FoldedScalar_SurvivesRepeatedRoundTrips(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ctx := t.Context() | ||
|
|
||
| doc, validationErrs, err := openapi.Unmarshal(ctx, strings.NewReader(foldedScalarDocument)) | ||
| require.NoError(t, err) | ||
| require.Empty(t, validationErrs) | ||
|
|
||
| want := doc.Info.GetDescription() | ||
| require.Contains(t, want, "| acme |") | ||
|
|
||
| current := foldedScalarDocument | ||
| for i := range 3 { | ||
| doc, _, err := openapi.Unmarshal(ctx, strings.NewReader(current)) | ||
| require.NoError(t, err) | ||
|
|
||
| var buf bytes.Buffer | ||
| require.NoError(t, openapi.Marshal(ctx, doc, &buf)) | ||
|
|
||
| current = buf.String() | ||
| assert.Equal(t, want, doc.Info.GetDescription(), "value changed after %d round trips", i+1) | ||
| } | ||
| } |
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| package oq_test | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/speakeasy-api/openapi/graph" | ||
| "github.com/speakeasy-api/openapi/openapi" | ||
| "github.com/speakeasy-api/openapi/oq" | ||
| "github.com/speakeasy-api/openapi/references" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| // The trailing table row is indented one space further than the rows above it, which | ||
| // makes yaml.v3 emit an extra line break before it on every encode. The break lands | ||
| // inside the scalar, so it changes the decoded value rather than just the layout. | ||
| const foldedScalarSpec = `openapi: 3.1.0 | ||
| info: | ||
| title: Test | ||
| version: 1.0.0 | ||
| paths: {} | ||
| components: | ||
| schemas: | ||
| Widget: | ||
| type: object | ||
| description: >- | ||
| ### Widgets | ||
|
|
||
| | Name | Kind | | ||
| | ---- | ---- | | ||
| | acme | ` + "`petstore`" + ` | | ||
| ` | ||
|
|
||
| func loadFoldedScalarGraph(t *testing.T) *graph.SchemaGraph { | ||
| t.Helper() | ||
|
|
||
| ctx := t.Context() | ||
|
|
||
| doc, _, err := openapi.Unmarshal(ctx, strings.NewReader(foldedScalarSpec), openapi.WithSkipValidation()) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, doc) | ||
|
|
||
| idx := openapi.BuildIndex(ctx, doc, references.ResolveOptions{ | ||
| RootDocument: doc, | ||
| TargetDocument: doc, | ||
| TargetLocation: "spec.yaml", | ||
| }) | ||
|
|
||
| return graph.Build(ctx, idx) | ||
| } | ||
|
|
||
| // formattedDescription pulls the description back out of a `key:\n <schema>` wrapper. | ||
| func formattedDescription(t *testing.T, formatted string) string { | ||
| t.Helper() | ||
|
|
||
| var decoded map[string]struct { | ||
| Description string `yaml:"description"` | ||
| } | ||
| require.NoError(t, yaml.Unmarshal([]byte(formatted), &decoded)) | ||
| require.Len(t, decoded, 1) | ||
|
|
||
| for _, schema := range decoded { | ||
| return schema.Description | ||
| } | ||
|
|
||
| return "" | ||
| } | ||
|
|
||
| // `openapi spec query --format yaml` marshals graph nodes with its own encoder, so it | ||
| // needs the same stabilization as every other encode boundary. | ||
| func TestFormatYAML_FoldedScalarKeepsItsValue(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| var want struct { | ||
| Components struct { | ||
| Schemas map[string]struct { | ||
| Description string `yaml:"description"` | ||
| } `yaml:"schemas"` | ||
| } `yaml:"components"` | ||
| } | ||
| require.NoError(t, yaml.Unmarshal([]byte(foldedScalarSpec), &want)) | ||
| wantDescription := want.Components.Schemas["Widget"].Description | ||
| require.Contains(t, wantDescription, "| acme |") | ||
|
|
||
| g := loadFoldedScalarGraph(t) | ||
|
|
||
| result, err := oq.Execute(`schemas | where(name == "Widget")`, g) | ||
| require.NoError(t, err) | ||
| require.Len(t, result.Rows, 1) | ||
|
|
||
| formatted := oq.FormatYAML(result, g) | ||
| require.NotEmpty(t, formatted) | ||
|
|
||
| assert.Equal(t, wantDescription, formattedDescription(t, formatted)) | ||
| assert.NotContains(t, formatted, ">-", "affected folded scalars should be emitted as literal blocks") | ||
| } | ||
|
|
||
| // Formatting the same result twice must not drift, since FormatYAML restyles graph | ||
| // nodes in place. | ||
| func TestFormatYAML_FoldedScalarIsAFixedPoint(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| g := loadFoldedScalarGraph(t) | ||
|
|
||
| result, err := oq.Execute(`schemas | where(name == "Widget")`, g) | ||
| require.NoError(t, err) | ||
|
|
||
| first := oq.FormatYAML(result, g) | ||
| for i := range 3 { | ||
| assert.Equal(t, first, oq.FormatYAML(result, g), "output changed on format %d", i+2) | ||
| } | ||
| } |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| package overlay | ||
|
|
||
| import ( | ||
| "github.com/speakeasy-api/openapi/yml" | ||
| ) | ||
|
|
||
| // stabilizeFoldedScalars restyles folded block scalars in the overlay's own update | ||
| // payloads so that serializing the overlay round trips unchanged. | ||
| func (o *Overlay) stabilizeFoldedScalars() { | ||
| if o == nil { | ||
| return | ||
| } | ||
|
|
||
| for i := range o.Actions { | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| yml.StabilizeFoldedScalars(&o.Actions[i].Update) | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be worth adding a test fixture with a more-indented string in https://github.com/speakeasy-api/openapi/tree/main/openapi/testdata/localize
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 77720c4.
input/components.yamlnow gives theUserschema a folded description whose last table row is indented one space further, and bothoutput_counter/components.yamlandoutput_pathbased/components.yamlpin the localized result as a literal block.I checked it is a real guard and not just a snapshot: neutralizing the
StabilizeFoldedScalarscall inlocalize.goflips the goldens back to>-with the injected breaks, and both localize tests fail.