Skip to content
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

Generating OpenAPI file has wrong schemas used #3242

Open
michaelgambold opened this issue Feb 7, 2023 · 6 comments
Open

Generating OpenAPI file has wrong schemas used #3242

michaelgambold opened this issue Feb 7, 2023 · 6 comments

Comments

@michaelgambold
Copy link

michaelgambold commented Feb 7, 2023

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;
  }
}

object_a.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")
})

object_b.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

  • In one of the child objects change "ref" to "ref1"
  • Add another field to ObjectB to make it different from ObjectA

What does not work getting the types generated correctly

  • Changing the examples to be different
  • Ensuring different descriptions
@michaelgambold
Copy link
Author

Upgraded to 3.11.0 and have the same issue.

@raphael
Copy link
Member

raphael commented Feb 9, 2023

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
Copy link

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 17, 2023
@mokiat
Copy link
Contributor

mokiat commented Aug 1, 2024

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
Copy link
Member

raphael commented Aug 6, 2024

Makes sense, I'm reopening the issue.

@raphael
Copy link
Member

raphael commented Aug 8, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants