[Refactor] : thumb-up API를 thumb_logs 개별 로그 기반으로 변경 (#169)#171
Merged
[Refactor] : thumb-up API를 thumb_logs 개별 로그 기반으로 변경 (#169)#171
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
|
/describe |
Owner
Author
|
/review |
Owner
Author
|
/improve |
Review Summary by QodoRefactor thumb-up API to use individual thumb_logs records
WalkthroughsDescription• Refactored thumb-up API from total_stats aggregation to individual thumb_logs records • Changed POST endpoint from PATCH to POST with simplified insert logic • Implemented app-level aggregation for song thumb counts with top 50 ranking • Updated search API to join thumb_logs and calculate thumbs via reduce • Renamed field from total_thumb to thumb_count across codebase Diagramflowchart LR
A["thumb_logs table"] -->|"fetch all records"| B["App-level aggregation"]
B -->|"group by song_id"| C["Calculate thumb counts"]
C -->|"sort & slice top 50"| D["Fetch song details"]
D -->|"merge & return"| E["ThumbUpSong[] response"]
F["POST request"] -->|"insert log entry"| A
File Changes1. apps/web/src/app/api/songs/thumb-up/route.ts
|
Code Review by QodoNew Review StartedThis review has been superseded by a new analysisⓘ The new review experience is currently in Beta. Learn more |
1 similar comment
Code Review by QodoNew Review StartedThis review has been superseded by a new analysisⓘ The new review experience is currently in Beta. Learn more |
Code Review by Qodo
1. Unbounded thumb_logs scan
|
|
PR Description updated to latest commit (3d30b35)
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+16
to
55
| // 1) thumb_logs 전체 조회 | ||
| const { data: thumbData, error: thumbError } = await supabase | ||
| .from('thumb_logs') | ||
| .select('song_id, thumb_count'); | ||
|
|
||
| if (thumbError) throw thumbError; | ||
| if (!thumbData || thumbData.length === 0) { | ||
| return NextResponse.json({ success: true, data: [] }); | ||
| } | ||
|
|
||
| // 2) 앱에서 song_id별 합계 집계 | ||
| const thumbMap = new Map<string, number>(); | ||
| for (const row of thumbData) { | ||
| thumbMap.set(row.song_id, (thumbMap.get(row.song_id) ?? 0) + row.thumb_count); | ||
| } | ||
|
|
||
| // 3) 상위 50개 song_id 추출 | ||
| const sorted = [...thumbMap.entries()] | ||
| .sort((a, b) => b[1] - a[1]) | ||
| .slice(0, 50); | ||
|
|
||
| const songIds = sorted.map(([songId]) => songId); | ||
|
|
||
| // 4) 해당 song 상세 정보 조회 | ||
| const { data: songs, error: songError } = await supabase | ||
| .from('songs') | ||
| .select('*') | ||
| .in('id', songIds); | ||
|
|
||
| if (songError) throw songError; | ||
|
|
||
| // 5) 병합 후 thumb_count 내림차순 정렬 | ||
| const songMap = new Map(songs?.map(song => [song.id, song])); | ||
| const data = sorted | ||
| .filter(([songId]) => songMap.has(songId)) | ||
| .map(([songId, thumbCount]) => ({ | ||
| ...songMap.get(songId)!, | ||
| thumb_count: thumbCount, | ||
| })); | ||
|
|
There was a problem hiding this comment.
1. Unbounded thumb_logs scan 🐞 Bug ➹ Performance
thumb-up GET loads the entire thumb_logs table and aggregates/sorts in application memory, so latency and memory usage grow linearly with log volume and can time out under load. This regresses from DB-side ordering/limiting and will get worse as logs accumulate.
Agent Prompt
### Issue description
`GET /api/songs/thumb-up` currently reads **all** rows from `thumb_logs` and aggregates/sorts in the app. This will not scale and can cause timeouts.
### Issue Context
We only need the **top 50 songs by SUM(thumb_count)**.
### Fix Focus Areas
- apps/web/src/app/api/songs/thumb-up/route.ts[16-55]
### Suggested approach
- Move aggregation to the database:
- Create a SQL view (e.g., `song_thumb_totals(song_id, thumb_count_total)`) or an RPC function that returns `song_id` + `sum(thumb_count)` ordered desc limited 50.
- Query that aggregated result and then fetch/join song details.
- Ensure the final list preserves the aggregated ordering (use a single query that joins songs to the aggregate if possible).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
49
to
55
| if (!authenticated) { | ||
| const baseQuery = supabase.from('songs').select( | ||
| `*, | ||
| total_stats ( | ||
| thumb_logs ( | ||
| * | ||
| ) | ||
| `, |
There was a problem hiding this comment.
2. Search overfetches thumb logs 🐞 Bug ➹ Performance
Search API selects thumb_logs(*) for each song and then reduces client-side to a sum, so response size and query cost scale with the number of logs per song even though only one aggregated number is returned. This can severely slow search and inflate bandwidth/costs as thumb_logs grows.
Agent Prompt
### Issue description
Search API fetches `thumb_logs(*)` for each song and then sums them in JS. This overfetches and will become slow/expensive as logs increase.
### Issue Context
The API response only needs `thumb` as a number.
### Fix Focus Areas
- apps/web/src/app/api/search/route.ts[49-57]
- apps/web/src/app/api/search/route.ts[99-116]
- apps/web/src/app/api/search/route.ts[77-87]
- apps/web/src/app/api/search/route.ts[136-148]
### Suggested approach
- Replace embedded `thumb_logs(*)` with a DB-side aggregate:
- Prefer joining to an aggregated view (e.g., `song_thumb_totals`) or using an RPC function that returns songs with `thumb_sum`.
- If using a view, select `songs.*` plus `song_thumb_totals.thumb_sum` and default to 0 when null.
- Keep pagination/order behavior unchanged.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+75
to
+86
| export async function POST(request: Request): Promise<NextResponse<ApiResponse<void>>> { | ||
| try { | ||
| const supabase = await createClient(); | ||
| const userId = await getAuthenticatedUser(supabase); | ||
|
|
||
| const { point, songId } = await request.json(); | ||
|
|
||
| const { data } = await supabase | ||
| .from('total_stats') | ||
| .select('total_thumb') | ||
| .eq('song_id', songId) | ||
| .single(); | ||
|
|
||
| if (data) { | ||
| const totalThumb = data.total_thumb + point; | ||
|
|
||
| const { error: updateError } = await supabase | ||
| .from('total_stats') | ||
| .update({ total_thumb: totalThumb }) | ||
| .eq('song_id', songId); | ||
| const { error: insertError } = await supabase | ||
| .from('thumb_logs') | ||
| .insert({ song_id: songId, user_id: userId, thumb_count: point }); | ||
|
|
||
| if (updateError) throw updateError; | ||
| } else { | ||
| const { error: insertError } = await supabase | ||
| .from('total_stats') | ||
| .insert({ song_id: songId, total_thumb: point }); | ||
|
|
||
| if (insertError) throw insertError; | ||
| } | ||
| if (insertError) throw insertError; |
There was a problem hiding this comment.
3. Thumb-up points unenforced 🐞 Bug ⛨ Security
thumb-up POST inserts a thumb log for the authenticated user without validating point or enforcing that the user actually spent/has enough points, so clients can mint arbitrary thumb_count (including negative/huge values) and manipulate rankings. Because spending points is handled by a separate endpoint, thumbs and user points can also diverge on partial failures.
Agent Prompt
### Issue description
`POST /api/songs/thumb-up` writes `thumb_count` directly from client input and does not enforce any point rules. This allows abuse (free/infinite thumbs, negative thumbs) and data inconsistencies.
### Issue Context
Point deduction currently happens via `/api/user/spend-point` which updates `users.point` independently.
### Fix Focus Areas
- apps/web/src/app/api/songs/thumb-up/route.ts[75-86]
- apps/web/src/app/api/user/spend-point/route.ts[7-18]
### Suggested approach
- Add strict validation in thumb-up POST:
- `songId` required, valid UUID format (if applicable), and exists.
- `point` must be a positive integer within reasonable bounds.
- Enforce points server-side atomically:
- Implement a DB transaction via Postgres function/RPC (recommended) that:
1) checks `users.point >= point`
2) decrements user points
3) inserts into `thumb_logs`
- Call that RPC from the API route and return 400/403 on insufficient points.
- If you keep separate endpoints, at minimum ensure thumb-up checks the user’s current points and fails if insufficient (still non-atomic, but blocks free thumbs).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
📌 PR 제목
[Refactor] : thumb-up API를 thumb_logs 개별 로그 기반으로 변경
📌 변경 사항
total_stats단일 행 조회 →thumb_logs전체 조회 후 앱 레벨에서 song_id별thumb_count합계 집계, 상위 50개 반환getAuthenticatedUser()로 인증 후song_id+user_id+thumb_count단순 inserttotal_stats→thumb_logs조인 변경, 배열 합산(reduce)으로 thumb 계산patchSongThumb→postSongThumb, HTTP 메서드 PATCH → POSTThumbUpSong.total_thumb→thumb_count필드명 변경💬 추가 참고 사항
PR Type
Enhancement, Refactor
Description
Refactored thumb-up API from aggregated stats to individual log-based approach
Changed GET endpoint to query
thumb_logstable and aggregate at application levelConverted PATCH to POST method with simplified insert-only logic
Updated search API to calculate thumbs from
thumb_logsarray using reduceRenamed field from
total_thumbtothumb_countacross codebaseDiagram Walkthrough
File Walkthrough
route.ts
Refactor thumb-up endpoint to log-based aggregationapps/web/src/app/api/songs/thumb-up/route.ts
thumb_logstable and aggregateresults at application level instead of querying
total_statsremoving upsert complexity
getAuthenticatedUser()call for user authentication in POSThandler
route.ts
Update search API to use thumb_logs aggregationapps/web/src/app/api/search/route.ts
thumb_logsarray instead oftotal_statsobjecttotal_thumbtothumb_countin type definitionsthumb_logsinstead oftotal_statsreduce()to sumthumb_countfrom logsarray
thumbSong.ts
Update client API method from PATCH to POSTapps/web/src/lib/api/thumbSong.ts
patchSongThumbtopostSongThumbsongThumbQuery.ts
Update TanStack Query hook to use POST methodapps/web/src/queries/songThumbQuery.ts
postSongThumbinstead ofpatchSongThumbpostSongThumbsong.ts
Update ThumbUpSong type field nameapps/web/src/types/song.ts
ThumbUpSonginterface field fromtotal_thumbtothumb_countPopularRankingList.tsx
Update popular ranking to use thumb_count fieldapps/web/src/app/popular/PopularRankingList.tsx
item.total_thumbtoitem.thumb_count