-
Notifications
You must be signed in to change notification settings - Fork 2
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
밸런스 게임 임시저장 시 파일 중복 오류 해결 #775
Changes from 9 commits
897a6c1
8bf8d56
b1e5cb7
04df535
e5b9d84
7d7b75a
b4503b2
227244c
0dd7855
48c9a23
f78df04
89d09d4
3617968
a0ac4d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,6 +3,7 @@ | |||||||||||||||||||||||||
import static balancetalk.file.domain.FileType.TEMP_GAME_OPTION; | ||||||||||||||||||||||||||
import static balancetalk.game.dto.TempGameDto.CreateTempGameRequest; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
import balancetalk.file.domain.File; | ||||||||||||||||||||||||||
import balancetalk.file.domain.FileHandler; | ||||||||||||||||||||||||||
import balancetalk.file.domain.repository.FileRepository; | ||||||||||||||||||||||||||
import balancetalk.game.domain.MainTag; | ||||||||||||||||||||||||||
|
@@ -21,7 +22,10 @@ | |||||||||||||||||||||||||
import balancetalk.member.dto.ApiMember; | ||||||||||||||||||||||||||
import java.util.ArrayList; | ||||||||||||||||||||||||||
import java.util.Collections; | ||||||||||||||||||||||||||
import java.util.LinkedHashMap; | ||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||
import java.util.Map; | ||||||||||||||||||||||||||
import java.util.stream.Collectors; | ||||||||||||||||||||||||||
import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||
import org.springframework.stereotype.Service; | ||||||||||||||||||||||||||
import org.springframework.transaction.annotation.Transactional; | ||||||||||||||||||||||||||
|
@@ -44,14 +48,23 @@ public void createTempGame(CreateTempGameSetRequest request, ApiMember apiMember | |||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<CreateTempGameRequest> tempGames = request.getTempGames(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<TempGame> newTempGames = tempGames.stream() | ||||||||||||||||||||||||||
.map(game -> game.toEntity(fileRepository)) | ||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||
List<TempGame> newTempGames = request.getTempGames().stream() | ||||||||||||||||||||||||||
.map(game -> game.toEntity(fileRepository)) | ||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if (member.hasTempGameSet()) { // 기존 임시저장이 존재하는 경우 | ||||||||||||||||||||||||||
TempGameSet existGame = member.getTempGameSet(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if (request.isNewRequest()) { // 새롭게 임시저장 하는 경우, 파일 모두 삭제 | ||||||||||||||||||||||||||
List<Long> oldFileIds = existGame.getAllFileIds(); | ||||||||||||||||||||||||||
if (!oldFileIds.isEmpty()) { | ||||||||||||||||||||||||||
fileRepository.deleteByResourceIdInAndFileType(oldFileIds, TEMP_GAME_OPTION); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
// 새롭게 불러온 경우, 파일만 재배치 (isLoaded: true) | ||||||||||||||||||||||||||
relocateFiles(request, existGame); | ||||||||||||||||||||||||||
existGame.updateTempGameSet(request.getTitle(), request.getSubTag(), mainTag, newTempGames); | ||||||||||||||||||||||||||
processTempGameFiles(tempGames, existGame.getTempGames()); | ||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
|
@@ -67,6 +80,100 @@ public void createTempGame(CreateTempGameSetRequest request, ApiMember apiMember | |||||||||||||||||||||||||
processTempGameFiles(tempGames, tempGameSet.getTempGames()); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private void relocateFiles(CreateTempGameSetRequest request, TempGameSet tempGameSet) { | ||||||||||||||||||||||||||
if (request.getAllFileIds().isEmpty()) { | ||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<Long> deletedFileIds = deleteFiles(request, tempGameSet); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<Long> newFileIds = getNewNonDuplicateFileIds(request, deletedFileIds, tempGameSet); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
relocateFilesToGameOption(request, newFileIds, tempGameSet); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private void relocateFilesToGameOption( | ||||||||||||||||||||||||||
CreateTempGameSetRequest request, | ||||||||||||||||||||||||||
List<Long> newFileIds, | ||||||||||||||||||||||||||
TempGameSet tempGameSet | ||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sonarqube 라인 120이 넘어서 저렇게 설정했는데
형태가 되어야 할까요?? |
||||||||||||||||||||||||||
if (!newFileIds.isEmpty()) { | ||||||||||||||||||||||||||
List<TempGame> tempGames = tempGameSet.getTempGames(); | ||||||||||||||||||||||||||
Map<Long, Long> fileToOptionMap = getFileToOptionMap(tempGames, request, newFileIds); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<File> files = fileRepository.findAllById(newFileIds); | ||||||||||||||||||||||||||
Map<Long, File> fileMap = files.stream() | ||||||||||||||||||||||||||
.collect(Collectors.toMap(File::getId, file -> file)); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
Comment on lines
+99
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Map 생성 시 NPE 방지 필요
다음과 같이 수정을 제안합니다: Map<Long, File> fileMap = files.stream()
- .collect(Collectors.toMap(File::getId, file -> file));
+ .filter(file -> file != null)
+ .collect(Collectors.toMap(
+ File::getId,
+ file -> file,
+ (existing, replacement) -> existing,
+ LinkedHashMap::new
+ )); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||
for (Map.Entry<Long, Long> entry : fileToOptionMap.entrySet()) { | ||||||||||||||||||||||||||
Long fileId = entry.getKey(); | ||||||||||||||||||||||||||
Long tempGameOptionId = entry.getValue(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
File file = fileMap.get(fileId); | ||||||||||||||||||||||||||
if (file == null) { | ||||||||||||||||||||||||||
throw new BalanceTalkException(ErrorCode.NOT_FOUND_FILE); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
fileHandler.relocateFile(file, tempGameOptionId, TEMP_GAME_OPTION); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private Map<Long, Long> getFileToOptionMap( | ||||||||||||||||||||||||||
List<TempGame> tempGames, | ||||||||||||||||||||||||||
CreateTempGameSetRequest request, | ||||||||||||||||||||||||||
List<Long> newFileIds | ||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||
Map<Long, Long> fileToOptionMap = new LinkedHashMap<>(); | ||||||||||||||||||||||||||
List<CreateTempGameRequest> tempGameRequests = request.getTempGames(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
for (int i = 0; i < tempGameRequests.size(); i++) { | ||||||||||||||||||||||||||
CreateTempGameRequest gameRequest = tempGameRequests.get(i); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<TempGameOptionDto> tempGameOptions = gameRequest.getTempGameOptions(); | ||||||||||||||||||||||||||
for (TempGameOptionDto optionDto : tempGameOptions) { | ||||||||||||||||||||||||||
Long fileId = optionDto.getFileId(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if (fileId != null && newFileIds.contains(fileId)) { | ||||||||||||||||||||||||||
TempGameOption tempGameOption = tempGames.get(i).getTempGameOptions() | ||||||||||||||||||||||||||
.stream() | ||||||||||||||||||||||||||
.filter(option -> option.getOptionType().equals(optionDto.getOptionType())) | ||||||||||||||||||||||||||
.findFirst() | ||||||||||||||||||||||||||
.orElseThrow(() -> new BalanceTalkException(ErrorCode.FILE_ID_GAME_OPTION_ID_MISMATCH)); | ||||||||||||||||||||||||||
fileToOptionMap.put(fileId, tempGameOption.getId()); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
return fileToOptionMap; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private List<Long> getNewNonDuplicateFileIds( | ||||||||||||||||||||||||||
CreateTempGameSetRequest request, | ||||||||||||||||||||||||||
List<Long> deletedFileIds, | ||||||||||||||||||||||||||
TempGameSet tempGameSet | ||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||
List<Long> existingFileIds = tempGameSet.getAllFileIds(); | ||||||||||||||||||||||||||
return request.getAllFileIds().stream() | ||||||||||||||||||||||||||
.filter(fileId -> !deletedFileIds.contains(fileId)) | ||||||||||||||||||||||||||
.filter(fileId -> !existingFileIds.contains(fileId)) | ||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private List<Long> deleteFiles(CreateTempGameSetRequest request, TempGameSet tempGameSet) { | ||||||||||||||||||||||||||
List<Long> existingFileIds = tempGameSet.getAllFileIds(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<Long> newFileIds = request.getAllFileIds(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
List<Long> filesIdsToDelete = existingFileIds.stream() | ||||||||||||||||||||||||||
.filter(existingFileId -> !newFileIds.contains(existingFileId)) | ||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
if (!filesIdsToDelete.isEmpty()) { | ||||||||||||||||||||||||||
List<File> filesToDelete = fileRepository.findAllById(filesIdsToDelete); | ||||||||||||||||||||||||||
fileHandler.deleteFiles(filesToDelete); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
Comment on lines
+165
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion 파일 삭제 실패 시 예외 처리 필요 파일 삭제 작업이 실패할 경우에 대한 예외 처리가 없습니다. 다음과 같이 수정을 제안합니다: if (!filesIdsToDelete.isEmpty()) {
List<File> filesToDelete = fileRepository.findAllById(filesIdsToDelete);
- fileHandler.deleteFiles(filesToDelete);
+ try {
+ fileHandler.deleteFiles(filesToDelete);
+ } catch (Exception e) {
+ throw new BalanceTalkException(ErrorCode.FILE_DELETE_ERROR,
+ String.format("파일 삭제 중 오류가 발생했습니다. 파일 ID: %s", filesIdsToDelete));
+ }
}
|
||||||||||||||||||||||||||
return filesIdsToDelete; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
private void processTempGameFiles(List<CreateTempGameRequest> requests, List<TempGame> tempGames) { | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
for (int i = 0; i < requests.size(); i++) { | ||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ public class TempGameOption { | |
@Enumerated(value = EnumType.STRING) | ||
private VoteOption optionType; | ||
|
||
private Long imgId; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GameOption과 일관성을 위해 imgUrl도 추가 부탁드립니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 처음에 Game 엔티티에서 imgUrl이 불필요한 값이다보니 TempGame에도 불필요하다고 생각해서 제외했습니다. 근데 다시 생각해보니 임시게임 불러오기의 응답값을 밸런스 게임 생성할 때 쓰기 때문에 추가하는게 맞는것 같네요 |
||
@ManyToOne(fetch = FetchType.LAZY) | ||
@JoinColumn(name = "temp_game_id") | ||
private TempGame tempGame; | ||
|
@@ -52,5 +54,6 @@ public void assignTempGame(TempGame tempGame) { | |
public void updateTempGameOption(TempGameOption newTempGameOption) { | ||
this.name = newTempGameOption.getName(); | ||
this.description = newTempGameOption.getDescription(); | ||
this.imgId = newTempGameOption.getImgId(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,5 @@ | ||
package balancetalk.game.dto; | ||
|
||
import balancetalk.file.domain.File; | ||
import balancetalk.file.domain.repository.FileRepository; | ||
import balancetalk.game.domain.TempGameOption; | ||
import balancetalk.global.exception.BalanceTalkException; | ||
|
@@ -37,6 +36,7 @@ public TempGameOption toEntity(FileRepository fileRepository) { | |
|
||
return TempGameOption.builder() | ||
.name(name) | ||
.imgId(fileId) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 응답할 때도 TempGameOptionDto를 사용하는 것 같은데, imgUrl도 추가해야 하는거 아닌가요? |
||
.description(description) | ||
.optionType(optionType) | ||
.build(); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,8 +9,10 @@ | |||||||||||||||||||||||||||||||||||
import balancetalk.global.exception.ErrorCode; | ||||||||||||||||||||||||||||||||||||
import balancetalk.member.domain.Member; | ||||||||||||||||||||||||||||||||||||
import io.swagger.v3.oas.annotations.media.Schema; | ||||||||||||||||||||||||||||||||||||
import jakarta.validation.constraints.NotNull; | ||||||||||||||||||||||||||||||||||||
import jakarta.validation.constraints.Size; | ||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||
import java.util.Objects; | ||||||||||||||||||||||||||||||||||||
import lombok.AllArgsConstructor; | ||||||||||||||||||||||||||||||||||||
import lombok.Builder; | ||||||||||||||||||||||||||||||||||||
import lombok.Data; | ||||||||||||||||||||||||||||||||||||
|
@@ -33,6 +35,10 @@ public static class CreateTempGameSetRequest { | |||||||||||||||||||||||||||||||||||
@Size(max = 10, message = "서브 태그는 최대 10자까지 입력 가능합니다") | ||||||||||||||||||||||||||||||||||||
private String subTag; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
@Schema(description = "최근 임시저장된 밸런스 게임 불러오기 여부", example = "true") | ||||||||||||||||||||||||||||||||||||
@NotNull(message = "isLoaded 필드는 NULL을 허용하지 않습니다.") | ||||||||||||||||||||||||||||||||||||
private Boolean isLoaded; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
private List<CreateTempGameRequest> tempGames; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
public TempGameSet toEntity(MainTag mainTag, Member member) { | ||||||||||||||||||||||||||||||||||||
|
@@ -46,6 +52,18 @@ public TempGameSet toEntity(MainTag mainTag, Member member) { | |||||||||||||||||||||||||||||||||||
.member(member) | ||||||||||||||||||||||||||||||||||||
.build(); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
public List<Long> getAllFileIds() { | ||||||||||||||||||||||||||||||||||||
return tempGames.stream() | ||||||||||||||||||||||||||||||||||||
.flatMap(game -> game.getTempGameOptions().stream()) | ||||||||||||||||||||||||||||||||||||
.map(TempGameOptionDto::getFileId) | ||||||||||||||||||||||||||||||||||||
.filter(Objects::nonNull) | ||||||||||||||||||||||||||||||||||||
.toList(); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
Comment on lines
+46
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tempGames가 null일 경우에 대한 방어 로직이 필요합니다. 현재 구현은 tempGames가 null일 경우 NullPointerException이 발생할 수 있습니다. 다음과 같이 수정하는 것을 제안드립니다: public List<Long> getAllFileIds() {
+ if (tempGames == null) {
+ return List.of();
+ }
return tempGames.stream()
.flatMap(game -> game.getTempGameOptions().stream())
.map(TempGameOptionDto::getFileId)
.filter(Objects::nonNull)
.toList();
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
public boolean isNewRequest() { | ||||||||||||||||||||||||||||||||||||
return !isLoaded; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
@Data | ||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
왜 수정했는지 알 수 있을까요??
위에
request.getTempGamse()
를 이미 호출했는데 또 호출하는 방식으로 변경돼서 궁금합니다