Skip to content

Commit

Permalink
[BE] refactor: question 타입과 답변 내용 일치 검증 및 매핑 로직 개선 (#958)
Browse files Browse the repository at this point in the history
* refactor: 타입에 대한 분기를 answerMapper 안에서 처리할 수 있도록

* refactor: 사용하지 않는 코드 제거

* refactor: 하나의 테스트에 한가지를 테스트하도록

* refactor: 추상화를 더욱 활용하도록

* style: 개행 통일

* refactor: getQuestionType 없애고 바로 supports를 사용하도록

* refactor: 다른 유형의 답변이 동시에 입력되는지에 대한 검증 삭제

* refactor: 함수 이름 변경

* style: 띄어쓰기 추가

* refactor: 접근제한자 변경

* refactor: 접근제한자 변경

- 팀의 컨벤션을 위해 & 유지보수 편한 코드를 위해 접근 제한자를 public & private 으로 통일

* refactor: AnswerMapper 변경

- abstract class -> interface

* fix: 컴파일 에러 해결

- abstract class -> interface 변환 하면서 extends -> implements 변경되지 않은 부분 변경
  • Loading branch information
nayonsoso authored Nov 22, 2024
1 parent fa7d3d3 commit 0522c5f
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ public record ReviewAnswerRequest(
@Nullable
String text
) {
public boolean hasTextAnswer() {
return text != null && !text.isEmpty();

public boolean hasNoText() {
return text == null || text.isBlank();
}

public boolean hasCheckboxAnswer() {
return selectedOptionIds != null && !selectedOptionIds.isEmpty();
public boolean hasNoSelectedOptions() {
return selectedOptionIds == null || selectedOptionIds.isEmpty();
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package reviewme.review.service.mapper;

import reviewme.template.domain.QuestionType;
import reviewme.review.domain.Answer;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.template.domain.QuestionType;

public interface AnswerMapper {

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package reviewme.review.service.mapper;

import org.springframework.stereotype.Component;
import reviewme.template.domain.QuestionType;
import reviewme.review.domain.CheckboxAnswer;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.review.service.exception.CheckBoxAnswerIncludedTextException;
import reviewme.template.domain.QuestionType;

@Component
public class CheckboxAnswerMapper implements AnswerMapper {
Expand All @@ -16,8 +15,8 @@ public boolean supports(QuestionType questionType) {

@Override
public CheckboxAnswer mapToAnswer(ReviewAnswerRequest answerRequest) {
if (answerRequest.text() != null) {
throw new CheckBoxAnswerIncludedTextException(answerRequest.questionId());
if (answerRequest.hasNoSelectedOptions()) {
return null;
}
return new CheckboxAnswer(answerRequest.questionId(), answerRequest.selectedOptionIds());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,10 @@ private List<Answer> getAnswersByQuestionType(ReviewRegisterRequest request) {

private Answer mapRequestToAnswer(Map<Long, Question> questions, ReviewAnswerRequest answerRequest) {
Question question = questions.get(answerRequest.questionId());

if (question == null) {
throw new SubmittedQuestionNotFoundException(answerRequest.questionId());
}

// TODO: 아래 코드를 삭제해야 한다
if (question.isSelectable() && answerRequest.selectedOptionIds() != null && answerRequest.selectedOptionIds().isEmpty()) {
return null;
}
if (!question.isSelectable() && answerRequest.text() != null && answerRequest.text().isEmpty()) {
return null;
}
// END

AnswerMapper answerMapper = answerMapperFactory.getAnswerMapper(question.getQuestionType());
return answerMapper.mapToAnswer(answerRequest);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
package reviewme.review.service.mapper;

import org.springframework.stereotype.Component;
import reviewme.template.domain.QuestionType;
import reviewme.review.domain.TextAnswer;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.review.service.exception.TextAnswerIncludedOptionItemException;
import reviewme.template.domain.QuestionType;

@Component
public class TextAnswerMapper implements AnswerMapper {
Expand All @@ -16,12 +15,9 @@ public boolean supports(QuestionType questionType) {

@Override
public TextAnswer mapToAnswer(ReviewAnswerRequest answerRequest) {
if (!answerRequest.hasTextAnswer()) {
if (answerRequest.hasNoText()) {
return null;
}
if (answerRequest.selectedOptionIds() != null) {
throw new TextAnswerIncludedOptionItemException(answerRequest.questionId());
}
return new TextAnswer(answerRequest.questionId(), answerRequest.text());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,23 @@
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.boot.test.system.CapturedOutput;
import org.springframework.boot.test.system.OutputCaptureExtension;
import reviewme.template.domain.QuestionType;
import reviewme.review.domain.Answer;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.template.domain.QuestionType;

@ExtendWith(OutputCaptureExtension.class)
class AnswerMapperFactoryTest {

private final AnswerMapper answerMapper = new AnswerMapper() {

@Override
public boolean supports(QuestionType questionType) {
return questionType == QuestionType.CHECKBOX;
public Answer mapToAnswer(ReviewAnswerRequest answerRequest) {
return null;
}

@Override
public Answer mapToAnswer(ReviewAnswerRequest answerRequest) {
return null;
public boolean supports(QuestionType questionType) {
return questionType == QuestionType.CHECKBOX;
}
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package reviewme.review.service.mapper;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.NullAndEmptySource;
import reviewme.review.domain.CheckboxAnswer;
import reviewme.review.domain.CheckboxAnswerSelectedOption;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.review.service.exception.CheckBoxAnswerIncludedTextException;

class CheckboxAnswerMapperTest {

Expand All @@ -28,16 +28,17 @@ class CheckboxAnswerMapperTest {
.containsExactly(1L, 2L, 3L);
}

@Test
void 체크박스_답변_요청에_텍스트가_포함되어_있으면_예외를_발생시킨다() {
@ParameterizedTest
@NullAndEmptySource
void 체크박스_답변이_비어있는_경우_null로_매핑한다(List<Long> selectedOptionIds) {
// given
ReviewAnswerRequest request = new ReviewAnswerRequest(1L, List.of(1L, 2L, 3L), "text");
ReviewAnswerRequest request = new ReviewAnswerRequest(1L, selectedOptionIds, null);
CheckboxAnswerMapper mapper = new CheckboxAnswerMapper();

// when
CheckboxAnswerMapper mapper = new CheckboxAnswerMapper();
CheckboxAnswer actual = mapper.mapToAnswer(request);

// then
assertThatThrownBy(() -> mapper.mapToAnswer(request))
.isInstanceOf(CheckBoxAnswerIncludedTextException.class);
assertThat(actual).isNull();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.assertAll;
import static reviewme.fixture.OptionGroupFixture.선택지_그룹;
import static reviewme.fixture.OptionItemFixture.선택지;
import static reviewme.fixture.QuestionFixture.서술형_옵션_질문;
Expand All @@ -16,12 +15,6 @@
import java.util.List;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import reviewme.template.domain.OptionGroup;
import reviewme.template.domain.OptionItem;
import reviewme.template.domain.Question;
import reviewme.template.repository.OptionGroupRepository;
import reviewme.template.repository.OptionItemRepository;
import reviewme.template.repository.QuestionRepository;
import reviewme.review.domain.CheckboxAnswer;
import reviewme.review.domain.Review;
import reviewme.review.domain.TextAnswer;
Expand All @@ -31,7 +24,13 @@
import reviewme.reviewgroup.domain.ReviewGroup;
import reviewme.reviewgroup.repository.ReviewGroupRepository;
import reviewme.support.ServiceTest;
import reviewme.template.domain.OptionGroup;
import reviewme.template.domain.OptionItem;
import reviewme.template.domain.Question;
import reviewme.template.domain.Section;
import reviewme.template.repository.OptionGroupRepository;
import reviewme.template.repository.OptionItemRepository;
import reviewme.template.repository.QuestionRepository;
import reviewme.template.repository.SectionRepository;
import reviewme.template.repository.TemplateRepository;

Expand Down Expand Up @@ -60,7 +59,7 @@ class ReviewMapperTest {
private TemplateRepository templateRepository;

@Test
void 텍스트가_포함된_리뷰를_생성한다() {
void 서술형_답변을_매핑한다() {
// given
ReviewGroup reviewGroup = reviewGroupRepository.save(리뷰_그룹());

Expand All @@ -81,7 +80,7 @@ class ReviewMapperTest {
}

@Test
void 체크박스가_포함된_리뷰를_생성한다() {
void 선택형_답변을_매핑한다() {
// given
ReviewGroup reviewGroup = reviewGroupRepository.save(리뷰_그룹());

Expand All @@ -106,57 +105,47 @@ class ReviewMapperTest {
}

@Test
void 필수가_아닌_질문에_답변이_없을_경우_답변을_생성하지_않는다() {
void 필수가_아닌_서술형_질문에_답변이_없으면_매핑하지_않는다() {
// given
ReviewGroup reviewGroup = reviewGroupRepository.save(리뷰_그룹());
Question question = questionRepository.save(서술형_옵션_질문());
Section section = sectionRepository.save(항상_보이는_섹션(List.of(question.getId())));
templateRepository.save(템플릿(List.of(section.getId())));

Question requiredTextQuestion = questionRepository.save(서술형_필수_질문());
Question optionalTextQuestion = questionRepository.save(서술형_옵션_질문());
ReviewAnswerRequest answerRequest = new ReviewAnswerRequest(question.getId(), null, "");
ReviewRegisterRequest reviewRegisterRequest = new ReviewRegisterRequest(
reviewGroup.getReviewRequestCode(), List.of(answerRequest));

Question requeiredCheckBoxQuestion = questionRepository.save(선택형_필수_질문());
OptionGroup optionGroup1 = optionGroupRepository.save(선택지_그룹(requeiredCheckBoxQuestion.getId()));
OptionItem optionItem1 = optionItemRepository.save(선택지(optionGroup1.getId()));
OptionItem optionItem2 = optionItemRepository.save(선택지(optionGroup1.getId()));
// when
Review review = reviewMapper.mapToReview(reviewRegisterRequest);

Question optionalCheckBoxQuestion = questionRepository.save(선택형_옵션_질문());
OptionGroup optionGroup2 = optionGroupRepository.save(선택지_그룹(optionalCheckBoxQuestion.getId()));
OptionItem optionItem3 = optionItemRepository.save(선택지(optionGroup2.getId()));
OptionItem optionItem4 = optionItemRepository.save(선택지(optionGroup2.getId()));
// then
assertThat(review.getAnswersByType(TextAnswer.class))
.extracting(TextAnswer::getQuestionId)
.isEmpty();
}

Section section = sectionRepository.save(항상_보이는_섹션(
List.of(requiredTextQuestion.getId(), optionalTextQuestion.getId(),
requeiredCheckBoxQuestion.getId(), optionalCheckBoxQuestion.getId())));
@Test
void 필수가_아닌_선택형_질문에_답변이_없으면_매핑하지_않는다() {
// given
ReviewGroup reviewGroup = reviewGroupRepository.save(리뷰_그룹());
Question question = questionRepository.save(선택형_옵션_질문());
OptionGroup optionGroup = optionGroupRepository.save(선택지_그룹(question.getId()));

Section section = sectionRepository.save(항상_보이는_섹션(List.of(question.getId())));
templateRepository.save(템플릿(List.of(section.getId())));

String textAnswer = "답".repeat(20);
ReviewAnswerRequest requiredTextAnswerRequest = new ReviewAnswerRequest(
requiredTextQuestion.getId(), null, textAnswer
);
ReviewAnswerRequest optionalTextAnswerRequest = new ReviewAnswerRequest(
optionalTextQuestion.getId(), null, ""
);
ReviewAnswerRequest requiredCheckBoxAnswerRequest = new ReviewAnswerRequest(
requeiredCheckBoxQuestion.getId(), List.of(optionItem1.getId()), null
);
ReviewAnswerRequest optionalCheckBoxAnswerRequest = new ReviewAnswerRequest(
optionalCheckBoxQuestion.getId(), List.of(), null
);
ReviewRegisterRequest reviewRegisterRequest = new ReviewRegisterRequest(reviewGroup.getReviewRequestCode(),
List.of(requiredTextAnswerRequest, optionalTextAnswerRequest,
requiredCheckBoxAnswerRequest, optionalCheckBoxAnswerRequest));
ReviewAnswerRequest answerRequest = new ReviewAnswerRequest(question.getId(), List.of(), null);
ReviewRegisterRequest reviewRegisterRequest = new ReviewRegisterRequest(
reviewGroup.getReviewRequestCode(), List.of(answerRequest));

// when
Review review = reviewMapper.mapToReview(reviewRegisterRequest);

// then
assertAll(
() -> assertThat(review.getAnswersByType(TextAnswer.class))
.extracting(TextAnswer::getQuestionId)
.containsExactly(requiredTextQuestion.getId()),
() -> assertThat(review.getAnswersByType(CheckboxAnswer.class))
.extracting(CheckboxAnswer::getQuestionId)
.containsExactly(requeiredCheckBoxQuestion.getId())
);
assertThat(review.getAnswersByType(CheckboxAnswer.class))
.extracting(CheckboxAnswer::getQuestionId)
.isEmpty();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,16 @@
package reviewme.review.service.mapper;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.NullSource;
import org.junit.jupiter.params.provider.ValueSource;
import reviewme.review.domain.TextAnswer;
import reviewme.review.service.dto.request.ReviewAnswerRequest;
import reviewme.review.service.exception.TextAnswerIncludedOptionItemException;

class TextAnswerMapperTest {

/*
TODO: Request를 추상화해야 할까요?
떠오르는 방법은 아래와 같습니다.
1: static factory method를 사용 -> 걷잡을 수 없어지지 않을까요?
2: 다른 방식으로 추상화 ?
*/

@Test
void 텍스트_답변을_요청으로부터_매핑한다() {
// given
Expand All @@ -31,16 +24,18 @@ class TextAnswerMapperTest {
assertThat(actual.getContent()).isEqualTo("text");
}

@Test
void 텍스트_답변_요청에_옵션이_포함되어_있으면_예외를_발생시킨다() {
@ParameterizedTest
@NullSource
@ValueSource(strings = {"", " "})
void 텍스트_답변이_비어있는_경우_null로_매핑한다(String text) {
// given
ReviewAnswerRequest request = new ReviewAnswerRequest(1L, List.of(1L), "text");
ReviewAnswerRequest request = new ReviewAnswerRequest(1L, null, text);

// when
TextAnswerMapper mapper = new TextAnswerMapper();
TextAnswer actual = mapper.mapToAnswer(request);

// then
assertThatThrownBy(() -> mapper.mapToAnswer(request))
.isInstanceOf(TextAnswerIncludedOptionItemException.class);
assertThat(actual).isNull();
}
}

0 comments on commit 0522c5f

Please sign in to comment.