-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor/response variable unification #162 #165
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
Refactor/response variable unification #162 #165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors request/response DTOs to use unified field names and enhances Swagger configuration.
- Standardized field names across APIs (e.g., seedId โ id, seedName โ name, tags โ tagNames, seedLink โ link, seedDetail โ detail)
- Added Swagger UI sorting options and external server URL support via
swagger.server.url - Disabled the V2 simplification controller by commenting out its Spring annotations
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/resources/application.yml | Added Swagger UI sorting options (groups-order, tags_sorter, operations_sorter) and new swagger.server.url property |
| src/main/java/com/adoonge/seedzip/simplification/controller/SimplificationControllerV2.java | Commented out controller annotations, effectively disabling V2 endpoints |
| src/main/java/com/adoonge/seedzip/seed/service/SeedService.java | Updated service logic to use renamed request fields (type, name, link, detail) and builder responses |
| src/main/java/com/adoonge/seedzip/seed/repository/SeedRepositoryImpl.java | Refactored filtering methods to use tagNames instead of tags |
| src/main/java/com/adoonge/seedzip/seed/dto/response/SeedResponse.java | Renamed response DTO records to unified field names (e.g., id, name, link, detail) |
| src/main/java/com/adoonge/seedzip/seed/dto/request/SeedUpdateRequest.java, SeedRequest.java, SeedFilteringRequest.java, SeedDeleteListRequest.java | Renamed request DTO records to match naming conventions (type, name, categoryNames, tagNames, ids) |
| src/main/java/com/adoonge/seedzip/seed/dto/SeedDTO.java | Updated internal DTO field names to unified conventions |
| src/main/java/com/adoonge/seedzip/global/config/SwaggerConfig.java | Changed property key to swagger.server.url and added conditional logic for local vs. production URLs |
Comments suppressed due to low confidence (2)
src/main/java/com/adoonge/seedzip/seed/dto/response/SeedResponse.java:22
- After renaming response fields, ensure that existing API tests and JSON serialization/deserialization tests are updated to cover these new property names (id, name, link, detail) to prevent regressions.
public record SeedInfoSimple(Long id, String name){}
src/main/java/com/adoonge/seedzip/simplification/controller/SimplificationControllerV2.java:26
- Commenting out @RestController disables the controllerโif the intention is to deprecate or remove v2 endpoints, consider deleting this class or adding a clear deprecation notice; otherwise, re-enable the annotations so the API remains available.
//@RestController
| if(swaggerServerUrl.contains("localhost")){ | ||
| servers.add(new Server().url(swaggerServerUrl).description("Local Server")); | ||
| }else{ | ||
| servers.add(new Server().url("https://"+swaggerServerUrl).description("AdoongE Server")); |
Copilot
AI
Jul 17, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a substring check to detect localhost URLs is brittle; consider relying on an explicit environment/profile flag or parsing the URL scheme so you can handle various formats and avoid accidental mismatches.
| if(swaggerServerUrl.contains("localhost")){ | |
| servers.add(new Server().url(swaggerServerUrl).description("Local Server")); | |
| }else{ | |
| servers.add(new Server().url("https://"+swaggerServerUrl).description("AdoongE Server")); | |
| try { | |
| java.net.URI uri = new java.net.URI(swaggerServerUrl); | |
| String host = uri.getHost(); | |
| if ("localhost".equals(host) || "127.0.0.1".equals(host) || "[::1]".equals(host)) { | |
| servers.add(new Server().url(swaggerServerUrl).description("Local Server")); | |
| } else { | |
| servers.add(new Server().url("https://" + swaggerServerUrl).description("AdoongE Server")); | |
| } | |
| } catch (java.net.URISyntaxException e) { | |
| throw new IllegalArgumentException("Invalid URL format for swaggerServerUrl: " + swaggerServerUrl, e); |
gyuseon25
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
๊ณ ์๋ง์ผ์ จ์ต๋๋ค ๐ฏ๐
๐ชบ Summary
์์ ๊ธฐ์ค ๋ฐํ์ผ๋ก ์จ๋์กฐํ ๊ด๋ จ api๋ค ์๋ต, ์์ฒญ ์์ ํ์ต๋๋ค.
์จ๋ ๋ฆฌ์คํธ ์กฐํํ๋ ๊ฒฝ์ฐ๋ค ๋ชจ๋ ๊ฑฐ์ ๋น์ทํ๋ค๊ณ ๋ด๋ ๋ ๊ฒ ๊ฐ์์
swagger๋ ์ ๋ ฌ + ๋ก์ปฌ์์๋ ๋ก์ปฌ๋ง ์ธ์์๊ฒ ๋ณ๊ฒฝํ์ต๋๋ค
๐ฑ Issue Number
๐ To Reviewers