Skip to content

refactor: 레포지토리 저장 로직 변경 #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
coli-geonwoo merged 2 commits into from
Aug 29, 2025
Merged

refactor: 레포지토리 저장 로직 변경 #83

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6bd0b3
refactor: 재포지토리 저장 로직 변경
coli-geonwoo Aug 27, 2025
84745bd
style: 오버라이드 애너테이션 추가
coli-geonwoo Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import com.devoops.service.github.WebHookService;
import com.devoops.service.repository.RepositoryService;
import java.util.List;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
Expand All @@ -36,24 +35,23 @@ public class RepositoryFacadeService {
public GithubRepository save(RepositorySaveRequest request, User user) {
try {
GithubRepoUrl repoUrl = new GithubRepoUrl(request.url());
GithubRepository savedRepository = saveRepository(repoUrl, user);
webHookService.registerWebhook(user, savedRepository.getId());
GithubRepoInfoResponse repositoryInfo = gitHubService.getRepositoryInfo(repoUrl, user.getGithubToken());
GithubRepository savedRepository = repositoryService.findByUserAndExternalId(user, repositoryInfo.id())
.map(alreadyRegisteredRepo -> reTrackingOrThrowException(user, alreadyRegisteredRepo))
.orElseGet(() -> registerNewRepo(repositoryInfo, repoUrl, user));

Comment on lines +38 to +42
Copy link

@coderabbitai coderabbitai bot Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

신규/기존 분기 로직은 명확합니다. 다만 동시성 레이스를 방지하는 보강이 필요합니다.

동일 유저가 같은 repo를 거의 동시에 저장하면 둘 다 findByUserAndExternalId에서 미존재로 판단 후 save로 진입할 수 있습니다. DB 유니크 제약이 있다면 DataIntegrityViolationException으로 끝나고, 없다면 중복 데이터가 생깁니다. registerNewRepo 내부에서 저장 시 예외를 캐치해 재조회/재연결로 폴백하는 방식을 권장합니다.

가능한 보강 예시(핵심 아이디어만 제시):

- GithubRepository savedRepository = repositoryService.findByUserAndExternalId(user, repositoryInfo.id())
+ GithubRepository savedRepository = repositoryService.findByUserAndExternalId(user, repositoryInfo.id())
     .map(alreadyRegisteredRepo -> reTrackingOrThrowException(user, alreadyRegisteredRepo))
     .orElseGet(() -> registerNewRepo(repositoryInfo, repoUrl, user));

registerNewRepo에서 save를 try/catch로 감싸 폴백 처리(아래 코멘트 참고).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java
around lines 38 to 42, the branch correctly chooses registerNewRepo for missing
repos but is vulnerable to a race where two concurrent requests both decide to
insert; update registerNewRepo to wrap the repositoryService.save(...) call in a
try/catch that catches DataIntegrityViolationException (and other relevant
persistence exceptions), and in the catch re-query
repositoryService.findByUserAndExternalId(user, repositoryInfo.id()) and return
that existing entity if found, otherwise rethrow or convert to a clear
application exception; ensure the save remains in a transactional context (or
uses REQUIRES_NEW if appropriate) so the fallback query sees committed state.

eventPublisher.publishEvent(new AnalyzeMyPrEvent(repoUrl, user, this));
return savedRepository;
} catch (GithubNotFoundException githubNotFoundException) {
throw new GssException(ErrorCode.REGISTRY_GITHUB_REPOSITORY_NOT_FOUND);
}
}

private GithubRepository saveRepository(GithubRepoUrl url, User user) {
GithubRepoInfoResponse repositoryInfo = gitHubService.getRepositoryInfo(url, user.getGithubToken());
long externalId = repositoryInfo.id();
Optional<GithubRepository> alreadyRegisteredRepo = repositoryService.findByUserAndExternalId(user, externalId);

if(alreadyRegisteredRepo.isPresent()) {
return reTrackingOrThrowException(user, alreadyRegisteredRepo.get());
}

private GithubRepository registerNewRepo(
GithubRepoInfoResponse repositoryInfo,
GithubRepoUrl url,
User user
) {
RepositoryCreateCommand createCommand = new RepositoryCreateCommand(
user.getId(),
repositoryInfo.name(),
Expand All @@ -62,7 +60,9 @@ private GithubRepository saveRepository(GithubRepoUrl url, User user) {
INITIAL_PULL_REQUEST_COUNT,
repositoryInfo.id()
);
return repositoryService.save(createCommand);
GithubRepository savedRepository = repositoryService.save(createCommand);
webHookService.registerWebhook(user, savedRepository.getId());
return savedRepository;
}
Comment on lines +63 to 66
Copy link

@coderabbitai coderabbitai bot Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

웹훅 등록은 트랜잭션 커밋 후로 지연시켜야 외부 부작용과 롤백 불일치를 막을 수 있습니다.

현재는 DB 저장 → 웹훅 호출 순이며, 이후 예외로 롤백되면 GitHub에는 훅이 남고 DB에는 레코드가 없는 불일치가 생길 수 있습니다. 커밋 이후에 호출하도록 전환해 주세요(예: TransactionSynchronizationManager 또는 @TransactionalEventListener 사용).

아주 국소 수정 예시:

-        GithubRepository savedRepository = repositoryService.save(createCommand);
-        webHookService.registerWebhook(user, savedRepository.getId());
-        return savedRepository;
+        GithubRepository savedRepository = repositoryService.save(createCommand);
+        // 커밋 이후 실행
+        org.springframework.transaction.support.TransactionSynchronizationManager
+                .registerSynchronization(new org.springframework.transaction.support.TransactionSynchronization() {
+                    @Override
+                    public void afterCommit() {
+                        webHookService.registerWebhook(user, savedRepository.getId());
+                    }
+                });
+        return savedRepository;

또는 도메인 이벤트 발행 → @TransactionalEventListener(phase = AFTER_COMMIT) 리스너에서 registerWebhook 수행도 좋습니다.

📝 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
GithubRepository savedRepository = repositoryService.save(createCommand);
webHookService.registerWebhook(user, savedRepository.getId());
return savedRepository;
}
GithubRepository savedRepository = repositoryService.save(createCommand);
// 커밋 이후 실행
org.springframework.transaction.support.TransactionSynchronizationManager
.registerSynchronization(new org.springframework.transaction.support.TransactionSynchronization() {
@Override
public void afterCommit() {
webHookService.registerWebhook(user, savedRepository.getId());
}
});
return savedRepository;
}
🤖 Prompt for AI Agents 
In
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java
around lines 63-66, the code calls webHookService.registerWebhook immediately
after saving the repository which can leave a webhook registered if the
surrounding transaction later rolls back; change this so webhook registration
runs after the transaction commits — either publish a domain event (e.g.
RepositoryCreatedEvent) and handle it in a @TransactionalEventListener(phase =
AFTER_COMMIT) that calls webHookService.registerWebhook(user,
savedRepository.getId()), or register a TransactionSynchronization (via
TransactionSynchronizationManager.registerSynchronization) that invokes
registerWebhook in afterCommit; ensure the save still returns the saved entity
and that exception handling/logging remains in the async/after-commit path.


private GithubRepository reTrackingOrThrowException(User user, GithubRepository registeredRepo) {
Expand Down
24 changes: 17 additions & 7 deletions gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.devoops.service.facade;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertAll;
import static org.mockito.ArgumentMatchers.any;
Expand Down Expand Up @@ -69,7 +70,7 @@ class Save {
}

@Test
void 레포지토리를_중복_저장할_수_없다() {
void 동일_유저가_레포지토리를_중복_저장할_수_없다() {
User user = userGenerator.generate("김건우");
RepositorySaveRequest request = new RepositorySaveRequest("https://github.com/octocat/Hello-World");
mockingGithubClient();
Expand All @@ -81,24 +82,33 @@ class Save {
.hasMessage(ErrorCode.ALREADY_SAVED_REPOSITORY.getMessage());
}

@Test
void 다른_유저가_레포지토리를_중복_저장할_수_있다() {
User user1 = userGenerator.generate("김건우1");
User user2 = userGenerator.generate("김건우2");
RepositorySaveRequest request = new RepositorySaveRequest("https://github.com/octocat/Hello-World");
mockingGithubClient();

assertThatCode(() -> {
repositoryFacadeService.save(request, user1);
repositoryFacadeService.save(request, user2);
}).doesNotThrowAnyException();
}

@Test
void 레포지토리를_재연결_할_수_있다() {
User user = userGenerator.generate("김건우");
GithubRepository unTrackingRepo = repoGenerator.generate(user, "연결 끊긴 레포지토리", 123L, false);
RepositorySaveRequest request = new RepositorySaveRequest("https://github.com/octocat/Hello-World");
mockingGithubClient();

GithubRepository reTrackingRepository = repositoryFacadeService.save(request, user);
repositoryFacadeService.save(request, user);

GithubRepository actual = githubRepoDomainRepository.findByIdAndUserId(
unTrackingRepo.getId(),
user.getId()
);

assertAll(
() -> Mockito.verify(gitHubClient, times(1)).createWebhook(any(), any(), any(), any()),
() -> assertThat(actual.isTracking()).isTrue()
);
assertThat(actual.isTracking()).isTrue();
}

private void mockingGithubClient() {
Expand Down
1 change: 1 addition & 0 deletions gss-domain/src/main/java/com/devoops/domain/entity/analysis/ClaudeAiModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public enum ClaudeAiModel implements AiModel {
private final double inputTokenCharge; //달러
private final double outputTokenCharge; //달러

@Override
public double getCharge(int promptToken, int completionTokens) {
double inputCharge = CurrencyUtil.usdToKrw(inputTokenCharge * promptToken);
double outputCharge = CurrencyUtil.usdToKrw(outputTokenCharge * completionTokens);
Expand Down
1 change: 1 addition & 0 deletions gss-domain/src/main/java/com/devoops/domain/entity/analysis/OpenAiModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ private boolean isBetween(double currentUsage, double min, double max) {
return currentUsage >= min && currentUsage <= max;
}

@Override
public double getCharge(int promptToken, int completionTokens) {
double inputCharge = CurrencyUtil.usdToKrw(inputTokenCharge * promptToken);
double outputCharge = CurrencyUtil.usdToKrw(outputTokenCharge * completionTokens);
Expand Down
Loading