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] feature/#251 최신 지도 조회 API 구현 #258

Merged
merged 29 commits into from
Aug 16, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
e9d7017
feat: 모아보기 기능 구현
junpakPark Aug 9, 2023
1de1b01
refactor: controller 필드 final 추가
junpakPark Aug 10, 2023
62ba5f1
refactor: URI 및 메서드 네이밍 변경
junpakPark Aug 10, 2023
10df876
test: atlas 테스트 추가
junpakPark Aug 10, 2023
53ec9b4
test: 테스트 코드 리팩터링
junpakPark Aug 10, 2023
8af79ae
feat: 최신 토픽 조회 기능 구현
junpakPark Aug 10, 2023
b075d3a
test: 테스트 추가 및 리팩터링
junpakPark Aug 10, 2023
b6e45e7
test: 테스트 코드 로직 오류 수정
junpakPark Aug 10, 2023
fd65611
Merge branch 'feature/atlas' into feature/#251-updatedTopic
junpakPark Aug 10, 2023
fe0225a
refactor: null값 validate 추가 및 개행 추가
junpakPark Aug 11, 2023
e8ac42a
test: 개행 및 DisplayName 수정
junpakPark Aug 11, 2023
8167a2c
test: 개행 및 DisplayName 수정
junpakPark Aug 11, 2023
573fc31
docs: restDocs snippets 추가
junpakPark Aug 11, 2023
4faba89
refactor: 메서드 순서 변경
junpakPark Aug 13, 2023
d4429bb
feat: Response에 isInAtlas 추가
junpakPark Aug 13, 2023
9905b19
refactor: convert 로직 간소화
junpakPark Aug 13, 2023
14296c4
refactor: 메서드 분리 및 정리
junpakPark Aug 14, 2023
1ef8041
chore: git conflict 해결
junpakPark Aug 14, 2023
f8b18d3
doc: restDocs 적용
junpakPark Aug 14, 2023
e530d4d
chore: git conflict 해결
junpakPark Aug 14, 2023
a7f1392
refactor: 메서드 네이밍 수정
junpakPark Aug 14, 2023
88ff4e6
refactor: git conflict 해결 및 Atlas 및 Bookmark 관련 조회 기능 MemberQueryServ…
junpakPark Aug 14, 2023
0c2d500
refactor: MemberController 내 토픽 및 핀 조회 메서드 분리
junpakPark Aug 14, 2023
e84a463
chore: git Conflict 해결
junpakPark Aug 16, 2023
8b479c3
refactor: 코드 리뷰 반영
junpakPark Aug 16, 2023
4d87239
refactor: 코드 리뷰 반영
junpakPark Aug 16, 2023
5e216db
refactor: DTO에 작성자 이름 추가
junpakPark Aug 16, 2023
7c45b11
Merge branch 'develop' into feature/#251-updatedTopic
junpakPark Aug 16, 2023
9c98f8a
chore: git conflict 해결
junpakPark Aug 16, 2023
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,74 @@
package com.mapbefine.mapbefine.atlas.application;

import com.mapbefine.mapbefine.atlas.domain.Atlas;
import com.mapbefine.mapbefine.atlas.domain.AtlasRepository;
import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.member.domain.MemberRepository;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import java.util.NoSuchElementException;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
public class AtlasCommandService {

private final TopicRepository topicRepository;
private final MemberRepository memberRepository;
private final AtlasRepository atlasRepository;

public AtlasCommandService(
TopicRepository topicRepository,
MemberRepository memberRepository,
AtlasRepository atlasRepository
) {
this.topicRepository = topicRepository;
this.memberRepository = memberRepository;
this.atlasRepository = atlasRepository;
}

public void addTopic(AuthMember authMember, Long topicId) {
Long memberId = authMember.getMemberId();

// TODO: 2023/08/10 memberId가 없는 경우 터짐 (Guest인 경우) (단, loginRequired로 일차적으로 막아놓긴 함)
if (isTopicAlreadyAdded(topicId, memberId)) {
return;
}

Topic topic = findTopicById(topicId);
validateReadPermission(authMember, topic);

Member member = findMemberById(memberId);

Atlas atlas = Atlas.from(topic, member);
atlasRepository.save(atlas);
}

private boolean isTopicAlreadyAdded(Long topicId, Long memberId) {
return atlasRepository.existsByMemberIdAndTopicId(memberId, topicId);
}

private Member findMemberById(Long memberId) {
return memberRepository.findById(memberId)
.orElseThrow(NoSuchElementException::new);
}

private Topic findTopicById(Long topicId) {
return topicRepository.findById(topicId)
.orElseThrow(NoSuchElementException::new);
}

private void validateReadPermission(AuthMember authMember, Topic topic) {
if (authMember.canRead(topic)) {
return;
}
throw new IllegalArgumentException("해당 지도에 접근권한이 없습니다.");
}

public void removeTopic(AuthMember authMember, Long topicId) {
atlasRepository.deleteByMemberIdAndTopicId(authMember.getMemberId(), topicId);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.mapbefine.mapbefine.atlas.application;

import com.mapbefine.mapbefine.atlas.domain.Atlas;
import com.mapbefine.mapbefine.atlas.domain.AtlasRepository;
import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.topic.dto.response.TopicResponse;
import java.util.List;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional(readOnly = true)
public class AtlasQueryService {

private final AtlasRepository atlasRepository;

public AtlasQueryService(AtlasRepository atlasRepository) {
this.atlasRepository = atlasRepository;
}

public List<TopicResponse> findTopicsByMember(AuthMember member) {
return atlasRepository.findAllByMemberId(member.getMemberId())
.stream()
.map(Atlas::getTopic)
.filter(member::canRead)
.map(TopicResponse::from)
.toList();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package com.mapbefine.mapbefine.atlas.domain;

import static lombok.AccessLevel.PROTECTED;

import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.topic.domain.Topic;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import java.util.Objects;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@NoArgsConstructor(access = PROTECTED)
@Getter
public class Atlas {

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@ManyToOne
@JoinColumn(name = "topic_id", nullable = false)
private Topic topic;

@ManyToOne
@JoinColumn(name = "member_id", nullable = false)
private Member member;

private Atlas(Topic topic, Member member) {
this.topic = topic;
this.member = member;
}

public static Atlas from(Topic topic, Member member) {
validateNotNull(topic, member);
return new Atlas(topic, member);
}

private static void validateNotNull(Topic topic, Member member) {
if (Objects.isNull(topic) || Objects.isNull(member)) {
throw new IllegalArgumentException("토픽과 멤버는 Null이어선 안됩니다.");
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.mapbefine.mapbefine.atlas.domain;

import java.util.List;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

@Repository
public interface AtlasRepository extends JpaRepository<Atlas, Long> {

List<Atlas> findAllByMemberId(Long memberId);

boolean existsByMemberIdAndTopicId(Long memberId, Long topicId);

void deleteByMemberIdAndTopicId(Long memberId, Long topicId);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package com.mapbefine.mapbefine.atlas.presentation;

import com.mapbefine.mapbefine.atlas.application.AtlasCommandService;
import com.mapbefine.mapbefine.atlas.application.AtlasQueryService;
import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.common.interceptor.LoginRequired;
import com.mapbefine.mapbefine.topic.dto.response.TopicResponse;
import java.util.List;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.DeleteMapping;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping("/atlas")
public class AtlasController {

private final AtlasCommandService atlasCommandService;
private final AtlasQueryService atlasQueryService;

public AtlasController(AtlasCommandService atlasCommandService, AtlasQueryService atlasQueryService) {
this.atlasCommandService = atlasCommandService;
this.atlasQueryService = atlasQueryService;
}

@LoginRequired
@GetMapping
public ResponseEntity<List<TopicResponse>> findTopicsFromAtlas(AuthMember member) {
List<TopicResponse> topicResponses = atlasQueryService.findTopicsByMember(member);

return ResponseEntity.ok(topicResponses);
}

@LoginRequired
@PostMapping("/{topicId}")
public ResponseEntity<Void> addTopicToAtlas(AuthMember authMember, @PathVariable Long topicId) {
atlasCommandService.addTopic(authMember, topicId);

return ResponseEntity.status(HttpStatus.CREATED).build();
}

@LoginRequired
@DeleteMapping("/{topicId}")
public ResponseEntity<Void> removeTopicFromAtlas(AuthMember authMember, @PathVariable Long topicId) {
atlasCommandService.removeTopic(authMember, topicId);

return ResponseEntity.noContent().build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ public interface PinRepository extends JpaRepository<Pin, Long> {

List<Pin> findByCreatorId(Long creatorId);

List<Pin> findAllByOrderByUpdatedAtDesc();
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.mapbefine.mapbefine.topic.application;

import com.mapbefine.mapbefine.auth.domain.AuthMember;
import com.mapbefine.mapbefine.pin.domain.Pin;
import com.mapbefine.mapbefine.pin.domain.PinRepository;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import com.mapbefine.mapbefine.topic.dto.response.TopicDetailResponse;
Expand All @@ -13,9 +15,11 @@
@Transactional(readOnly = true)
public class TopicQueryService {

private final PinRepository pinRepository;
private final TopicRepository topicRepository;

public TopicQueryService(final TopicRepository topicRepository) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋ 먼저 발견하셨군요 !

public TopicQueryService(PinRepository pinRepository, TopicRepository topicRepository) {
this.pinRepository = pinRepository;
this.topicRepository = topicRepository;
}

Expand Down Expand Up @@ -75,4 +79,13 @@ private void validateReadableTopics(AuthMember member, List<Topic> topics) {
}
}

public List<TopicResponse> findAllByOrderByUpdatedAtDesc(AuthMember member) {
return pinRepository.findAllByOrderByUpdatedAtDesc()
.stream()
.map(Pin::getTopic)
.distinct()
.filter(member::canRead)
.map(TopicResponse::from)
.toList();
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

질문이 있습니다 !

Topic에 대해서 distinct를 통해 중복을 제거하려고 하신 것 같은데, 해당 동작이 올바르게 수행 되나요 ?
Topic에 별다른 equals&hascode가 정의되어있지 않아서 여쭤봅니다 !
뭔가 될 것 같기도 한데... 잘 몰라서 ㅎㅎ..

만약, 올바르게 동작하지 않는다면
Repository에서 topicId에 대해 중복되지 않은 것들을 갖고오도록 하는 방법도 있을 것 같아요 !

Copy link
Collaborator Author

@junpakPark junpakPark Aug 10, 2023

Choose a reason for hiding this comment

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

Topic에 별다른 equals&Hashcode가 정의 되어있지 않더라도
같은 객체로 처리해주더라구요!
JPA 학습이 짧아서 왜 그런지 설명은 못하겠지만... (아마 영속성 컨텍스트에서 처리해줄 것 같습니다)
테스트 코드를 통해서 해당 동작이 정상적으로 수행되는 건 확인 완료했습니다 ㅎㅎ

Copy link
Collaborator

Choose a reason for hiding this comment

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

준팍 말씀대로, 영속성 컨텍스트에서 수행해주는 것이 맞는 것 같네요 ~

한 가지 걱정이되는 부분은, 서비스 특징상(핀 복사 등) findAll로 데이터를 가져올 경우 너무 많은 데이터를 가져오지 않을까 ? 싶습니다.
단순히 limit을 걸어두자니, 특정 토픽에 있는 핀들만 최근에 업데이트 되어 있을 경우 원하는 결과를 못가져올 것 같긴 해서..

Topic에도 updatedAt이라는 컬럼이 있다보니 위의 문제를 개선할 수 있지 않을까요 ?

처음 리뷰 당시에 이 부분을 인지하지 못했었네요 ㅠㅠ 죄송합니다 !

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 알기로, Topic의 updatedAt은
TOPIC 테이블에 존재한 row가 변화할 때만 업데이트 된다고 알고 있습니다.

그래서 지금과 같은 상황에서 Topic의 updatedAt으로 해결할 경우,
Topic이 새로 생기거나, Topic의 설명이나 이름이 바뀐 경우를 기준으로 나열되게 됩니다.

저희의 도메인 요구사항은
새로운 게시글이 추가되거나, 게시글의 내용이 변경된 게시판을 찾는 게 목적이지
새로 만들어진 게시판이나, 게시판의 이름이나 내용이 변경된 게시판을 찾는게 목적이 아니라는 점에서

Topic의 updatedAt을 사용하는 건 어려울 것으로 판단되는데
혹시 제가 잘못 이해한 부분이 있을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

현재로서는 topic 에 마지막으로 핀이 추가된 시각 등을 컬럼으로 두고, pin 을 추가할 때 해당 컬럼을 갱신한 뒤, 정렬하여 가져오는 것이 가장 부하가 적어보이긴하네용!

Copy link
Collaborator

Choose a reason for hiding this comment

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

비즈니스적인 updatedAt이 필요하다.. 정도로 결론을 내리면 될까요 허헛

Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ public ResponseEntity<List<TopicDetailResponse>> findByIds(
return ResponseEntity.ok(responses);
}

@GetMapping("/newest")
public ResponseEntity<List<TopicResponse>> findAllByOrderByUpdatedAtDesc(AuthMember member) {
List<TopicResponse> responses = topicQueryService.findAllByOrderByUpdatedAtDesc(member);

return ResponseEntity.ok(responses);
}

@LoginRequired
@PutMapping("/{topicId}")
public ResponseEntity<Void> update(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.mapbefine.mapbefine.atlas;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.http.HttpHeaders.AUTHORIZATION;

import com.mapbefine.mapbefine.atlas.domain.Atlas;
import com.mapbefine.mapbefine.atlas.domain.AtlasRepository;
import com.mapbefine.mapbefine.common.IntegrationTest;
import com.mapbefine.mapbefine.member.MemberFixture;
import com.mapbefine.mapbefine.member.domain.Member;
import com.mapbefine.mapbefine.member.domain.MemberRepository;
import com.mapbefine.mapbefine.member.domain.Role;
import com.mapbefine.mapbefine.topic.TopicFixture;
import com.mapbefine.mapbefine.topic.domain.Topic;
import com.mapbefine.mapbefine.topic.domain.TopicRepository;
import io.restassured.RestAssured;
import io.restassured.response.ExtractableResponse;
import io.restassured.response.Response;
import org.apache.tomcat.util.codec.binary.Base64;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpStatus;

public class AtlasIntegrationTest extends IntegrationTest {

@Autowired
TopicRepository topicRepository;

@Autowired
MemberRepository memberRepository;

@Autowired
AtlasRepository atlasRepository;

private Member member;
private Topic topic;
private String authHeader;

@BeforeEach
void setMember() {
member = memberRepository.save(
MemberFixture.create("other", "[email protected]", Role.USER)
);
topic = topicRepository.save(TopicFixture.createPublicAndAllMembersTopic(member));
authHeader = Base64.encodeBase64String(
("Basic " + member.getMemberInfo().getEmail()).getBytes()
);
}

@Test
@DisplayName("모아보기에 추가되어있는 지도 목록을 조회한다")
void findTopicsFromAtlas_Success() {
// when
ExtractableResponse<Response> response = RestAssured.given()
.log().all()
.header(AUTHORIZATION, authHeader)
.when().get("/atlas")
.then().log().all()
.extract();

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.OK.value());
}

@Test
@DisplayName("모아보기에 지도를 추가한다")
void addTopicToAtlas_Success() {
//given
Long topicId = topic.getId();

// when
ExtractableResponse<Response> response = RestAssured.given()
.log().all()
.header(AUTHORIZATION, authHeader)
.when().post("/atlas/{topicId}", topicId)
.then().log().all()
.extract();

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.CREATED.value());
}


@Test
@DisplayName("모아보기에 추가되어있는 지도를 삭제한다")
void removeTopicFromAtlas_Success() {
//given
Long topicId = topic.getId();
atlasRepository.save(Atlas.from(topic, member));

// when
ExtractableResponse<Response> response = RestAssured.given()
.log().all()
.header(AUTHORIZATION, authHeader)
.when().delete("/atlas/{topicId}", topicId)
.then().log().all()
.extract();

// then
assertThat(response.statusCode()).isEqualTo(HttpStatus.NO_CONTENT.value());
}
}
Loading