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

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

Open
pboguslawski opened this issue Jan 14, 2023 · 2 comments

Comments

@pboguslawski
Copy link
Contributor

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

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

pboguslawski commented Jan 15, 2023

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.

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

No branches or pull requests

2 participants