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

[feat]: Bookmark to survey #376

Merged
merged 13 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
Expand Up @@ -28,6 +28,7 @@ public Long saveTargetAndGetId(String name, Instant date) {
.nickname(name)
.createdAt(date)
.updatedAt(date)
.imageUrl("empty image")
.build();
entityManager.persist(targetEntity);
return targetEntity.getId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ public abstract class AbstractSurveyTestSupporter {
private static final String API_VERSION = "/v1";
private static final Set<String> tableNameSet = Set.of("target", "survey", "form_question", "choice");

protected ResultActions bookmarkSurvey(String token, Long surveyId) throws Exception {
return mockMvc.perform(MockMvcRequestBuilders
.post(API_VERSION + "/surveys/" + surveyId + "/bookmarks")
dojinyou marked this conversation as resolved.
Show resolved Hide resolved
.accept(MediaType.APPLICATION_JSON)
dojinyou marked this conversation as resolved.
Show resolved Hide resolved
.header(HttpHeaders.AUTHORIZATION, token));
}

protected ResultActions createSurvey(String token, String content) throws Exception {
return mockMvc.perform(MockMvcRequestBuilders
.post(API_VERSION + "/surveys")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;

import me.nalab.survey.web.adaptor.bookmark.response.SurveyBookmarkResponse;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.ResultActions;

Expand Down Expand Up @@ -101,4 +102,16 @@ public static void assertIsSurveyDoesNotExists(ResultActions resultActions) thro
);
}

public static void assertIsBookmarked(ResultActions resultActions) throws Exception {
resultActions.andExpectAll(
status().isOk(),
content().contentType(MediaType.APPLICATION_JSON),
jsonPath("$.target_id").isString(),
jsonPath("$.survey_id").isString(),
jsonPath("$.nickname").isString(),
jsonPath("$.position").doesNotExist(),
jsonPath("$.image_url").doesNotExist()
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package me.nalab.luffy.api.acceptance.test.survey.bookmark;

import static me.nalab.luffy.api.acceptance.test.survey.SurveyAcceptanceValidator.assertIsBookmarked;

import java.time.LocalDateTime;
import java.time.ZoneOffset;
import me.nalab.auth.mock.api.MockUserRegisterEvent;
import me.nalab.luffy.api.acceptance.test.TargetInitializer;
import me.nalab.luffy.api.acceptance.test.survey.AbstractSurveyTestSupporter;
import me.nalab.luffy.api.acceptance.test.survey.RequestSample;
import me.nalab.survey.jpa.adaptor.findid.repository.SurveyIdFindJpaRepository;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.domain.EntityScan;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
import org.springframework.test.context.TestPropertySource;

@SpringBootTest
@AutoConfigureMockMvc
@TestPropertySource("classpath:h2.properties")
@ComponentScan("me.nalab")
@EnableJpaRepositories(basePackages = {"me.nalab"})
@EntityScan(basePackages = {"me.nalab"})
class SurveyBookmarkAcceptanceTest extends AbstractSurveyTestSupporter {

@Autowired
private ApplicationEventPublisher applicationEventPublisher;

@Autowired
private TargetInitializer targetInitializer;

@Autowired
private SurveyIdFindJpaRepository surveyIdFindJpaRepository;

@Test
@DisplayName("surveyBookmark api 는 token의 주인에게 survey를 북마크한다.")
void BOOKMARK_SURVEY_TO_TOKEN_OWNER() throws Exception {
// given
var targetId = targetInitializer.saveTargetAndGetId("luffy",
LocalDateTime.now().minusYears(24).toInstant(ZoneOffset.UTC));
var token = "luffy's-double-token";
applicationEventPublisher.publishEvent(MockUserRegisterEvent.builder()
.expectedToken(token)
.expectedId(targetId)
.build());
createSurvey(token, RequestSample.DEFAULT_JSON);

var surveyId = getSurveyId(targetId);

// when
var result = bookmarkSurvey(token, surveyId);

// then
assertIsBookmarked(result);
}

private Long getSurveyId(Long targetId) {
return surveyIdFindJpaRepository.findAllIdByTargetId(targetId).get(0);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
alter table target add `job` TEXT;
alter table target add image_url TEXT;

create table if not exists bookmarked_survey (
dojinyou marked this conversation as resolved.
Show resolved Hide resolved
target_id BIGINT not null,
bookmarked_survey_id BIGINT not null,
foreign key (target_id) references target (target_id),
foreign key (bookmarked_survey_id) references survey (survey_id)
);
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ public class JwtDecryptInterceptorConfigurer implements WebMvcConfigurer {
"/v1/reviewers/summary*",
"/v2/surveys/*/feedbacks",
"/v1/feedbacks/bookmarks",
"/v1/users"
"/v1/users",
"/v1/surveys/*/bookmarks",
};

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public class MockAuthConfigurer implements WebMvcConfigurer {
"/v1/reviewers*",
"/v1/reviewers/summary*",
"/v2/surveys/*/feedbacks",
"/v1/surveys/*/bookmarks",
};

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package me.nalab.core.data.target;

import javax.persistence.Column;
import javax.persistence.Embeddable;
import javax.persistence.JoinColumn;

@Embeddable
public class SurveyBookmarkEntity {

@Column(name = "bookmarked_survey_id")
@JoinColumn(name = "survey_id", nullable = false)
private Long surveyId;

}
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package me.nalab.core.data.target;

import java.util.Set;
import javax.persistence.CollectionTable;
import javax.persistence.Column;
import javax.persistence.ElementCollection;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.Table;

import lombok.AllArgsConstructor;
Expand Down Expand Up @@ -31,4 +35,17 @@ public class TargetEntity extends TimeBaseEntity {
@Column(name = "position")
private String position;

@Column(name = "job", columnDefinition = "TEXT")
private String job;

@Column(name = "image_url", columnDefinition = "TEXT")
private String imageUrl;

@ElementCollection
@CollectionTable(
name = "bookmarked_survey",
joinColumns = @JoinColumn(name = "target_id")
)
private Set<SurveyBookmarkEntity> bookmarkedSurveys;
dojinyou marked this conversation as resolved.
Show resolved Hide resolved

}
113 changes: 113 additions & 0 deletions support/e2e/v1_7_bookmark_survey.hurl
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
POST http://nalab-server:8080/v1/oauth/default # Default provider를 통해서 로그인 진행
{
"nickname": "bookmark_survey",
"email": "hello@123456"
}

HTTP 200
[Asserts]
header "Content-type" == "application/json"

jsonpath "$.access_token" exists
jsonpath "$.token_type" exists

[Captures]
token_type: jsonpath "$.token_type"
auth_token: jsonpath "$.access_token"

##########

POST http://nalab-server:8080/v1/surveys # 발급받은 토큰으로 survey를 생성한다.
Authorization: {{ token_type }} {{ auth_token }}
{
"question_count": 2,
"question": [
{
"type": "choice",
"form_type": "tendency",
"title": "저는 UI, UI, GUI 중에 어떤 분야를 가장 잘하는 것 같나요?",
"choices": [
{
"content": "UI",
"order": 1
},
{
"content": "UX",
"order": 2
},
{
"content": "GUI",
"order": 3
}
],
"max_selectable_count": 1,
"order": 1
},
{
"type": "short",
"form_type": "strength",
"title": "저는 UX, UI, GUI 중에 어떤 분야에 더 강점이 있나요?",
"order": 2
}
]
}

HTTP 201
[Asserts]
header "Content-type" == "application/json"

jsonpath "$.survey_id" exists

[Captures]
survey_id: jsonpath "$.survey_id"

##########

GET http://nalab-server:8080/v1/surveys/{{ survey_id }} # 생성된 survey를 조회한다.

HTTP 200
[Asserts]
header "Content-type" == "application/json"

jsonpath "$.survey_id" exists

jsonpath "$.target.id" exists
jsonpath "$.target.nickname" == "bookmark_survey"

jsonpath "$.question_count" == 2
jsonpath "$.question.[0].question_id" exists
jsonpath "$.question.[0].type" == "choice"
jsonpath "$.question.[0].form_type" == "tendency"
jsonpath "$.question.[0].title" == "저는 UI, UI, GUI 중에 어떤 분야를 가장 잘하는 것 같나요?"
jsonpath "$.question.[0].order" == 1
jsonpath "$.question.[0].max_selectable_count" == 1
jsonpath "$.question.[0].choices.[0].choice_id" exists
jsonpath "$.question.[0].choices.[0].content" == "UI"
jsonpath "$.question.[0].choices.[0].order" == 1
jsonpath "$.question.[0].choices.[1].choice_id" exists
jsonpath "$.question.[0].choices.[1].content" == "UX"
jsonpath "$.question.[0].choices.[1].order" == 2
jsonpath "$.question.[0].choices.[2].choice_id" exists
jsonpath "$.question.[0].choices.[2].content" == "GUI"
jsonpath "$.question.[0].choices.[2].order" == 3
jsonpath "$.question.[1].question_id" exists
jsonpath "$.question.[1].type" == "short"
jsonpath "$.question.[1].form_type" == "strength"
jsonpath "$.question.[1].title" == "저는 UX, UI, GUI 중에 어떤 분야에 더 강점이 있나요?"
jsonpath "$.question.[1].order" == 2

[Captures]
target_id: jsonpath "$.target.id"

##########

POST http://nalab-server:8080/v1/surveys/{{ survey_id }}/bookmarks
Authorization: {{ token_type }} {{ auth_token }}

HTTP 200
[Asserts]
header "Content-type" == "application/json"

jsonpath "$.target_id" == {{ target_id }}
jsonpath "$.survey_id" == {{ survey_id }}
jsonpath "$.nickname" == "bookmark_survey"
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package me.nalab.survey.application.common.survey.dto;

import lombok.Builder;

@Builder
public record SurveyBookmarkDto(
Long targetId,
Long surveyId,
String nickname,
String position,
String job,
String imageUrl
) {
dojinyou marked this conversation as resolved.
Show resolved Hide resolved

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package me.nalab.survey.application.port.in.web.bookmark;

import me.nalab.survey.application.common.survey.dto.SurveyBookmarkDto;

public interface SurveyBookmarkReplaceUseCase {

/**
* targetId에 해당하는 유저에게 survey를 북마크합니다.
* 이미 북마크되어있다면 북마크를 취소합니다.
*/
SurveyBookmarkDto flipBookmark(Long targetId, Long surveyId);
Copy link
Member

Choose a reason for hiding this comment

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

플립보다는 북마크하기 취소하기 나눠서 만들고 멱등하게 동작시키는게 동시성 제어에 더 편할 것 같은데

Copy link
Member Author

Choose a reason for hiding this comment

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

음.. 그런가?
나눠서 만들어도 동시요청 들어오면 갱신손실은 똑같이 발생할 수 있는거아냐? (survey 모듈쪽이 클린아키텍처라서 읽고 업데이트 해야함)
결국 두 방식 모두 락 획득해서 작업해야하는데, 북마크는 갱신손실 나도 문제없을거라 생각했고
클라 입장에서 현재 북마크상태 상관없이 flip 호출만하면 되니까 이게 편할거라 생각해서 이렇게 구현했어
별루??

Copy link
Member

Choose a reason for hiding this comment

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

첫 따닥은 어차피 둘 다 막아야되고 나머지는 updatedAt 같은 거 받아서 낙관적락 같은 느낌이 멱등성 가져가도 될 거 같은디?

Copy link
Member Author

Choose a reason for hiding this comment

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

d8d662c

낙관적 락 추가함 이 로직이 동시성 문제나면 한쪽은 실패하는게 맞는거 같아서 낙관적락 예외 retry는 안했어

근데, 내가 잘 이해를 못하고있는거 같은데 이 로직에서 멱등성 가져가는게 API 사용성 가져가는거보다 장점인게 아직 와닿지 않는달까나
나는 여기서 멱등하지 않는게 사용성을 높여주는거 같아서 (누르면 반대상태로 전환됨) 흠흠...

Copy link
Member

@dojinyou dojinyou Feb 23, 2024

Choose a reason for hiding this comment

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

이런 기능들은 클라이언트쪽에서 디바운스를 걸어주는게 좋음. 북마크했다가 바로 취소하거나 할 수 있기 때문에. 그러면 최종적 상태에 대한 요청을 보내도록 유도할 수 있음.
근데 지금 위와 같은 형태는 빠르게 사용자가 요청 했다가 취소 하면 요청 2번을 처리해야 함.

Copy link
Member Author

Choose a reason for hiding this comment

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

아하 이해했어 따닥 했을때 멱등성이 없으면, 클라이언트 요청을 맞춰주기위해서 요청처리를 2번해야하는구만
수정완
e08d6d8

나중에 취소 API도 하나 더 뚫겠음


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package me.nalab.survey.application.service.bookmark;

import lombok.RequiredArgsConstructor;
import me.nalab.survey.application.common.survey.dto.SurveyBookmarkDto;
import me.nalab.survey.application.exception.SurveyDoesNotExistException;
import me.nalab.survey.application.exception.TargetDoesNotExistException;
import me.nalab.survey.application.port.in.web.bookmark.SurveyBookmarkReplaceUseCase;
import me.nalab.survey.application.port.out.persistence.findfeedback.SurveyExistCheckPort;
import me.nalab.survey.application.port.out.persistence.findtarget.TargetFindPort;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
public class SurveyBookmarkReplaceService implements SurveyBookmarkReplaceUseCase {

private final TargetFindPort targetFindPort;
private final SurveyExistCheckPort surveyExistCheckPort;

@Override
@Transactional
public SurveyBookmarkDto flipBookmark(Long targetId, Long surveyId) {
var target = targetFindPort.findTargetById(targetId)
dojinyou marked this conversation as resolved.
Show resolved Hide resolved
.orElseThrow(() -> new TargetDoesNotExistException(targetId));

if (!surveyExistCheckPort.isExistSurveyBySurveyId(surveyId)) {
throw new SurveyDoesNotExistException(surveyId);
}

target.flipBookmark(surveyId);

return SurveyBookmarkDto.builder()
.surveyId(surveyId)
.targetId(target.getId())
.nickname(target.getNickname())
.job(target.getJob())
.imageUrl(target.getImageUrl())
.position(target.getPosition())
.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package me.nalab.survey.domain.target;

public record SurveyBookmark(
Long surveyId
) {

}
Loading
Loading