Open
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
개요WebSocket 하트비트 기능을 추가하고, 사회자용 토큰 발급 및 검증 로직을 구현합니다. 새로운 REST 엔드포인트를 통해 사회자 토큰을 발급하고, 기존 WebSocket 메시지 핸들러는 사회자 인증을 위해 업데이트됩니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client as 클라이언트
participant REST as SharingRestController
participant Service as SharingService
participant Repo as CustomizeTableDomainRepository
participant JWT as JwtTokenProvider
participant DB as 데이터베이스
Client->>REST: GET /api/share/{tableId}/chairman-token
REST->>Service: issueChairmanToken(tableId, member)
Service->>Repo: getCustomizeTable(tableId, member)
Repo->>DB: 테이블 조회
DB-->>Repo: CustomizeTable
Repo-->>Service: CustomizeTable
Service->>Repo: getTotalTimeBoxTimes(tableId)
Repo->>DB: SUM(timeBox.time)
DB-->>Repo: total time
Repo-->>Service: total time
Service->>JWT: createChairmanToken(memberInfo, debateTime*2)
JWT-->>Service: chairman token
Service-->>REST: ChairmanTokenResponse
REST-->>Client: {chairmanToken: "..."}
sequenceDiagram
participant Client as WebSocket 클라이언트
participant WS as SharingWebSocketController
participant Resolver as WebSocketAuthMemberResolver
participant AuthMgr as AuthManager
participant Service as SharingService
participant DB as 데이터베이스
Client->>WS: /app/event/{roomId} (STOMP 메시지)
WS->>Resolver: resolveArgument (chairman token)
Resolver->>AuthMgr: resolveChairmanToken(token)
AuthMgr-->>Resolver: member email
Resolver->>DB: getMember(email)
DB-->>Resolver: Member
Resolver-->>WS: Member
WS->>Service: share(member, roomId, request)
Service-->>WS: SharingResponse
WS-->>Client: SharingResponse (WebSocket)
예상 코드 리뷰 노력🎯 3 (Moderate) | ⏱️ ~35 minutes 관련 PR
추천 레이블
추천 리뷰어
🚥 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 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 19s ⏱️ Results for commit 0821655. ♻️ 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); |
Comment on lines
+16
to
+17
| @Query("SELECT SUM(ctb.time) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId") | ||
| long sumTimeByTableId(long tableId); |
There was a problem hiding this comment.
JPQL의 SUM 함수는 조회 결과가 없을 경우 null을 반환합니다. 현재 반환 타입이 Java 기본형인 long으로 되어 있어, null 반환 시 언박싱 과정에서 예외가 발생할 수 있습니다. COALESCE를 사용하여 결과가 없을 때 0을 반환하도록 처리하는 것이 안전합니다.
Suggested change
| @Query("SELECT SUM(ctb.time) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId") | |
| long sumTimeByTableId(long tableId); | |
| @Query("SELECT COALESCE(SUM(ctb.time), 0) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId") | |
| long sumTimeByTableId(long tableId); |
![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("[email protected]"); 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
+16
to
+17
| @Query("SELECT SUM(ctb.time) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId") | ||
| long sumTimeByTableId(long tableId); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
JPQL/Hibernate에서 SUM(...)은 조건에 맞는 row가 0개일 때 null을 반환하나요? 공식 문서 또는 신뢰 가능한 레퍼런스 기준으로 확인해 주세요.
💡 Result:
네, JPQL/Hibernate에서 SUM(...) 집계 함수는 조건에 맞는 row가 0개일 때 null을 반환합니다. 이는 JPA 사양에 명시된 표준 동작입니다.
Citations:
🏁 Script executed:
# 파일 위치 확인
find . -name "CustomizeTimeBoxRepository.java" -type fRepository: debate-timer/debate-timer-be
Length of output: 156
🏁 Script executed:
# sumTimeByTableId 메서드 호출처 검색
rg "sumTimeByTableId" --type java -B 2 -A 2Repository: debate-timer/debate-timer-be
Length of output: 2045
🏁 Script executed:
# CustomizeTimeBoxRepository 파일 내용 확인
cat -n src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.javaRepository: debate-timer/debate-timer-be
Length of output: 1190
🏁 Script executed:
# getTotalTimeBoxTimes 호출처 검색
rg "getTotalTimeBoxTimes" --type java -B 3 -A 3Repository: debate-timer/debate-timer-be
Length of output: 1815
🏁 Script executed:
# 토큰 생성/발급 관련 로직과의 연결 확인
rg "token|Token" --type java -i | grep -i "customize\|TimeBox" | head -20Repository: debate-timer/debate-timer-be
Length of output: 1979
🏁 Script executed:
# 테스트 코드 전체 확인 - empty 케이스 있는지 확인
cat -n src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java | head -80Repository: debate-timer/debate-timer-be
Length of output: 4474
🏁 Script executed:
# sumTimeByTableId 테스트 부분 확인
rg "sumTimeByTableId" src/test/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepositoryTest.java -A 15 -B 5Repository: debate-timer/debate-timer-be
Length of output: 598
🏁 Script executed:
# issueChairmanToken 메서드 전체 확인
cat -n src/main/java/com/debatetimer/service/sharing/SharingService.java | grep -A 10 "issueChairmanToken"Repository: debate-timer/debate-timer-be
Length of output: 586
🏁 Script executed:
# empty 케이스 테스트 존재 여부 확인
rg "sumTimeByTableId|getTotalTimeBoxTimes" src/test -A 10 -B 5Repository: debate-timer/debate-timer-be
Length of output: 1555
SUM 쿼리가 null을 반환할 때 primitive long 언박싱 예외 발생 위험
매칭 데이터가 없을 때 SUM(ctb.time)은 null을 반환하는데, 반환형이 primitive long이므로 NullPointerException이 발생합니다. 이 메서드는 SharingService.issueChairmanToken() (토큰 발급 API)에서 직접 호출되므로, 타임박스가 없는 테이블에서 토큰 발급 요청 시 500 에러로 이어집니다.
🔧 제안하는 수정
-@Query("SELECT SUM(ctb.time) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId")
+@Query("SELECT COALESCE(SUM(ctb.time), 0) FROM CustomizeTimeBoxEntity ctb WHERE ctb.customizeTable.id = :tableId")
long sumTimeByTableId(long tableId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/debatetimer/repository/customize/CustomizeTimeBoxRepository.java`
around lines 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.
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!
Comment on lines
+36
to
+41
| void 회원_소유의_테이블이_아니면_에러가_발생한다() { | ||
| Member member = memberGenerator.generate("[email protected]"); | ||
|
|
||
| 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 protected]");
+ Member requester = memberGenerator.generate("[email protected]");
+ Member owner = memberGenerator.generate("[email protected]");
+ 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 protected]"); | |
| assertThatThrownBy(() -> sharingService.issueChairmanToken(1L, member)) | |
| .isInstanceOf(DTClientErrorException.class) | |
| .hasMessage(ClientErrorCode.TABLE_NOT_FOUND.getMessage()); | |
| void 회원_소유의_테이블이_아니면_에러가_발생한다() { | |
| Member requester = memberGenerator.generate("[email protected]"); | |
| Member owner = memberGenerator.generate("[email protected]"); | |
| 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 단에서 처리하기로 합의하지 않았었나요?
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
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
2 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
릴리스 노트
신규 기능
개선 사항