Carrot/#3 #14

sansan20535 · 2024-06-02T10:01:48Z

-close #13

이 주의 과제 ❗️

image upload 로직✅

sansan20535/carrot/src/main/java/clone/carrot/domain/Sell.java

Line 38 in 88748df

private String imageUrl;

imageUrl 필드를 추가했습니다.

sansan20535/carrot/src/main/java/clone/carrot/dto/SellCreateDto.java

Line 16 in 88748df

MultipartFile image

sansan20535/carrot/src/main/java/clone/carrot/service/SellService.java

Lines 28 to 48 in 88748df

    
           private final S3Service s3Service; 
        
           private static final String SELL_S3_UPLOAD_FOLDER = "sell/"; 
        
           @Transactional 
        
           public String createSell( 
        
                   Long customerId, SellCreateDto sellCreateDto 
        
           ) { 
        
               //orElseThrow()를 안 붙이면 실행이 안되는 이유 
        
               Customer customer = customerRepository.findById(customerId).orElseThrow( 
        
                       () -> new EntityNotFoundException("해당 고객님을 찾을 수 없습니다.") 
        
               ); 
        
               try { 
        
                   Sell sell = Sell.create(customer.getNickname(), sellCreateDto.title(), sellCreateDto.cost(), sellCreateDto.costPropose(), sellCreateDto.detail(), sellCreateDto.place(), sellCreateDto.soldOut() 
        
                           , s3Service.uploadImage(SELL_S3_UPLOAD_FOLDER, sellCreateDto.image())); 
        
                   sellRepository.save(sell); 
        
                   return sell.getId().toString(); 
        
               } catch (RuntimeException | IOException e) { 
        
                   throw new RuntimeException(e.getMessage()); 
        
               } 
        
           }

이미지를 받아 S3와 데이터베이스에 저장했습니다.
S3Service, AwsConfig는 6차 세미나의 코드를 참고했습니다.

S3Service : https://github.com/NOW-SOPT-SERVER/sansan20535/blob/88748dfddbb9a056d11511628ab820cdcfaf96bd/carrot/src/main/java/clone/carrot/external/S3Service.java
AwsConfig : https://github.com/NOW-SOPT-SERVER/sansan20535/blob/88748dfddbb9a056d11511628ab820cdcfaf96bd/carrot/src/main/java/clone/carrot/external/AwsConfig.java

image delete 로직✅

API

path : /api/v1/sell/{sellId}

pathvariable : sellId, 판매 물품 Id

sansan20535/carrot/src/main/java/clone/carrot/service/SellService.java

Lines 62 to 71 in 88748df

    
           @Transactional 
        
           public void deleteSell(Long sellId) { 
        
               Sell sell = findById(sellId); 
        
               try { 
        
                   s3Service.deleteImage(sell.getImageUrl()); 
        
               } catch (IOException e) { 
        
                   throw new RuntimeException(e.getMessage()); 
        
               } 
        
               sellRepository.delete(sell); 
        
           }

sellId를 통해 S3의 이미지를 먼저 삭제하고 데이터베이스에서 삭제를 했습니다.

요구사항 분석 ❗️

image upload 로직✅

image delete 로직✅

- 고객 등록 API 추가 - 판매 정보에 판매자의 별명과 판매여부 추가 - 판매 정보 중 판매자의 별명은 판매자의 아이디를 이용하여 판매자의 정보를 찾고 별명 저장

unanchoi

안녕하세요 ~ 클론코딩 과제를 리뷰하게 된 명예오비 최윤한이라고 합니다..!
저도 부족하지만 궁금한 점이나 아니면 제 생각에 대해서 리뷰 해봤습니다!! 잘 부탁드립니다 ㅎㅎ
API 테스트 하신 것 PR에 올려두신 부분 너무 좋은 것 같습니다 ~ !

unanchoi · 2024-06-03T15:17:27Z

carrot/build.gradle

+	//Multipart file
+	implementation("software.amazon.awssdk:bom:2.21.0")
+	implementation("software.amazon.awssdk:s3:2.21.0")
+}


//Multipart file 보다는 s3 혹은 aws 라고 표현해도 좋을 것 같습니다 !

좋은 의견 감사합니다!

unanchoi · 2024-06-03T15:18:52Z

carrot/src/main/java/clone/carrot/common/SuccessResponse.java

+        int status,
+        String message,
+
+        List<T> data


data를 담는 필드의 타입을 List<T>로 사용하신 이유가 있는지 궁금합니다!

2차시 클론코딩에서 지역별 판매 '리스트'를 반환하는 API를 구현하는 중에 '리스트'를 구현하는 것에 초점을 맞췄던 것 같습니다..! 말씀해주신 것을 생각하면 그냥 제네릭 타입 T를 사용하는 것이 더 적절한 것 같다고 생각하게 되었습니다!

API 명세에 따라 달라지는 부분일 것 같아서 ... API 기본 응답 포맷을 배열로 사용하는 것이 아니라면 T로 바꿔도 좋을 것 같아요 ~

넵 감사합니다~!

unanchoi · 2024-06-03T15:21:31Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+
+    private final HeartService heartService;
+
+    @Transactional


Service에서 작성하는 로직들이 Transaction 관리에 조금 더 이점이 있기 떄문에, @Transactional 어노테이션은 Controller가 아닌 Service 계층에서 처리해도 좋을 것 같습니다 ...!

엇 이 부분은 잘못 작성했던 것 같은데 미처 발견하지 못했던 것 같습니다! 감사합니다! Controller에 Transactional 어노테이션을 붙이는 것보다 Service에 붙이면 생기는 이점에 대해서 더 찾아보도록 하겠습니다!

해당 이점에 대해서는 다양한 자료가 있으니 찾아보시면 좋을 것 같습니다!

넵!! 찾아보니 JPA와 관련되어 있어서 더 공부하면 좋을 것 같습니다. 감사합니다 : )

unanchoi · 2024-06-03T15:22:41Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+        @RequestHeader("customerId") Long customerId,
+        @RequestHeader("sellId") Long sellId


Request Header에서 customerId, sellId를 받아오시는 이유가 있는지 궁금합니다 ..!🤔

Id라는 속성이 중요한 정보라 생각해서 Header에 포함하여 안전하게 넘긴다고 생각했습니다. 하지만 다른 분들의 코드를 보고 jwt를 배우면서, 이러한 Id는 객체를 식별하는 용도로 사용하는 정도고 PathVariable이나 RequestParam을 통해 받아오는 것이 더 적절한 것 같다고 생각하게 되었습니다!

REST API를 사용한다면 URL에 최대한 표현되면 좋을 것 같아요!

아하! 좋은 의견 감사합니다!! 바로 적용해보도록 하겠습니다.

unanchoi · 2024-06-03T15:23:06Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+        return ResponseEntity.status(HttpStatus.CREATED)
+                .header("Location", heartService.create(customerId, sellId))


ResponseEntity.created() 로 한번에 처리할 수 있을 것 같습니다 ~

엇 그런 방법을 생각치 못했습니다..! 감사합니다 : )

unanchoi · 2024-06-03T15:32:25Z

carrot/src/main/java/clone/carrot/external/AwsConfig.java

+    @Bean
+    public SystemPropertyCredentialsProvider systemPropertyCredentialsProvider() {
+        System.setProperty(AWS_ACCESS_KEY_ID, accessKey);
+        System.setProperty(AWS_SECRET_ACCESS_KEY, secretKey);
+        return SystemPropertyCredentialsProvider.create();


AWS에서 credential을 setting하는 방법이 있는데, 해당 방법 말고도 다른 방법이 어떤 방법이 있는지 알아봐도 좋을 것 같습니다 ~

좋은 정보 감사합니다!!

unanchoi · 2024-06-03T15:34:34Z

carrot/src/main/java/clone/carrot/service/SellService.java

+            sellRepository.save(sell);
+            return sell.getId().toString();
+        } catch (RuntimeException | IOException e) {
+            throw new RuntimeException(e.getMessage());


RuntimeException을 던지고 있는데, 해당 예외보다 개발하고 있는 프로젝트에서 사용하는 적절한 커스텀 Exception을 정의하여 그 Exception을 RuntimeException 대신에 사용해도 좋을 것 같아요! RuntimeException은 Java API 에서 정의된 예외이기 때문에, 예외 상황 추적이나 의도하지 않은 예외가 발생할 수도 있기 때문입니다 ~!

아하 그런 문제점이 있을 수 있군요..! 감사합니다!!

unanchoi · 2024-06-03T15:34:49Z

carrot/src/main/java/clone/carrot/external/S3Service.java

+    private String generateImageFileName() {
+        return UUID.randomUUID() + ".jpg";
+    }


uploadImage method에서 private method로 image의 사이즈와 확장자를 validation하고 있는데요. 이 때 확장자로 네가지를 제한하고 있는데, 현재 method로 이미지 파일 이름을 생성할 때 jpg만을 붙이고 있습니다. 이렇게 되면 .webp 확장자 파일을 올려도 jpg 이름으로 저장되기 때문에 이후에 조회할 때 문제가 생길수도 있을 것 같아요 ! 어떻게 해결하면 좋을까요 ? 🤔🤔

그러한 생각은 못했던 것 같습니다! 음.. 말씀해주신 것을 통해 생각해 봤을 땐 validateExtension에서 확장자를 반환하여 그 확장자를 붙이는 방법을 생각해보았습니다. 하지만 더 좋은 의견이나 제가 말씀드린 것 중 잘못된 부분이 있으면 수정해 나가도록 하겠습니다!! 감사합니다 : )

확장자에 맞춰서 파일 이름을 생성해주는 형태로 리팩토링 시도해보시면 좋을 것 같습니다 !!!!

아하! 감사합니다!!

unanchoi · 2024-06-03T15:35:30Z

carrot/src/main/java/clone/carrot/external/S3Service.java

+                .bucket(bucketName)
+                .key(key)
+                .contentType(image.getContentType())
+                .contentDisposition("inline")


contentDisposition을 inline으로 해두신 이유가 궁금합니다 ~

큰 이유 없이 세미나를 참고했습니다..! 하지만 말씀하신 것을 듣고 찾아보니 바로 실행과 첨부파일로 인식하여 확인을 하는 차이라는 것을 알게 되었습니다!! 감사합니다 ㅜㅜ

unanchoi · 2024-06-03T15:40:37Z

carrot/src/main/java/clone/carrot/domain/Place.java

+    Gwanak,
+    Gangnam,
+    Dongjak


enum 자체는

GWANAK ,GANGNAM, DONGJAK으로 선언하고, 실제 사용하는 값은 필드로 선언하는 방법도 있을 것 같습니다..!

아래는 예시코드 입니다!

public enum Place { GWANAK("Gwanak"), GANGNAM("Gangnam"), DONGJAK("Dongjak"), private String value; }

좋은 의견 감사합니다!! 혹시 이렇게 필드로 따로 선언했을 때 얻을 수 있는 편리함이나 이점이 따로 있을까요?!

Enum의 상수 자체를 그대로 사용하게 되면, 변경에 취약해집니다.
예를들어 현재는 Gwanak으로 되어있는 값을 gwanak으로 바꿔야한다고 가정했을 때, Enum의 상수 값을 그대로 사용하면 상수 자체를 변경해야합니다. 이렇게 되면 Place라는 Enum을 사용하는 모든 class의 코드가 변경되어야 합니다. 하지만 필드로 따로 선언해주면 필드안에 선언했던
GWANAK("Gwanak") -> GWANAK("gwanak')으로만 바꿔주면 되기 때문에 변경이 적어집니다.

자세하게 설명해 주셔서 감사합니다!! 바로 적용해 보도록 하겠습니다 : )

sansan20535

바쁘신 와중에도 자세하게 의견을 말씀해셔서 감사합니다!! 덕분에 디테일한 부분이나 당연하다고 생각했던 부분에 대해서도 다시 한 번 생각해볼 수 있었습니다 : )

sansan20535 · 2024-06-04T07:29:10Z

carrot/build.gradle

+	//Multipart file
+	implementation("software.amazon.awssdk:bom:2.21.0")
+	implementation("software.amazon.awssdk:s3:2.21.0")
+}


좋은 의견 감사합니다!

sansan20535 · 2024-06-04T07:29:23Z

carrot/src/main/java/clone/carrot/common/SuccessResponse.java

+        int status,
+        String message,
+
+        List<T> data


2차시 클론코딩에서 지역별 판매 '리스트'를 반환하는 API를 구현하는 중에 '리스트'를 구현하는 것에 초점을 맞췄던 것 같습니다..! 말씀해주신 것을 생각하면 그냥 제네릭 타입 T를 사용하는 것이 더 적절한 것 같다고 생각하게 되었습니다!

sansan20535 · 2024-06-04T07:29:32Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+
+    private final HeartService heartService;
+
+    @Transactional


엇 이 부분은 잘못 작성했던 것 같은데 미처 발견하지 못했던 것 같습니다! 감사합니다! Controller에 Transactional 어노테이션을 붙이는 것보다 Service에 붙이면 생기는 이점에 대해서 더 찾아보도록 하겠습니다!

sansan20535 · 2024-06-04T07:29:36Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+        @RequestHeader("customerId") Long customerId,
+        @RequestHeader("sellId") Long sellId


Id라는 속성이 중요한 정보라 생각해서 Header에 포함하여 안전하게 넘긴다고 생각했습니다. 하지만 다른 분들의 코드를 보고 jwt를 배우면서, 이러한 Id는 객체를 식별하는 용도로 사용하는 정도고 PathVariable이나 RequestParam을 통해 받아오는 것이 더 적절한 것 같다고 생각하게 되었습니다!

sansan20535 · 2024-06-04T07:30:05Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+        return ResponseEntity.status(HttpStatus.CREATED)
+                .header("Location", heartService.create(customerId, sellId))


엇 그런 방법을 생각치 못했습니다..! 감사합니다 : )

sansan20535 · 2024-06-04T07:44:43Z

carrot/src/main/java/clone/carrot/domain/Sell.java

+    private long cost;
+
+    private boolean costPropose;
+
+    private String detail;
+
+    @Enumerated(EnumType.STRING)
+    private Place place;
+
+    private boolean soldOut;
+
+    @ColumnDefault("0")
+    private long likeCount;


좋은 의견 감사합니다! 리팩토링하면서, 또 앞으로 코드를 작성하면서 적용해보도록 하겠습니다!

sansan20535 · 2024-06-04T07:45:28Z

carrot/src/main/java/clone/carrot/external/AwsConfig.java

+    @Bean
+    public SystemPropertyCredentialsProvider systemPropertyCredentialsProvider() {
+        System.setProperty(AWS_ACCESS_KEY_ID, accessKey);
+        System.setProperty(AWS_SECRET_ACCESS_KEY, secretKey);
+        return SystemPropertyCredentialsProvider.create();


좋은 정보 감사합니다!!

sansan20535 · 2024-06-04T07:58:03Z

carrot/src/main/java/clone/carrot/external/S3Service.java

+                .bucket(bucketName)
+                .key(key)
+                .contentType(image.getContentType())
+                .contentDisposition("inline")


큰 이유 없이 세미나를 참고했습니다..! 하지만 말씀하신 것을 듣고 찾아보니 바로 실행과 첨부파일로 인식하여 확인을 하는 차이라는 것을 알게 되었습니다!! 감사합니다 ㅜㅜ

sansan20535 · 2024-06-04T08:06:42Z

carrot/src/main/java/clone/carrot/external/S3Service.java

+    private String generateImageFileName() {
+        return UUID.randomUUID() + ".jpg";
+    }


그러한 생각은 못했던 것 같습니다! 음.. 말씀해주신 것을 통해 생각해 봤을 땐 validateExtension에서 확장자를 반환하여 그 확장자를 붙이는 방법을 생각해보았습니다. 하지만 더 좋은 의견이나 제가 말씀드린 것 중 잘못된 부분이 있으면 수정해 나가도록 하겠습니다!! 감사합니다 : )

sansan20535 · 2024-06-04T08:07:24Z

carrot/src/main/java/clone/carrot/service/SellService.java

+            sellRepository.save(sell);
+            return sell.getId().toString();
+        } catch (RuntimeException | IOException e) {
+            throw new RuntimeException(e.getMessage());


아하 그런 문제점이 있을 수 있군요..! 감사합니다!!

sansan20535

시험기간 때문에 리뷰를 지금에서야 보았습니다.. 좋은 말씀 많이 해주셨는데 너무 늦게 봐서 죄송하고, 정성스럽게 써주신 만큼 많이 배울 수 있었습니다! 감사합니다 : )

sansan20535 · 2024-06-20T11:57:15Z

carrot/src/main/java/clone/carrot/common/SuccessResponse.java

+        int status,
+        String message,
+
+        List<T> data


넵 감사합니다~!

sansan20535 · 2024-06-20T12:02:10Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+
+    private final HeartService heartService;
+
+    @Transactional


넵!! 찾아보니 JPA와 관련되어 있어서 더 공부하면 좋을 것 같습니다. 감사합니다 : )

sansan20535 · 2024-06-20T12:02:48Z

carrot/src/main/java/clone/carrot/controller/HeartController.java

+        @RequestHeader("customerId") Long customerId,
+        @RequestHeader("sellId") Long sellId


아하! 좋은 의견 감사합니다!! 바로 적용해보도록 하겠습니다.

sansan20535 · 2024-06-20T12:03:36Z

carrot/src/main/java/clone/carrot/controller/SellController.java

+
+    @GetMapping("/list")
+    public ResponseEntity<SuccessResponse> findSellListByPlace(
+            @RequestParam("place") Place place


아 그런 방법이 또 있었군요! 감사합니다!! (Entity대신 객체를 정의해서 받아오면 이점은 개인적으로 질문하는 것으로..)

sansan20535 · 2024-06-20T12:05:51Z

carrot/src/main/java/clone/carrot/domain/Customer.java

+
+@Entity
+@Getter
+@NoArgsConstructor


아.. NoArgsConstructor에 대해서 자세하게는 알지 못해서 그 안에 access option이 있다는 것을 알게 되었습니다! 감사합니다!! 더 공부해보고 적용해보도록 하겠습니다 : )

sansan20535 · 2024-06-20T12:07:04Z

carrot/src/main/java/clone/carrot/domain/Place.java

+    Gwanak,
+    Gangnam,
+    Dongjak


자세하게 설명해 주셔서 감사합니다!! 바로 적용해 보도록 하겠습니다 : )

sansan20535 · 2024-06-20T12:07:35Z

carrot/src/main/java/clone/carrot/external/S3Service.java

+    private String generateImageFileName() {
+        return UUID.randomUUID() + ".jpg";
+    }


아하! 감사합니다!!

sansan20535 added 9 commits April 14, 2024 21:14

feat : create Sell POST API

062db50

feat : create Customer POST API and modify Sell POST API

c79bc94

- 고객 등록 API 추가 - 판매 정보에 판매자의 별명과 판매여부 추가 - 판매 정보 중 판매자의 별명은 판매자의 아이디를 이용하여 판매자의 정보를 찾고 별명 저장

[FEAT] : add Sell Find By Place GET API

bda77d4

feat : add Like feature

8e15c4d

feat : add Like feature2

5bac5b9

feat : start clonecoding/#3

9015f1d

refact : modify gitignore

329da4c

fixed untracked files

263947a

feat : Add image upload and delete function

88748df

unanchoi approved these changes Jun 3, 2024

View reviewed changes

sansan20535 commented Jun 4, 2024

View reviewed changes

sansan20535 commented Jun 20, 2024

View reviewed changes

bo-ram-bo-ram mentioned this pull request Jul 12, 2024

[fix] enum 참조 방식 수정 PICK-PLE/PICKPLE-server#62

Merged

	private final S3Service s3Service;
	private static final String SELL_S3_UPLOAD_FOLDER = "sell/";

	@Transactional
	public String createSell(
	Long customerId, SellCreateDto sellCreateDto
	) {
	//orElseThrow()를 안 붙이면 실행이 안되는 이유
	Customer customer = customerRepository.findById(customerId).orElseThrow(
	() -> new EntityNotFoundException("해당 고객님을 찾을 수 없습니다.")
	);
	try {
	Sell sell = Sell.create(customer.getNickname(), sellCreateDto.title(), sellCreateDto.cost(), sellCreateDto.costPropose(), sellCreateDto.detail(), sellCreateDto.place(), sellCreateDto.soldOut()
	, s3Service.uploadImage(SELL_S3_UPLOAD_FOLDER, sellCreateDto.image()));
	sellRepository.save(sell);
	return sell.getId().toString();
	} catch (RuntimeException \| IOException e) {
	throw new RuntimeException(e.getMessage());
	}

	}

	@Transactional
	public void deleteSell(Long sellId) {
	Sell sell = findById(sellId);
	try {
	s3Service.deleteImage(sell.getImageUrl());
	} catch (IOException e) {
	throw new RuntimeException(e.getMessage());
	}
	sellRepository.delete(sell);
	}

		@RequestHeader("customerId") Long customerId,
		@RequestHeader("sellId") Long sellId

		return ResponseEntity.status(HttpStatus.CREATED)
		.header("Location", heartService.create(customerId, sellId))

Carrot/#3 #14

Are you sure you want to change the base?

Carrot/#3 #14

Conversation

sansan20535 commented Jun 2, 2024

이 주의 과제 ❗️

image upload 로직✅

image delete 로직✅

API

path : /api/v1/sell/{sellId}

pathvariable : sellId, 판매 물품 Id

요구사항 분석 ❗️

image upload 로직✅

image delete 로직✅

unanchoi 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

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

sansan20535 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

sansan20535 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

unanchoi left a comment •

edited

Loading