Merge allOf #828
Merge allOf #828
Conversation
…to merge-allof
Merge allof
update doc
Merge allof
openapi3/schema_merge_test.go
Outdated
| }, | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Equal(t, []string{"string", "boolean"}, merged.OneOf[0].Value.Required) |
There was a problem hiding this comment.
I think it should be boolean only.
| }, | ||
| }) | ||
|
|
||
| require.EqualError(t, err, TypeErrorMessage) |
There was a problem hiding this comment.
This error is used for two different scenarios.
I recommend to use a dedicated error here.
openapi3/schema_merge.go
Outdated
| orderMap := make(map[string]int) | ||
| orderMap[formatInt32] = 1 | ||
| orderMap[formatInt64] = 2 | ||
| orderMap[formatDouble] = 3 |
There was a problem hiding this comment.
Shouldn't the order be reversed?
According to the OpenAPI Spec, Double is wider than Float:
| Type | Format | Description |
|---|---|---|
| number | float | Floating-point numbers. |
| number | double | Floating-point numbers with double precision. |
openapi3/schema_merge_test.go
Outdated
| require.Equal(t, "base schema", merged.Title) | ||
| } | ||
|
|
||
| func TestMerge_NumericFormat(t *testing.T) { |
There was a problem hiding this comment.
I recommend to split this into 3 tests
openapi3/schema_merge_test.go
Outdated
| schema := Schema{} | ||
| merged, err := Merge(schema) | ||
| require.NoError(t, err) | ||
| require.Equal(t, &schema, merged) //todo &schema |
There was a problem hiding this comment.
I suggest to clarify the TODO message or remove it
openapi3/schema_merge_test.go
Outdated
| } | ||
| merged, err := Merge(schema) | ||
| require.NoError(t, err) | ||
| require.Equal(t, &schema, merged) //todo &schema |
There was a problem hiding this comment.
I suggest to clarify the TODO message or remove it
Merge allof
|
Hi @fenollp, Please note that we expect frequent tweaks on this functionality during the coming months until it is stable, so Thank you, |
|
Hi @reuvenharrison ! Does this seem reasonable to you? I don't kind this func to change in the future. I agree its impl could leave in your repo but I'd rather the lib offers this kind of thing from upstream somewhat. But if you prefer let's have the visitor in this lib and you impl your merge in your repo and once it's stable enough for you then open a second PR with your merge impl. That's probably the easiest for everyone. |
|
Hi @fenollp, |
|
We implemented this in oasdiff in order to answer the primary requirement ASAP: Happy to assist integration/migration to this repo if anyone wants to lead this effort. |
Objective
This PR introduces a function that simplifies OpenAPI specifications by removing subschemas under
allOfand replacing them with a single merged (or flattened) top-level schema.The resulting schema remains equivalent to the original one in terms of validation.
The use case we had in mind for this PR is for detecting breaking-changes which is implemented in https://github.com/Tufin/oasdiff using kin-openapi3. There may be other use cases which we did not think about.
The
MergefunctionMergefunction accepts a schema as input and returns a new equivalent schema withoutallOfMergereturns an errorUnit testing
Mergeworks for different kinds of subschemas underallOfValidation testing
ValidateRequestto various inputs and verify that either the input matches both schemas or the input doesn't match both schemas, if one matches and the other doesn't, then the schemas are not equivalent.Development Status