Conversation
…d use it in UserRestaurantVoteService
…fe/votepage # Conflicts: # be/src/main/java/com/yumst/be/restaurant/repository/RestaurantRepository.java
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @hangyeoli, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request focuses on fixing and enhancing the vote page functionality. It includes changes to the backend to streamline data retrieval for restaurants and votes, as well as frontend updates to display and interact with this data. The changes include adding a new ErrorResponse class, modifying repository queries for restaurant and review features, implementing a new controller and service for handling user votes, and updating the frontend to support voting and display restaurant information with like/dislike counts and scrap status. The shared preferences and manager related to batch processing of votes have been removed.
Highlights
- Backend Restructuring: The backend has been refactored to efficiently retrieve restaurant data, user votes, and review features. This includes new queries in the
RestaurantRepositoryandNaverReviewFeatureCountRepositoryto fetch nearby restaurants with vote counts and top features. - Vote Functionality Implementation: A new
UserRestaurantVoteControllerandUserRestaurantVoteServicehave been added to handle user votes. This includes API endpoints for retrieving votable restaurants and submitting votes, as well as rate limiting to prevent abuse. - Frontend Enhancements: The frontend has been updated to display restaurant information, including like/dislike counts and scrap status. Users can now vote on restaurants and toggle scrap status directly from the vote page. The UI also includes loading indicators and error handling.
- Model and State Management: New models (
VoteRestaurant) and state management (VotePageStateNotifier) have been introduced in the frontend to handle the display and interaction with restaurant vote data. This includes pagination, sorting, and optimistic UI updates.
Changelog
Click here to see the changelog
- be/src/main/java/com/yumst/be/global/exception/ErrorResponse.java
- Added a new
ErrorResponseclass to handle exceptions and provide a consistent error response format.
- Added a new
- be/src/main/java/com/yumst/be/restaurant/repository/NaverReviewFeatureCountRepository.java
- Added a new query
findTop2FeaturesForRestaurantIdsto retrieve top features for a list of restaurant IDs. - Added
@Paramannotation for restaurantIds parameter
- Added a new query
- be/src/main/java/com/yumst/be/restaurant/repository/RestaurantRepository.java
- Added new queries (
findNearbyRestaurantsOrderByDistance,findNearbyRestaurantsOrderByLikes,findNearbyRestaurantsOrderByDislikes) to retrieve nearby restaurants sorted by distance, likes, or dislikes. - These queries use native SQL with PostGIS functions to calculate distances and sort results.
- Added new queries (
- be/src/main/java/com/yumst/be/restaurant/service/RestaurantService.java
- Added a new method
findNearbyRestaurantsto retrieve nearby restaurants based on location and sorting criteria. - Added
validateRestaurantExistsmethod to validate if a restaurant exists by its ID. - Modified
getResponseRestaurantto include user vote status. - Refactored to use
collect(Collectors.toList())instead of.toList().
- Added a new method
- be/src/main/java/com/yumst/be/restaurant/vo/ResponseRestaurant.java
- Added a
userVoteStatusfield to theResponseRestaurantrecord. - Added a new
fromSearchResultmethod to create aResponseRestaurantfrom a search result.
- Added a
- be/src/main/java/com/yumst/be/user/config/SecurityConfig.java
- Corrected a typo in the
permitAllListto ensure the guest login endpoint is correctly whitelisted.
- Corrected a typo in the
- be/src/main/java/com/yumst/be/user/repository/UserRestaurantScrapRepository.java
- Added a new query
findScrappedRestaurantIdsto retrieve scrapped restaurant IDs for a user.
- Added a new query
- be/src/main/java/com/yumst/be/user/service/UserService.java
- Added
validateUserExistsmethod to validate if a user exists by their ID.
- Added
- be/src/main/java/com/yumst/be/vote/controller/UserRestaurantVoteController.java
- Added a new controller
UserRestaurantVoteControllerto handle user votes. - Includes endpoints for retrieving votable restaurants and submitting votes.
- Added a new controller
- be/src/main/java/com/yumst/be/vote/domain/UserRestaurantVote.java
- Added a new entity
UserRestaurantVoteto represent user votes on restaurants. - Includes fields for user ID, restaurant ID, vote type, and a version field for optimistic locking.
- Added a new entity
- be/src/main/java/com/yumst/be/vote/dto/RestaurantRequest.java
- Added a new DTO
RestaurantRequestto encapsulate restaurant request parameters (latitude, longitude, radius, sort, page).
- Added a new DTO
- be/src/main/java/com/yumst/be/vote/dto/VoteRequest.java
- Added a new DTO
VoteRequestto encapsulate the vote type.
- Added a new DTO
- be/src/main/java/com/yumst/be/vote/dto/VoteResponse.java
- Added a new DTO
VoteResponseto encapsulate the vote response (message, restaurant ID, likes, dislikes).
- Added a new DTO
- be/src/main/java/com/yumst/be/vote/exception/VoteErrorCode.java
- Added a new enum
VoteErrorCodeto define vote-related error codes.
- Added a new enum
- be/src/main/java/com/yumst/be/vote/exception/VoteException.java
- Added a new exception class
VoteExceptionto represent vote-related exceptions.
- Added a new exception class
- be/src/main/java/com/yumst/be/vote/exception/VoteExceptionHandler.java
- Added a new exception handler
VoteExceptionHandlerto handleVoteExceptionand return an appropriate error response.
- Added a new exception handler
- be/src/main/java/com/yumst/be/vote/repository/UserRestaurantVoteRepository.java
- Added a new repository
UserRestaurantVoteRepositoryto manage user votes. - Includes methods for finding votes by user and restaurant, counting votes by restaurant and vote type, and counting votes by restaurant IDs.
- Added a new repository
- be/src/main/java/com/yumst/be/vote/service/UserRestaurantVoteService.java
- Added a new service
UserRestaurantVoteServiceto handle user vote logic. - Includes methods for retrieving votable restaurants and submitting votes, with rate limiting and optimistic locking.
- Added a new service
- be/src/main/java/com/yumst/be/vote/util/VoteRateLimiter.java
- Added a new utility class
VoteRateLimiterto limit the number of vote requests from a user within a time window.
- Added a new utility class
- fe/ios/Podfile.lock
- Added
shared_preferences_foundationdependency for FlutterMacOS.
- Added
- fe/lib/data/token_interceptor.g.dart
- Updated the hash for the dio provider.
- fe/lib/main.dart
- Changed
MyAppto aConsumerWidgetto accessWidgetRef. - Added
navigatorKeytoMaterialApp.
- Changed
- fe/lib/model/restaurant.dart
- Modified
Restaurant.fromJsonto handle null or non-listtop2Features.
- Modified
- fe/lib/model/vote_restaurant.dart
- Added a new model
VoteRestaurantto represent restaurant data on the vote page. - Includes fields for restaurant ID, name, category, thumbnail URL, latitude, longitude, business hours, top features, like count, dislike count, distance, scrap status, and user vote status.
- Added a new model
- fe/lib/model/vote_restaurant.g.dart
- Generated code for
VoteRestaurantmodel.
- Generated code for
- fe/lib/model/vote_types.dart
- Added a new enum
VoteTypeto represent vote types (LIKE, DISLIKE).
- Added a new enum
- fe/lib/provider/vote_state_provider.dart
- Added a new state provider
votePageStateProviderto manage the state of the vote page. - Includes state classes (
VoteRestaurantState,VotePageCombinedState) and a state notifier (VotePageStateNotifier) to handle data fetching, voting, and scrap toggling.
- Added a new state provider
- fe/lib/repository/auth_repository.dart
- Removed blank line at the top of the file.
- fe/lib/repository/vote_repository.dart
- Added a new repository
VoteRepositoryto handle API calls related to voting. - Includes methods for retrieving votable restaurants, submitting votes, and toggling scrap status.
- Added a new repository
- fe/lib/repository/vote_repository.g.dart
- Generated code for
VoteRepositoryprovider.
- Generated code for
- fe/lib/view/screen/main_page.dart
- Replaced placeholder text with the new
VotePagecomponent. - Added logic to refresh the vote page data when the tab is double-tapped or when navigating to the vote page.
- Replaced placeholder text with the new
- fe/lib/view/screen/vote_page.dart
- Added a new screen
VotePageto display votable restaurants. - Includes UI for sorting restaurants, displaying restaurant information, and interacting with vote and scrap actions.
- Added a new screen
- fe/lib/view/widget/vote_restaurant_card.dart
- Added a new widget
VoteRestaurantCardto display individual restaurant information on the vote page. - Includes UI for displaying restaurant details, like/dislike counts, scrap status, and vote/scrap actions.
- Added a new widget
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A vote cast, a choice made,
In digital realms displayed.
Likes and dislikes take their flight,
Guiding taste with pure insight.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: a votable restaurants page with functionality for fetching nearby restaurants, sorting, pagination, user voting (like/dislike/cancel), and scraping. The changes span both backend (Java/Spring) and frontend (Flutter/Dart) and are quite comprehensive.
Backend Highlights:
- New REST endpoints for fetching votable restaurants and submitting votes.
- Complex native SQL queries for finding nearby restaurants with different sorting options (distance, likes, dislikes), including user-specific data like scrap status and vote status.
- Implementation of optimistic locking (
@Version) for vote entities to handle concurrent updates. - A simple in-memory rate limiter for vote submissions.
- Necessary DTOs, service logic, and exception handling for the new vote feature.
Frontend Highlights:
- A new 'Vote' page with a
GridViewto display restaurant cards. - State management using Riverpod (
StateNotifierProvider) for fetching, paginating, sorting, voting, and scraping restaurants. - Optimistic UI updates for vote and scrap actions, enhancing user experience.
- Integration with a location service to fetch restaurants based on user's current position.
- New models (
VoteRestaurant,VoteType) and repository (VoteRepository) for the vote feature.
Overall, the PR is a substantial step forward. The backend correctly implements core voting logic and data retrieval. The frontend provides a reactive UI for these features. Key areas for improvement revolve around the maintainability of duplicated SQL queries on the backend and a couple of points in the frontend state management and backend rate limiting strategy.
A critical security configuration fix for guest login path is included, which is excellent.
Summary of Findings
- Critical Security Path Fix: A missing leading slash in
SecurityConfigfor the guest login path was corrected, which is essential for proper security rule application. - SQL Query Duplication: Significant code duplication exists in
RestaurantRepositoryacross three native queries for fetching nearby restaurants with different sort orders. This impacts maintainability. - Recursive Retry for Optimistic Locking: The
UserRestaurantVoteServiceuses a direct recursive call for retrying on optimistic lock failures, which could be made more robust with bounded retries or Spring Retry. - In-Memory Rate Limiter Scalability: The
VoteRateLimiteris single-instance and won't scale effectively in a multi-instance deployment. A distributed solution might be needed for future scalability. - Dart Provider Type Safety: The
_readparameter inVotePageStateNotifier(Flutter) is dynamically typed, which could be improved for type safety. - Missing Newlines at EOF (Backend & Frontend): Several Java and Dart files are missing a newline character at the end of the file. This is a minor stylistic issue. (Not commented inline due to review settings)
- Potentially Unused Repository Method (Backend): The method
UserRestaurantVoteRepository.countVotesByRestaurantIdsappears to be unused in the current scope of this PR. (Not commented inline due to review settings) - Client-Side Counts in
Restaurant.toJson(Frontend): TheRestaurant.toJsonmethod in Flutter now includeslikeCountanddislikeCount. If these are intended for local state, it's fine. If they were to be sent to the server for updates, the server should rely on its own calculations. (Not commented inline due to review settings) - Unused Provider (Frontend): The
navigatorKeyProviderdefined invote_state_provider.dartseems to be unused within that file. (Not commented inline due to review settings)
Merge Readiness
This pull request introduces valuable new functionality for the voting page. The core logic is well-implemented on both backend and frontend.
However, before merging, I strongly recommend addressing the following:
- The critical security configuration fix in
SecurityConfig.java(line 48) is already correctly implemented, which is great. - The high severity issue regarding SQL query duplication in
RestaurantRepository.java(lines 18-171) should be addressed to improve long-term maintainability. - The medium severity concerns regarding the recursive retry mechanism in
UserRestaurantVoteService.java(lines 91-94), the scalability of the in-memoryVoteRateLimiter.java(lines 10-31), and the dynamic typing of_readinVotePageStateNotifier.dart(lines 124-133) should also be considered for improvement.
Once these points, especially the high-severity duplication, are discussed and ideally resolved, the PR will be in a much better state for merging. As a reviewer, I am not authorized to approve pull requests, so please ensure further review and approval from authorized team members.
| @Query(value = | ||
| "WITH RankedRestaurants AS (" + | ||
| " SELECT " + | ||
| " r.restaurant_id, r.name, r.category, r.thumbnail_url, " + | ||
| " ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326))" + | ||
| " ) as dist, " + | ||
| " COALESCE(bool_or(us.restaurant_id IS NOT NULL), false) as is_scrapped, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'LIKE' THEN 1 ELSE 0 END), 0) as like_count, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'DISLIKE' THEN 1 ELSE 0 END), 0) as dislike_count, " + | ||
| " MAX(v_user.vote_type) as user_vote_status, " + | ||
| " ROW_NUMBER() OVER (PARTITION BY r.name ORDER BY ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)))\n" + | ||
| " ) as row_num " + | ||
| " FROM " + | ||
| " restaurant r " + | ||
| " LEFT JOIN user_restaurant_scrap us ON r.restaurant_id = us.restaurant_id AND us.user_id = :userId " + | ||
| " LEFT JOIN user_restaurant_vote v_all ON r.restaurant_id = v_all.restaurant_id " + | ||
| " LEFT JOIN user_restaurant_vote v_user ON r.restaurant_id = v_user.restaurant_id AND v_user.user_id = :userId " + | ||
| " WHERE " + | ||
| " r.crawl_complete = true AND " + | ||
| " ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| " ) " + | ||
| " GROUP BY r.restaurant_id, r.name, r.category, r.thumbnail_url, r.longitude, r.latitude" + | ||
| ") " + | ||
| "SELECT restaurant_id, name, category, thumbnail_url, dist, is_scrapped, like_count, dislike_count, user_vote_status " + | ||
| "FROM RankedRestaurants " + | ||
| "WHERE row_num = 1 " + | ||
| "ORDER BY dist ASC", | ||
| countQuery = "SELECT COUNT(DISTINCT r.name) " + | ||
| "FROM restaurant r " + | ||
| "WHERE r.crawl_complete = true AND " + | ||
| "ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| ")", | ||
| nativeQuery = true) | ||
| Page<Object[]> findNearbyRestaurantsOrderByDistance( | ||
| @Param("userId") String userId, | ||
| @Param("latitude") Double latitude, | ||
| @Param("longitude") Double longitude, | ||
| @Param("radius") Double radius, | ||
| Pageable pageable | ||
| ); | ||
|
|
||
| // 좋아요순 정렬 | ||
| @Query(value = | ||
| "WITH RankedRestaurants AS (" + | ||
| " SELECT " + | ||
| " r.restaurant_id, r.name, r.category, r.thumbnail_url, " + | ||
| " ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326))" + | ||
| " ) as dist, " + | ||
| " COALESCE(bool_or(us.restaurant_id IS NOT NULL), false) as is_scrapped, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'LIKE' THEN 1 ELSE 0 END), 0) as like_count, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'DISLIKE' THEN 1 ELSE 0 END), 0) as dislike_count, " + | ||
| " MAX(v_user.vote_type) as user_vote_status, " + | ||
| " ROW_NUMBER() OVER (PARTITION BY r.name ORDER BY ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)))\n" + | ||
| " ) as row_num " + | ||
| " FROM " + | ||
| " restaurant r " + | ||
| " LEFT JOIN user_restaurant_scrap us ON r.restaurant_id = us.restaurant_id AND us.user_id = :userId " + | ||
| " LEFT JOIN user_restaurant_vote v_all ON r.restaurant_id = v_all.restaurant_id " + | ||
| " LEFT JOIN user_restaurant_vote v_user ON r.restaurant_id = v_user.restaurant_id AND v_user.user_id = :userId " + | ||
| " WHERE " + | ||
| " r.crawl_complete = true AND " + | ||
| " ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| " ) " + | ||
| " GROUP BY r.restaurant_id, r.name, r.category, r.thumbnail_url, r.longitude, r.latitude" + | ||
| ") " + | ||
| "SELECT restaurant_id, name, category, thumbnail_url, dist, is_scrapped, like_count, dislike_count, user_vote_status " + | ||
| "FROM RankedRestaurants " + | ||
| "WHERE row_num = 1 " + | ||
| "ORDER BY like_count DESC, dist ASC", | ||
| countQuery = "SELECT COUNT(DISTINCT r.name) " + | ||
| "FROM restaurant r " + | ||
| "WHERE r.crawl_complete = true AND " + | ||
| "ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| ")", | ||
| nativeQuery = true) | ||
| Page<Object[]> findNearbyRestaurantsOrderByLikes( | ||
| @Param("userId") String userId, | ||
| @Param("latitude") Double latitude, | ||
| @Param("longitude") Double longitude, | ||
| @Param("radius") Double radius, | ||
| Pageable pageable | ||
| ); | ||
|
|
||
| // 싫어요순 정렬 | ||
| @Query(value = | ||
| "WITH RankedRestaurants AS (" + | ||
| " SELECT " + | ||
| " r.restaurant_id, r.name, r.category, r.thumbnail_url, " + | ||
| " ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326))" + | ||
| " ) as dist, " + | ||
| " COALESCE(bool_or(us.restaurant_id IS NOT NULL), false) as is_scrapped, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'LIKE' THEN 1 ELSE 0 END), 0) as like_count, " + | ||
| " COALESCE(SUM(CASE WHEN v_all.vote_type = 'DISLIKE' THEN 1 ELSE 0 END), 0) as dislike_count, " + | ||
| " MAX(v_user.vote_type) as user_vote_status, " + | ||
| " ROW_NUMBER() OVER (PARTITION BY r.name ORDER BY ST_Distance(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)))\n" + | ||
| " ) as row_num " + | ||
| " FROM " + | ||
| " restaurant r " + | ||
| " LEFT JOIN user_restaurant_scrap us ON r.restaurant_id = us.restaurant_id AND us.user_id = :userId " + | ||
| " LEFT JOIN user_restaurant_vote v_all ON r.restaurant_id = v_all.restaurant_id " + | ||
| " LEFT JOIN user_restaurant_vote v_user ON r.restaurant_id = v_user.restaurant_id AND v_user.user_id = :userId " + | ||
| " WHERE " + | ||
| " r.crawl_complete = true AND " + | ||
| " ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| " ) " + | ||
| " GROUP BY r.restaurant_id, r.name, r.category, r.thumbnail_url, r.longitude, r.latitude" + | ||
| ") " + | ||
| "SELECT restaurant_id, name, category, thumbnail_url, dist, is_scrapped, like_count, dislike_count, user_vote_status " + | ||
| "FROM RankedRestaurants " + | ||
| "WHERE row_num = 1 " + | ||
| "ORDER BY dislike_count DESC, dist ASC", | ||
| countQuery = "SELECT COUNT(DISTINCT r.name) " + | ||
| "FROM restaurant r " + | ||
| "WHERE r.crawl_complete = true AND " + | ||
| "ST_DWithin(" + | ||
| " geography(ST_SetSRID(ST_MakePoint(CAST(r.longitude AS float), CAST(r.latitude AS float)), 4326)), " + | ||
| " geography(ST_SetSRID(ST_MakePoint(:longitude, :latitude), 4326)), " + | ||
| " :radius * 1000" + | ||
| ")", | ||
| nativeQuery = true) | ||
| Page<Object[]> findNearbyRestaurantsOrderByDislikes( | ||
| @Param("userId") String userId, | ||
| @Param("latitude") Double latitude, | ||
| @Param("longitude") Double longitude, | ||
| @Param("radius") Double radius, | ||
| Pageable pageable | ||
| ); |
There was a problem hiding this comment.
The three native queries findNearbyRestaurantsOrderByDistance, findNearbyRestaurantsOrderByLikes, and findNearbyRestaurantsOrderByDislikes are almost identical except for the final ORDER BY clause in the outer query (e.g., ORDER BY dist ASC, ORDER BY like_count DESC, dist ASC, ORDER BY dislike_count DESC, dist ASC). This leads to significant code duplication, making maintenance harder. If one part of the common logic needs to change, it has to be updated in three places.
Could we explore ways to reduce this duplication? For example:
- Could a base query string be defined and the
ORDER BYclause appended dynamically in the service layer before creating the query? (Care must be taken to prevent SQL injection if not using named parameters for the dynamic part, though Spring's@Paramhandles the fixed parts well). - Alternatively, if Spring Data JPA doesn't easily support dynamic
ORDER BYwith such complex native CTEs, perhaps a single repository method that fetches the data and the service layer performs the final sort in-memory if the dataset per page isn't excessively large? (This might shift complexity or performance characteristics). - Another advanced option could be a database view or function, but that moves logic into the database.
What are your thoughts on refactoring this to improve maintainability?
| } catch (ObjectOptimisticLockingFailureException e) { | ||
| // 낙관적 락 실패 시 재시도 로직 | ||
| return vote(userId, restaurantId, voteType); | ||
| } |
There was a problem hiding this comment.
The recursive call return vote(userId, restaurantId, voteType); to handle ObjectOptimisticLockingFailureException is a simple way to implement a retry. However, without a limit on the number of retries or a backoff strategy, could this potentially lead to a StackOverflowError if optimistic locking conflicts are very frequent or persistent for some reason (though unlikely for typical user voting patterns)?
Consider using Spring Retry (@Retryable) for a more robust and configurable retry mechanism, or at least implementing a counter to limit the number of recursive calls.
| @Component | ||
| public class VoteRateLimiter { | ||
| private final Map<String, Queue<Long>> userRequests = new ConcurrentHashMap<>(); | ||
| private static final int MAX_REQUESTS = 20; | ||
| private static final long TIME_WINDOW_MS = 60 * 1000; // 1분 | ||
|
|
||
| public boolean allowRequest(String userId) { | ||
| Queue<Long> requests = userRequests.computeIfAbsent(userId, k -> new LinkedList<>()); | ||
| long now = System.currentTimeMillis(); | ||
|
|
||
| // 요청 시간 큐에서 TIME_WINDOW_MS보다 오래된 요청 제거 | ||
| while (!requests.isEmpty() && requests.peek() < now - TIME_WINDOW_MS) { | ||
| requests.poll(); | ||
| } | ||
|
|
||
| if (requests.size() >= MAX_REQUESTS) { | ||
| return false; // 제한 초과 | ||
| } | ||
|
|
||
| requests.add(now); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This VoteRateLimiter is implemented as an in-memory, single-instance component. This works well for a single-node deployment. However, if the application is deployed across multiple instances (e.g., for scalability or high availability), this rate limit will be per instance, not global. This means a user could potentially exceed the intended global rate limit by hitting different instances.
Is the current deployment strategy single-instance, or are there plans for multi-instance deployment in the near future? If multi-instance deployment is anticipated, a distributed rate limiting solution (e.g., using Redis with a sliding window log or token bucket algorithm) would be more appropriate.
| VotePageStateNotifier({ | ||
| required VoteRepository voteRepository, | ||
| required dynamic read, | ||
| LocationService? locationService, | ||
| }) : _voteRepository = voteRepository, | ||
| _read = read, | ||
| _locationService = locationService, | ||
| super(const VotePageCombinedState()) { | ||
| _fetchInitialRestaurants(); | ||
| } |
There was a problem hiding this comment.
In the VotePageStateNotifier constructor, the _read parameter is typed as dynamic. For better type safety and code clarity, could this be typed more specifically, for example, as Reader (if using older Riverpod syntax) or directly use ref.read from a Ref instance if the notifier structure allows?
// Example if Ref is available directly or passed differently
// final Ref _ref;
// VotePageStateNotifier({required VoteRepository voteRepository, required Ref ref, ...}) : _ref = ref, ...
// Or if _read is indeed a function like ref.read:
// typedef Reader = T Function<T>(ProviderListenable<T> provider);
// final Reader _read;This would make it clearer what _read is and provide better static analysis.
1 participant
PULL REQUEST
🎋 작업 브랜치
🔑 주요 변경사항
🏞 스크린샷 (선택)