See if we can use more info from Go's type system

[arp242]

Right now there is quite a lot of duplication in the Kommentaar comment blocks:

// GET /v1/foo/bar category
// Some description.
//
// Request body: $ref: fooRequest
// Response 200: $ref: fooResponse
// Response 400: $default
func (Foo cn) foo(c echo.Context) error {
    var args fooRequest
    c.Bind(&args)

    v := validate.New()
    if v.HasErrors() {
        return v
    }

    foo, err := models.GetFoo()
    if err != nil {
        return err
    }
    return c.JSON(200, fooResponse{foo})
}

Pretty much every element except the description is duplicated from somewhere:

• The path is in the echo router
• The category/tag is in the controller/type name
• The request body is in the var args ...
• The 200 response is the c.JSON call
• The 400 and 404 responses are in the error returns (can get this like guru whicherrs)

The upshot of getting this from the type system is that it avoids duplication and typos, and that all the standard Go tooling will work (e.g. renames). I've had a number of typos on e.g. the path name and such.

The downside is that it's more complex to implement in Kommentaar, and that it will be much stronger tied to the way we structure our handlers. Right now, there is almost no relation between the code in the handler and the documentation. This is not accidental: one of the big problems with the previous go-swagger approach in Desk was that we needed to refactor code in a way that no one liked to make it work.

I don't know if there is a better way to do this. Perhaps the current approach is the best possible/least worst one; but on the other hand, I think that if eliminating at least the duplicate Request body: or Response 200: would be a good win.

For example, if we could restructure things to be like:

// GET /v1/foo/bar category
// Some description.
//
// Response 400: $default
func (Foo cn) foo(c echo.Context, args fooRequest) (*fooResponse, error) {
    c.Bind(&args)

    v := validate.New()
    if v.HasErrors() {
        return nil, v
    }

    foo, err := models.GetFoo()
    if err != nil {
        return nil, err
    }
    return &fooResponse{foo}, nil
}

Then that might be nice? Question is, is it worth is, and will it work with echo?

[arp242]

Thoughts @Teamwork/gophers ?

[ready4god2513]

Problem with that is somehow we also need to add the StatusCode in there, which c.JSON gives us (we could do it on our own, but then how do we tie it to the documentation).

Doing your change would work, but then we would need to wrap that method in the router mapping to just return the error. Or maybe something like this (but it seems to complicate the code for the sake of the documentation):

cn.POST("/foo/bar", func(c echo.Context) error {
    resp, err := cn.bar(c)
    if err != nil {
        return err
    }
    return c.JSON(resp.StatusCode, resp)
})

[abiosoft]

Changing to a custom fooResponse is not something I think is worth it.
Also, how is the response code gonna be detected ?

[ready4god2513]

See my answer there @abiosoft

[arp242]

it seems to complicate the code for the sake of the documentation

Yeah, that is my concern as well. OTOH, it does make writing documentation easier, so I don't know. There are good points to be made for both approaches I think; although I'm kind of leaning towards keeping it as-is, simple because that's the least effort (I'm lazy 😄).

[ripexz]

Also, there are some projects where we want to return a specific error message with c.JSON rather than fall back to default error handler, or will this take it into account?

[0xor1]

fwiw this is the pattern I use in my hobby projects:

type doSomethingArgs struct {
    // some properties
}

type doSomethingResponse struct {
    // some properties
}

var doSomething = &endpoint.Endpoint{
	Method:                   "POST",
	Path:                     "/api/v1/centralAccount/doSomething",
	RequiresSession:          false,
	GetArgsStruct:            func() interface{} { return &doSomethingArgs{}},
	ExampleResponseStructure: &doSomethingResponse{}
	CtxHandler:               func(ctx echo.Context, a interface{}) interface{} {
		args := a.(*doSomethingArgs)
		// Some business logic, to build up a *doSomethingResponse value
		// any errors are paniced immediately and the server will
		// recover from them and return the associated http status code
		// and message from the error
		return doSomethingResponse{}
	},
}

this and other *endpoint.Endpoint are passed to a server which knows what to do with []*endpoint.Endpoints and auto generates all the docs every time the server starts up and it doesnt require any special documentation comments. though I doubt this meets the requirements teamwork has, but just incase it sparks any other ideas or thoughts. It returns basic json docs at /api/docs e.g.:

[
    {
        "method": "GET",
        "path": "/api/v1/centralAccount/getRegions",
        "requiresSession": false,
        "exampleResponseStructure": [
            "use",
            "usw",
            "eu"
        ]
    },
    {
        "method": "POST",
        "path": "/api/v1/centralAccount/register",
        "requiresSession": false,
        "argsLocation": "Body",
        "argsStructure": {
            "name": "",
            "email": "",
            "pwd": "",
            "region": "",
            "language": "",
            "displayName": null,
            "theme": 0
        }
    },
    {
        "method": "POST",
        "path": "/api/v1/centralAccount/resendActivationEmail",
        "requiresSession": false,
        "argsLocation": "Body",
        "argsStructure": {
            "email": ""
        }
    },
]