goadesign / goa

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

OpenAPI schema for `Any` DSL type assumes nested serialization into a string #3560

Closed kjedruczyk closed 3 months ago

kjedruczyk commented 3 months ago

In the GOA DSL, the Any type is described as:

// Any is the type for an arbitrary JSON value (any in Go).
Any = expr.Any

This is quite useful interface to capture schemaless JSON values as any type in GO, while also easily allowing us to control how JSON responses would be formed using standard go JSON serialization tags. In other words, I'd like to be able to catch/emit an arbitrary JSON value in an attribute declared like this:

Attribute("any", Any)

The trouble is that GOA openapi generator decides Any type should be encoded as serialized binary string. This is arguably inconsistent with the idea that Any represents JSON value (it now is just a string that the client would have to explicitly parse):

any:
    type: string
    format: binary

Note, that the go code doesn't seem to do or expect any kind of nested binary string encoding/decoding. The expectation to handle the "Any JSON" encoding/decoding seems to be implicitly passed onto the client in this scenario.

It is also inconsistent with handling of Any type in different contexts. Consider MapOf(String,Any), which will generate following openapi type (as I would expect):

mapofstringany:
    type: object
    additionalProperties: true

In this case, an arbitrary JSON object will have Any-typed attributes passed to go layer directly as any go type without collapsing these attributes into string/binary encoding.

Can we make GOA, by explicit DSL support if needed, emit AnyValue or equivalent oneOf schema for Any DSL type? (I'd be happy to submit a PR if we agree on an approach).

kjedruczyk commented 3 months ago

Here's an example design to try and illustrate the issue more precisely:

package design

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

var _ = API("any-test", func() {
    Server("srv", func() {
        Services("any-test")
    })
})

var TestAny = Type("TestAny", func() {
    Attribute("AnyAttribute", Any)
    Attribute("MapOfAny", MapOf(String,Any))
})

var _ = Service("any-test", func() {
    Method("test", func() {
        Payload(TestAny)
        Result(TestAny)
        HTTP(func() {
            GET("/")
        })
    })
})

The above, will give us following schema in the OpenAPI3.yaml (I removed examples for clarity):

components:
    schemas:
        TestRequestBody:
            type: object
            properties:
                AnyAttribute:
                    type: string
                    format: binary
                MapOfAny:
                    type: object
                    additionalProperties: true

The AnyAttribute property type/format is what gives me trouble, and I haven't found a way to override this that would let me retain any type in go for the attribute.

kjedruczyk commented 3 months ago

FWIW, I found a DSL workaround to let me force different codepath in the Any type generator for openapi, but that involves calling some DSL internals to get around explicit restrictions, that ,I assume, were there for a reason:

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

func ExtendAny(t expr.DataType) {
    switch def := eval.Current().(type) {
    case *expr.ResultTypeExpr:
        def.Bases = append(def.Bases, t)
    case *expr.AttributeExpr:
        def.Bases = append(def.Bases, t)
    default:
        eval.IncompatibleDSL()
    }
}

ExtendAny defined above will allow me to add Bases to a primitive-type (Any) argument, and that actually triggers interesting codepath in the code generator: the codepath seemingly intended to generate anyOf type covering all the bases.

raphael commented 3 months ago

Thank you for the report this all makes sense! A few considerations:

Although the Any header comment mentions JSON, Goa is agnostic of the underlying encoding. By default it supports JSON, XML and Gob but it's also possible to provide custom encoders for any other encoding (e.g. form encoding). We may want to update the comment...

There are cases where generating the OpenAPI specification the way it is today is what is needed, for example when used in conjunction with Meta("struct:field:type","json.RawMessage","encoding/json").

The semantic of Extend is "inlay the attributes of the extended object into the current object", making it possible to extend basic types muddles that a bit. It could also have unintended side-effects in code generation algorithms.

Given that the issue is with the way Goa generates the OpenAPI specification maybe we could introduce a new Meta key, e.g. openapi:object that would tell the OpenAPI generators to render the corresponding attribute as a extensible object.

tchssk commented 3 months ago

The OpenAPI Specification says

https://swagger.io/docs/specification/data-models/data-types/#string

Strings

A string of text is defined as:

type: string

https://swagger.io/docs/specification/data-models/data-types/#format

String Formats

An optional format modifier serves as a hint at the contents and format of the string.

  • binary – binary data, used to describe files (see Files below)

https://swagger.io/docs/specification/data-models/data-types/#file

Files

Unlike OpenAPI 2.0, Open API 3.0 does not have the file type. Files are defined as strings:

type: string
format: binary  # binary file contents

or

type: string
format: byte    # base64-encoded file contents

https://swagger.io/docs/specification/data-models/data-types/#any

Any Type

A schema without a type matches any data type – numbers, strings, objects, and so on. {} is shorthand syntax for an arbitrary-type schema:

components:
    schemas:
        AnyValue: {}

On the other hand, Goa handles Any type as

I think these should be handled as untyped. (We can use Bytes for struct:field:type Meta instead) I'll try to create a fix.

kjedruczyk commented 3 months ago

I think these should be handled as untyped. (We can use Bytes for struct:field:type Meta instead) I'll try to create a fix.

That was my thinking when we originally run into this issue - I had a quick hack that seemed to work when I applied it locally: https://github.com/goadesign/goa/compare/v3...kjedruczyk:goa:openapi-anykind , but I have to admit I don't fully understand why the 'Bases' iteration is even there (as I mentioned above, it seems that trickery is required to force Bases into a primitive type), so it was a bit of a blind edit to see if this is the piece of code responsible for the problematic behaviour.

kjedruczyk commented 3 months ago

Thank you for the report this all makes sense! A few considerations:

Although the Any header comment mentions JSON, Goa is agnostic of the underlying encoding. By default it supports JSON, XML and Gob but it's also possible to provide custom encoders for any other encoding (e.g. form encoding). We may want to update the comment...

There are cases where generating the OpenAPI specification the way it is today is what is needed, for example when used in conjunction with Meta("struct:field:type","json.RawMessage","encoding/json").

The semantic of Extend is "inlay the attributes of the extended object into the current object", making it possible to extend basic types muddles that a bit. It could also have unintended side-effects in code generation algorithms.

Given that the issue is with the way Goa generates the OpenAPI specification maybe we could introduce a new Meta key, e.g. openapi:object that would tell the OpenAPI generators to render the corresponding attribute as a extensible object.

I thought about something like this too, if changing the default behaviour is a no-go.

raphael commented 3 months ago

Closing this issue, thank you @tchssk for the PR!