Feature/chung step5 #12

kochungcheon · 2024-05-17T15:50:32Z

구현 기능

파일 공유 링크 생성 / 다운로드
링크 일괄 삭제

파일 공유 링크 생성

UUID.randomUUID 기반 링크 생성
링크 중복 검사

파일 공유 다운로드

링크 유효 기간은 3시간
유효 기간 내 요청만 다운로드 가능
파일 공유 이후 원본 파일 삭제 발생 시 다운 불가능
스케줄러를 통해 유효하지 않은 링크 삭제 ( 5분 단위)

- link는 LocalDate.now + UUID로 구성 - link에 대한 중복 검사 진행 - link에 인덱스 설정

- 삭제된 파일 다운 시 다운 받지 못함

kochungcheon · 2024-05-17T15:56:55Z

src/main/java/com/c4cometrue/mystorage/exception/ErrorCode.java

+	MEMBER_NOT_FOUND(HttpStatus.NOT_FOUND, "해당 맴버를 찾지 못했습니다"),
+	EXCEEDED_CAPACITY(HttpStatus.INSUFFICIENT_STORAGE, "더 이상 업로드 할 수 없습니다"),
+	INVALID_OPERATION(HttpStatus.BAD_REQUEST, "사용 중인 공간보다 많은 공간은 해제할 수 없습니다"),
+	VALIDATION_ERROR(HttpStatus.BAD_REQUEST, "유효하지 않은 요청입니다."),



공유된 이슈로 개행 문제가 있습니다.
아래 세 개가 이번 수정 사항에 반영된 것입니다.

DUPLICATE_SHARE_LINK

NOT_FOUND_SHARE_LINK

NOT_FRESH_LINK

kochungcheon · 2024-05-17T15:57:34Z

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

+	public void deleteAll(List<FileMetadata> fileMetadataList) {
+		fileRepository.deleteAll(fileMetadataList);
+	}
+


공유된 이슈로 개행 문제가 있습니다.
아래 findBy가 이번에 추가된 사항입니다.

kochungcheon · 2024-05-17T15:57:58Z

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

@@ -5,23 +5,22 @@

 import org.springframework.data.domain.Pageable;
 import org.springframework.data.jpa.repository.JpaRepository;
-import org.springframework.data.jpa.repository.Query;



개행 수정했습니다. 로직 상 수정 사항은 없습니다.

kariskan

수고하셨습니다~ 많이 배웠네요.
근데 이제 봤는데
https://github.com/C4-ComeTrue/c4-cometrue-assignment/blob/main/assignments/my-storage.md
이거랑
https://github.com/C4-ComeTrue/my-storage/tree/main/assignment
이거랑 요구사항이 약간 다르네요. 폴더 공유에 대한 로직도 작성해보시면 좋을 것 같기도..?

kariskan · 2024-05-18T08:14:14Z

src/main/java/com/c4cometrue/mystorage/fileshare/DeleteLinkScheduler.java

+		List<Long> expirationLinkIds;
+
+		do {
+			expirationLinkIds = fileShareRepository.findExpirations(expirationTime, PageRequest.of(0, DELETE_SIZE));


전 native query로 limit 사용했는데 이런 방법도 있군요. 신기..

전 native query로 limit 사용했는데 이런 방법도 있군요. 신기..

아 ! limit 거는 형식으로 가야겠네요. 기존 방법대로 하면 불필요하게 정렬되니.. 수정했습니다.

kariskan · 2024-05-18T08:19:02Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareRepository.java

+	Optional<FileShare> findByShareLink(String shareLink);
+
+	@Query("SELECT fs.id FROM FileShare fs WHERE fs.createdAt < :expirationTime")
+	List<Long> findExpirations(@Param("expirationTime") ZonedDateTime expirationTime, Pageable pageable);


createdAt에 index는 필요 없나요?

createdAt에 index는 필요 없나요?

인덱스 추가하였습니다 감사합니다~!

kariskan · 2024-05-18T08:22:36Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareController.java

+	}
+
+	@GetMapping("/download")
+	public ResponseEntity<DownloadShareFileRes> downloadShareFile(@Valid @RequestBody DownloadShareFileReq req) {


GetMapping에서는 RequestBody 보다는 PathVariable이나 RequestParam을 이용하는 것은 어떨까요?

GetMapping에서는 RequestBody 보다는 PathVariable이나 RequestParam을 이용하는 것은 어떨까요?

아 맞네요 ModelAttribute 로 수정하였습니다!

kariskan · 2024-05-18T08:31:09Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareService.java

+		Duration duration = Duration.between(createTimeInLink, now);
+		Duration maxTime = Duration.ofHours(3);
+
+		if (duration.compareTo(maxTime) > 0) {


duration.toHours() >= 3
이렇게 해도 될 거 같아요

duration.toHours() >= 3 이렇게 해도 될 거 같아요

오 그게 더 깔끔하네요 duration.toHours() 형태로 수정했습니다!
3 을 int maxTime = 3; 처리하는 것과 상수로 나두는 것 둘 중 어떤 걸 선호하시나요

저는 상수는 스페이스바 이런거 말고는 다 나누긴 해요.
하나의 클래스에서만 사용되면 그냥 전역 변수로 분리하고 여러개에서 사용되면 따로 constant 클래스로 분리해서 사용합니다

의 클래스에서만 사용되면 그냥 전역 변수로 분리하고 여러개에서 사용되면 따로 constant 클래스로 분리해서 사용합니다

오호 그렇군요 배우고 갑니다! 감사합니다!

- Get request

pear96

코드 보고 많이 배우고 갑니다!!
수정사항을 찾아낼게 없어서 조금이나마 남기고 갑니다.

pear96 · 2024-05-19T07:17:14Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareService.java

+
+	private void checkDuplicate(String sharedLink) {
+		if (fileShareRepository.existsByShareLink(sharedLink)) {
+			throw ErrorCode.DUPLICATE_SHARE_LINK.serviceException();


공유 링크 중복을 검사를 하는 이유를 혼자서 생각해봤습니다.

다른 사람끼리의 파일들의 공유 링크가 겹칠까봐

한 파일의 공유 링크가 중복되면 만료 시간이 달라야 하는데 원래보다 늦게 혹은 일찍 만료될까봐

1번과 같은 경우를 생각해본다면 error를 발생시킨다기보단 파일 id와 같은 파일의 고유한 정보도 shareLink에 추가해줘야하지 않을까 싶습니다.
2번과 같은 경우라면 애초에 LocalDateTime이 달랐을 테니 발생하지 않을 상황 같습니다.

그래서 저라면 shareLink에 파일의 고유한 정보를 넣고, 중복 처리는 딱히 필요 없을 것 같습니다.
(..그냥 저의 생각일 뿐이므로 그렇구나 하고 넘기시면 됩니다. 너무 잘해서 쓸게 없어서 이런 얘기라도 써봤습니다.)

공유 링크 중복을 검사를 하는 이유를 혼자서 생각해봤습니다.

다른 사람끼리의 파일들의 공유 링크가 겹칠까봐

한 파일의 공유 링크가 중복되면 만료 시간이 달라야 하는데 원래보다 늦게 혹은 일찍 만료될까봐

1번과 같은 경우를 생각해본다면 error를 발생시킨다기보단 파일 id와 같은 파일의 고유한 정보도 shareLink에 추가해줘야하지 않을까 싶습니다. 2번과 같은 경우라면 애초에 LocalDateTime이 달랐을 테니 발생하지 않을 상황 같습니다.

그래서 저라면 shareLink에 파일의 고유한 정보를 넣고, 중복 처리는 딱히 필요 없을 것 같습니다. (..그냥 저의 생각일 뿐이므로 그렇구나 하고 넘기시면 됩니다. 너무 잘해서 쓸게 없어서 이런 얘기라도 써봤습니다.)

자세히 의견 이야기 해주셔서 다른 관점도 많이 배우고 갑요 감사합니다!
확실히 이야기 하신 것처럼 고유 정보 + 시간 + UUID 와 같은 형태로 구성하면 중복 체크를 방지할 수 있겠네요!! 오호 배우고 갑니다.

공유 링크 중복을 검사를 하는 이유는 중복 링크 생성 가능성을 방지하기 위함이였습니다. 또한 외부 사용자에게는 (공유 파일을 이용하는) 파일 고유 정보 노출을 하지 않기 위해 시간 + UUID 조합을 이용했습니다!

pear96 · 2024-05-19T07:18:08Z

src/main/java/com/c4cometrue/mystorage/fileshare/dto/CreateShareFileRes.java

+package com.c4cometrue.mystorage.fileshare.dto;
+
+public record CreateShareFileRes(
+	String fileName,


혹시 Req에는 @NotBlank를 검사하고, Res들에는 검사하지 않는 이유가 있을까요?
Res를 반환해주는 것은 앞선 유효검사 로직을 다 통과한걸텐데, 비어있는 링크가 가도 되는 상황은 무엇인지 궁금합니다.

혹시 Req에는 @NotBlank를 검사하고, Res들에는 검사하지 않는 이유가 있을까요?
Res를 반환해주는 것은 앞선 유효검사 로직을 다 통과한걸텐데, 비어있는 링크가 가도 되는 상황은 무엇인지 궁금합니다.

오호 생각해보니 붙이는 게 좋겠네요..! 감사합니다!

- settings.gradle

VSFe

작업량이 많지 않아서 짧게 리뷰 남기고 갑니다 ㅎㅎ~~

VSFe · 2024-05-22T14:28:13Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareController.java

+	private final FileShareService fileShareService;
+
+	@PostMapping
+	public ResponseEntity<CreateShareFileRes> createShareFile(@Valid @RequestBody CreateShareFileReq req) {


https://github.com/C4-ComeTrue/my-storage/blob/base/chung/src/main/java/com/c4cometrue/mystorage/folder/FolderController.java

Controller 형식을 이전 코드와 맞춰주세요.

넵 수정하였습니다

VSFe · 2024-05-22T14:30:44Z

src/main/java/com/c4cometrue/mystorage/fileshare/DeleteLinkScheduler.java

+		List<Long> expirationLinkIds;
+
+		do {
+			expirationLinkIds = fileShareRepository.findExpirations(expirationTime, DELETE_SIZE);


혹시 바로 삭제하지 않는 의도가 있을까요?
(delete 계열은 long을 반환할 수 있습니다.)

혹시 바로 삭제하지 않는 의도가 있을까요? (delete 계열은 long을 반환할 수 있습니다.)

앗...!!!!! 덕분에 하나 배워 갑니다 감사합니다! 수정했습니다 💯

VSFe · 2024-05-22T14:32:39Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareService.java

+		FileMetadata file = fileDataHandlerService.findBy(fileId, userId);
+
+		// shareLink 생성
+		String shareLink = LocalDateTime.now() + UUID.randomUUID().toString();


UUID는 일반적으로 timeStamp를 활용해서 만들어집니다.
UUID와 LocalDateTime이 만들 데이터는 유사할 확률이 높아요.

앗

UUID는 일반적으로 timeStamp를 활용해서 만들어집니다. UUID와 LocalDateTime이 만들 데이터는 유사할 확률이 높아요.

앗 그러네요 UUID 를 통해서만 만드는 것으로 수정하겠습니다.

VSFe · 2024-05-22T14:35:54Z

src/main/java/com/c4cometrue/mystorage/fileshare/FileShareService.java

+		ZonedDateTime now = ZonedDateTime.now();
+		Duration duration = Duration.between(createTimeInLink, now);
+
+		int maxTime = 3;


이게 Lazy하게 데이터를 Validation 하는게 좋을 수도 있긴 하지만... 여기서는 그렇지가 않아요.
링크 목록을 가져올 때 (특히 FE 관점에서는 목록을 구하는 API 를 호출할 가능성이 있을텐데) Valid 한 것 처럼 보일 가능성이 있고, 이걸 접근하려고 할 때 비로소 isValid가 호출될 것 같거든요.

차라리 Schedule의 Term을 좀 더 짧게 가져가는게 나을 것 같습니다.

이게 Lazy하게 데이터를 Validation 하는게 좋을 수도 있긴 하지만... 여기서는 그렇지가 않아요. 링크 목록을 가져올 때 (특히 FE 관점에서는 목록을 구하는 API 를 호출할 가능성이 있을텐데) Valid 한 것 처럼 보일 가능성이 있고, 이걸 접근하려고 할 때 비로소 isValid가 호출될 것 같거든요.

차라리 Schedule의 Term을 좀 더 짧게 가져가는게 나을 것 같습니다.

5분 주기로 삭제 되도록 수정했습니다!
확실히 짧게 가져가니 삭제 테스크 당 서버 부하가 줄겠네요

- delete return id

- 삭제 빈도 상승으로 삭제 당 부하를 낮추기 위함

- 중복된 기능

sonarcloud · 2024-05-22T15:22:24Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

This reverts commit 9919fac.

kochungcheon added 9 commits May 13, 2024 23:38

feat : Add shareLink

dbdd8e9

- link는 LocalDate.now + UUID로 구성 - link에 대한 중복 검사 진행 - link에 인덱스 설정

test: Add create shareLink

4df23ee

feat: Add downloadShareFile

fd17400

refacot: FileUtil.download

916c153

test: Add download shareFile

4f3be8a

test : 공유파일 다운로드 실패 테스트

3335d28

- 삭제된 파일 다운 시 다운 받지 못함

feat : Add 파일공유링크 삭제 스케줄러

28c28d2

test : 링크삭제 스케줄러

4432e2c

refactor : 파일 공유 다운로드 에러코드 정정

fc21cc3

kochungcheon requested review from VSFe, yunsik0115, MinYeongPark, kkkmj, semi-cloud, kariskan, seungh1024, pear96, swonny, Hugh-KR, MilanoBeer and youngsoosoo May 17, 2024 15:50

chore : 코드스멜 제거

733c6ad

kochungcheon commented May 17, 2024

View reviewed changes

chore : 코드스멜 제거

dc7a49f

kariskan reviewed May 18, 2024

View reviewed changes

kochungcheon added 5 commits May 18, 2024 20:00

refactor : PageRequest -> limit

ada9402

refactor : Duration.ofHours -> duration.toHours()

4d1e597

refactor : requestBody -> modelAttribute

b53fe7c

- Get request

refactor : createAt index

d87cae5

chore : code smell

3949129

kochungcheon added the enhancement New feature or request label May 18, 2024

pear96 reviewed May 19, 2024

View reviewed changes

kochungcheon added 2 commits May 20, 2024 00:41

refactor : 파일 공유 Response Dto에 유효성 검사 추가

f3fd490

chore : remove line

7c43fed

- settings.gradle

VSFe requested changes May 22, 2024

View reviewed changes

kochungcheon requested a review from VSFe May 22, 2024 14:45

kochungcheon added 6 commits May 22, 2024 23:46

refactor : mapping 형식 변경

70010f1

refactor : shareLink 생성 방법 변경

a3be9b6

refactor : cron 매일 3시간 단위로 수정

85002ab

refactor : findExpirations -> deleteByExpirations

26be5ed

- delete return id

refactor : DeleteLinkSchedler 3시간 단위 삭제 -> 5분 단위 삭제

38ed73f

- 삭제 빈도 상승으로 삭제 당 부하를 낮추기 위함

refactor : deleteByExpirations 도입에 따른 deleteById 삭제

32737ab

- 중복된 기능

kochungcheon merged commit 9919fac into base/chung May 29, 2024
2 checks passed

kochungcheon added a commit that referenced this pull request Jun 2, 2024

Revert "Feature/chung step5 (#12)"

78f957d

This reverts commit 9919fac.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/chung step5 #12

Feature/chung step5 #12

kochungcheon commented May 17, 2024 •

edited

Loading

kochungcheon May 17, 2024

kochungcheon May 17, 2024

kochungcheon May 17, 2024

kariskan left a comment •

edited

Loading

kariskan May 18, 2024

kochungcheon May 18, 2024

kariskan May 18, 2024

kochungcheon May 18, 2024

kariskan May 18, 2024

kochungcheon May 18, 2024

kariskan May 18, 2024

kochungcheon May 18, 2024

kariskan May 18, 2024

kochungcheon May 18, 2024

pear96 left a comment

pear96 May 19, 2024

kochungcheon May 19, 2024

pear96 May 19, 2024

kochungcheon May 19, 2024

VSFe left a comment

VSFe May 22, 2024

kochungcheon May 22, 2024

VSFe May 22, 2024

kochungcheon May 22, 2024

VSFe May 22, 2024

kochungcheon May 22, 2024

VSFe May 22, 2024

kochungcheon May 22, 2024 •

edited

Loading

sonarcloud bot commented May 22, 2024

Feature/chung step5 #12

Feature/chung step5 #12

Conversation

kochungcheon commented May 17, 2024 • edited Loading

구현 기능

파일 공유 링크 생성

파일 공유 다운로드

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kariskan left a comment • edited Loading

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

pear96 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

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

kochungcheon May 22, 2024 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented May 22, 2024

Quality Gate passed

kochungcheon commented May 17, 2024 •

edited

Loading

kariskan left a comment •

edited

Loading

kochungcheon May 22, 2024 •

edited

Loading