Skip to content

Commit

Permalink
전남대 BE_26조 9주차 코드리뷰 요청 (#69)
Browse files Browse the repository at this point in the history
* docs: Update README.md

불필요 내용 제거

* [Weekly/9/Hotfix/Issue-49] Issue-49(#49) 해결 (#58)

* fix: Member 조회시 null이 들어가는 증상 (#49)

* fix: mappedBy 수정

* [Weekly/9/Refactor/ExtractMemberServiceFactory] 멤버서비스 팩토리 추출 (#59)

* feat: Swagger 설정 (#61)

- Swagger 의존성 추가
- AUTH_WHITELIST 추가

* [Weekly/9/Refactor/RemoveEntityGettable] EntityGettable 제거 (#63)

* [Weekly/9/Refactor/Image] 이미지 도메인 리팩터링 (#64)

* [Weekly/9/Hotfix/BrokenTest-1101] 테스트코드 컴파일 되게 수정 (#65)

* [Weekly/9/Chore/DeploySetting] 프로필 변경 가능하게 수정 (#66)

---------

Co-authored-by: ariimo <[email protected]>
Co-authored-by: 조홍식 <[email protected]>
  • Loading branch information
3 people authored Nov 4, 2024
1 parent da1183a commit 2342924
Show file tree
Hide file tree
Showing 92 changed files with 1,133 additions and 571 deletions.
43 changes: 0 additions & 43 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,45 +1,2 @@
# Team26_BE
26조 백엔드


### 코드리뷰 요청 링크
https://www.notion.so/830084caab6c4d7bab816740974e263f#1ea368223b1849baa55991c4fbc55c0b

![image](https://github.com/user-attachments/assets/2f32ac86-0817-47fb-af3b-7f488c4e58ad)

### 기본적인 ERD 설계가 잘 되어있는지 궁금합니다.

- Image : curation 과 event에서는 여러 개의 이미지를 등록할 수 있습니다. 또한, curation에서는 아래와 같은 대표 이미지가 따로 필요합니다. 이 경우 이미지 테이블을 어떻게 생성해야 하는 지 궁금합니다.

![image](https://github.com/user-attachments/assets/9815bb8c-6e9a-4341-ac3b-157fc9a8accb)


- 일반 멤버와 큐레이터는 ‘큐레이터는 글을 쓸 수 있다’ 이 한 가지 기능 차이만 있습니다. 이 경우 큐레이터 테이블과 일반 멤버 테이블을 합쳐야 하는 걸까요?
- 상호베타적 관계가 실무에서 자주 사용되는지, 보통 어떻게 구현되는지 궁금합니다.
- Image 테이블을 보시면 attach_id 속성이 있는데 여기에는 curation/event/member id가 모두 들어갈 수 있습니다. 모든 테이블의 primary key로 UUID를 사용하기로 해서 혼용해서 저장할 수 있을 것 같아서입니다.
- 이런 경우 Jpa 코드로 어떻게 구현되어야 하는지 궁금합니다.
- ERD에 나와있는 host, curator, member를 jpa로 어떻게 매핑해야 하는지 궁금합니다.


### 빌더 패턴 관련하여 궁금합니다.

1. 4~5개 이상의 필드를 가지는 클래스가 있고, 그 필드가 모두 NotNull이어야 합니다. 즉 모든 필드를 초기화하는 생성자(AllArgsConstructor) 하나만 있으면 되는 상황인데, 이때 @Builder 어노테이션을 적용해 빌더를 생성하는 것이 적절한지 궁금합니다. `MyClass.builder().field1(val1).field2(val2)...field_n(val_n).build()` 와 같이 모든 필드에 대한 메서드를 호출해야만 하는데, 이런 경우 그냥 생성자 호출로 `MyClass(val1, val2, ... , val_n)` (파라미터수는 5개 이상일 수 있음) 와 같은 방법이 더 나을까요?
2. 위 상황에서 빌더로 필드를 초기화하는 과정에서 실수로 필수 필드를 빼먹은 경우, 즉 예를 들어 7번째 필드에 대한 `.field_7(val7)` 을 빼먹은 채 `.build()` 를 호출하였을 때 이를 감지하는 가장 나은 방법이 무엇인지 궁금합니다. `@NotNull` / `@Column(nullable = false)` / `Objects.requireNonNull(field)` 등등의 방법을 생각해 봤는데 더 나은 방법이 있을까요?
3. 빌더로 초기화하다가 필수 필드를 빼먹는 실수와, 빌더를 사용하지 않고 생성자만 있을 때 파라미터 순서나 개수 등을 헷갈리는 실수 중 어떤 실수가 더 치명적인지 궁금합니다. 2번과 같은 상황을 고려하여, 1번과 같은 클래스에 대해서는 빌더 패턴을 적용하지 않는 것이 좋을까요?


### Enum 관련하여 궁금합니다.

```java
package org.ktc2.cokaen.wouldyouin.domain;

public enum Category {
밴드,
연극,
뮤지컬,
원데이클래스,
전시회
}
```

- domain 패키지의 Category.java에서, 위처럼 Enum 타입으로 한글을 사용해도 괜찮은 건지 궁금합니다. 데이터베이스에서도 한글로 저장되게 할 예정입니다.
3 changes: 3 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ dependencies {

//for test
implementation 'org.springframework.boot:spring-boot-starter-thymeleaf'

//for Swagger
implementation 'org.springdoc:springdoc-openapi-starter-webmvc-ui:2.1.0'
}

tasks.named('test') {
Expand Down

This file was deleted.

12 changes: 0 additions & 12 deletions src/main/java/org/ktc2/cokaen/wouldyouin/Image/api/GetSubPath.java

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.ktc2.cokaen.wouldyouin.Image.application.ImageServiceFactory;
import org.ktc2.cokaen.wouldyouin._common.api.ApiResponse;
import org.ktc2.cokaen.wouldyouin._common.api.ApiResponseBody;
import org.ktc2.cokaen.wouldyouin._common.exception.FailToReadImageException;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseEntity;
Expand All @@ -30,7 +31,7 @@ public class ImageController {

@PostMapping("/images")
public ResponseEntity<ApiResponseBody<List<ImageResponse>>> uploadImages(@RequestParam List<MultipartFile> images,
@RequestParam ImageDomain imageDomain) {
@RequestParam(value = "type") ImageDomain imageDomain) {
return ApiResponse.ok(imageServiceFactory.getImageServiceByImageType(imageDomain).saveAndCreateImages(images));
}

Expand All @@ -39,7 +40,7 @@ public ResponseEntity<byte[]> getImage(@PathVariable String path) {
try {
return ResponseEntity.status(HttpStatus.OK).body(Files.readAllBytes(Paths.get(path)));
} catch (IOException e) {
throw new RuntimeException("failed to get image.");
throw new FailToReadImageException();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
public enum ImageDomain {
MEMBER,
CURATION,
EVENT
EVENT,
ADVERTISEMENT
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@

import lombok.Builder;
import lombok.Getter;
import org.ktc2.cokaen.wouldyouin.Image.persist.Image;

@Getter
@Builder
public class ImageRequest {

private String url;
private Long size;
private String extension;

public static ImageRequest of(String url, Long size) {
public static ImageRequest of(String url, Long size, String extension) {
return ImageRequest.builder()
.url(url)
.size(size)
.extension(extension)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package org.ktc2.cokaen.wouldyouin.Image.application;

import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.ktc2.cokaen.wouldyouin.Image.api.ImageDomain;
import org.ktc2.cokaen.wouldyouin.Image.api.dto.ImageRequest;
import org.ktc2.cokaen.wouldyouin.Image.persist.AdvertisementImage;
import org.ktc2.cokaen.wouldyouin.Image.persist.AdvertisementImageRepository;
import org.ktc2.cokaen.wouldyouin.Image.persist.ImageRepository;
import org.ktc2.cokaen.wouldyouin._common.exception.EntityParamIsNullException;
import org.ktc2.cokaen.wouldyouin.advertisement.persist.Advertisement;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.web.multipart.MultipartFile;

@Service
@RequiredArgsConstructor
public class AdvertisementImageService extends ImageService<AdvertisementImage> {

@Value("${image.upload.ad.sub-path}")
private String subPath;
private final ImageStorage imageStorage;
private final AdvertisementImageRepository adImageRepository;

@Override
protected ImageRepository<AdvertisementImage> getImageRepository() {
return adImageRepository;
}

@Override
protected ImageDomain getImageDomain() {
return ImageDomain.ADVERTISEMENT;
}

@Override
protected String getSubPath() {
return subPath;
}

@Override
protected AdvertisementImage toEntity(ImageRequest imageRequest) {
return AdvertisementImage.builder()
.name(imageRequest.getUrl())
.size(imageRequest.getSize())
.build();
}

@Transactional
public AdvertisementImage saveAndCreateImage(MultipartFile image) {
String path = imageStorage.save(image, getSubPath());
return adImageRepository.save(toEntity(ImageRequest.of(path, image.getSize(), ImageStorage.getExtension(image))));
}

@Transactional
public void setAd(AdvertisementImage image, Advertisement ad) {
Optional.ofNullable(image).orElseThrow(() -> new EntityParamIsNullException(getImageDomain().name() + " image"));
Optional.ofNullable(ad).orElseThrow(() -> new EntityParamIsNullException("advertisement"));
image.setAdvertisement(ad);
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
package org.ktc2.cokaen.wouldyouin.Image.application;

import java.util.List;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.ktc2.cokaen.wouldyouin.Image.api.ImageDomain;
import org.ktc2.cokaen.wouldyouin.Image.api.dto.ImageRequest;
import org.ktc2.cokaen.wouldyouin.Image.persist.CurationImage;
import org.ktc2.cokaen.wouldyouin.Image.persist.CurationImageRepository;
import org.ktc2.cokaen.wouldyouin.Image.persist.ImageRepository;
import org.ktc2.cokaen.wouldyouin._common.api.EntityGettable;
import org.ktc2.cokaen.wouldyouin._common.exception.EntityParamIsNullException;
import org.ktc2.cokaen.wouldyouin.curation.persist.CurationCard;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class CurationImageService extends ImageService<CurationImage> implements EntityGettable<List<Long>, List<CurationImage>> {
public class CurationImageService extends ImageService<CurationImage> {

private final CurationImageRepository curationImageRepository;

Expand All @@ -26,25 +28,27 @@ public ImageRepository<CurationImage> getImageRepository() {
}

@Override
protected String getSubPath() {
return subPath;
protected ImageDomain getImageDomain() {
return ImageDomain.CURATION;
}

@Override
protected ImageDomain getImageDomain() {
return ImageDomain.CURATION;
protected String getSubPath() {
return subPath;
}

@Override
protected CurationImage mapToEntityFrom(ImageRequest imageRequest) {
protected CurationImage toEntity(ImageRequest imageRequest) {
return CurationImage.builder()
.name(imageRequest.getUrl())
.size(imageRequest.getSize())
.build();
}

@Override
public List<CurationImage> getByIdOrThrow(List<Long> ids) throws RuntimeException {
return ids.stream().map(id -> curationImageRepository.findById(id).orElseThrow(RuntimeException::new)).toList();
@Transactional
public void setCuration(CurationImage image, CurationCard curationCard) {
Optional.ofNullable(image).orElseThrow(() -> new EntityParamIsNullException(getImageDomain().name() + " image"));
Optional.ofNullable(curationCard).orElseThrow(() -> new EntityParamIsNullException("curationCard"));
image.setCurationCard(curationCard);
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
package org.ktc2.cokaen.wouldyouin.Image.application;

import java.util.List;
import java.util.Optional;
import lombok.RequiredArgsConstructor;
import org.ktc2.cokaen.wouldyouin.Image.api.ImageDomain;
import org.ktc2.cokaen.wouldyouin.Image.api.dto.ImageRequest;
import org.ktc2.cokaen.wouldyouin.Image.persist.CurationImage;
import org.ktc2.cokaen.wouldyouin.Image.persist.EventImage;
import org.ktc2.cokaen.wouldyouin.Image.persist.EventImageRepository;
import org.ktc2.cokaen.wouldyouin.Image.persist.ImageRepository;
import org.ktc2.cokaen.wouldyouin._common.api.EntityGettable;
import org.ktc2.cokaen.wouldyouin._common.exception.EntityNotFoundException;
import org.ktc2.cokaen.wouldyouin._common.exception.EntityParamIsNullException;
import org.ktc2.cokaen.wouldyouin.advertisement.persist.Advertisement;
import org.ktc2.cokaen.wouldyouin.curation.persist.CurationCard;
import org.ktc2.cokaen.wouldyouin.event.persist.Event;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@RequiredArgsConstructor
@Service
public class EventImageService extends ImageService<EventImage> implements EntityGettable<List<Long>, List<EventImage>> {
public class EventImageService extends ImageService<EventImage> {

private final EventImageRepository eventImageRepository;

Expand All @@ -25,29 +32,25 @@ public ImageRepository<EventImage> getImageRepository() {
return eventImageRepository;
}

@Override
protected String getSubPath() {
return subPath;
}

@Override
protected ImageDomain getImageDomain() {
return ImageDomain.EVENT;
}

@Override
protected EventImage mapToEntityFrom(ImageRequest imageRequest) {
return EventImage.builder().name(imageRequest.getUrl()).size(imageRequest.getSize()).build();
protected String getSubPath() {
return subPath;
}

@Override
public void delete(Long id) {
eventImageRepository.findById(id).orElseThrow(RuntimeException::new);
eventImageRepository.deleteById(id);
protected EventImage toEntity(ImageRequest imageRequest) {
return EventImage.builder().name(imageRequest.getUrl()).size(imageRequest.getSize()).build();
}

@Override
public List<EventImage> getByIdOrThrow(List<Long> ids) throws RuntimeException {
return ids.stream().map(id -> eventImageRepository.findById(id).orElseThrow(RuntimeException::new)).toList();
@Transactional
public void setEvent(EventImage image, Event event) {
Optional.ofNullable(image).orElseThrow(() -> new EntityParamIsNullException(getImageDomain().name() + " image"));
Optional.ofNullable(event).orElseThrow(() -> new EntityParamIsNullException("event"));
image.setEvent(event);
}
}
Loading

0 comments on commit 2342924

Please sign in to comment.