swaggest / rest

Web services with OpenAPI and JSON Schema done quick in Go
https://pkg.go.dev/github.com/swaggest/rest
MIT License
362 stars 17 forks source link

Not obvious correlation between field name exporting and corresponding parameter visibility in API #110

Open pboguslawski opened 1 year ago

pboguslawski commented 1 year ago

Describe the bug

If you move example album handling stuff from https://dev.to/vearutop/tutorial-developing-a-restful-api-with-go-json-schema-validation-and-openapi-docs-2490 to separate package say mypackage and then (after exporting function names with capital letter) try to register interactors from main package with

    service.Get("/albums", mypackage.GetAlbums())
    service.Get("/albums/{id}", mypackage.GetAlbumByID())
    service.Post("/albums", mypackage.PostAlbums(), nethttp.SuccessStatus(http.StatusCreated))

everything works ok. But if you decide to change getAlbumByIDInput.ID to unexported getAlbumByIDInput.id than you'll get panic on app start saying

panic: failed to reflect API schema for GET /albums/{id}: undefined path parameter: id

If you restore getAlbumByIDInput.ID and add another query param say

    type getAlbumByIDInput struct {
        ID string `path:"id"`
        important string `query:"important"`
    }

no panics/warnings/errors will be thrown on start and important param won't be visible in OpenAPI nor defined in interactor even if present in request url.

If it's expected - consider adding info in manual about correlation between field name exporting and corresponding parameter visibility in API.

vearutop commented 1 year ago

This library is in the same boat with encoding/json, encoding/gob and many others, that use reflect to provide generalized implementations for user-defined types. Go allows visibility of unexported names (struct fields included) only in their own package, reflect honors that and can not read or write to unexported fields from an external package (unless unsafe tricks are applied).

There is a panic for path field because there is a condition that can be checked, if url pattern declares variable path segment, there must be a request parameter for it in structure. Query parameters are not enumerated in url pattern, so there is no way to tell if important query param is missing in request structure.

Having unexported fields in request/reponse structure is a legit situation, those fields can be an implementation detail of user code. Not having any exported fields is also ok, this may indicate request that has no parameters.

I'm not sure there is a need for specific documentation of unexported fields handling, given their lack of visibility is inherited from the Go language itself.

🤖 Bonus explanation from ChatGPT 🤖

hello, why can not I marshal unexported field with JSON in Go?

In Go, fields that are unexported (i.e., that begin with a lowercase letter) are not accessible from outside the package in which they are defined. This means that the JSON package, which is in a different package, cannot access unexported fields when marshalling or unmarshalling data. To make a field exported, you need to capitalize the first letter of the field name.

can a 3rd-party library access an unexported field of my Go program?

No, a 3rd-party library cannot access unexported fields of your Go program because unexported fields are not accessible from outside the package in which they are defined. This is a key feature of Go's encapsulation mechanism and it ensures that the internal state of your program is hidden from other packages.

If you want a 3rd-party library to have access to a field, you need to make the field exported by capitalizing the first letter of the field name. Additionally, you can use the tag feature of Go struct fields to specify a different name for the field when it is marshaled to JSON and other formats.

pboguslawski commented 1 year ago

Thank you for answer and great lib!

This library is in the same boat with encoding/json, encoding/gob and many others, that use reflect to provide generalized implementations for user-defined types.

Studying manuals of all dependencies should probably be not required for github.com/swaggest/rest users to use it.

Go allows visibility of unexported names (struct fields included) only in their own package

User will notice if try to use unexported name in other package (compilation error, IDE warning, etc.); example from this issue mentions not obvious situation (delegating all stuff connected with one "api object" to separate package with all its internals treated as "private" = unexported names where possible in first attempt) where no warnings where produced which may surprise github.com/swaggest/rest users.

Idea: add swaggest/rest manual section about decomposing big apis into separate packages with recommended/example implementation (would be nice - if possible - to see cleaner "service" way with "subservices" similar to "router" and "subrouters" in chi, without using chi wrapping) and mention there when using exported go field names is required in such separated packages for api params to be available.