goadesign / goa

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

Using Param() to map path parameters to attributes results in a runtime error #3462

Open Makirchn opened 7 months ago

Makirchn commented 7 months ago

Hi, I'm new to goa and I want to map a path param e.g. _productid to a payload attribute id.

My design:

Service:

var _ = Service("products", func() {
    Description("The product service provides an API for managing products.")
    HTTP(func() {
        Path("/products")
    })

    // update product by id
    Method("update by id", func() {
        Payload(UpdateProductPayload)
        Result(UpdateProductPayload, func() {
            View("default")
        })
        Error("no_criteria", String, "Missing criteria")
        Error("no_match", String, "No product found with specified id")
        HTTP(func() {
            POST("/{product_id}")
            Param("product_id:id")
            Response(StatusOK)
            Response("no_criteria", StatusBadRequest)
            Response("no_match", StatusNotFound)
        })
    })
})

Payload Model:

var UpdateProductPayload = Type("UpdateProductPayload", func() {
    Attribute("id", String, "Product ID", func() {
        Example("83d83ec2-d2ca-49ff-bbea-b92b5c3be202")
    })
    Attribute("name", String, "Product Name", func() {
        Example("Apple")
    })
})

Generating the code results in:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x60 pc=0x1023a3028]
raphael commented 6 months ago

Just a note that there is indeed a bug in that Goa should never panic. That being said the correct DSL here would be:

var _ = Service("products", func() {
    Description("The product service provides an API for managing products.")
    HTTP(func() {
        Path("/products")
    })

    // update product by id
    Method("update by id", func() {
        Payload(UpdateProductPayload)
        Result(UpdateProductPayload, func() {
            View("default")
        })
        Error("no_criteria", String, "Missing criteria")
        Error("no_match", String, "No product found with specified id")
        HTTP(func() {
            POST("/{id}")
            Response(StatusOK)
            Response("no_criteria", StatusBadRequest)
            Response("no_match", StatusNotFound)
        })
    })
})

Param defines query string parameters so in the original DSL the id field gets mapped twice - once in the HTTP route and once as a query string parameter. So Param should be removed and instead the mapping done within the route definition (POST("/{id}")).

Makirchn commented 6 months ago

So as I understand your answer, it is not possible to map a path param element name to an attribute name like it is possible for query, header or body elements?

raphael commented 6 months ago

Yes you are correct, the name of the path param needs to match the name of the attribute. The idea is that the name of the path params should not affect how clients build requests. I am curious to understand the use case for wanting to use a different name for the path param than the name of the attribute?

Makirchn commented 6 months ago

I saw the http element mapping of header, body and query elements on https://goa.design/design/http_mapping/ and simply tried to use it with path parameters. My intention was to use a speaking route like /users/{user_id}/products/{product_id} instead of using /users/{id}/products/{product_id} while having a consistent schema for my structs by having them all have an id parameter regardless of whether it is a user, product or is something else.

raphael commented 6 months ago

That's fair, it does set the expectation. I guess the one place where that might matter is in the generated OpenAPI specification. I'm reopening the issue so we can add this feature.