Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BE] docs: 답변 하이라이트 API 문서 작성 #800

Merged
merged 8 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
3 changes: 3 additions & 0 deletions backend/src/docs/asciidoc/highlight-answers.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
==== 리뷰 하이라이트 변경 (추가, 삭제, 수정 포함)

operation::highlight-answer[snippets="curl-request,http-response,request-fields"]
Copy link
Contributor

Choose a reason for hiding this comment

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

request-cookies가 추가되어야겠네요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍🏻

6 changes: 5 additions & 1 deletion backend/src/docs/asciidoc/index.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,8 @@ include::review-list.adoc[]

=== 리뷰 모아보기

include::review-gather.adoc[]
include::review-gather.adoc[]

=== 답변 하이라이트

include::highlight-answers.adoc[]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package reviewme.highlight.controller;

import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RestController;
import reviewme.highlight.service.HighlightService;
import reviewme.highlight.service.dto.HighlightsRequest;

@RestController
@RequiredArgsConstructor
public class HighlightController {

private final HighlightService highlightService;

@PostMapping("/v2/highlight")
public ResponseEntity<Void> highlight(@Valid @RequestBody HighlightsRequest request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

세션 reviewRequestCode는 api 작성에서는 안 붙혀도 되는가요? (레독 잘 모릅니다...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

세션은 클라이언트 입장에서 JSESSIONID로만 구분해요. 이미 반영돼 있습니다 👍🏻

Copy link
Contributor

@nayonsoso nayonsoso Oct 9, 2024

Choose a reason for hiding this comment

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

@SessionAttribute("reviewRequestCode") String reviewRequestCode 이거 인자로 추가할 필욘 없나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

으엉 그거 말한거였네요 😢

highlightService.highlight(request);
return ResponseEntity.ok().build();
Copy link
Contributor

Choose a reason for hiding this comment

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

자원 개념으로 보았을 때 새로운 자원이 생성되었는데 created가 아닌 ok를 쓴 이유가 궁금해요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

생성 뿐만 아니라 수정 또한 이 API에서 일어납니다~

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package reviewme.highlight.service;

import org.springframework.stereotype.Service;
import reviewme.highlight.service.dto.HighlightsRequest;

@Service
public class HighlightService {

public void highlight(HighlightsRequest request) {
// TODO: implement method
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package reviewme.highlight.service.dto;

import jakarta.validation.constraints.NotNull;

public record HighlightIndexRangeRequest(

@NotNull(message = "시작 인덱스를 입력해주세요.")
Long startIndex,

@NotNull(message = "끝 인덱스를 입력해주세요.")
Long endIndex
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package reviewme.highlight.service.dto;

import jakarta.validation.constraints.NotNull;
import java.util.List;

public record HighlightRequest(

@NotNull(message = "답변 ID를 입력해주세요.")
Long answerId,

@NotNull(message = "하이라이트 된 라인을 입력해주세요.")
List<HighlightedLineRequest> lines
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분도 빈 리스트 받지 않는 게 좋을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

위 답변과 같아요~!

Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 빈리스트를 못받게 해야하지 않을까요?
왜냐면 answerId가 있는 request의 경우 lines가 빈 리스트가 올 수 없으니까요!
answerId가 있다 → 하이라이트가 있는 부분이어서 요청에 포함되었다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

최외곽 하나가 비어있다면 그냥 무시하고, 내부로 한 번 들어가기만 하면 매핑이 되어야한다는 이야기네요. 아래 질문 내용과 함께 반영할게요 🚀

Copy link
Contributor

Choose a reason for hiding this comment

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

댓츠 롸잇👍

) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package reviewme.highlight.service.dto;

import jakarta.validation.constraints.NotNull;
import java.util.List;

public record HighlightedLineRequest(

@NotNull(message = "인덱스를 입력해주세요.")
Long index,
Copy link
Contributor

Choose a reason for hiding this comment

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

(나머지 request도 동일하게) long 타입이어도 되지 않을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

request라 null이 들어올 수 있슴다~!

Copy link
Contributor

Choose a reason for hiding this comment

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

아차차😂


@NotNull(message = "하이라이트 범위를 입력해주세요.")
List<HighlightIndexRangeRequest> ranges
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package reviewme.highlight.service.dto;

import jakarta.validation.constraints.NotNull;
import java.util.List;

public record HighlightsRequest(

@NotNull(message = "하이라이트할 부분을 입력해주세요.")
List<HighlightRequest> highlights
Copy link
Contributor

Choose a reason for hiding this comment

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

빈 리스트가 오는 것도 막는 것이 나을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

삭제 요청을 같은 API에서 하니 빈 리스트도 괜찮지 않을까? 라는 생각입니다

Copy link
Contributor

Choose a reason for hiding this comment

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

삭제 요청을 같은 API에서 하니 빈 리스트도 괜찮지 않을까?

어떤 뜻인지 좀 더 설명해줄 수 있나요?
프론트에서 보낸다면 절대 빈 리스트가 들어올 수 없는데, 빈 리스트가 들어왔다면 잘못된 형식의 요청이니 dto에서 막는 게 맞다고 생각해서요!

Copy link
Contributor Author

@donghoony donghoony Oct 9, 2024

Choose a reason for hiding this comment

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

요청으로 항상 덮어쓴다고 생각하면 편할 듯 합니다 😁 빈 리스트가 들어온다면 하이라이트를 하나도 안 칠한 경우예요

Copy link
Contributor

Choose a reason for hiding this comment

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

validation annotation 이 붙은 컬렉션의 각 원소에도 valid 검증을 하기 위해서는 여기에됴 @Valid 붙여줘야 합니다!

Suggested change
@NotNull(message = "하이라이트할 부분을 입력해주세요.")
List<HighlightRequest> highlights
@NotNull(message = "하이라이트할 부분을 입력해주세요.") @Valid
List<HighlightRequest> highlights

지금은 아래 요청이 통과되네요

image

Copy link
Contributor

Choose a reason for hiding this comment

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

헉 그럼 다른데에도 붙여줘야겠네요? 확인해보니 속에는 안 붙인데가 있는데
예: ReviewRegisterRequest 속의 ReviewAnswerRequest를 위해서 붙여야함

Copy link
Contributor

@nayonsoso nayonsoso Oct 9, 2024

Choose a reason for hiding this comment

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

네네 그래서 이번에 싹 다 붙여도 될 것 같아용~

) {
Copy link
Contributor

Choose a reason for hiding this comment

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

현재는 각각 상위, 하위 dto가 여러 Highlight에 대한 dto, 하나의 Highlight에 대한 dto의 느낌의 네이밍인데,
Highlight가 question당 달린 하이라이트를 말하는 건지, answer 당 달린 하이라이트를 말하는 건지, line당을 말하는 건지 헷갈릴 수 있다고 생각해요!

따라서 현재 dto는 각 answer에 대한 하이라이트를 리스트로 갖고있는 dto이고, 하위 dto는 하나의 answer에 대한 하이라이트를 갖는 dto라는 의미를 담아 이런 네이밍은 어떨까요?

HighlightsRequest → AnswersHighlightRequest
HighlightRequest → AnswerHighlightRequest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네이밍에서 Answer에 대한 의존이 필요할까요? 하이라이트 그 자체의 자원 생성에 대해서만 보았어요.

Copy link
Contributor

@skylar1220 skylar1220 Oct 9, 2024

Choose a reason for hiding this comment

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

하이라이트 자체의 자원 생성에 있어서도 Highlight 테이블 내에 answerId가 있는 만큼 하이라이트-answer는 의존이 강하다고 생각했어요!
더불어 전 서비스, 컨트롤러에서 반환타입으로 해당 dto의 이름을 보았을 때 잘 이해가 되어야한다를 기준으로 생각했을 때 이런 네이밍이 어떨까 생각했는데, 이건 다른 사람들 의견도 궁금합니다!

Copy link
Contributor

Choose a reason for hiding this comment

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

저는 여기에 대해서는 강한 주장이 생기지 않는 것 같습니다😅 두 의견 다 일리가 있다고 생각해요.

}
8 changes: 7 additions & 1 deletion backend/src/test/java/reviewme/api/ApiTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.context.WebApplicationContext;
import reviewme.highlight.controller.HighlightController;
import reviewme.highlight.service.HighlightService;
import reviewme.review.controller.ReviewController;
import reviewme.review.service.GatheredReviewLookupService;
import reviewme.review.service.ReviewDetailLookupService;
Expand All @@ -44,7 +46,8 @@
ReviewGroupController.class,
ReviewController.class,
TemplateController.class,
SectionController.class
SectionController.class,
HighlightController.class
})
@ExtendWith(RestDocumentationExtension.class)
public abstract class ApiTest {
Expand Down Expand Up @@ -78,6 +81,9 @@ public abstract class ApiTest {
@MockBean
protected GatheredReviewLookupService gatheredReviewLookupService;

@MockBean
protected HighlightService highlightService;

Filter sessionCookieFilter = (request, response, chain) -> {
chain.doFilter(request, response);
HttpSession session = ((HttpServletRequest) request).getSession(false);
Expand Down
60 changes: 60 additions & 0 deletions backend/src/test/java/reviewme/api/HighlightApiTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package reviewme.api;

import static org.springframework.restdocs.cookies.CookieDocumentation.cookieWithName;
import static org.springframework.restdocs.cookies.CookieDocumentation.requestCookies;
import static org.springframework.restdocs.mockmvc.MockMvcRestDocumentation.document;
import static org.springframework.restdocs.payload.PayloadDocumentation.fieldWithPath;
import static org.springframework.restdocs.payload.PayloadDocumentation.requestFields;

import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
import org.springframework.restdocs.cookies.CookieDescriptor;
import org.springframework.restdocs.mockmvc.RestDocumentationResultHandler;
import org.springframework.restdocs.payload.FieldDescriptor;

class HighlightApiTest extends ApiTest {

@Test
void 존재하는_답변에_하이라이트를_생성한다() {
String request = """
{
"highlights": [{
"answerId": 3,
"lines": [{
"index": 5,
"ranges": [{
"startIndex": 6,
"endIndex": 9
}]
}]
}]
}
""";

CookieDescriptor[] cookieDescriptors = {
cookieWithName("JSESSIONID").description("세션 ID")
};

FieldDescriptor[] requestFields = {
fieldWithPath("highlights").description("하이라이트 목록"),
fieldWithPath("highlights[].answerId").description("답변 ID"),
fieldWithPath("highlights[].lines[].index").description("개행으로 구분되는 라인 번호, 0-based"),
fieldWithPath("highlights[].lines[].ranges[].startIndex").description("하이라이트 시작 인덱스, 0-based"),
fieldWithPath("highlights[].lines[].ranges[].endIndex").description("하이라이트 끝 인덱스, 0-based")
};

RestDocumentationResultHandler handler = document(
"highlight-answer",
requestFields(requestFields),
requestCookies(cookieDescriptors)
);

givenWithSpec().log().all()
.cookie("JSESSIONID", "AVEBNKLCL13TNVZ")
.body(request)
.when().post("/v2/highlight")
.then().log().all()
.apply(handler)
.status(HttpStatus.OK);
}
}