Backwards compatibility break in v0.36.4 and up

[boekkooi-impossiblecloud]

Good day,

Today I was updating one of our project to v0.37.0 and hit an issue where a lot of allOf where suddenly appearing.
I tracked it issue down to v0.36.4 which seems to break backward compatibility, the following test works with v0.36.3 but fails with v0.36.4.

Maybe you can point me in the right direction to fix this issue?
Thanks in advance and have a great day 😄

package openapi

import (
	"testing"

	"github.com/pb33f/libopenapi"
	"github.com/stretchr/testify/assert"
	"github.com/stretchr/testify/require"
)

func TestAllOf(t *testing.T) {
	docYAML := `openapi: 3.1.2
paths: {}
components:
    schemas:
        Timestamp:
            $schema: https://spec.openapis.org/oas/3.1/dialect/base
            type: string
            title: Timestamp
            format: date-time
        cmd.protoc_gen_go.testdata.proto.GetShipperRequest:
            $schema: https://spec.openapis.org/oas/3.1/dialect/base
            type: object
            properties:
                name:
                    type: string
                    description: |-
                        The resource name of the shipper to retrieve.
                         Format: shippers/{shipper}
            title: Get Shipper Request
            required:
                - name
            additionalProperties: true
            description: Request message for FreightService.GetShipper.
        cmd.protoc_gen_go.testdata.proto.Shipper:
            $schema: https://spec.openapis.org/oas/3.1/dialect/base
            type: object
            properties:
                name:
                    type: string
                    description: The resource name of the shipper.
                createTime:
                    $ref: '#/components/schemas/Timestamp'
                    description: The creation timestamp of the shipper.
                updateTime:
                    $ref: '#/components/schemas/Timestamp'
                    title: The last update timestamp of the shipper.
                    description: Updated when create/update/delete operation is performed.
                deleteTime:
                    $ref: '#/components/schemas/Timestamp'
                    description: The deletion timestamp of the shipper.
                displayName:
                    type: string
                    description: The display name of the shipper.
                billingAccount:
                    type: string
                    description: The billing account of the shipper.
                etag:
                    type: string
                    description: The current etag value of the shipper.
            title: Shipper
            required:
                - billingAccount
                - displayName
                - name
            additionalProperties: true
            description: A shipper is a supplier or owner of goods to be transported.
`

	doc, err := libopenapi.NewDocument([]byte(docYAML))
	require.NoError(t, err)

	model, err := doc.BuildV3Model()
	require.NoError(t, err)

	modelBytes, err := model.Model.Render()
	require.NoError(t, err)

	assert.Equal(t, docYAML, string(modelBytes))
}

[daveshanley]

looking into it...

[daveshanley]

Yeah, this is a regression after fixing #568

[daveshanley]

fix inbound.

[daveshanley]

Fixed in v0.37.1. With regression tests.

[boekkooi-impossiblecloud]

Hey @daveshanley

Thanks for the quick fix, now I can at least generate the yaml.
However, I think this only patched the output and didn't actually fix the underlying breaking change.

Now it seems I hit a second regression as a lot of my proxy.IsReference() calls now return false and proxy.Description fields are empty.

Due to your PR I can now manage to work around this by creating the following helper which looks a bit hacky but it seems to work.
Now for the other properties (Description, Title etc.) I will also have to create similar helpers.

func getReference(proxy *base.SchemaProxy) (string, bool) {
	if proxy.IsReference() {
		return proxy.GetReference(), true
	}

	// Fix issues an issue introduced in https://github.com/pb33f/libopenapi/issues/568
	// and partially fixed in https://github.com/pb33f/libopenapi/pull/576/changes#diff-66ce5cf67ff3d7fa89aa05c5d559dc33bd1ba0576e0051d5ef8ac4050af3a90cR422
	s := proxy.Schema()
	if s != nil && len(s.AllOf) == 2 && s.AllOf[0] != nil && s.AllOf[1] != nil && s.AllOf[1].IsReference() {
		return s.AllOf[1].GetReference(), true
	}

	return "", false
}

I'm not sure if there is a way to resolve this but maybe you have an idea?

[daveshanley]

Will fix this for you in an ergonomic way, no hacks!

[daveshanley]

This should now be fixed properly in v0.37.2.

The deeper issue was the OpenAPI 3.1 $ref + sibling-keyword transform leaking through the high-level model in inconsistent ways. v0.37.2 preserves the authored $ref + sibling shape on render, keeps SchemaProxy.IsReference() / GetReference() ergonomic for this case, exposes sibling keywords like description / title directly on the high-level schema, and keeps renderer/mock/generator paths from treating these schemas as pure aliases.

I added regression tests, you should be able to remove the workaround helper now.

[boekkooi-impossiblecloud]

Thanks a lot @daveshanley and wow you are fast!
I can confirm that with v0.37.2 I can safely remove the workaround and everything is back to what it was 😄

[daveshanley]

Glad to hear it! And speed is the name of the game!