Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
@@ -0,0 +1,16 @@
package org.sopt.jaksim.category.dto;

import org.sopt.jaksim.mset.domain.Mset;
import org.sopt.jaksim.mset.domain.MsetDTO;
import org.sopt.jaksim.task.domain.Task;

import java.util.List;

public record TaskMsetLinkResponse(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

네이밍 좋네요 ! 👍🏻👍🏻

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

근데, TaskMsetLinkResponse에는 Task는 제외해도 됩니다 :) 모립세트만 포함되면 됩니다!

Task task,
List<MsetDTO> msets
) {
public static TaskMsetLinkResponse of(Task task, List<MsetDTO> msets) {
return new TaskMsetLinkResponse(task, msets);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.sopt.jaksim.category.domain.Category;
import org.sopt.jaksim.category.dto.CategoryCheckResponse;
import org.sopt.jaksim.category.dto.CategoryTaskLink;
import org.sopt.jaksim.category.dto.FilteredResourceResponse;
import org.sopt.jaksim.category.dto.TaskWithTaskTimer;
import org.sopt.jaksim.category.dto.*;
import org.sopt.jaksim.category.service.CategoryService;
import org.sopt.jaksim.mset.domain.CategoryMset;
import org.sopt.jaksim.mset.domain.Mset;
import org.sopt.jaksim.mset.domain.TaskMset;
import org.sopt.jaksim.mset.service.CategoryMsetService;
import org.sopt.jaksim.task.domain.Task;
import org.sopt.jaksim.task.domain.TaskTimer;
import org.sopt.jaksim.task.service.TaskService;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.sopt.jaksim.category.facade;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.sopt.jaksim.category.domain.Category;
import org.sopt.jaksim.category.dto.CategoryMsetLinkResponse;
import org.sopt.jaksim.category.dto.TaskMsetLinkResponse;
import org.sopt.jaksim.category.service.CategoryService;
import org.sopt.jaksim.mset.domain.CategoryMset;
import org.sopt.jaksim.mset.domain.Mset;
import org.sopt.jaksim.mset.domain.MsetDTO;
import org.sopt.jaksim.mset.domain.TaskMset;
import org.sopt.jaksim.mset.service.CategoryMsetService;
import org.sopt.jaksim.mset.service.MsetService;
import org.sopt.jaksim.mset.service.TaskMsetService;
import org.sopt.jaksim.task.domain.Task;
import org.sopt.jaksim.task.service.TaskService;
import org.springframework.stereotype.Service;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

@Slf4j
@Service
@RequiredArgsConstructor
public class TaskMsetFacade {
private final TaskService taskService;
private final MsetService msetService;
private final TaskMsetService taskMsetService;

public TaskMsetLinkResponse getFromOtherTask(Long taskId) {
Task task = taskService.getTaskById(taskId);
List<TaskMset> taskMsetList = taskMsetService.getByTaskId(taskId);
List<MsetDTO> msets = taskMsetList.stream() //

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

map을 두번 엮어서 간결하게 코드를 표현해주신 것 아주 좋네요 :) !!

.map(taskMset -> msetService.getMsetById(taskMset.getMsetId()))
.map(mset -> new MsetDTO(mset.getId(), mset.getName(), mset.getUrl()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이렇게 new 연산자로 객체를 무분별하게 만들지 않기 위해 우리는 패턴을 적용했었는데요 ...! 패턴을 적용해주면 좋을 것 같네요 :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

넵넵...

.collect(Collectors.toList());
return TaskMsetLinkResponse.of(task, msets);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
import org.apache.coyote.Response;
import org.sopt.jaksim.category.api.CategoryApi;
import org.sopt.jaksim.category.dto.CategoryMsetLinkResponse;
import org.sopt.jaksim.category.dto.TaskMsetLinkResponse;
import org.sopt.jaksim.category.facade.CategoryMsetFacade;
import org.sopt.jaksim.category.facade.TaskMsetFacade;
import org.sopt.jaksim.global.common.ApiResponseUtil;
import org.sopt.jaksim.global.common.BaseResponse;
import org.sopt.jaksim.global.message.SuccessMessage;
Expand All @@ -21,11 +23,19 @@
@RequestMapping("/api/v1")
public class MsetApiController implements MsetApi {
private final CategoryMsetFacade categoryMsetFacade;
private final TaskMsetFacade taskMsetFacade;

@GetMapping("/mset/categories/{categoryId}")
@Override
public ResponseEntity<BaseResponse<?>> getFromOtherCategory(@PathVariable("categoryId") Long categoryId) {
CategoryMsetLinkResponse response = categoryMsetFacade.getFromOtherCategory(categoryId);
return ApiResponseUtil.success(SuccessMessage.SUCCESS, response);
}

@GetMapping("/mset/tasks/{taskId}")
//@Override
public ResponseEntity<BaseResponse<?>> getFromOtherTask(@PathVariable("taskId") Long taskId) {
TaskMsetLinkResponse response = taskMsetFacade.getFromOtherTask(taskId);
return ApiResponseUtil.success(SuccessMessage.SUCCESS, response);
}
}
17 changes: 17 additions & 0 deletions jaksim/src/main/java/org/sopt/jaksim/mset/domain/MsetDTO.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.sopt.jaksim.mset.domain;

import lombok.Getter;

@Getter
public class MsetDTO {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

dto는 클라이언트와 controller 레벨에서 주고받는 객체의 형태입니다.

여기서 MsetDTO라는 객체는 TaskMsetLinkResponse 안의 mset 객체를 표현하기 위한 VO 인것으로 보여지는데요, 조금 더 적절한 이름으로 수정해보는 것은 어떨까요?

예를 들어, MsetInTask 같은 것이 있겠네요!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

넵 의견 감사합니다

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

record가 아닌 class를 사용한 이유가 있을까요?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

고치겠슴니다....

private final Long id;
private final String name;
private final String url;

public MsetDTO(Long id, String name, String url) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TaskMsetLinkResponse의 형태처럼 정적 팩토리 메소드 패턴을 이용한 것이 아닌, 매개변수가 있는 생성자를 이용한 특별한 이유가 있을까요?

또한, 매개변수를 모두 파라미터로 받는 생성자를 작성할 때는 Lombok의 @AllArgsConstructor가 있기 때문에 코드를 더 간결하게 만들 수도 있겠네요!

this.id = id;
this.name = name;
this.url = url;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.sopt.jaksim.mset.repository;

import org.sopt.jaksim.mset.domain.TaskMset;
import org.springframework.data.jpa.repository.JpaRepository;

import java.util.List;

public interface TaskMsetRepository extends JpaRepository<TaskMset, Long> {
List<TaskMset> findByTaskId(Long taskId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package org.sopt.jaksim.mset.service;

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.sopt.jaksim.mset.domain.CategoryMset;
import org.sopt.jaksim.mset.domain.TaskMset;
import org.sopt.jaksim.mset.repository.CategoryMsetRepository;
import org.sopt.jaksim.mset.repository.TaskMsetRepository;
import org.springframework.stereotype.Service;

import java.util.List;

@Slf4j
@Service
@RequiredArgsConstructor
public class TaskMsetService {
private final TaskMsetRepository taskMsetRepository;
public List<TaskMset> getByTaskId(Long taskId) {
return taskMsetRepository.findByTaskId(taskId);

@geniusYoo geniusYoo Jul 17, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

taskId가 존재하지 않는 경우를 핸들링해주는 코드가 필요할 것 같아요 !

Suggested change
return taskMsetRepository.findByTaskId(taskId);
taskMsetRepository.findByTaskId(taskId).orElseThrow(
() -> /* Error Handling */
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

추가적으로, orElseThrow()를 쓰려면 repository의 코드를 수정해야할텐데요 ! 같이 고민해보면 좋을 것 같네요 :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

아하 넵!

@eunseo5343 eunseo5343 Jul 18, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

말씀해주신대로

taskMsetRepository.findByTaskId(taskId).orElseThrow(
    () -> new NotFoundException(ErrorMessage.NOT_FOUND)
)

이렇게 바꾸고

repository는

Optional을 추가해줘야할 것 같군요!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.sopt.jaksim.category.domain.Category;
import org.sopt.jaksim.category.domain.CategoryTask;
import org.sopt.jaksim.category.dto.TaskWithTaskTimer;
import org.sopt.jaksim.category.repository.CategoryTaskRepository;
import org.sopt.jaksim.global.exception.NotFoundException;
import org.sopt.jaksim.global.message.ErrorMessage;
import org.sopt.jaksim.mset.repository.TaskMsetRepository;
import org.sopt.jaksim.mset.service.MsetService;
import org.sopt.jaksim.task.domain.Task;
import org.sopt.jaksim.task.dto.TaskCreateRequest;
import org.sopt.jaksim.task.domain.TodoTask;
Expand All @@ -24,6 +27,9 @@
public class TaskService {
private final TaskRepository taskRepository;
private final CategoryTaskRepository categoryTaskRepository;
private final TaskMsetRepository taskMsetRepository;
private final MsetService msetService;
private final TaskService taskService;

public Task getTaskById(Long taskId) {
return taskRepository.findById(taskId).orElseThrow(
Expand Down Expand Up @@ -64,5 +70,4 @@ public List<Task> getTasksByTodoTask(List<TodoTask> todoTaskList) {
List<Long> taskIdList = todoTaskList.stream().map(TodoTask::getTaskId).collect(Collectors.toList());
return taskRepository.findAllById(taskIdList);
}

}