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

Feature/chung step2 #6

Merged
merged 17 commits into from
Dec 28, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@

import org.springframework.web.multipart.MultipartFile;

import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;

public record FileUploadRequest(@NotBlank(message = "파일을 전달해주세요") MultipartFile multipartFile,
@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,
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

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

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

@NotNull(message = "사용자 id는 null 될 수 없습니다") long userId,
Long parentId) {
public static FileUploadRequest of(MultipartFile multipartFile, long userId, Long parentId) {
return new FileUploadRequest(multipartFile, userId, parentId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.c4cometrue.mystorage.dto;

import jakarta.validation.constraints.NotNull;

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

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

) {
public static FolderContentsRequest of(Long folderId, long userId) {
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

return new FolderContentsRequest(folderId, userId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.c4cometrue.mystorage.dto;

import jakarta.validation.constraints.NotNull;

public record FolderCreateRequest(@NotNull long userId, @NotNull String userFolderName, Long parentId) {
public static FolderCreateRequest of(long userId, String userFolderName, long parentId) {
return new FolderCreateRequest(userId, userFolderName, parentId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.c4cometrue.mystorage.dto;

import jakarta.validation.constraints.NotNull;

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

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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

String folderName
) {
public static FolderNameChangeRequest of(
long userId,
Long folderId,
String folderName) {
return new FolderNameChangeRequest(userId, folderId, folderName);
}
}
30 changes: 30 additions & 0 deletions src/main/java/com/c4cometrue/mystorage/dto/MetaResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.c4cometrue.mystorage.dto;

import com.c4cometrue.mystorage.file.FileMetadata;
import com.c4cometrue.mystorage.folder.FolderMetadata;
import com.c4cometrue.mystorage.meta.MetadataType;

public record MetaResponse(
Long id,
String metadataOriginalName,
Long parentId,
MetadataType type
) {
public static MetaResponse from(FileMetadata metadata) {
return new MetaResponse(
metadata.getId(),
metadata.getOriginalFileName(),
metadata.getParentId(),
metadata.getMetadataType()
);
}

public static MetaResponse from(FolderMetadata metadata) {
return new MetaResponse(
metadata.getId(),
metadata.getOriginalFolderName(),
metadata.getParentId(),
metadata.getMetadataType()
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import lombok.AllArgsConstructor;
import lombok.Getter;


@Getter
@AllArgsConstructor
public enum ErrorCode {
Expand All @@ -17,7 +16,12 @@ public enum ErrorCode {

FILE_DELETE_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "파일 삭제 중 오류가 발생했습니다."),

DUPLICATE_FILE_NAME(HttpStatus.BAD_REQUEST, "파일 업로드에 중복이 발생 했습니다");
DUPLICATE_FILE_NAME(HttpStatus.BAD_REQUEST, "파일 업로드에 중복이 발생 했습니다"),

FOLDER_CREATE_ERROR(HttpStatus.INTERNAL_SERVER_ERROR, "폴더 생성 중 오류가 발생했습니다"),
UNAUTHORIZED_FOLDER_ACCESS(HttpStatus.FORBIDDEN, "비정상적인 요청입니다."),
DUPLICATE_FOLDER_NAME(HttpStatus.BAD_REQUEST, "폴더 업로드에 중복이 발생 했습니다"),
CANNOT_FOUND_FOLDER(HttpStatus.NOT_FOUND, "해당 폴더를 찾을 수 없습니다.");

private final HttpStatus httpStatus;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
import org.springframework.http.HttpStatus;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseStatus;
import org.springframework.web.bind.annotation.RestController;

import com.c4cometrue.mystorage.dto.FileDeleteRequest;
import com.c4cometrue.mystorage.dto.FileDownloadRequest;
import com.c4cometrue.mystorage.dto.FileUploadRequest;

import lombok.RequiredArgsConstructor;

Expand All @@ -20,12 +18,6 @@
public class FileController {
private final FileService fileService;

@PostMapping
Copy link

Choose a reason for hiding this comment

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

Metadata 컨트롤러로 인해 파일 업로드를 이동하셨군요..
그리고 메타데이터 서비스가 파일과 폴더 서비스를 다시 의존하고 있군요..

Copy link
Author

Choose a reason for hiding this comment

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

파일과 폴더 서비스에 의존성이 생기는 게 자연스러운가 생각해서
Metadata 서비스를 만들어서 의존을 관리하고자 했습니다
지금 생각해보니
네이밍이 MetadataFasade 쪽으로 됐어야 하고
파일과 폴더 사이 의존에 대해 좀 더 생각해봐야겠네요

@ResponseStatus(HttpStatus.CREATED)
public void uploadFile(FileUploadRequest fileUploadRequest) {
fileService.uploadFile(fileUploadRequest.multipartFile(), fileUploadRequest.userId());
}

@DeleteMapping
@ResponseStatus(HttpStatus.NO_CONTENT)
public void deleteFile(FileDeleteRequest request) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,41 +1,57 @@
package com.c4cometrue.mystorage.file;

import java.util.List;

import org.springframework.stereotype.Service;

import com.c4cometrue.mystorage.exception.ErrorCode;

import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;

@Service
@RequiredArgsConstructor
public class FileDataAccessService implements FileReader, FileWriter {
private final FileRepository fileRepository;

@Transactional
public void deleteBy(Long fileId) {
existBy(fileId);
fileRepository.deleteById(fileId);
}

public Metadata findBy(Long fileId, Long userId) {
private void existBy(Long fileId) {
if (!fileRepository.existsById(fileId)) {
throw ErrorCode.CANNOT_FOUND_FILE.serviceException("fileId : {}", fileId);
}
}

public FileMetadata findBy(Long fileId, Long userId) {
Copy link
Member

Choose a reason for hiding this comment

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

existBy랑 매칭이 좀 어색해보이네요.

  • existBy는 fileId로 Unique 하게 찾을 수 있어보이는데, findBy는 userId를 받는 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

악성 유저가 다른 사람의 파일에 접근하는 것을 막고자 의도 했습니다!

Copy link
Author

Choose a reason for hiding this comment

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

delete의 경우 FileService 에서 유저가 해당 파일 소유주인지 확인을 한 다음 오기 때문에 별도로 userId 는 추가 하지 않았습니다!

return fileRepository.findByIdAndUploaderId(fileId, userId)
.orElseThrow(() -> ErrorCode.CANNOT_FOUND_FILE.serviceException("fileId : {}, userId : {}", fileId,
userId));
}

public void persist(Metadata metadata, Long userId) {
duplicateBy(metadata.getOriginalFileName(), userId);
fileRepository.save(metadata);
@Transactional
public void persist(FileMetadata fileMetadata, Long userId, Long parentId) {
validateFileOwnershipBy(parentId, userId);
duplicateBy(fileMetadata.getOriginalFileName(), userId, parentId);
fileRepository.save(fileMetadata);
}

public void existBy(Long fileId) {
if (!fileRepository.existsById(fileId)) {
throw ErrorCode.CANNOT_FOUND_FILE.serviceException("fileId : {}", fileId);
private void duplicateBy(String fileName, Long userId, Long parentId) {
if (fileRepository.checkDuplicateFileName(parentId, userId, fileName)) {
throw ErrorCode.DUPLICATE_FILE_NAME.serviceException("fileName : {}", fileName);
}
}

public void duplicateBy(String fileName, Long userId) {
if (fileRepository.checkDuplicateFileName(fileName, userId)) {
throw ErrorCode.DUPLICATE_FILE_NAME.serviceException("fileName : {}", fileName);
public List<FileMetadata> findChildBy(Long parentId, Long userId) {
Copy link
Member

Choose a reason for hiding this comment

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

fileId는 Unique 하게 만들었으면서, folderId는 Unique하게 만들지 않은 이유가 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

좀더 설명 부탁드려도 되나요!?
fileId는 Unique 라는 부분을 이해못했습니다..

validateFileOwnershipBy(parentId, userId);
return fileRepository.findByParentIdAndUploaderId(parentId, userId);
}

private void validateFileOwnershipBy(Long folderId, Long userId) {
if (folderId != null && !fileRepository.existsByIdAndUploaderId(folderId, userId)) {
throw ErrorCode.UNAUTHORIZED_FILE_ACCESS.serviceException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,27 @@

import java.util.UUID;

import com.c4cometrue.mystorage.meta.MetadataType;

import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.Index;
import jakarta.persistence.Table;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@Getter
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Table(name = "Metadata", indexes = @Index(name = "index_uploaderId", columnList = "uploaderId"))
public class Metadata {
@Table(name = "file_metadata", indexes = @Index(name = "index_parentId", columnList = "parentId"))
public class FileMetadata {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
Expand All @@ -30,19 +35,24 @@ public class Metadata {
private String filePath;
@Column(nullable = false)
private Long uploaderId;
private Long parentId;

@Column(nullable = false)
@Enumerated(EnumType.STRING)
private MetadataType metadataType;

public static String storedName() {
return UUID.randomUUID().toString();
}

private Metadata(String originalFileName, String storedFileName, String filePath, Long uploaderId) {
@Builder
public FileMetadata(String originalFileName, String storedFileName, String filePath, Long uploaderId,
Long parentId) {
this.originalFileName = originalFileName;
this.storedFileName = storedFileName;
this.filePath = filePath;
this.uploaderId = uploaderId;
}

public static Metadata of(String originalFileName, String storedFileName, String filePath, Long uploaderId) {
return new Metadata(originalFileName, storedFileName, filePath, uploaderId);
this.parentId = parentId;
this.metadataType = MetadataType.FILE;
}
}
8 changes: 4 additions & 4 deletions src/main/java/com/c4cometrue/mystorage/file/FileReader.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.c4cometrue.mystorage.file;

public interface FileReader {
Metadata findBy(Long fileId, Long userId);
import java.util.List;

void existBy(Long fileId);
public interface FileReader {
FileMetadata findBy(Long fileId, Long userId);

void duplicateBy(String fileName, Long userId);
List<FileMetadata> findChildBy(Long parentId, Long userId);
}
17 changes: 12 additions & 5 deletions src/main/java/com/c4cometrue/mystorage/file/FileRepository.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
package com.c4cometrue.mystorage.file;

import java.util.List;
import java.util.Optional;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;

public interface FileRepository extends JpaRepository<Metadata, Long> {
Optional<Metadata> findByIdAndUploaderId(Long id, Long uploaderId);
public interface FileRepository extends JpaRepository<FileMetadata, Long> {
Optional<FileMetadata> findByIdAndUploaderId(Long id, Long uploaderId);

@Query("SELECT CASE WHEN COUNT(m) > 0 "
Copy link
Member

Choose a reason for hiding this comment

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

JDK 17에서는 """ 으로 Text Block 생성이 가능합니다.

Copy link
Author

Choose a reason for hiding this comment

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

오 감사합니다

+ "THEN TRUE "
+ "ELSE FALSE END "
+ "FROM Metadata m "
+ "WHERE m.uploaderId = :uploaderId "
+ "FROM FileMetadata m "
+ "WHERE "
+ "(m.parentId = :parentId OR (m.parentId IS NULL AND :parentId IS NULL)) "
Copy link
Member

Choose a reason for hiding this comment

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

(parent_id, uploader_id, original_file_name) 의 인덱스가 필요하겠네요.

Copy link
Author

Choose a reason for hiding this comment

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

넵 추가하겠습니다!

+ "AND m.uploaderId = :uploaderId "
+ "AND m.originalFileName = :fileName ")
boolean checkDuplicateFileName(String fileName, Long uploaderId);
boolean checkDuplicateFileName(Long parentId, Long uploaderId, String fileName);

List<FileMetadata> findByParentIdAndUploaderId(Long parentId, Long userId);

boolean existsByIdAndUploaderId(Long parentId, Long userId);

}
43 changes: 25 additions & 18 deletions src/main/java/com/c4cometrue/mystorage/file/FileService.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,52 +2,59 @@

import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;

import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.web.multipart.MultipartFile;

import com.c4cometrue.mystorage.util.FileUtil;

import jakarta.transaction.Transactional;
import lombok.RequiredArgsConstructor;

@Service
@RequiredArgsConstructor
public class FileService {
private final FileDataAccessService fileDataAccessService;

@Value("${file.storage-path}")
private String storagePath;
private final FileReader fileReader;
private final FileWriter fileWriter;

@Value("${file.buffer}")
private int bufferSize;

@Transactional
public void uploadFile(MultipartFile file, Long userId) {
public void uploadFile(MultipartFile file, Long userId, Long parentId, String basePath) {
String originalFileName = file.getOriginalFilename();
String storedFileName = Metadata.storedName();
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);
Copy link

Choose a reason for hiding this comment

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

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

FileMetadata fileMetadata = FileMetadata.builder()
.originalFileName(originalFileName)
.storedFileName(storedFileName)
.filePath(path.toString())
.uploaderId(userId)
.parentId(parentId)
.build();

fileDataAccessService.persist(metadata, userId);
FileUtil.uploadFile(file, path, bufferSize);
fileWriter.persist(fileMetadata, userId, parentId);
}

public void downloadFile(Long fileId, String userPath, Long userId) {
Metadata metadata = fileDataAccessService.findBy(fileId, userId);
Path originalPath = Paths.get(metadata.getFilePath());
Path userDesignatedPath = Paths.get(userPath).resolve(metadata.getOriginalFileName()).normalize();
FileMetadata fileMetadata = fileReader.findBy(fileId, userId);
Path originalPath = Paths.get(fileMetadata.getFilePath());
Path userDesignatedPath = Paths.get(userPath).resolve(fileMetadata.getOriginalFileName()).normalize();

FileUtil.download(originalPath, userDesignatedPath, bufferSize);
}

@Transactional
public void deleteFile(Long fileId, Long userId) {
Metadata metadata = fileDataAccessService.findBy(fileId, userId);
fileDataAccessService.deleteBy(fileId);
Path path = Paths.get(metadata.getFilePath());
FileMetadata fileMetadata = fileReader.findBy(fileId, userId);
Path path = Paths.get(fileMetadata.getFilePath());

FileUtil.delete(path);

fileWriter.deleteBy(fileId);
}

public List<FileMetadata> findChildBy(Long parentId, Long userId) {
return fileReader.findChildBy(parentId, userId);
}
}
Loading