[FEAT] 하트비트 및 인증로직 추가#246
Open
coli-geonwoo wants to merge 10 commits intodevelopfrom
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughWebSocket에 STOMP/SockJS 하트비트를 추가하고, 사회자용 JWT 토큰 생성/검증 흐름(토큰 타입·발급 REST 엔드포인트·WebSocket 검증)을 도입 및 관련 서비스·레포지토리·테스트를 추가/수정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant REST as SharingRestController
participant Service as SharingService
participant Repo as CustomizeTableDomainRepository
participant JWT as JwtTokenProvider
participant DB as Database
Client->>REST: GET /api/share/{tableId}/chairman-token
REST->>Service: issueChairmanToken(tableId, member)
Service->>Repo: getCustomizeTable(tableId, member)
Repo->>DB: SELECT table
DB-->>Repo: CustomizeTable
Repo-->>Service: CustomizeTable
Service->>Repo: getTotalTimeBoxTimes(tableId)
Repo->>DB: SUM(time)
DB-->>Repo: total time
Repo-->>Service: total time
Service->>JWT: createChairmanToken(memberInfo, expiration)
JWT-->>Service: chairman token
Service-->>REST: ChairmanTokenResponse
REST-->>Client: {chairmanToken: "..."}
sequenceDiagram
participant WSClient as WebSocket Client
participant WS as SharingWebSocketController
participant Resolver as WebSocketAuthMemberResolver
participant AuthMgr as AuthManager
participant MemberRepo as MemberRepository
participant Service as SharingService
WSClient->>WS: STOMP SEND /app/event/{roomId} (Authorization header)
WS->>Resolver: resolveArgument (Authorization header)
Resolver->>AuthMgr: resolveChairmanToken(token)
AuthMgr-->>Resolver: member email
Resolver->>MemberRepo: getMemberByEmail(email)
MemberRepo-->>Resolver: Member
Resolver-->>WS: Member
WS->>Service: share(member, roomId, request)
Service-->>WS: SharingResponse
WS-->>WSClient: SharingResponse (메시지 발행)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results142 files 142 suites 18s ⏱️ Results for commit a93ab77. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'Chairman Token' mechanism to manage permissions for debate sharing via WebSockets. It adds new endpoints for token issuance, updates JWT handling to support the new token type, and configures WebSocket heartbeats for improved connection stability. Review feedback highlights a bug in token resolution where the wrong resolver method was used and a potential null-pointer issue in a database query when summing debate times.
| } | ||
|
|
||
| public String resolveChairmanToken(String chairmanToken) { | ||
| return jwtTokenResolver.resolveAccessToken(chairmanToken); |
There was a problem hiding this comment.
resolveChairmanToken 메서드에서 jwtTokenResolver.resolveAccessToken을 호출하고 있습니다. JwtTokenResolver에는 CHAIRMAN_TOKEN 타입을 검증하는 전용 메서드인 resolveChairmanToken이 구현되어 있으므로, 이를 호출하도록 수정해야 합니다. 현재 상태로는 토큰 타입 불일치로 인해 인증에 실패하게 됩니다.
Suggested change
| return jwtTokenResolver.resolveAccessToken(chairmanToken); | |
| return jwtTokenResolver.resolveChairmanToken(chairmanToken); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java (1)
84-100: 엣지 케이스 테스트 추가 고려현재 테스트는 정상 케이스를 잘 커버하고 있습니다. 다만, 다음 엣지 케이스에 대한 테스트 추가를 고려해 보세요:
- 테이블에 타임박스가 없을 때 0을 반환하는지 확인
- 존재하지 않는 테이블 ID에 대한 동작 확인
💡 엣지 케이스 테스트 예시
`@Test` void 타임박스가_없는_테이블은_0을_반환한다() { Member chan = memberGenerator.generate("default@gmail.com"); CustomizeTableEntity emptyTable = customizeTableEntityGenerator.generate(chan); long summedTime = customizeTimeBoxRepository.sumTimeByTableId(emptyTable.getId()); assertThat(summedTime).isEqualTo(0L); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java` around lines 84 - 100, Add edge-case tests for CustomizeTimeBoxRepository.sumTimeByTableId: one test that creates a CustomizeTableEntity with no CustomizeTimeBoxEntity entries and asserts sumTimeByTableId(table.getId()) returns 0L, and another test that calls sumTimeByTableId with a non-existent table id (e.g., Long.MAX_VALUE or table.getId()+1) and asserts the expected behavior (0L or throws/handled result consistent with repository contract); add these new `@Test` methods in the SumTimeByTableId nested class (use memberGenerator and customizeTableEntityGenerator as in existing test) and name them clearly (e.g., 타임박스가_없는_테이블은_0을_반환한다 and 존재하지_않는_테이블_ID_처리_확인).src/test/java/com/debatetimer/fixture/HeaderGenerator.java (1)
26-27: 테스트용 사회자 토큰 만료시간(5초) 하드코딩은 플래키를 유발할 수 있습니다.Line 27의
5L은 CI 지연 시 만료를 일으킬 수 있습니다. 최소 하트비트 주기(10초)보다 충분히 큰 테스트 상수로 분리해 두는 편이 안전합니다.만료시간 상수화 예시
public class HeaderGenerator { + private static final long TEST_CHAIRMAN_TOKEN_EXPIRATION_SECONDS = 60L; private final JwtTokenProvider jwtTokenProvider; @@ public StompHeaders generateChairmanTokenHeader(String destination, Member member) { - String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), 5L); + String chairmanToken = jwtTokenProvider.createChairmanToken( + new MemberInfo(member), TEST_CHAIRMAN_TOKEN_EXPIRATION_SECONDS + ); StompHeaders stompHeaders = new StompHeaders(); stompHeaders.setDestination(destination); stompHeaders.add(HttpHeaders.AUTHORIZATION, chairmanToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/debatetimer/fixture/HeaderGenerator.java` around lines 26 - 27, The 5L literal in generateChairmanTokenHeader (jwtTokenProvider.createChairmanToken(new MemberInfo(member), 5L)) hardcodes a short expiry that can cause flaky tests; replace it with a named test constant (e.g., TEST_CHAIRMAN_TOKEN_EXPIRY_SECONDS) defined in HeaderGenerator and set it to a value safely above the minimum heartbeat (≥10s, e.g., 30L), then call createChairmanToken(new MemberInfo(member), TEST_CHAIRMAN_TOKEN_EXPIRY_SECONDS).src/test/java/com/debatetimer/controller/sharing/SharingDocumentTest.java (1)
51-56: 성공 테스트에서 응답 바디 검증을 추가해 회귀 탐지력을 높여주세요.현재는 200만 확인해서
chairmanToken누락/빈 값 회귀를 놓칠 수 있습니다.응답 필드 검증 추가 예시
+import static org.hamcrest.Matchers.emptyOrNullString; +import static org.hamcrest.Matchers.not; @@ given(document) .contentType(ContentType.JSON) .headers(EXIST_MEMBER_HEADER) .pathParam("tableId", String.valueOf(requestTableId)) .when().get("/api/share/{tableId}/chairman-token") - .then().statusCode(200); + .then() + .statusCode(200) + .body("chairmanToken", not(emptyOrNullString()));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/debatetimer/controller/sharing/SharingDocumentTest.java` around lines 51 - 56, The test currently only asserts HTTP 200 for the GET call built with given(document).headers(EXIST_MEMBER_HEADER).pathParam("tableId", String.valueOf(requestTableId)).when().get("/api/share/{tableId}/chairman-token") but doesn't validate the response body; update the test to assert the JSON response contains a non-null, non-empty "chairmanToken" field (e.g., add assertions after then() to check "chairmanToken" exists and is not an empty string) so regressions that drop or return an empty token are caught.src/test/java/com/debatetimer/service/sharing/SharingServiceTest.java (1)
31-32: 성공 케이스가 핵심 결과를 검증하지 않습니다.예외 미발생만 확인하면 잘못된 토큰(빈 문자열 등)도 통과합니다. 반환된
chairmanToken자체를 검증해 주세요.결과 검증 강화 예시
import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThat; @@ import com.debatetimer.dto.sharing.response.ChairmanTokenResponse; @@ - assertThatCode(() -> sharingService.issueChairmanToken(tableEntity.getId(), member)) - .doesNotThrowAnyException(); + ChairmanTokenResponse response = sharingService.issueChairmanToken(tableEntity.getId(), member); + assertThat(response.chairmanToken()).isNotBlank();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/debatetimer/service/sharing/SharingServiceTest.java` around lines 31 - 32, The test currently only asserts that sharingService.issueChairmanToken(tableEntity.getId(), member) does not throw, which misses validating the actual token; change the test in SharingServiceTest to capture the returned value (e.g., String chairmanToken = sharingService.issueChairmanToken(...)) and add assertions that chairmanToken is not null and not empty (and optionally matches the expected format such as a UUID or token regex) instead of using assertThatCode(...).doesNotThrowAnyException(); this ensures the method returns a valid chairmanToken rather than merely not throwing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/debatetimer/controller/tool/jwt/AuthManager.java`:
- Around line 41-43: The method resolveChairmanToken currently delegates to
jwtTokenResolver.resolveAccessToken which verifies ACCESS token type; change the
delegation to the chairman-specific resolver so CHAIRMAN_TOKEN validation is
used (e.g., call jwtTokenResolver.resolveChairmanToken or the generic resolver
that accepts a TokenType/enum and pass TokenType.CHAIRMAN) in
resolveChairmanToken to ensure correct token-type verification.
In
`@src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.java`:
- Around line 16-17: The SUM query can return null and cause a primitive long
unboxing NPE in sumTimeByTableId; modify the repository query to return zero
instead of null by using COALESCE (e.g. change the JPQL in
CustomizeTimeBoxRepository.sumTimeByTableId to "SELECT COALESCE(SUM(ctb.time),
0) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId" so
the method can safely remain returning long), and then verify
SharingService.issueChairmanToken() uses that value directly.
In `@src/main/java/com/debatetimer/service/sharing/SharingService.java`:
- Around line 34-39: The method issueChairmanToken currently multiplies
debateTime from customizeTableDomainRepository.getTotalTimeBoxTimes(...) by 2
and may pass 0 to jwtTokenProvider.createChairmanToken, creating an immediately
expired token; update issueChairmanToken to handle debateTime == 0 by either
enforcing a minimum expiry (e.g., compute expiry = Math.max(debateTime * 2,
MIN_EXPIRY_MS) and pass expiry to jwtTokenProvider.createChairmanToken) or throw
a domain/client exception (e.g., DTClientErrorException with a
NO_TIME_BOXES_CONFIGURED code) so that ChairmanTokenResponse is not created with
a zero TTL; locate this logic in issueChairmanToken and adjust the flow
accordingly.
In
`@src/test/java/com/debatetimer/controller/sharing/SharingWebSocketControllerTest.java`:
- Around line 61-81: The test method 사회자가_아니면_이벤트를_발행할_수_없다() contains an unused
local variable member and a misleading comment; remove the unused Member member
= memberGenerator.generate(...) declaration (or use it to authenticate if that
was intended) and update the inline comment on stompSession.send("/app/event/" +
roomId, request) to reflect that this is sending an unauthenticated/non-host
event (e.g., "인증되지 않은(또는 사회자가 아닌) 요청 전송") so the test intent matches the code
using handler, stompSession.subscribe, stompSession.send, SharingRequest and
handler.getCompletableFuture().
In `@src/test/java/com/debatetimer/service/sharing/SharingServiceTest.java`:
- Around line 36-41: Replace the current non-owner table test input (hardcoded
1L) with a real table owned by another member so the ownership check is
exercised: create a different Member via memberGenerator (e.g., other =
memberGenerator.generate(...)), create a Table/Room entity owned by that other
member and persist it, then call
sharingService.issueChairmanToken(table.getId(), member) using the original test
Member; assert the same DTClientErrorException and
ClientErrorCode.TABLE_NOT_FOUND.getMessage() to verify non-ownership is detected
(references: issueChairmanToken, memberGenerator, Member, sharingService,
ClientErrorCode.TABLE_NOT_FOUND).
---
Nitpick comments:
In `@src/test/java/com/debatetimer/controller/sharing/SharingDocumentTest.java`:
- Around line 51-56: The test currently only asserts HTTP 200 for the GET call
built with given(document).headers(EXIST_MEMBER_HEADER).pathParam("tableId",
String.valueOf(requestTableId)).when().get("/api/share/{tableId}/chairman-token")
but doesn't validate the response body; update the test to assert the JSON
response contains a non-null, non-empty "chairmanToken" field (e.g., add
assertions after then() to check "chairmanToken" exists and is not an empty
string) so regressions that drop or return an empty token are caught.
In `@src/test/java/com/debatetimer/fixture/HeaderGenerator.java`:
- Around line 26-27: The 5L literal in generateChairmanTokenHeader
(jwtTokenProvider.createChairmanToken(new MemberInfo(member), 5L)) hardcodes a
short expiry that can cause flaky tests; replace it with a named test constant
(e.g., TEST_CHAIRMAN_TOKEN_EXPIRY_SECONDS) defined in HeaderGenerator and set it
to a value safely above the minimum heartbeat (≥10s, e.g., 30L), then call
createChairmanToken(new MemberInfo(member), TEST_CHAIRMAN_TOKEN_EXPIRY_SECONDS).
In
`@src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java`:
- Around line 84-100: Add edge-case tests for
CustomizeTimeBoxRepository.sumTimeByTableId: one test that creates a
CustomizeTableEntity with no CustomizeTimeBoxEntity entries and asserts
sumTimeByTableId(table.getId()) returns 0L, and another test that calls
sumTimeByTableId with a non-existent table id (e.g., Long.MAX_VALUE or
table.getId()+1) and asserts the expected behavior (0L or throws/handled result
consistent with repository contract); add these new `@Test` methods in the
SumTimeByTableId nested class (use memberGenerator and
customizeTableEntityGenerator as in existing test) and name them clearly (e.g.,
타임박스가_없는_테이블은_0을_반환한다 and 존재하지_않는_테이블_ID_처리_확인).
In `@src/test/java/com/debatetimer/service/sharing/SharingServiceTest.java`:
- Around line 31-32: The test currently only asserts that
sharingService.issueChairmanToken(tableEntity.getId(), member) does not throw,
which misses validating the actual token; change the test in SharingServiceTest
to capture the returned value (e.g., String chairmanToken =
sharingService.issueChairmanToken(...)) and add assertions that chairmanToken is
not null and not empty (and optionally matches the expected format such as a
UUID or token regex) instead of using
assertThatCode(...).doesNotThrowAnyException(); this ensures the method returns
a valid chairmanToken rather than merely not throwing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 174ac2d0-e160-4c12-a7a2-6e9bdbc166b4
📒 Files selected for processing (19)
src/main/java/com/debatetimer/config/sharing/WebSocketAuthMemberResolver.javasrc/main/java/com/debatetimer/config/sharing/WebSocketConfig.javasrc/main/java/com/debatetimer/controller/sharing/SharingRestController.javasrc/main/java/com/debatetimer/controller/sharing/SharingWebSocketController.javasrc/main/java/com/debatetimer/controller/tool/jwt/AuthManager.javasrc/main/java/com/debatetimer/controller/tool/jwt/JwtTokenProvider.javasrc/main/java/com/debatetimer/controller/tool/jwt/JwtTokenResolver.javasrc/main/java/com/debatetimer/controller/tool/jwt/TokenType.javasrc/main/java/com/debatetimer/domainrepository/customize/CustomizeTableDomainRepository.javasrc/main/java/com/debatetimer/dto/sharing/response/ChairmanTokenResponse.javasrc/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.javasrc/main/java/com/debatetimer/service/sharing/SharingService.javasrc/test/java/com/debatetimer/controller/BaseDocumentTest.javasrc/test/java/com/debatetimer/controller/Tag.javasrc/test/java/com/debatetimer/controller/sharing/SharingDocumentTest.javasrc/test/java/com/debatetimer/controller/sharing/SharingWebSocketControllerTest.javasrc/test/java/com/debatetimer/fixture/HeaderGenerator.javasrc/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.javasrc/test/java/com/debatetimer/service/sharing/SharingServiceTest.java
Comment on lines
+34
to
+39
| public ChairmanTokenResponse issueChairmanToken(long tableId, Member member) { | ||
| CustomizeTable customizeTable = customizeTableDomainRepository.getByIdAndMember(tableId, member); | ||
| long debateTime = customizeTableDomainRepository.getTotalTimeBoxTimes(customizeTable.getId()); | ||
| String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), debateTime * 2); | ||
| return new ChairmanTokenResponse(chairmanToken); | ||
| } |
There was a problem hiding this comment.
타임박스가 없을 경우 토큰 만료 시간이 0이 됨
getTotalTimeBoxTimes가 0을 반환하면(타임박스가 없는 테이블) debateTime * 2 = 0이 되어 즉시 만료되는 토큰이 생성됩니다. 이는 의도된 동작이 아닐 수 있습니다.
최소 만료 시간을 설정하거나, 타임박스가 없는 경우에 대한 처리를 고려해 주세요.
🛡️ 최소 만료 시간 적용 제안
public ChairmanTokenResponse issueChairmanToken(long tableId, Member member) {
+ private static final long MINIMUM_EXPIRATION_SECONDS = 300; // 최소 5분
+
CustomizeTable customizeTable = customizeTableDomainRepository.getByIdAndMember(tableId, member);
long debateTime = customizeTableDomainRepository.getTotalTimeBoxTimes(customizeTable.getId());
- String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), debateTime * 2);
+ long expirationSeconds = Math.max(debateTime * 2, MINIMUM_EXPIRATION_SECONDS);
+ String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), expirationSeconds);
return new ChairmanTokenResponse(chairmanToken);
}또는 타임박스가 없는 테이블에 대해서는 예외를 발생시키는 방법도 있습니다:
if (debateTime == 0) {
throw new DTClientErrorException(ClientErrorCode.NO_TIME_BOXES_CONFIGURED);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public ChairmanTokenResponse issueChairmanToken(long tableId, Member member) { | |
| CustomizeTable customizeTable = customizeTableDomainRepository.getByIdAndMember(tableId, member); | |
| long debateTime = customizeTableDomainRepository.getTotalTimeBoxTimes(customizeTable.getId()); | |
| String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), debateTime * 2); | |
| return new ChairmanTokenResponse(chairmanToken); | |
| } | |
| private static final long MINIMUM_EXPIRATION_SECONDS = 300; // 최소 5분 | |
| public ChairmanTokenResponse issueChairmanToken(long tableId, Member member) { | |
| CustomizeTable customizeTable = customizeTableDomainRepository.getByIdAndMember(tableId, member); | |
| long debateTime = customizeTableDomainRepository.getTotalTimeBoxTimes(customizeTable.getId()); | |
| long expirationSeconds = Math.max(debateTime * 2, MINIMUM_EXPIRATION_SECONDS); | |
| String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), expirationSeconds); | |
| return new ChairmanTokenResponse(chairmanToken); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/debatetimer/service/sharing/SharingService.java` around
lines 34 - 39, The method issueChairmanToken currently multiplies debateTime
from customizeTableDomainRepository.getTotalTimeBoxTimes(...) by 2 and may pass
0 to jwtTokenProvider.createChairmanToken, creating an immediately expired
token; update issueChairmanToken to handle debateTime == 0 by either enforcing a
minimum expiry (e.g., compute expiry = Math.max(debateTime * 2, MIN_EXPIRY_MS)
and pass expiry to jwtTokenProvider.createChairmanToken) or throw a
domain/client exception (e.g., DTClientErrorException with a
NO_TIME_BOXES_CONFIGURED code) so that ChairmanTokenResponse is not created with
a zero TTL; locate this logic in issueChairmanToken and adjust the flow
accordingly.
Contributor
Author
There was a problem hiding this comment.
이미 타임박스가 없는 경우는 도메인적으로 제한하고 있어
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for domain-level restrictions on time boxes (e.g., validation, exceptions when empty)
rg -n "timeBox|TimeBox" --type java -C3 | grep -i "empty\|size\|isEmpty\|validate\|minimum\|least\|required\|exception\|error" | head -40
# Look at CustomizeTable domain class for constraints
fd "CustomizeTable.java" --exec cat {}
# Look for any validation related to time boxes being required
rg -n "NO_TIME\|EMPTY.*TIME\|TIME.*EMPTY\|timeBox.*size\|size.*timeBox" --type java -C3 | head -30
Length of output: 8190
🏁 Script executed:
#!/bin/bash
# Look for the request DTO that contains the table/timebox list with validation
fd "CustomizeTableCreateRequest.java\|CustomizeTableRequest.java\|TableCreateRequest.java" --exec cat {}
# Also search for `@NotEmpty` or `@Size` on table-related fields
rg -n "@NotEmpty|@Size|@Min" --type java -B2 -A2 | grep -A4 -B4 "table\|timeBox\|TimeBox" | head -60
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find all request DTOs in the customize package
fd -e java . src/main/java --exec grep -l "table\|timeBox" {} | head -20
# Look for validation annotations in dto/request files
rg -n "NotEmpty\|Size\|NotNull" src/main/java --type java -B3 -A1 | head -80
# Find the customize request DTOs
fd -e java "Request" src/main/java/com/debatetimer/dto --exec cat {}
Length of output: 7461
🏁 Script executed:
#!/bin/bash
# Read CustomizeService to find validation logic for empty time boxes
fd "CustomizeService.java" --exec cat {}
# Also check if there's a domain class that validates the time box list
fd "CustomizeTimeBoxes.java\|CustomizeTableTimeBoxes.java" --exec cat {}
# Search for any `@NotEmpty` or `@Size`(min on table fields
rg -n "NotEmpty|Size\(min" --type java -B5 -A2 | head -60
Length of output: 2613
🏁 Script executed:
#!/bin/bash
# Read CustomizeTableDomainRepository for any validation on time boxes
fd "CustomizeTableDomainRepository.java" --exec cat {}
# Look for any validation in the domain entities or value objects about minimum timebox count
rg -n "empty\|isEmpty\|size.*0\|EMPTY_TABLE\|NO_TIMEBOX\|EMPTY_TIMEBOX" --type java -C3 | head -60
Length of output: 5082
@coli-geonwoo, 확인해주셔서 감사합니다. 도메인 레벨에서 이미 타임박스가 없는 테이블 생성을 제한하고 있다면, 해당 우려는 적용되지 않습니다.
다만 코드를 살펴보면서 한 가지 추가로 확인하실 사항이 있습니다. sumTimeByTableId의 JPQL 쿼리는 SUM(ctb.time)을 사용하는데, TimeBasedTimeBox 타입의 타임박스는 time 컬럼 대신 timePerTeam/timePerSpeaking을 사용합니다. 만약 테이블에 TimeBasedTimeBox 타입의 타임박스만 있을 경우 SUM 결과가 null이 되어 반환 타입인 primitive long에 할당 시 NullPointerException이 발생할 수 있습니다. 의도된 동작인지 확인해보시면 좋을 것 같습니다.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Contributor
Author
There was a problem hiding this comment.
timebasedtimebox 같은 경우에는 저장될 때 timeperteam *2로 시간을 계산하여 time 컬럼의 총 시간이 저장되므로 time만 계산하더라도 계산에 틀린 부분은 없어
Comment on lines
+36
to
+41
| void 회원_소유의_테이블이_아니면_에러가_발생한다() { | ||
| Member member = memberGenerator.generate("email@email.com"); | ||
|
|
||
| assertThatThrownBy(() -> sharingService.issueChairmanToken(1L, member)) | ||
| .isInstanceOf(DTClientErrorException.class) | ||
| .hasMessage(ClientErrorCode.TABLE_NOT_FOUND.getMessage()); |
There was a problem hiding this comment.
“비소유 테이블” 시나리오를 실제로 검증하지 못하고 있습니다.
Line 39의 1L은 단순 미존재 테이블 케이스로도 통과할 수 있어, 소유권 검증 로직이 깨져도 잡지 못합니다. 다른 회원 소유 테이블을 생성한 뒤 요청하도록 바꿔 주세요.
실제 비소유 시나리오로 수정 예시
`@Test`
void 회원_소유의_테이블이_아니면_에러가_발생한다() {
- Member member = memberGenerator.generate("email@email.com");
+ Member requester = memberGenerator.generate("requester@email.com");
+ Member owner = memberGenerator.generate("owner@email.com");
+ CustomizeTableEntity ownersTable = customizeTableEntityGenerator.generate(owner);
- assertThatThrownBy(() -> sharingService.issueChairmanToken(1L, member))
+ assertThatThrownBy(() -> sharingService.issueChairmanToken(ownersTable.getId(), requester))
.isInstanceOf(DTClientErrorException.class)
.hasMessage(ClientErrorCode.TABLE_NOT_FOUND.getMessage());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void 회원_소유의_테이블이_아니면_에러가_발생한다() { | |
| Member member = memberGenerator.generate("email@email.com"); | |
| assertThatThrownBy(() -> sharingService.issueChairmanToken(1L, member)) | |
| .isInstanceOf(DTClientErrorException.class) | |
| .hasMessage(ClientErrorCode.TABLE_NOT_FOUND.getMessage()); | |
| void 회원_소유의_테이블이_아니면_에러가_발생한다() { | |
| Member requester = memberGenerator.generate("requester@email.com"); | |
| Member owner = memberGenerator.generate("owner@email.com"); | |
| CustomizeTableEntity ownersTable = customizeTableEntityGenerator.generate(owner); | |
| assertThatThrownBy(() -> sharingService.issueChairmanToken(ownersTable.getId(), requester)) | |
| .isInstanceOf(DTClientErrorException.class) | |
| .hasMessage(ClientErrorCode.TABLE_NOT_FOUND.getMessage()); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/com/debatetimer/service/sharing/SharingServiceTest.java` around
lines 36 - 41, Replace the current non-owner table test input (hardcoded 1L)
with a real table owned by another member so the ownership check is exercised:
create a different Member via memberGenerator (e.g., other =
memberGenerator.generate(...)), create a Table/Room entity owned by that other
member and persist it, then call
sharingService.issueChairmanToken(table.getId(), member) using the original test
Member; assert the same DTClientErrorException and
ClientErrorCode.TABLE_NOT_FOUND.getMessage() to verify non-ownership is detected
(references: issueChairmanToken, memberGenerator, Member, sharingService,
ClientErrorCode.TABLE_NOT_FOUND).
unifolio0
reviewed
Apr 15, 2026
Contributor
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@coli-geonwoo 리뷰 몇개 남겼어요
| @RequiredArgsConstructor | ||
| public class SharingService { | ||
|
|
||
| private final JwtTokenProvider jwtTokenProvider; |
Contributor
There was a problem hiding this comment.
저희 이거 예전에 Oauth 로그인 할 때 AuthManager로 Controller 단에서 처리하기로 합의하지 않았었나요?
Contributor
Author
There was a problem hiding this comment.
일단 늦게 확인하고 답변 남겨 염치없지만 이부분은 의견이 달라 남겨둡니다.
1) 내가 기억하는 이전에 우리가 Controller 단에서 인증처리를 하려고 했던 이유
- 인증이 프로토콜 경계에서 일어나기 때문에
- Http 프로토콜에 종속되지 않고 서비스 확장가능성을 보장하기 위해
즉, 서비스 단에서는 '인증된 사용자'를 받으면 되지 토큰 해석이나 발급까지 서비스에서 할 필요가 없다는데 동의했기 때문에 그런 결정이 있었던 것으로 기억합니다.
그런데 해당 상황은 다음 부분에서 다르다고 생각합니다.
-
인증된 사용자를 대상으로 일정 시간 동안, 사회자용 권한 자격증을 발급해주는 로직임.
이 토큰 같은 경우에는누구인지 식별하는 인증 토큰보다 사회자용 이벤트 발행에 대한 권한이 있는지 검증하는 권한 토큰의 성격이 더 크다고 생각했어요. -
도메인이 많이 묻어있음
사회자용 인증 토큰을 발급하는 과정에서 도메인에 꽤 깊은 관여가 되어 있습니다. 사회자를 위해 유효기간이 전체 토론 시간 *2인 토큰을 발급해야 하는데, 도메인 레이어를 각각 호출해서 이를 controller에서 조립해 발급한다는 생각이 들었어요. -
만약 토큰 발급에 필요한 도메인이 많아진다면..?
예전 매쉬업 프로젝트에서도 컨트롤러에서 authService를 통해 토큰을 발급하는 로직을 고수해보았는데 점점 토큰 발급에 필요한 도메인이 많아지다보니까 low layer에서 필요한 정보들을 모아 컨트롤러에서 조합해 토큰 발급 로직에 넣어주어야 하는 경우가 생겼습니다.
사실 컨트롤러로 바꾸는 건 그리 어려운 일은 아닌데 비토 생각 듣고 싶어 한번 팅겨봅니다.
Contributor
Author
There was a problem hiding this comment.
claude 한테도 맥락 넘겨 리뷰받아보았는데 이런 의견을 주었어요.
claude 답변
사회자용 토큰 발급, 어디에 둘 것인가
좋은 질문이고, 사실 이 고민의 핵심은 "토큰이라는 것이 정말 HTTP/프로토콜에 종속된 개념인가, 아니면 도메인 개념인가" 라는 지점이라고 봐. 결론부터 말하면, 나는 이 경우에는 Service 레이어에서 발급하고, Controller는 단순히 그것을 HTTP 응답으로 감싸기만 하는 구조를 택할 거야. 이유를 차근차근 설명해줄게.
1. 이전 결정과 지금 상황의 본질적 차이
팀에서 OAuth/JWT 인증 토큰을 Controller에서 발급하기로 한 결정은 옳았어. 그 이유를 다시 분해해보면:
- 인증(Authentication)은 프로토콜 경계에서 일어나는 일이야. "이 HTTP 요청을 보낸 사람이 누구인가"를 식별하는 건 transport layer의 관심사지.
- JWT는 그 자체가 stateless 인증을 위한 HTTP 친화적 메커니즘이야. 쿠키, Authorization 헤더, CORS 같은 것과 한 묶음으로 다뤄지지.
- 그래서 Service는 "인증된 User"를 받는 것이지, 토큰을 알 필요가 없었어. 프로토콜이 gRPC로 바뀌어도 Service는 그대로 재사용 가능했지.
그런데 지금 발급하려는 "사회자용 토큰"은 성격이 완전히 달라:
- 이건 누구인지 식별하는 인증 토큰이 아니라, 특정 도메인 이벤트(토론 시작) 시점에, 특정 도메인 역할(사회자)에게, 특정 도메인 컨텍스트(이 토론, 이 시간 동안)에서 사용할 **권한 자격증(capability token)**이야.
- 발급 트리거가 HTTP 요청이 아니라 "토론이 시작된다"는 도메인 이벤트야. 토론 시작이 스케줄러로 자동화되거나, 다른 진입점(예: 관리자 콘솔, 배치 작업)이 생긴다면? Controller에 발급 로직이 있으면 그곳에서도 똑같은 코드를 또 작성하거나 Controller를 호출해야 해.
- 만료 시간이 **"토론 전체 시간의 2배"**라는 도메인 규칙으로 결정돼. 이건 명백히 비즈니스 규칙이야.
2. "토큰 발급 = Controller"는 규칙이 아니라 사례였다
여기서 중요한 포인트야. 팀의 이전 결정은 **"토큰은 무조건 Controller에서 발급한다"**가 아니라, **"프로토콜 종속적인 인증 토큰은 Controller에서 발급한다"**였어. 둘은 달라.
지금 토큰의 본질을 따져보면:
관점 | OAuth JWT | 사회자 토큰
-- | -- | --
발급 트리거 | 로그인 HTTP 요청 | 토론 시작 도메인 이벤트
만료 정책 결정 주체 | 보안 정책 (인프라/플랫폼) | 토론 길이 (도메인 규칙)
식별 대상 | "이 사용자가 누구인가" | "이 사람이 이 토론의 사회자인가"
다른 프로토콜에서 재사용? | 보통 안 함 (프로토콜별 인증) | 가능성 매우 높음 (WS, SSE, 추후 RPC 등)
검증 위치 | API Gateway / 미들웨어 | WebSocket 핸들러 비즈니스 로직 진입부
특히 마지막 두 줄을 보면, 사회자 토큰은 오히려 재사용성과 도메인 결합도 측면에서 Service에 있어야 할 이유가 더 강해.
3. Controller에 두면 생기는 실질적 문제
추상적인 "레이어드 아키텍처 위반"을 넘어서, 실제로 발생할 문제들:
(1) 만료 시간 규칙의 분산
"토론 시간의 2배"라는 규칙이 Controller에 있으면, Service의 Debate 엔티티가 getDuration()을 알고, Controller가 그걸 가져다 ×2 해서 토큰을 만들어. 도메인 규칙이 Controller에 흘러나가는 거지. 나중에 "프리미엄 토론은 3배" 같은 규칙이 추가되면? Controller에 if문이 박히기 시작해.
(2) 진입점이 늘어나면 중복 발생 지금은 사회자가 "토론 시작" 버튼을 누르는 HTTP 요청 하나뿐일 수 있어. 하지만 이런 게 추가될 가능성이 높아:
- 예약된 토론을 자동 시작하는 스케줄러
- 관리자가 강제로 시작시키는 어드민 API
- 다른 서비스가 호출하는 내부 API
이 모든 진입점에서 같은 토큰 발급 로직을 호출해야 해. Service에 있으면 한 줄, Controller에 있으면 복붙.
(3) 테스트 어려움 토큰 발급 로직을 테스트하려면 Controller 테스트를 돌려야 해. HTTP 목킹, 요청/응답 컨텍스트... Service에 있으면 순수 단위 테스트 한 방.
(4) "토큰" 단어에 속지 않기 이게 진짜 중요한데, "토큰"이라는 단어가 주는 인프라/보안 뉘앙스 때문에 자동으로 Controller에 두려는 관성이 생겨. 하지만 본질은 **"사회자에게 발급되는, 이 토론에서 유효한, 시간 제한 있는 권한 증서"**야. 이걸 그냥 도메인 객체로 보면 어디에 둘지 자명해져.
Comment on lines
+22
to
+25
| public class SharingDocumentTest extends BaseDocumentTest { | ||
|
|
||
|
|
||
| @Nested |
Contributor
There was a problem hiding this comment.
Suggested change
| public class SharingDocumentTest extends BaseDocumentTest { | |
| @Nested | |
| public class SharingDocumentTest extends BaseDocumentTest { | |
| @Nested |
Comment on lines
+34
to
+39
| public ChairmanTokenResponse issueChairmanToken(long tableId, Member member) { | ||
| CustomizeTable customizeTable = customizeTableDomainRepository.getByIdAndMember(tableId, member); | ||
| long debateTime = customizeTableDomainRepository.getTotalTimeBoxTimes(customizeTable.getId()); | ||
| String chairmanToken = jwtTokenProvider.createChairmanToken(new MemberInfo(member), debateTime * 2); | ||
| return new ChairmanTokenResponse(chairmanToken); | ||
| } |
unifolio0
requested changes
Apr 15, 2026
leegwichan
approved these changes
Apr 20, 2026
Member
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti 제 코멘트 이외 코멘트만 확인 부탁드립니다.
coli-geonwoo
commented
Apr 28, 2026
Contributor
Author
coli-geonwoo
left a comment
coli-geonwoo
left a comment
There was a problem hiding this comment.
/noti
요새 힘드네요.. 취준하랴 소마하랴.. 토큰 발급 계층에 대해 한번 비토 의견 듣고 싶어 생각남겨놓았으니 확인 부탁드립니다.
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
3 participants
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.
🚩 연관 이슈
closed #245
🗣️ 리뷰 요구사항 (선택)
하트비트 문제
기존 문제
양쪽모두 하트비트 설정
두 하트비트의 목적이 다르고 신뢰성 있는 기능 개발을 위해 10초 간격으로 양방향 설정
STOMP가 하트비트를 통해 자동으로 처리해주는 것
해당 세션을 죽었다고 판단
WebSocket 연결 close
DISCONNECT이벤트 발행 → Spring의 내부 세션 정리해당 세션의 구독 정보 제거
SimpUserRegistry에서 사용자/세션 제거추후 처리할 에러 처리 문제 : Role 별 비정상종료 시 비즈니스 로직 처리 문제
⇒ 종료시 비즈니스 로직 처리를 위해서는 SessionDisconnectEvent 처리 필요
인증 로직
토큰이 3종류로 넘어가면서 중복 코드가 조금 생겼는데 이부분에 대한 의견 부탁합니다
Summary by CodeRabbit
릴리스 노트
신규 기능
개선 사항
테스트 / 문서