goadesign / goa

🌟 Goa: Elevate Go API development! 🚀 Streamlined design, automatic code generation, and seamless HTTP/gRPC support. ✨
https://goa.design
MIT License
5.69k stars 559 forks source link

Generating OpenAPI file has wrong schemas used #3242

Open michaelgambold opened 1 year ago

michaelgambold commented 1 year ago

I'm having an issue where the generated OpenAPI file is using a different type (though identical in format) than what is specified. We are running ~3.10.2~ 3.11.0 of goa.

Say I have two files that contain design documentation (ObjectA & ObjectB) and each has a PUT api method and both bodies have a "child" field object that contains the field "ref" that is a string.

Body Objects

// ObjectA Body Object
{
  child: {
    ref: string;
  }
}

// ObjectB Body Object
{
  child: {
    ref: string;
  }
}

_objecta.go

package design

import (
    . "goa.design/goa/v3/dsl"
)

var _ = Service("objectA", func() {

    HTTP(func() {
        Path("/objectA")
        CanonicalMethod("push")
    })

    Method("push", func() {
        Payload(func() {
            Attribute("objectA", ObjectAPayload, "")
        })

        HTTP(func() {
            PUT("/")
            Body("objectA")
        })
    })
})

var ObjectAPayload = Type("ObjectAPayload", func() {
    Attribute("child", ObjectAChild, "")

    Required("child")
})

var ObjectAChild = Type("ObjectAChild", func() {
    Attribute("ref", String, "Unique reference", func() {
        Example("aac4894c-357b-46b5-a57e-bd6dfcee8a88")
    })

    Required("ref")
})

_objectb.go

package design

import (
    . "goa.design/goa/v3/dsl"
)

var _ = Service("objectB", func() {

    HTTP(func() {
        Path("/objectA")
        CanonicalMethod("push")
    })

    Method("push", func() {
        Payload(func() {
            Attribute("objectB", ObjectBPayload, "")
        })

        HTTP(func() {
            PUT("/")
            Body("objectB")
        })
    })
})

var ObjectBPayload = Type("ObjectBPayload", func() {
    Attribute("child", ObjectBChild, "")

    Required("child")
})

var ObjectBChild = Type("ObjectBChild", func() {
    Attribute("ref", String, "Unique reference", func() {
        Example("aac4894c-357b-46b5-a57e-bd6dfcee8a89")
    })

    Required("ref")
})

When running "gen" I get the following in the OpenApi yaml file (abbreviated version)

ObjectAPayload:
    type: object
    properties:
        child:
            $ref: '#/components/schemas/ObjectAChild'

...

ObjectBPayload:
    type: object
    properties:
        child:
            $ref: '#/components/schemas/ObjectAChild'

If you notice it has used ObjectAChild (incorrectly) as the type for ObjectBPayload.child instead of ObjectBChild. If I rename the object_b.go file to aobject_b.go (i.e. earlier lexicographically) then ObjectB get's set correctly but ObjectA.child is ObjectBChild instead.

So it seams to check if any objects match the structure and it that structure is already defined then it uses the existing one instead of the type specified.

What get's the types generated correctly

What does not work getting the types generated correctly

michaelgambold commented 1 year ago

Upgraded to 3.11.0 and have the same issue.

raphael commented 1 year ago

Hello, as you guessed the algorithm compares the types structurally to avoid generating different schemas for types that are generated dynamically and structurally equivalent. This behavior can be overridden by using the openapi:typename Meta which overrides the name computed by Goa. The following designs behave as you would like them to:

package design

import (
    . "goa.design/goa/v3/dsl"
)

var _ = Service("objectA", func() {

    HTTP(func() {
        Path("/objectA")
        CanonicalMethod("push")
    })

    Method("push", func() {
        Payload(func() {
            Attribute("objectA", ObjectAPayload, "")
        })

        HTTP(func() {
            PUT("/")
            Body("objectA")
        })
    })
})

var ObjectAPayload = Type("ObjectAPayload", func() {
    Attribute("child", ObjectAChild, "")

    Required("child")
    Meta("openapi:typename", "ObjectAPayload")
})

var ObjectAChild = Type("ObjectAChild", func() {
    Attribute("ref", String, "Unique reference", func() {
        Example("aac4894c-357b-46b5-a57e-bd6dfcee8a88")
    })

    Required("ref")
    Meta("openapi:typename", "ObjectAChild")
})

and

package design

import (
    . "goa.design/goa/v3/dsl"
)

var _ = Service("objectB", func() {

    HTTP(func() {
        Path("/objectB")
        CanonicalMethod("push")
    })

    Method("push", func() {
        Payload(func() {
            Attribute("objectB", ObjectBPayload, "")
        })

        HTTP(func() {
            PUT("/")
            Body("objectB")
        })
    })
})

var ObjectBPayload = Type("ObjectBPayload", func() {
    Attribute("child", ObjectBChild, "")

    Required("child")
    Meta("openapi:typename", "ObjectBPayload")
})

var ObjectBChild = Type("ObjectBChild", func() {
    Attribute("ref", String, "Unique reference", func() {
        Example("aac4894c-357b-46b5-a57e-bd6dfcee8a89")
    })

    Required("ref")
    Meta("openapi:typename", "ObjectBChild")
})

We may want to change the behavior for non-dynamically generated types, this is a little bit of work (the generator has no idea whether the type was defined explicitly in the design or as a result of an inline definition for example).

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

mokiat commented 3 months ago

This issue is still valid. I came across it today. Any plans to have a more general fix for this (like a cli flag)?

Otherwise, it is hard to predict when two types would get matching fields in the future, leading to broken (backward-incompatible) OpenAPI changes. Having to specify the Meta property on each type might workaround that but is tedious to write and maintain.

raphael commented 3 months ago

Makes sense, I'm reopening the issue.

raphael commented 3 months ago

I put together a PR that attempt to address this issue. The results for the example given here seems to be what is expected. I would appreciate it if others could give it a shot:

go get goa.design/goa/v3@openapi_user_type_name