Upgrade spectra - #8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the spectra dependency from version 0.1.1 to 0.4.0, which introduces breaking API changes that require significant code updates throughout the codebase.
Changes:
- Upgraded spectra dependency from 0.1.1 to 0.4.0 in rebar.config and rebar.lock
- Migrated from tuple-based field representation (
{map_field_exact, Name, Type}) to record-based representation (#literal_map_field{}) - Updated spectra API calls (
spectra_type_info:get_function→spectra_type_info:find_function) - Added logic to exclude request bodies from HTTP methods that don't support them (GET, HEAD, OPTIONS, TRACE)
- Updated all test assertions to use binary keys instead of atoms to match new spectra output format
- Added new test case to verify GET requests don't generate requestBody in OpenAPI spec
- Created user_handler.erl as a new example handler
- Updated demo.escript and README.md to reference the new user_handler module
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rebar.config | Updated spectra dependency constraint from ~> 0.2.0 to ~> 0.4.0 |
| rebar.lock | Updated locked spectra version from 0.1.1 to 0.4.0 with new hash values |
| src/elli_openapi.erl | Migrated to new spectra API using #literal_map_field records, updated function calls, added http_method_supports_body/1 to exclude request bodies for GET/HEAD/OPTIONS/TRACE, removed unused elli include |
| src/elli_openapi_handler.erl | Simplified handle_event spec, added request_complete event handler with logging |
| test/elli_openapi_integration_SUITE.erl | Updated test assertions to use binary keys, added new test for GET request body exclusion |
| src/user_handler.erl | Added new example handler module demonstrating get_user and create_user endpoints |
| demo.escript | Updated to use new user_handler module instead of elli_openapi_demo |
| README.md | Updated example code to reference user_handler module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| %% {<<"POST">>, <<"/api/echo">>, fun elli_openapi_demo:echo_text/3}, | ||
| %% {<<"PUT">>, <<"/api/items/{itemId}">>, fun elli_openapi_demo:update_item/3} | ||
| {<<"GET">>, <<"/api/users/{userId}">>, fun user_handler:get_user/3}, | ||
| {<<"POST">>, <<"/api/users/">>, fun user_handler:create_user/3} |
There was a problem hiding this comment.
The POST route has a trailing slash in the path which is inconsistent with the rest of the codebase. All other POST routes to /api/users use the path without a trailing slash. For example, in the test file test/elli_openapi_integration_SUITE.erl lines 78, 392, 449, and 534, the path is consistently <<"/api/users">> without a trailing slash.
| ok; | ||
| handle_event(_Event, _Args, _Config) -> | ||
| handle_event(request_complete, [Req, ReturnCode, _, _, _], _Config) -> | ||
| io:format("Req complete: ~s ~p ~n", [elli_request:raw_path(Req), ReturnCode]), |
There was a problem hiding this comment.
This logging statement should not be in production code. The handle_event callback should not have side effects like console logging for every request, as this can produce excessive output and impact performance in production. This appears to be debug code that should either be removed or made conditional based on a debug flag.
| io:format("Req complete: ~s ~p ~n", [elli_request:raw_path(Req), ReturnCode]), | |
| case application:get_env(elli_openapi, debug_logging, false) of | |
| true -> | |
| io:format("Req complete: ~s ~p ~n", [elli_request:raw_path(Req), ReturnCode]); | |
| _ -> | |
| ok | |
| end, |
| -spec create_user(#{}, #{}, #user{}) -> | ||
| {201, #{'Location' => binary()}, #user{}}. | ||
| create_user(#{}, #{}, User) -> | ||
| io:format("Creating user: ~s with role ~p~n", [User#user.name, User#user.role]), |
There was a problem hiding this comment.
This logging statement should not be in a handler function. The create_user function should not have side effects like console logging, as this mixes concerns and makes the function less testable. Production handler code should focus on business logic, with logging handled at the framework level if needed.
| io:format("Creating user: ~s with role ~p~n", [User#user.name, User#user.role]), |
No description provided.