Feature/chung step2 #6

kochungcheon · 2023-11-03T03:45:22Z

[FolderMetadata]

폴더 속 폴더를 위해 부모 폴더 레코드 생성
폴더와 파일 구분을 위해 타입 레코드 생성
자주 사용되는 부모 폴더id는 인덱스 지정

[폴더 생성]

부모 폴더가 널 일 경우 루트 폴더에 폴더가 생성되도록 구현

[폴더 조회]

개별 유저의 폴더 및 파일 조회 가능하도록 구현, 루트 폴더에는 여러 유저의 폴더와 파일이 있을 수 있기 때문입니다
커서 기반 페이지 네이션을 도입했습니다. 폴더 조회 시 하위 폴더/파일이 많을 경우 데이터 로드에 많은 시간이 걸릴 수 있기 때문입니다. 오프셋 기반은 매 요청 마다 데이터 첫 행부터 검색해야 해서 성능적으로 아쉬워 커서 기반 페이지 네이션을 선택하였습니다

[파일 로직 변경]

폴더 기능 도입에 따른 변경
부모 폴더 레코드 추가
파일 조회 시 부모 폴더가 널 일 경우 처리

- FileReader. FileWriter 의존 변화 (기존 dataAccessService) - 사유: DIP

[FolderController] - createFolder 성공 시 201 반환 [FolderService] - 인터페이스에 의존 - createBy 부모 폴더는 null 이 될 수 있다 null 일 경우 스토리지 최상단 폴더 아래 생성 [FolderReader] - findPathBy - findBy - verifyBy - checkDuplicateBy [FolderWriter] - persist [FolderDataAccessService] - FolderReader 구현 클래스 - FolderWriter 구현 클래스 [ErrorCode] - FOLDER_CREATE_ERROR - UNAUTHORIZED_FOLDER_ACCESS - DUPLICATE_FOLDER_NAME - CANNOT_FOUND_FOLDER [FolderRepository] - existsByParentIdAndUserId - existsByParentIdAndUserIdAndOriginalFolderName 인덱스인 ParentId를 활용 [FolderMetadata] - 인덱스 설정 자주 사용되는 ParentId를 인덱스로 설정 - 빌더 패턴 사용 인자 개수에 따른 대응 [FolderUtil] - createFolder 폴더 생성 메서드 [테스트 코드 작성] - 기능 개발 따른 테스트 코드 작성

[FolderController] - changeFolderNameBy 성공 시 204 반환 Patch : 데이터 일부 변환 [FolderService] - changeFolderNameBy 폴더 이름 변경 [FolderWriter] - changeFolderNameBy [FolderDataAccessService] - validateFolderOwnershipBy 폴더에 접근 권한 있는 유저인지 확인 - changeFolderNameBy(String folderName, Long folderId, Long userId) validateFolderOwnershipBy, changeFolderNameBy 로 폴더 이름 변경 로직 수행 - changeFolderNameBy(String folderName, Long folderId) 폴더 이름 변경 후 저장 [ErrorCode] - FOLDER_CREATE_ERROR - UNAUTHORIZED_FOLDER_ACCESS - DUPLICATE_FOLDER_NAME - CANNOT_FOUND_FOLDER [FolderRepository] - existsByIdAndUserId [요청 DTO 생성] - FolderNameChangeRequest [테스트 코드 작성] - 기능 개발 따른 테스트 코드 작성

- file, folder 구분 목적

@builder

[FileDataAccessService] - 클래스 내부에서만 쓰이는 메서드 접근 제어자 private으로 변경 [FileMetadata] - @builder 적용 인자 개수 증가로 변경 [FileRepository] - checkDuplicateFileName 부모 Id 널일때 고려 [FileService] - findChildBy 파일 조회 메서드 [FolderDataAccessService] - 클래스 내부에서만 쓰이는 메서드 접근 제어자 private으로 변경 [FolderMetadata] - parentId에 인덱스 적용 [FolderReader] - FolderReader 생성 [FolderRepository] - existsByParentIdAndUploaderIdAndOriginalFolderName 생성 - existsByIdAndUploaderId 생성 - findByParentIdAndUploaderId 생성 - 전부 pk나 세컨더리 인덱스를 타게 유도 [FolderService] - findPathBy MetaService 에서도 활용하기 때문에 접근 제어자 public - findChildBy 생성 [MetadataController] - MetadataService의 컨트롤러 [MetadataService] - 파사드 패턴 적용 FolderService, FileService 의존성 관리 - uploadFile 생성 - uploadFile 생성 [기타] - 전송, 수신 Dto 생성 - 변경 사항에 따른 테스트 코드 추가

VSFe · 2023-11-03T11:56:26Z

gradlew.bat

@@ -2,7 +2,7 @@
 @rem Copyright 2015 the original author or authors.
 @rem
 @rem Licensed under the Apache License, Version 2.0 (the "License");
-@rem you may not use this metadata except in compliance with the License.
+@rem you may not use this fileMetadata except in compliance with the License.


아 기존 metadata를 전체 바꿈 단축기로 fileMetadata 로 바꿨었는 데 그것때문에 발생만 문제 같습니다
다시 push 하겠습니다

- fileMetadata -> metadata

- 트랜잭션 범위를 저장 로직/ 삭제 로직에 한정했습니다. 기존의 방식은 IO 작업(실제 파일을 업로드, 삭제)으로 인해 트랜잭션이 오래 열려있게 되기 때문입니다. - I0 작업과 DB 저장 작업 순서를 바꿨습니다. IO 은 취소할 수 없는 작업이기 때문입니다. 기존 방식에 따르면 데이터 불일치 문제가 일어날 수 있어 DB 작업 이후 IO 작업을 하게 구성하였습니다

sonarcloud · 2023-11-06T10:03:59Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

95.5% Coverage
0.0% Duplication

VSFe

고생하라는 의미에서 (?) 리뷰 난이도를 올려봤습니다.

VSFe · 2023-11-04T07:59:31Z

src/main/java/com/c4cometrue/mystorage/folder/FolderDataAccessService.java

+		}
+	}
+
+	// 적절한 예외 던져라


VSFe · 2023-11-08T17:15:07Z

src/main/java/com/c4cometrue/mystorage/meta/MetadataService.java

+
+@Service
+@RequiredArgsConstructor
+public class MetadataService {


굳이 분리할 필요가 있나하는 생각이 드네요.
MetadataService가 아닌 File 및 FolderService에도 메타데이터를 다루고 있지 않나요?

폴더 서비스와 파일 서비스에서 발생한 의존성을 별도로 관리하고 싶어 만들었습니다!

VSFe · 2023-11-08T17:15:57Z

src/main/java/com/c4cometrue/mystorage/meta/MetadataController.java

+public class MetadataController {
+	private final MetadataService metadataService;
+	@GetMapping("/metadata")
+	public ResponseEntity<List<MetaResponse>> getFolderContents(FolderContentsRequest req) {


역할이 정확히 이해가 안 가네요.
폴더 목록 조회는 FolderController로 가는게 더 자연스럽지 않을까요?

이 부분에서 고민을 좀 많이 해봤는 데
파일과 폴더는 의존성이 생길 수 밖에 없는 관계네요 의존성 형성하는 방향으로 가고
컨트롤러 위치도 그에 맞게 수정하겠습니다

VSFe · 2023-11-08T17:17:48Z

src/main/java/com/c4cometrue/mystorage/folder/FolderWriter.java

@@ -0,0 +1,7 @@
+package com.c4cometrue.mystorage.folder;
+
+public interface FolderWriter {


2개 이상의 구현체가 있지 않은 상황에서, 굳이 Interface를 만들어야 할지에 대한 의문이 들어요.
특히나 Reader, Writer 모두 하나의 Service로 때려박은 상황에선 더더욱요.

이 부분은 많이 고민 됩니다
결합도를 낮출수있어 가져오는 장점과 Reader, Writer 를 통해 해당 메서드가 어떤 역할을 하는지 알려줄수있다는 장점에서요
흡 좀 더 고민 해보겠습니다!

2개 이상의 구현체가 있지 않은 상황에서, 굳이 Interface를 만들어야 할지에 대한 의문이 들어요. 특히나 Reader, Writer 모두 하나의 Service로 때려박은 상황에선 더더욱요.

인터페이스 제거하였습니다!

VSFe · 2023-11-08T17:22:29Z

src/main/java/com/c4cometrue/mystorage/folder/FolderController.java

+	@PostMapping
+	@ResponseStatus(HttpStatus.CREATED)
+	public void createFolder(FolderCreateRequest req) {
+		folderService.createBy(req.userId(), req.userFolderName(), req.parentId());


~By 라는 네이밍을 사용한 이유가 있을까요?

뒤의 인자들로 만들어진다 라는 의미였는 데
이곳에서는 createFolder 라고 표현하는 게 더 좋을 거 같습니다

VSFe · 2023-11-08T17:40:05Z

src/main/java/com/c4cometrue/mystorage/meta/MetadataService.java

+		List<FolderMetadata> folderResponses = folderService.findChildBy(folderId, userId);
+		List<FileMetadata> fileResponses = fileService.findChildBy(folderId, userId);
+
+		return Stream.concat(


파일이 4700개이고, 폴더가 5300개라면 어떻게 될까요?

아아..!!! 다 보여 줄 필요가 없군요 고민해보겠습니다!

VSFe · 2023-11-08T17:41:09Z

src/main/java/com/c4cometrue/mystorage/util/FolderUtil.java

+
+	public static void createFolder(Path path) {
+		try {
+			Files.createDirectories(path);


폴더 생성이 실패하는 케이스가 무엇이 있을까요?
굳이 물리적인 파일시스템 장애가 아니더라도, 지금 로직으로는 충분히 폴더 생성이 실패할 수 있어요.

아아 폴더 경로가 중복될 수 있군요 UUID 로 대비된다 생각했는 데
UUID 로 생성된 폴더 경로가 중복일 경우 다시 생성 하는 로직이 필요하겠네요

VSFe · 2023-11-08T17:42:08Z

src/main/resources/application.yml

@@ -10,6 +10,7 @@ spring:
    properties:
      hibernate:
        dialect: org.hibernate.dialect.MySQL8Dialect
+        show_sql: true



위쪽에 적힌 ddl-auto: create는 제거하세요.
테스트할 데이터가 쌓일 수가 없는 구조네요...

통합 테스트를 구축하기 위한 property가 아닌 이상, 저걸 사용할 이유는 없습니다.

넵 알겠습니다!

VSFe · 2023-11-08T17:43:59Z

src/main/java/com/c4cometrue/mystorage/meta/MetadataController.java

+	}
+
+	@PostMapping("/files")
+	@ResponseStatus(HttpStatus.CREATED)


물리적인 파일 업로드랑, 메타데이터 업로드를 별도로 분리한건가요?
그렇다면 왜 폴더에 대한 메타데이터 업로드 메서드는 존재하지 않는걸까요?

위의 코멘트 처럼 수정하겠습니다!

VSFe · 2023-11-08T17:45:51Z

src/main/java/com/c4cometrue/mystorage/folder/FolderDataAccessService.java

+
+	private void changeFolderNameBy(String folderName, Long folderId) {
+		FolderMetadata folderMetadata = findBy(folderId);
+		folderMetadata.changeFolderName(folderName);


변경한 이름이 충돌되지 않는지는 확인하지 않네요.

아 UUID 로 생성 해도 충돌 될 경우가 존재하군요 ..! 수정하겠습니다!

변경한 이름이 충돌되지 않는지는 확인하지 않네요.

현재 폴더는 사용자가 저장한 이름과 서버에 저장용 이름 이렇게 두개로 저장됩니다
changeFolderNameBy 라는 메소드는 사용자가 저장한 이름을 바꾸는 메소드 입니다
이 경우에도 변경 이름이 충돌되는 지 검사 하는 게 좋을 까요!?

pear96 · 2023-11-07T18:52:10Z

src/main/java/com/c4cometrue/mystorage/dto/FileUploadRequest.java

-								@NotNull(message = "사용자 id는 null 될 수 없습니다") long userId) {
-	public static FileUploadRequest of(MultipartFile multipartFile, long userId) {
-		return new FileUploadRequest(multipartFile, userId);
+public record FileUploadRequest(@NotNull(message = "파일을 전달해주세요") MultipartFile multipartFile,


@NotNull로 바꾸셨군요! 그런데 저는 테스트해보면 다른 입력값은 MethodArgumentNotValidException가 발생하는데 Multipartfile은 파일을 담지 않고 보내면 ConstraintViolationException가 발생하더라구요. 예외 핸들러에서 한번에 처리해줄 수 없는데 청천님은 어떻게 하려고 하셨나요..

@NotNull로 바꾸셨군요! 그런데 저는 테스트해보면 다른 입력값은 MethodArgumentNotValidException가 발생하는데 Multipartfile은 파일을 담지 않고 보내면 ConstraintViolationException가 발생하더라구요. 예외 핸들러에서 한번에 처리해줄 수 없는데 청천님은 어떻게 하려고 하셨나요..

@NotNull로 바꾸셨군요! 그런데 저는 테스트해보면 다른 입력값은 MethodArgumentNotValidException가 발생하는데 Multipartfile은 파일을 담지 않고 보내면 ConstraintViolationException가 발생하더라구요. 예외 핸들러에서 한번에 처리해줄 수 없는데 청천님은 어떻게 하려고 하셨나요..

예외에 많이 미진해서 하은님 코드 보며 많이 배우고 있습니다!

pear96 · 2023-11-07T18:54:53Z

src/main/java/com/c4cometrue/mystorage/dto/FolderContentsRequest.java

+
+public record FolderContentsRequest(
+	Long folderId,
+	@NotNull(message = "사용자 id는 널이 될 수 없습니다") long userId


왜 Long과 long 으로 구분지으셨는지 궁금합니다.. 조회용 DTO같은데 folderId는 null이어도 되나요?

폴더 id 가 null 일 시 최상단 부모를 의미하게 했습니다! 그리고 userId 가 null 이 될 수 없게 했기 때문에 @NotNull 과 long 을 썼습니다

pear96 · 2023-11-07T18:55:37Z

src/main/java/com/c4cometrue/mystorage/dto/FolderContentsRequest.java

+	Long folderId,
+	@NotNull(message = "사용자 id는 널이 될 수 없습니다") long userId
+) {
+	public static FolderContentsRequest of(Long folderId, long userId) {


이 DTO를 생성하는 곳에서 new FolderRequest() 를 하지 않으려고 of로 만드신걸까요? 만약 그렇다면 더 좋은 점이 뭐죠?

https://tecoble.techcourse.co.kr/post/2020-05-26-static-factory-method/
메서드 명 가지는 장점이 있습니다. 메서드 내에서 제가 자유롭게 에러 코드를 던진다던가 다양하게 생성을 할 수 있다는 행위가 가능해서 new 보다는 of를 선호합니다

pear96 · 2023-11-07T18:57:15Z

src/main/java/com/c4cometrue/mystorage/dto/FolderNameChangeRequest.java

+
+public record FolderNameChangeRequest(
+	@NotNull(message = "유저id는 널이 될 수 없습니다") long userId,
+	Long folderId,


폴더의 이름을 바꾸는 경우에도 Null일 수 있나요?

오 아닙니다! 수정하겠습니다!

pear96 · 2023-11-07T19:01:17Z

src/main/java/com/c4cometrue/mystorage/folder/FolderDataAccessService.java

+
+@Service
+@RequiredArgsConstructor
+public class FolderDataAccessService implements FolderReader, FolderWriter {


Repository를 FolderService가 아닌 DataAccessService에서 의존하며 분리하신 이유가 있을까요?

비지니스로직과 데이터를 처리하는 로직을 분리하고 싶었습니다!

pear96 · 2023-11-07T19:03:38Z

src/main/java/com/c4cometrue/mystorage/folder/FolderController.java

+@RestController
+@RequestMapping("/folders")
+@RequiredArgsConstructor
+public class FolderController {


생각해보니 유효성 검사 어노테이션이 없군요.. 유효성 검사에 문제가 없었나요?

생각해보니 유효성 검사 어노테이션이 없군요.. 유효성 검사에 문제가 없었나요?

이 부분도 위에서 언급했 듯 많이 배우고 있습니다 감사합니다!

생각해보니 유효성 검사 어노테이션이 없군요.. 유효성 검사에 문제가 없었나요?

@Vaild 생각보다 재밌는 내용이 많군요 감사합니다!

pear96 · 2023-11-07T19:11:03Z

src/main/java/com/c4cometrue/mystorage/file/FileService.java

-		Path path = Paths.get(storagePath, storedFileName);
-		Metadata metadata = Metadata.of(originalFileName, storedFileName, path.toString(), userId);
+		String storedFileName = FileMetadata.storedName();
+		Path path = Paths.get(basePath, storedFileName);


헐 저도 get으로 바꿔야겠다.... 계속 resolve() 해주고 있었는데...

pear96 · 2023-11-07T19:12:39Z

src/main/java/com/c4cometrue/mystorage/folder/FolderController.java

+	@PostMapping
+	@ResponseStatus(HttpStatus.CREATED)
+	public void createFolder(FolderCreateRequest req) {
+		folderService.createBy(req.userId(), req.userFolderName(), req.parentId());


(정답은 모름 그냥 같이 고민하자는 뜻)
Service에 DTO를 바로 넘기는 것과 parsing해서 넘기는 것 중 어느게 더 좋은거란게 있을까요?

(정답은 모름 그냥 같이 고민하자는 뜻) Service에 DTO를 바로 넘기는 것과 parsing해서 넘기는 것 중 어느게 더 좋은거란게 있을까요?

그 부분에 대해 많은 의견이 갈리는 거 같더라고요
현재는 컨트롤러 까지 받아오는 걸 DTO 로 하고 있습니다. parsing 했을 때 장점은 명확하게 인자가 보이니 (ex. add(int a, int b) )Service 레이어에서 인자들을 보면서 작업하기 용이 하다는 것을 들 수 있을 거 같아요 DTO 를 통째로 넘기면 어떤 장점이 있을 까요

pear96 · 2023-11-09T14:06:53Z

src/test/java/com/c4cometrue/mystorage/file/FileDataAccessServiceTest.java

+	@DisplayName("파일 삭제 테스트 : 실패")
+	void shouldDeleteByFileIdFail() {
+		when(fileRepository.existsById(FILE_ID)).thenReturn(false);
+		assertThrows(ServiceException.class, () -> fileDataAccessService.deleteBy(FILE_ID));


원하는 exception의 이름이 맞는지 확인하는 검증 단계가 있으면 좋을 것 같아요

오 그러면 확실히 의도한 에러가 나온 지 알 수 있는 장점이 있네요

pear96 · 2023-11-09T14:10:22Z

src/test/java/com/c4cometrue/mystorage/folder/FolderDataAccessServiceTest.java

+	void getChildFolder() {
+		folderDataAccessService.findChildBy(null, USER_ID);
+
+		then(folderRepository).should(times(1)).findByParentIdAndUploaderId(null, USER_ID);


verify 대신 then을 쓰신 이유가 궁금합니다

https://brunch.co.kr/@springboot/292 부분 때문에요 사실 어떤 게 좋은 지 모르겠습니다

- MetadataController -> FileController - base위치 파악 위해 FileService 에 FolderService 의존 생성 - 변경 지점에 따른 테스트 코드 수정

- 인터페이스 제거에 따른 코드 수정 작업 - XXXDataAcessService -> XXXHandlerService 로 수정 작업 - 변화에 따른 테스트 코드 수정

- parentId, uploaderId, originalFolderName 을 인덱스로 생성 - parentId 에만 잡힌 인덱스 제거

- 명칭 변화에 따른 코드 수정

- FileDataHandlerService getFileList, hashNext - FileService getFiles - FolderDataHandlerService getFolderList hasNext - FolderService getFolders - StorageFacadeService getFolderContents - PagingUtil calculateSize createPageable

sonarcloud · 2023-12-28T07:19:02Z

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
92.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

kochungcheon added 5 commits October 27, 2023 12:28

refactor: FileService 의존 관계 변화

63bb89b

- FileReader. FileWriter 의존 변화 (기존 dataAccessService) - 사유: DIP

feat: MetadataType

70ffaa3

- file, folder 구분 목적

kochungcheon added the enhancement New feature or request label Nov 3, 2023

kochungcheon requested review from VSFe, semi-cloud, pear96, MilanoBeer and dawnmorning November 3, 2023 03:45

kochungcheon self-assigned this Nov 3, 2023

VSFe reviewed Nov 3, 2023

View reviewed changes

kochungcheon added 2 commits November 3, 2023 21:15

chore: gradlew.bat 수정

af8ff26

- fileMetadata -> metadata

VSFe requested changes Nov 8, 2023

View reviewed changes

pear96 reviewed Nov 9, 2023

View reviewed changes

kochungcheon added 6 commits December 8, 2023 17:33

refactor: 파일 조회 컨트롤러 위치 변경

539d0d2

- MetadataController -> FileController - base위치 파악 위해 FileService 에 FolderService 의존 생성 - 변경 지점에 따른 테스트 코드 수정

refactor: Reader, Writer 제거

576e60b

- 인터페이스 제거에 따른 코드 수정 작업 - XXXDataAcessService -> XXXHandlerService 로 수정 작업 - 변화에 따른 테스트 코드 수정

refactor: FolderMetadata index 추가 및 제거 작업

d5b94bb

- parentId, uploaderId, originalFolderName 을 인덱스로 생성 - parentId 에만 잡힌 인덱스 제거

refactor: MetadataService -> StoregeFasadeService

4149f7a

- 명칭 변화에 따른 코드 수정

chore: 오탈자 수정

99fb8e0

refactor: 유효성 검사 어노테이션 추가

4986865

kochungcheon requested a review from VSFe December 27, 2023 05:17

kochungcheon added 4 commits December 27, 2023 18:25

feat: 커서 기반 페이징 도입-

962485c

chore : 사용하지 않는 파일 정리

15ad8be

test : 변경 사항에 따른 테스트 코드 추가

a0fce68

- FileDataHandlerService getFileList, hashNext - FileService getFiles - FolderDataHandlerService getFolderList hasNext - FolderService getFolders - StorageFacadeService getFolderContents - PagingUtil calculateSize createPageable

chore : 코드 스멜 수정

bc03dbf

kochungcheon closed this Dec 27, 2023

kochungcheon deleted the feature/chung_step2 branch December 28, 2023 07:10

kochungcheon restored the feature/chung_step2 branch December 28, 2023 07:17

kochungcheon reopened this Dec 28, 2023

kochungcheon merged commit 821c22d into base/chung Dec 28, 2023
3 checks passed

kochungcheon deleted the feature/chung_step2 branch December 28, 2023 07:26

		@@ -0,0 +1,7 @@
		package com.c4cometrue.mystorage.folder;

		public interface FolderWriter {

Feature/chung step2 #6

Feature/chung step2 #6

Conversation

kochungcheon commented Nov 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 6, 2023

VSFe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 28, 2023

Quality Gate passed

kochungcheon commented Nov 3, 2023 •

edited

Loading