plzprayme commented 3 years ago

56

변경 사항

메모 생성 API
메모 생성 API 테스트

기타

컨트롤러 쪽에 try - catch가 또 반복되네요
테스트 코드 쪽에 delete를 안했더니 데이터가 남아서 수정했습니다.
뭔가 거대한 레거시 코드를 짠 것 같아서 마음이 무겁습니다..

plzprayme commented 3 years ago

정렬 기준 컬럼을 ENUM으로 입력 받는게 Swagger로 보기에 좋을 것 같아서 추가 작업 후 다시 리뷰 신청하겠습니다

plzprayme commented 3 years ago

정렬 기준 컬럼 ENUM을 작성하려고 했는데 작업 단위가 너무 커져서 일단은 머지하고 새로운 브랜치에서 작업하겠습니다~ 리뷰 부탁드립니다~!

plzprayme commented 3 years ago

작업 단위가 엄청 커졌네요 ㄷㄷ 꼼꼼한 리뷰 감사합니다 또 리뷰 부탁드립니다.. 굾굾

plzprayme commented 3 years ago

아.. 회사에서 인턴기간동안 사용하라고 맥북을 주셔서 잠깐 맥북으로 작업을 해봤는데요. 맥북 인텔리제이 환경에 네이버 핵데이 코드스타일을 적용을 안했더니 들여쓰기 때문에 File Change가 너무 보기 힘들어졌네요 너무 죄송합니다 ㅠㅠ 😭😭😭

그리고 중간에 PR 올라가 있던 feat: 폴더 저장 시 유저 관계 매핑 브랜치를 머지해서 작업해서 File Change가 더 커졌네요 ... 후.. 진짜 죄송합니다.. 절 매우 쳐주세요.. 🙇‍♂️🙇‍♀️🙇‍♂️🙇‍♀️🙇‍♂️🙇‍♀️

=====================================================================================

의견 반영해서 Repository만 사용해서 작업해봤습니다. 그런데 Repository만 사용해서 작업하다보니 각 엔티티들의 관계를 매핑하는 코드가 반복적으로 사용되고 있습니다.

        // FolderService.save()
        @Transactional
    public Long save(Long userId, FolderDto.CreateFolderReq req) {
        Integer limit = pagingConfig.getFolderConfig().get("limit");

        if (foldersExistMoreThanLimit(userId, limit)) {
            throw new IndexOutOfBoundsException(
                String.format("폴더는 최대 %d개까지 생성할 수 있습니다!", limit));
        }
                // 여기부터 @@@@@@@@@@@@@@@@@@@@@@@@@@@
        User user = userRepository.findById(userId)
            .orElseThrow(ModelNotFoundException::new);
        Folder folder = folderRepository.save(req.toEntity());
        user.addFolder(folder);
                // 여기까지 @@@@@@@@@@@@@@@@@@@@@@@@@@@
        return folder.getId();
    }

위와 같은 코드도 있고 아래와 같은 코드도 생겼습니다.

       // MemoService.save()
       @Transactional
    public Memo save(Long userId, MemoDto.CreateMemoReq body) {
        Folder folder = createOrReadFolder(body);
        User user = userRepository.findById(userId)
            .orElseThrow(ModelNotFoundException::new);
                // 여기부터 @@@@@@@@@@@@@@@@@@@@@@@@@@@
        user.addFolder(folder);
        Memo memo = memoRepository.save(body.toMemoEntity());
        folder.addMemo(memo);
                // 여기까지 @@@@@@@@@@@@@@@@@@@@@@@@@@@
        return memo;
    }

    private Folder createOrReadFolder(MemoDto.CreateMemoReq body) {
        if (body.hasFolderId()) {
            Long folderId = body.getFolderId();
            return folderRepository.findById(folderId)
                .orElseThrow(ModelNotFoundException::new);
        }

        return folderRepository.save(body.toFolderEntity());
    }

세 가지 정도 해결방법을 생각해봤습니다.

의견 주신대로 각 엔티티를 저장할 때 관계 매핑까지 한번에 한다. ex) Folder folder = folderRepository.save(req.toEntity(), user);
각 엔티티들끼리 관계를 매핑해주는 Service를 정의한다. 작성을 한다면 관계의 부모는 Id를 받고 자식은 엔티티를 받도록 작성할 것 같습니다.
그냥 쓴다.

그런데 2번은 의견 주신대로 트랜잭션도 꼬이고 의존성도 복잡해질 것 같아서 아쉽고

1번으로 작업을 한다고 하면

CreateMemoReq를 MemoEntity 로 변경
CreateMemoReq에서 받은 FolderId로 FolderEntity 조회
FolderEntity와 MemoEntity 관계 매핑
MemoEntity 저장 순으로 작업을 할 것 같은데요. 뭔가 석연치가 않네요.

CreateMemoReq 에는 FolderEntity에 대한 정보도 있고, MemoEntity에 대한 정보도 있어서 Entity로 변환하는 함수를 두개를 선언했거든요.. 예를들어 CreateMemoReq.toMemoEntity() 메소드와 CreateMemoReq.toFolderEntity() 메소드 두개를 선언했었는데 이것도 다른 Dto들은 모두 toEntity 메소드를 하나만 가지는데 이 녀석만 toEntity 메소드가 두 개니까 조금 아쉽더라구요.

이 작업을 계속 수정하고 있는 이유가 Service에 리턴 타입이 달라서 였는데 Dto에서도 통일성이 깨지는 느낌?

그래서 CreateMemoReq 안에 Folder, Memo에 대한 클래스를 멤버 변수로 받으려고 시도해봤습니다.

public static class CreateMemoReq {
  Folder folder;
  Memo memo;
  private static class Folder {
    Long id;
    String title;
    FolderColor color;

    public Folder toEntity() { ... }
  }

  private static class Memo {
    String content;

        public Memo toEntity() { ... }
  }
}

이런식으로요.. 흠.. 네이밍 짓기도 어렵고 FolderDto.CreateMemoReq.Folder 이런식으로 접근을 해야하는데 어떤 방식으로 작업을 하는게 가장 좋을지 혼자서 판단하기가 어렵네요..

어떻게 생각하시나요? 더 좋은 방법이 있을까요? 저는 3번으로 쫌 마음이 쓰이네요 ^^;;

hongbin-dev commented 3 years ago

의사소통이 바로바로안되니 조금 답답한 부분이 있네요 ㅠ

양방향관계에서 어려운점이 있네요.

두 코드만봤을때 user를 꺼낼 필요가 없어보여요

첫번째

Folder folder = folderRepository.save(req.toEntity());

두번째

Memo memo = memoRepository.save(body.toMemoEntity());

올려주신 코드만보면 저렇게 넣으면 될 것 같은데..

그리고 꼭 request dto에서 엔티티를 만들어야하나요? dto에서 entity를 만드시는 이유가 있을까요?

hongbin-dev commented 3 years ago

음 제가 글을 제대로 안봤군요

유저에서 폴더를 더하는방법. JPA를 활용하는방법입니다. user내에서 addFolder 메서드에서 폴더엔티티를 만들 수 있을 것 같아요. (Folder가 영속이 안돼서 별도에 설정이 필요할 것 같네요.)

User user = userRepository.findById(userId)
            .orElseThrow(ModelNotFoundException::new);
user.addFolder(req.getBody, req.Title...);

이렇게하면 아래코드를 서비스레이어에서 제거하고 addFolder()에 넣을 수 있겠군요. folders.size를 검사해서요

if (foldersExistMoreThanLimit(userId, limit)) {
            throw new IndexOutOfBoundsException(
                String.format("폴더는 최대 %d개까지 생성할 수 있습니다!", limit));
        }

그냥 다 꺼내기

User user = userRepository.findById(userId)
        .orElseThrow(ModelNotFoundException::new);
Folder folder = folderRepository.save(req.toEntity());
user.addFolder(folder);

연관관계 끊기 폴더와 메모는 연관관계가 확실히 맞고 자연스러운 것 같은데요. 유저와 폴더의 연관관계는 모호하네요. 폴더와 유저의 연관관계를 끊고, 폴더에서 Userid만 들고있게하는 것도 방법입니다.
```
folder = new Folder(req, userId...)
FolderRepository.save(folder)
folder.addMemo(memoReq.get...)
```

DDD-Community / undertheriver-sgsg-backend

feat: Memo 생성 API #67

56

변경 사항

기타