Skip to content
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.

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

변경된 요구사항에 따른 톡픽 본문 요약 기능 구현 #829

Merged
merged 11 commits into from
Dec 30, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import balancetalk.member.dto.GuestOrApiMember;
import balancetalk.talkpick.domain.TalkPick;
import balancetalk.talkpick.domain.TalkPickFileHandler;
import balancetalk.talkpick.domain.TalkPickSummarizer;
import balancetalk.talkpick.domain.repository.TalkPickRepository;
import balancetalk.vote.domain.TalkPickVote;
import java.util.List;
Expand All @@ -30,15 +31,20 @@ public class TalkPickService {
private final MemberRepository memberRepository;
private final TalkPickRepository talkPickRepository;
private final TalkPickFileHandler talkPickFileHandler;
private final TalkPickSummarizer talkPickSummarizer;

@Transactional
public Long createTalkPick(CreateTalkPickRequest request, ApiMember apiMember) {
Member member = apiMember.toMember(memberRepository);
TalkPick savedTalkPick = talkPickRepository.save(request.toEntity(member));
Long savedTalkPickId = savedTalkPick.getId();

if (request.containsFileIds()) {
talkPickFileHandler.handleFilesOnTalkPickCreate(request.getFileIds(), savedTalkPickId);
}

talkPickSummarizer.summary(savedTalkPick);

return savedTalkPickId;
}

Expand Down Expand Up @@ -81,6 +87,7 @@ public void updateTalkPick(Long talkPickId, UpdateTalkPickRequest request, ApiMe
talkPick.update(request.toEntity(member));
talkPickFileHandler
.handleFilesOnTalkPickUpdate(request.getNewFileIds(), request.getDeleteFileIds(), talkPickId);
talkPickSummarizer.summary(talkPick);
}

@Transactional
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package balancetalk.talkpick.domain;

public enum SummaryStatus {
PENDING, SUCCESS, FAIL, NOT_REQUIRED
}
14 changes: 14 additions & 0 deletions src/main/java/balancetalk/talkpick/domain/TalkPick.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.PositiveOrZero;
import jakarta.validation.constraints.Size;
import java.time.LocalDate;
Expand All @@ -40,6 +41,8 @@
@NoArgsConstructor(access = AccessLevel.PROTECTED)
public class TalkPick extends BaseTimeEntity {

private static final int MIN_CONTENT_LENGTH_FOR_SUMMARY = 200;

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
Expand All @@ -55,6 +58,9 @@ public class TalkPick extends BaseTimeEntity {
@Embedded
private Summary summary;

@NotNull
private SummaryStatus summaryStatus;
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

summaryStatus 필드의 초기화가 누락되었습니다

TalkPick 클래스의 코드를 검토한 결과:

  • @Builder 패턴을 사용하고 있으나 summaryStatus 필드의 기본값이 설정되어 있지 않습니다
  • hasShortContent() 메서드와 updateSummaryStatus() 메서드가 있어 요약 상태를 관리하고 있으나, 객체 생성 시점의 초기화는 보장되지 않습니다

다음 사항들을 수정해주세요:

  • @Builder.Default를 사용하여 summaryStatus 필드에 기본값 PENDING 또는 적절한 초기 상태를 지정하거나
  • 생성자나 빌더에서 hasShortContent()를 확인하여 적절한 초기 상태값을 설정하도록 구현해주세요
🔗 Analysis chain

summaryStatus 필드@NotNull 적용
summaryStatus는 반드시 null이 아니어야 하므로, TalkPick 생성 시점에 적절한 기본값으로 초기화하는 로직을 점검해 주세요.


Let me gather information about the initialization of summaryStatus field in TalkPick class.


Let me try a different approach to search for the relevant code.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Search for TalkPick class implementation to check constructor and initialization
ast-grep --pattern 'class TalkPick {
  $$$
}'

# Also search for SummaryStatus enum definition
ast-grep --pattern 'enum SummaryStatus {
  $$$
}'

Length of output: 99


Script:

#!/bin/bash
# Search for TalkPick.java file content
fd -e java "TalkPick.java" --exec cat {}

# Search for SummaryStatus.java file content
fd -e java "SummaryStatus.java" --exec cat {}

Length of output: 7344


@NotBlank
@Size(max = 2000)
private String content;
Expand Down Expand Up @@ -146,6 +152,14 @@ public NotificationHistory getNotificationHistory() {
return this.notificationHistory;
}

public boolean hasShortContent() {
return content.length() < MIN_CONTENT_LENGTH_FOR_SUMMARY;
}
Comment on lines +162 to +164
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

hasShortContent 메서드의 간소화
content가 null인 경우의 예외 처리가 필요할 수 있습니다. 외부나 다른 서비스에서 content를 누락하여 저장할 가능성이 있다면 방어 로직을 추가해주세요.


public void updateSummaryStatus(SummaryStatus summaryStatus) {
this.summaryStatus = summaryStatus;
}

public TodayTalkPick toTodayTalkPick() {
return TodayTalkPick.builder()
.pickDate(LocalDate.now())
Expand Down
104 changes: 104 additions & 0 deletions src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package balancetalk.talkpick.domain;

import static balancetalk.global.exception.ErrorCode.SUMMARY_SIZE_IS_OVER;
import static balancetalk.talkpick.domain.SummaryStatus.FAIL;
import static balancetalk.talkpick.domain.SummaryStatus.NOT_REQUIRED;
import static balancetalk.talkpick.domain.SummaryStatus.SUCCESS;

import balancetalk.global.exception.BalanceTalkException;
import balancetalk.talkpick.dto.fields.BaseTalkPickFields;
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.ai.chat.client.ChatClient;
import org.springframework.ai.chat.client.advisor.SimpleLoggerAdvisor;
import org.springframework.retry.annotation.Backoff;
import org.springframework.retry.annotation.Recover;
import org.springframework.retry.annotation.Retryable;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import org.springframework.transaction.annotation.Transactional;

@Slf4j
@Component
@RequiredArgsConstructor
public class TalkPickSummarizer {

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

private final ChatClient chatClient;

@Async
@Retryable(backoff = @Backoff(delay = 1000), maxAttempts = 3)
@Transactional
public void summary(TalkPick talkPick) {
Comment on lines +56 to +58
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

@async@transactional 어노테이션의 동시 사용이 확인되었습니다.

현재 코드에서 다음과 같은 잠재적인 문제가 있을 수 있습니다:

  • TalkPickSummarizer.summary() 메서드에 @async@transactional이 함께 사용되고 있어, 비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행됩니다.
  • 부모 트랜잭션과의 전파나 롤백이 예상대로 동작하지 않을 수 있습니다.

권장 사항:

  • 비동기 메서드와 트랜잭션 메서드를 분리하여 구현하는 것을 고려해보세요.
  • 트랜잭션 범위를 비동기 메서드 내부의 별도 메서드로 이동하는 것이 안전합니다.
🔗 Analysis chain

비동기와 트랜잭션 동시 사용에 유의하세요.
요약 메서드에 @async@transactional이 함께 사용될 경우, 예상치 못한 트랜잭션 범위 문제가 발생할 수 있습니다. 트랜잭션 롤백 범위나 프로그래시(proxy) 동작을 다시 한 번 점검해 보시는 것을 권장합니다.


Let me gather information about the usage of @async annotation in this class and related configurations.


Let me check the actual implementation and configuration of async and transaction handling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Check if @Async is used in TalkPickSummarizer
ast-grep --pattern 'class TalkPickSummarizer {
  $$$
  @Async
  $$$
}'

# Search for @Async configuration or AsyncConfig files
fd -e java -e xml -e properties -e yml | xargs rg -l "(@EnableAsync|@Async|AsyncConfig)"

# Check for any async-related configuration in application properties
fd "application.*properties" -e yml -e yaml | xargs cat

# Look for transaction configuration
fd -e java | xargs rg "@EnableTransactionManagement|@TransactionConfiguration"

Length of output: 492


Script:

#!/bin/bash
# Check AsyncConfig implementation
cat src/main/java/balancetalk/global/config/AsyncConfig.java

# Check TalkPickSummarizer implementation for @Async usage
cat src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java

# Check TalkPickFileHandler implementation
cat src/main/java/balancetalk/talkpick/domain/TalkPickFileHandler.java

Length of output: 7755

Copy link
Member Author

Choose a reason for hiding this comment

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

비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행되는 것을 인지하고 작성했습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

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!

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai 비동기 실행 시 새로운 스레드에서 트랜잭션이 독립적으로 실행되는 것을 인지하고 작성했습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Hanjaemo 네, 잘 알겠습니다. 말씀하신 대로 @async 메서드가 별도의 스레드에서 트랜잭션을 독립적으로 실행한다는 점을 인지하고 계시다면, 문제없이 진행하셔도 됩니다. 혹시 추후에 트랜잭션 경계에 대해 고민이 생기거나 다른 방식으로 구현할 필요가 있다면 언제든지 알려주세요. 추가로 필요한 사항 있으시면 말씀 부탁드립니다.

// 본문 글자수가 너무 짧으면 요약 제공 안함
if (talkPick.hasShortContent()) {
talkPick.updateSummaryStatus(NOT_REQUIRED);
return;
}

// 요약 수행
Summary summary = getSummary(talkPick);

// 요약 글자수 검증
if (summary.isOverSize()) {
throw new BalanceTalkException(SUMMARY_SIZE_IS_OVER);
}

// 톡픽 요약 내용 및 상태 업데이트
talkPick.updateSummary(summary);
talkPick.updateSummaryStatus(SUCCESS);
}

private Summary getSummary(TalkPick talkPick) {
return chatClient.prompt()
.advisors(new SimpleLoggerAdvisor())
.system("""
당신은 저에게 고용된 노동자입니다.
당신의 역할은 게시글을 작성한 사용자가 되어, 사용자의 어투 및 문체와 동일하게 3줄로 요약하는 것입니다.

다음은 이 작업에 대한 접근 방식입니다:
Step 1: 제목, 선택지 A, 선택지 B, 그리고 본문 내용을 바탕으로 맥락을 파악합니다.
Step 2: 파악한 맥락을 토대로 본문 중 불필요한 내용 혹은 표현을 제거합니다.
Step 3: 사용자가 두 가지 선택지 중 선택하길 원한다는 사실을 전제로 합니다.
Step 4: 마지막 3번째 문장에는 주요 비교군을 명확히 제시하는 질문을 배치합니다.
Step 5: 요약한 3개의 문장을 JSON 형태로 된 firstLine, secondLine, thirdLine 키에 각각 한 문장씩 담습니다.

지금부터 주어진 단계별로 요약을 진행하세요.
작업을 아주 잘 수행할 경우, 10만 달러의 인센티브를 제공할게요. 당신은 최고의 전문가니까 잘 해낼 수 있을 것이라 믿어요.
""")
.user(u -> u.text("""
제목: {title}
선택지 A: {optionA}
선택지 B: {optionB}
본문: {content}
""").params(getBaseTalkPickFieldsMap(talkPick)))
.call()
.entity(Summary.class);
}

private Map<String, Object> getBaseTalkPickFieldsMap(TalkPick talkPick) {
return OBJECT_MAPPER.convertValue(
getBaseTalkPickFields(talkPick),
new TypeReference<ConcurrentHashMap<String, Object>>() {});
}

private BaseTalkPickFields getBaseTalkPickFields(TalkPick talkPick) {
return BaseTalkPickFields.from(
talkPick.getTitle(),
talkPick.getContent(),
talkPick.getOptionA(),
talkPick.getOptionB());
}

@Recover
public void recoverSummary(Throwable ex, TalkPick talkPick) {
log.error("Fail to summary ID={} TalkPick", talkPick.getId());
log.error("exception message = {} {}", ex.getMessage(), ex.getStackTrace());
talkPick.updateSummaryStatus(FAIL);
}
}
6 changes: 6 additions & 0 deletions src/main/java/balancetalk/talkpick/dto/TalkPickDto.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import balancetalk.bookmark.domain.TalkPickBookmark;
import balancetalk.comment.domain.Comment;
import balancetalk.member.domain.Member;
import balancetalk.talkpick.domain.SummaryStatus;
import balancetalk.talkpick.domain.TalkPick;
import balancetalk.talkpick.dto.fields.BaseTalkPickFields;
import balancetalk.talkpick.dto.fields.ValidatedNotBlankTalkPickFields;
Expand Down Expand Up @@ -47,6 +48,7 @@ public TalkPick toEntity(Member member) {
.optionA(baseFields.getOptionA())
.optionB(baseFields.getOptionB())
.sourceUrl(baseFields.getSourceUrl())
.summaryStatus(SummaryStatus.PENDING)
.views(0L)
.bookmarks(0L)
.viewStatus(NORMAL)
Expand Down Expand Up @@ -82,6 +84,7 @@ public TalkPick toEntity(Member member) {
.optionA(baseFields.getOptionA())
.optionB(baseFields.getOptionB())
.sourceUrl(baseFields.getSourceUrl())
.summaryStatus(SummaryStatus.PENDING)
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the code analysis, I can now provide a final response:

요약 상태 초기화 문제 확인됨

톡픽 수정 시 update() 메서드에서 summaryStatus를 변경하지 않고 있으나, TalkPickDto에서 항상 PENDING 상태로 설정하고 있습니다. 이는 다음과 같은 문제를 야기할 수 있습니다:

  • 이미 요약이 완료(SUCCESS)되거나 실패(FAIL)한 톡픽의 내용을 수정할 때 불필요하게 다시 요약 프로세스가 시작될 수 있음
  • 요약이 불필요(NOT_REQUIRED)하다고 판단된 톡픽도 다시 PENDING 상태가 되어 혼란을 야기할 수 있음

수정 제안:

  • TalkPickDto에서 기존 톡픽의 summaryStatus를 유지하도록 수정
  • 내용이 변경된 경우에만 선택적으로 summaryStatusPENDING으로 변경하는 로직 추가
🔗 Analysis chain

수정 시에도 summaryStatus를 PENDING으로
UpdateTalkPickRequest에서도 요약 상태가 계속 PENDING으로 되돌아갈 수 있으니, 이미 실패하거나 요약 완료된 톡픽에 대해 다시 PENDING으로 세팅되는 것이 의도된 것인지 사용 시나리오를 재검토해보세요.


Let me gather more information about the usage of SummaryStatus and the update logic.


Let me check the SummaryStatus enum and any logic related to status transitions.


Let me check the update logic in TalkPick entity and how it handles the summary status during updates.

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Find files containing SummaryStatus
rg "SummaryStatus" -l

# Search for UpdateTalkPickRequest class definition and usage
ast-grep --pattern 'class UpdateTalkPickRequest {
  $$$
}'

# Search for methods that handle talk pick updates
rg "update.*TalkPick" -A 5

# Look for any status transition logic
rg "summaryStatus" -A 3

Length of output: 8089


Script:

#!/bin/bash
# Check SummaryStatus enum definition
cat src/main/java/balancetalk/talkpick/domain/SummaryStatus.java

# Look for TalkPickSummarizer implementation
cat src/main/java/balancetalk/talkpick/domain/TalkPickSummarizer.java

# Search for any update logic that changes summary status
rg "updateSummaryStatus" -B 5 -A 5

Length of output: 8219


Script:

#!/bin/bash
# Check TalkPick entity update method
rg "update.*\(" src/main/java/balancetalk/talkpick/domain/TalkPick.java -A 10

# Check UpdateTalkPickRequest implementation
rg -g "UpdateTalkPickRequest.java" "class|toEntity"

Length of output: 1335

Copy link
Member Author

Choose a reason for hiding this comment

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

톡픽이 생성된 직후에는 PENDING으로 설정되고, 요약이 수행되는 시점에 NOT_REQUIRED, SUCCESS, 혹은 FAIL로 설정되는 방식이에요.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Hanjaemo 감사합니다. 말씀해주신 것처럼 톡픽이 생성될 때 PENDING 상태로 시작하고, 이후 실제 요약 작업에서 NOT_REQUIRED, SUCCESS, 혹은 FAIL 상태로 변경되는 구조로 이해했습니다. 현재 구조라면, 톡픽 수정 시에도 새롭게 요약 과정을 진행하려는 의도라면 문제없이 동작하겠네요. 추가로 공유해주실 내용이 있으면 말씀 부탁드립니다.

.editedAt(LocalDateTime.now())
.build();
}
Expand Down Expand Up @@ -112,6 +115,8 @@ public static class TalkPickDetailResponse {

private SummaryResponse summary;

private SummaryStatus summaryStatus;

@Schema(description = "톡픽 작성 시 첨부한 이미지 URL 목록",
example = "["
+ "\"https://picko-image.amazonaws.com/temp-talk-pick/ad80-a94e083301d2_czz.png\",\n"
Expand Down Expand Up @@ -164,6 +169,7 @@ public static TalkPickDetailResponse from(TalkPick entity,
.sourceUrl(entity.getSourceUrl())
.build())
.summary(new SummaryResponse(entity.getSummary()))
.summaryStatus(entity.getSummaryStatus())
.imgUrls(imgUrls)
.fileIds(fileIds)
.votesCountOfOptionA(entity.votesCountOf(A))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package balancetalk.talkpick.dto.fields;

import com.fasterxml.jackson.annotation.JsonInclude;
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.Size;
import lombok.AllArgsConstructor;
Expand Down Expand Up @@ -28,6 +29,7 @@ public class BaseTalkPickFields {
private String optionB;

@Schema(description = "출처 URL", example = "https://github.com/CHZZK-Study/Balance-Talk-Backend/issues/506")
@JsonInclude(JsonInclude.Include.NON_NULL)
private String sourceUrl;

public static BaseTalkPickFields from(String title, String content,
Expand All @@ -41,4 +43,14 @@ public static BaseTalkPickFields from(String title, String content,
.sourceUrl(sourceUrl)
.build();
}

public static BaseTalkPickFields from(String title, String content,
String optionA, String optionB) {
return BaseTalkPickFields.builder()
.title(title)
.content(content)
.optionA(optionA)
.optionB(optionB)
.build();
}
}

This file was deleted.

2 changes: 1 addition & 1 deletion src/main/resources/config
Loading