Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import com.mr.domain.playing.dto.res.AnalysisContextResponse;
import com.mr.domain.playing.dto.res.PlayingStartResponse;
import com.mr.domain.playing.dto.res.RecordingUploadUrlResponse;
import com.mr.domain.playing.service.PlayingFileService;
import com.mr.domain.playing.service.PlayingQueryService;
import com.mr.domain.playing.service.PlayingService;
import com.mr.global.apipayload.ApiResponse;
import com.mr.global.security.principal.CustomUserDetails;
Expand All @@ -30,6 +32,8 @@
public class PlayingController {

private final PlayingService playingService;
private final PlayingFileService playingFileService;
private final PlayingQueryService playingQueryService;

@Operation(
summary = "연주 세션 시작 API",
Expand Down Expand Up @@ -61,7 +65,7 @@ public ApiResponse<RecordingUploadUrlResponse> createRecordingUploadUrl(
Long userId = userDetails.getUserId();

RecordingUploadUrlResponse response =
playingService.createRecordingUploadUrl(userId, playingId, request);
playingFileService.createRecordingUploadUrl(userId, playingId, request);
return ApiResponse.onSuccess(response);
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -99,7 +103,7 @@ public ApiResponse<AnalysisContextResponse> getAnalysisContext(
@Parameter(description = "연주 ID", example = "128")
@PathVariable Long playingId
) {
AnalysisContextResponse response = playingService.getAnalysisContext(
AnalysisContextResponse response = playingQueryService.getAnalysisContext(
userDetails.getUserId(),
playingId
);
Expand Down

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PlayingFileService의 책임 범위가 이름에서 조금 넓게 느껴져 확인 차 코멘트 드립니다!

현재 PlayingFileService는 createRecordingUploadUrl()을 통한 녹음 파일 Presigned Upload URL 발급만 담당하고 있고, 조회 시 필요한 Presigned Download URL 생성은 PlayingQueryService, 업로드된 녹음 파일 검증은 PlayingService에서 처리하고 있는 것으로 확인했습니다~

현재처럼 유스케이스별로 책임을 나누는 구조 자체는 괜찮아 보이는데, PlayingFileService라는 이름만 보면 Playing 도메인의 파일 관련 처리를 전반적으로 담당하는 서비스처럼 읽힐 수도 있을 것 같아요.

녹음 업로드 URL 발급만 담당하도록 의도한 서비스라면 PlayingRecordingFileService 또는 역할이 조금 더 드러나는 이름을 고려해봐도 좋을 것 같습니다! (꼭 고칠 필요 X... p3 정도로 봐주면 됨!)

Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.mr.domain.playing.service;

import com.mr.domain.playing.dto.req.RecordingUploadUrlRequest;
import com.mr.domain.playing.dto.res.RecordingUploadUrlResponse;
import com.mr.domain.playing.entity.Playing;
import com.mr.domain.playing.exception.PlayingErrorStatus;
import com.mr.domain.playing.repository.PlayingRepository;
import com.mr.global.apipayload.exception.GeneralException;
import com.mr.global.file.s3.enums.S3FileType;
import com.mr.global.file.s3.service.S3FileService;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class PlayingFileService {

private final PlayingRepository playingRepository;
private final S3FileService s3FileService;

@Transactional(readOnly = true)
public RecordingUploadUrlResponse createRecordingUploadUrl(
Long userId, Long playingId, RecordingUploadUrlRequest request
) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdAndDeletedAtIsNull(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateInProgress();

return RecordingUploadUrlResponse.from(s3FileService.createPresignedUpload(
userId, S3FileType.RECORDING, request.toCommand())
);
}

private void validatePlayingId(Long playingId) {
if (playingId == null || playingId < 1) {
throw new GeneralException(PlayingErrorStatus.INVALID_PLAYING_ID);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package com.mr.domain.playing.service;

import com.mr.domain.analysis.service.AnalysisBarCalculator;
import com.mr.domain.backingtrack.entity.BackingTrack;
import com.mr.domain.playing.dto.res.AnalysisContextResponse;
import com.mr.domain.playing.dto.res.PlayingDetailResponse;
import com.mr.domain.playing.entity.Playing;
import com.mr.domain.playing.exception.PlayingErrorStatus;
import com.mr.domain.playing.repository.PlayingRepository;
import com.mr.global.apipayload.exception.GeneralException;
import com.mr.global.file.s3.enums.S3FileType;
import com.mr.global.file.s3.service.S3FileService;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class PlayingQueryService {

private final PlayingRepository playingRepository;
private final S3FileService s3FileService;
private final AnalysisBarCalculator analysisBarCalculator;

@Transactional(readOnly = true)
public PlayingDetailResponse getPlayingDetail(Long userId, Long playingId) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdWithBackingTrack(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateCompleted();

String recordingFileUrl =
s3FileService.createPresignedDownload(
userId,
S3FileType.RECORDING,
playing.getRecordingObjectKey()
);

return PlayingDetailResponse.from(playing, recordingFileUrl);
}

@Transactional(readOnly = true)
public AnalysisContextResponse getAnalysisContext(Long userId, Long playingId) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdWithBackingTrack(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateCompleted();
if (playing.getBackingTrack() == null) {
throw new GeneralException(PlayingErrorStatus.BACKING_TRACK_NOT_FOUND);
}

String recordingFileUrl =
s3FileService.createPresignedDownload(
userId,
S3FileType.RECORDING,
playing.getRecordingObjectKey()
);

BackingTrack backingTrack = playing.getBackingTrack();

String backingTrackAudioFileUrl = null;

if (backingTrack.getAudioObjectKey() != null
&& !backingTrack.getAudioObjectKey().isBlank()) {

backingTrackAudioFileUrl =
s3FileService.createPresignedDownload(
backingTrack.getUser().getUserId(),
S3FileType.BACKING_TRACK,
backingTrack.getAudioObjectKey()
);
}

int totalBars = analysisBarCalculator.calculate(playing).totalBars();
return AnalysisContextResponse.from(playing, totalBars, recordingFileUrl, backingTrackAudioFileUrl);
}

private void validatePlayingId(Long playingId) {
if (playingId == null || playingId < 1) {
throw new GeneralException(PlayingErrorStatus.INVALID_PLAYING_ID);
}
}
}
84 changes: 0 additions & 84 deletions src/main/java/com/mr/domain/playing/service/PlayingService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,11 @@

import com.mr.domain.backingtrack.entity.BackingTrack;
import com.mr.domain.backingtrack.repository.BackingTrackRepository;
import com.mr.domain.analysis.service.AnalysisBarCalculator;
import com.mr.domain.playing.dto.req.MidiEventSaveRequest;
import com.mr.domain.playing.dto.req.PlayingStartRequest;
import com.mr.domain.playing.dto.req.RecordingUploadUrlRequest;
import com.mr.domain.playing.dto.res.MidiEventSaveResponse;
import com.mr.domain.playing.dto.res.AnalysisContextResponse;
import com.mr.domain.playing.dto.res.PlayingDeleteResponse;
import com.mr.domain.playing.dto.res.PlayingDetailResponse;
import com.mr.domain.playing.dto.res.PlayingStartResponse;
import com.mr.domain.playing.dto.res.RecordingUploadUrlResponse;
import com.mr.domain.playing.entity.MidiEventData;
import com.mr.domain.playing.entity.Playing;
import com.mr.domain.playing.exception.PlayingErrorStatus;
Expand Down Expand Up @@ -43,7 +38,6 @@
public class PlayingService {

private final PlayingRepository playingRepository;
private final AnalysisBarCalculator analysisBarCalculator;
private final UserRepository userRepository;
private final BackingTrackRepository backingTrackRepository;
private final S3FileService s3FileService;
Expand Down Expand Up @@ -91,25 +85,6 @@ public PlayingStartResponse startPlaying(

}

@Transactional(readOnly = true)
public RecordingUploadUrlResponse createRecordingUploadUrl(
Long userId, Long playingId, RecordingUploadUrlRequest request
) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdAndDeletedAtIsNull(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateInProgress();

return RecordingUploadUrlResponse.from(s3FileService.createPresignedUpload(
userId,
S3FileType.RECORDING,
request.toCommand())
);
}

public MidiEventSaveResponse saveMidiEvents(
Long userId, Long playingId, MidiEventSaveRequest request
) {
Expand Down Expand Up @@ -155,65 +130,6 @@ public MidiEventSaveResponse saveMidiEvents(
});
}

@Transactional(readOnly = true)
public PlayingDetailResponse getPlayingDetail(Long userId, Long playingId) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdWithBackingTrack(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateCompleted();

String recordingFileUrl =
s3FileService.createPresignedDownload(
userId,
S3FileType.RECORDING,
playing.getRecordingObjectKey()
);

return PlayingDetailResponse.from(playing, recordingFileUrl);
}

@Transactional(readOnly = true)
public AnalysisContextResponse getAnalysisContext(Long userId, Long playingId) {
validatePlayingId(playingId);

Playing playing = playingRepository.findByIdWithBackingTrack(playingId)
.orElseThrow(() -> new GeneralException(PlayingErrorStatus.PLAYING_NOT_FOUND));

playing.validatePlayingOwner(userId);
playing.validateCompleted();
if (playing.getBackingTrack() == null) {
throw new GeneralException(PlayingErrorStatus.BACKING_TRACK_NOT_FOUND);
}

String recordingFileUrl =
s3FileService.createPresignedDownload(
userId,
S3FileType.RECORDING,
playing.getRecordingObjectKey()
);

BackingTrack backingTrack = playing.getBackingTrack();

String backingTrackAudioFileUrl = null;

if (backingTrack.getAudioObjectKey() != null
&& !backingTrack.getAudioObjectKey().isBlank()) {

backingTrackAudioFileUrl =
s3FileService.createPresignedDownload(
backingTrack.getUser().getUserId(),
S3FileType.BACKING_TRACK,
backingTrack.getAudioObjectKey()
);
}

int totalBars = analysisBarCalculator.calculate(playing).totalBars();
return AnalysisContextResponse.from(playing, totalBars, recordingFileUrl, backingTrackAudioFileUrl);
}

@Transactional
public PlayingDeleteResponse deletePlaying(Long userId, Long playingId) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,23 @@

import com.fasterxml.jackson.databind.ObjectMapper;
import com.mr.domain.playing.dto.req.MidiEventSaveRequest;
import com.mr.domain.playing.dto.req.RecordingUploadUrlRequest;
import com.mr.domain.playing.dto.res.MidiEventSaveResponse;
import com.mr.domain.playing.dto.res.AnalysisContextResponse;
import com.mr.domain.playing.dto.res.RecordingUploadUrlResponse;
import com.mr.domain.playing.entity.enums.MidiType;
import com.mr.domain.playing.service.PlayingFileService;
import com.mr.domain.playing.service.PlayingQueryService;
import com.mr.domain.playing.service.PlayingService;
import com.mr.domain.user.entity.enums.UserRole;
import com.mr.global.apipayload.handler.GlobalExceptionHandler;
import com.mr.global.file.s3.dto.PresignedUrlUpload;
import com.mr.global.security.principal.CustomUserDetails;

import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -33,6 +40,7 @@
import static com.mr.domain.playing.constant.MidiEventConstants.MAX_MIDI_EVENT_COUNT;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.then;
import static org.mockito.Mockito.never;
Expand All @@ -56,14 +64,20 @@ class PlayingControllerTest {
@Mock
private PlayingService playingService;

@Mock
private PlayingFileService playingFileService;

@Mock
private PlayingQueryService playingQueryService;

@BeforeEach
void setUp() {
objectMapper = Jackson2ObjectMapperBuilder.json()
.findModulesViaServiceLoader(true)
.build();

mockMvc = MockMvcBuilders
.standaloneSetup(new PlayingController(playingService))
.standaloneSetup(new PlayingController(playingService, playingFileService, playingQueryService))
.setControllerAdvice(new GlobalExceptionHandler())
.setMessageConverters(
new MappingJackson2HttpMessageConverter(objectMapper)
Expand Down Expand Up @@ -173,7 +187,7 @@ void getAnalysisContextSuccess() throws Exception {
null,
60
);
given(playingService.getAnalysisContext(USER_ID, PLAYING_ID))
given(playingQueryService.getAnalysisContext(USER_ID, PLAYING_ID))
.willReturn(response);

mockMvc.perform(get(
Expand All @@ -190,7 +204,7 @@ void getAnalysisContextSuccess() throws Exception {
.value("https://example.com/backing.mp3"))
.andExpect(jsonPath("$.data.totalBars").value(60));

then(playingService).should()
then(playingQueryService).should()
.getAnalysisContext(USER_ID, PLAYING_ID);
}
}
Expand Down Expand Up @@ -392,4 +406,36 @@ private MidiEventSaveRequest createRequest() {

);
}

@Nested
@DisplayName("녹음 파일 업로드 URL 발급")
class CreateRecordingUploadUrl {

@Test
@DisplayName("녹음 파일 업로드 URL 발급 요청을 PlayingFileService에 위임한다")
void createRecordingUploadUrlSuccess() throws Exception {
RecordingUploadUrlRequest request =
new RecordingUploadUrlRequest("recording.mp3", "audio/mpeg", 1_024L);

PresignedUrlUpload presignedUpload =
new PresignedUrlUpload(
"recordings/1/recording.mp3",
"https://example.com/presigned-upload-url",
Instant.now().plusSeconds(600),
Map.of("Content-Type", "audio/mpeg")
);

RecordingUploadUrlResponse response = RecordingUploadUrlResponse.from(presignedUpload);

given(playingFileService.createRecordingUploadUrl(eq(USER_ID), eq(PLAYING_ID), any(RecordingUploadUrlRequest.class)))
.willReturn(response);

mockMvc.perform(post("/api/playings/{playingId}/recording-upload-url", PLAYING_ID)
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isOk());

then(playingFileService).should().createRecordingUploadUrl(eq(USER_ID), eq(PLAYING_ID), any(RecordingUploadUrlRequest.class));
}
}
}
Loading