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] feat: Club 생성 참여 탈퇴 API #207

Merged
merged 19 commits into from
Jul 29, 2024
Merged
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
Expand Up @@ -7,6 +7,7 @@
import java.util.Collections;
import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.support.DefaultMessageSourceResolvable;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageNotReadableException;
Expand Down Expand Up @@ -41,7 +42,7 @@ public ResponseEntity<ApiResponse<ErrorResponse>> handle(MethodArgumentNotValidE
List<String> detail = exception.getBindingResult()
.getFieldErrors()
.stream()
.map(fieldError -> fieldError.getDefaultMessage())
.map(DefaultMessageSourceResolvable::getDefaultMessage)
.toList();
Comment on lines 44 to 46
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


ErrorResponse errorResponse = new ErrorResponse(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,24 @@
package com.woowacourse.friendogly.club.controller;

import com.woowacourse.friendogly.auth.Auth;
import com.woowacourse.friendogly.club.dto.request.FindSearchingClubRequest;
import com.woowacourse.friendogly.club.dto.request.SaveClubMemberRequest;
import com.woowacourse.friendogly.club.dto.request.SaveClubRequest;
import com.woowacourse.friendogly.club.dto.response.FindSearchingClubResponse;
import com.woowacourse.friendogly.club.dto.response.SaveClubMemberResponse;
import com.woowacourse.friendogly.club.dto.response.SaveClubResponse;
import com.woowacourse.friendogly.club.service.ClubCommandService;
import com.woowacourse.friendogly.club.service.ClubQueryService;
import com.woowacourse.friendogly.common.ApiResponse;
import jakarta.validation.Valid;
import java.net.URI;
import java.util.List;
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.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

Expand All @@ -24,7 +35,37 @@ public ClubController(ClubCommandService clubCommandService, ClubQueryService cl
}

@GetMapping("/searching")
public ResponseEntity<List<FindSearchingClubResponse>> findSearching(@Valid FindSearchingClubRequest request) {
return ResponseEntity.ok(clubQueryService.findSearching(request));
public ApiResponse<List<FindSearchingClubResponse>> findSearching(@Valid FindSearchingClubRequest request) {
return ApiResponse.ofSuccess(clubQueryService.findSearching(request));
}

@PostMapping
public ResponseEntity<ApiResponse<SaveClubResponse>> save(
@Auth Long memberId,
@Valid @RequestBody SaveClubRequest request
) {
SaveClubResponse response = clubCommandService.save(memberId, request);
return ResponseEntity.created(URI.create("/clubs" + response.id())).body(ApiResponse.ofSuccess(response));
}

@PostMapping("/{clubId}/members")
public ResponseEntity<ApiResponse<SaveClubMemberResponse>> saveClubMember(
@PathVariable Long clubId,
@Auth Long memberId,
@Valid @RequestBody SaveClubMemberRequest request
) {
SaveClubMemberResponse response = clubCommandService.saveClubMember(clubId, memberId, request);
return ResponseEntity.created(URI.create("/clubs/" + clubId + "/members/" + response.clubMemberId()))
.body(ApiResponse.ofSuccess(response));
}

@DeleteMapping("/{clubId}/members")
public ResponseEntity<Void> deleteClubMember(
@Auth Long memberId,
@PathVariable("clubId") Long clubId
) {
clubCommandService.deleteClubMember(clubId, memberId);
return ResponseEntity.noContent().build();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import com.woowacourse.friendogly.exception.FriendoglyException;
import com.woowacourse.friendogly.member.domain.Member;
import com.woowacourse.friendogly.pet.domain.Gender;
import com.woowacourse.friendogly.pet.domain.Pet;
import com.woowacourse.friendogly.pet.domain.SizeType;
import jakarta.persistence.CascadeType;
import jakarta.persistence.CollectionTable;
import jakarta.persistence.Column;
import jakarta.persistence.ElementCollection;
Expand All @@ -17,7 +19,11 @@
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.OneToMany;
import java.time.LocalDateTime;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import lombok.AccessLevel;
import lombok.Builder;
Expand Down Expand Up @@ -71,8 +77,14 @@ public class Club {
@Column(name = "status", nullable = false)
private Status status;

@OneToMany(mappedBy = "club", orphanRemoval = true, cascade = CascadeType.ALL)
List<ClubMember> clubMembers = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

클럽이랑 생명주기가 완전 같으니까 oneToMany적용해서 cascade할 수 있는게 장점이네요~!


@OneToMany(mappedBy = "club", orphanRemoval = true, cascade = CascadeType.ALL)
List<ClubPet> clubPets = new ArrayList<>();

@Builder
public Club(
private Club(
Copy link
Contributor

Choose a reason for hiding this comment

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

굿

String title,
String content,
String address,
Expand All @@ -97,7 +109,6 @@ public Club(
this.createdAt = createdAt;
}


private void validateOwner(Member owner) {
if (owner == null) {
throw new FriendoglyException("모임 방장 정보는 필수 입니다.");
Expand All @@ -112,9 +123,10 @@ public static Club create(
Member owner,
Set<Gender> allowedGender,
Set<SizeType> allowedSize,
String imageUrl
String imageUrl,
List<Pet> participatingPets
) {
return Club.builder()
Club club = Club.builder()
.title(title)
.content(content)
.address(address)
Expand All @@ -126,6 +138,84 @@ public static Club create(
.createdAt(LocalDateTime.now())
.imageUrl(imageUrl)
.build();
club.addClubMember(club.owner);
club.addClubPet(participatingPets);
return club;
}

public void addClubMember(Member newMember) {
validateAlreadyExists(newMember);
validateMemberCapacity();
ClubMember clubMember = ClubMember.create(this, newMember);
clubMembers.add(clubMember);
clubMember.updateClub(this);
}
Comment on lines +146 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

Pet -> ClubPet 변환 로직이 Club에 들어있는게 부자연스러운 듯 합니다.
Club은 중간 테이블인 ClubPet을 통해 Pet과 상호작용해야 하는데, 어떻게 보면 Pet과 직접적으로 상호작용 하고 있네요.
해당 변환 로직을 ClubPet으로 밀어넣고, Service 에서 변환로직 호출 후 넣어주는 편이 좋겠어요.

이쪽도 마찬가지 입니다. 변환 로직을 ClubMember로 넣어주면 책임을 나눌 수 있을 것 같습니다.
또 이렇게 변경하면 바로 아래에 있는 vlidateAlreadyExists() 메서드 내부에서 cotains() 를 사용할 수도 있게 되겠지요?

Copy link
Member Author

Choose a reason for hiding this comment

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

엔티티에 동등성을 아직 부여하고 있지 않아서 contains 함수를 사용하는건 별개의 문제인 것 같아용


private void validateAlreadyExists(Member newMember) {
if (clubMembers.stream()
.anyMatch(clubMember -> Objects.equals(clubMember.getMember().getId(), newMember.getId()))
) {
throw new FriendoglyException("이미 참여 중인 모임입니다.");
}
}

private void validateMemberCapacity() {
if (memberCapacity.isCapacityReached(countClubMember())) {
throw new FriendoglyException("최대 인원을 초과하여 모임에 참여할 수 없습니다.");
}
}

public void addClubPet(List<Pet> pets) {
List<ClubPet> clubPets = pets.stream()
.peek(this::validateParticipatePet)
.map(pet -> new ClubPet(this, pet))
.peek(clubPet -> clubPet.updateClub(this))
.toList();
this.clubPets.addAll(clubPets);
}

private void validateParticipatePet(Pet pet) {
if (!allowedGenders.contains(pet.getGender()) || !allowedSizes.contains(pet.getSizeType())) {
throw new FriendoglyException("모임에 데려갈 수 없는 강아지가 있습니다.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new FriendoglyException("모임에 데려갈 수 없는 강아지가 있습니다.");
throw new FriendoglyException("모임에 데려갈 수 있는 강아지가 없습니다.");

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
Member Author

Choose a reason for hiding this comment

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

지금 보니깐 , 모임에 데려갈 수 없는 강아지가 요청에 포함되어 있을 경우라서 저게 맞는 것 같아요.

}
}

public int countClubMember() {
return clubMembers.size();
}

public boolean isEmpty() {
return clubMembers.isEmpty();
}

public void removeClubMember(Member member) {
ClubMember target = clubMembers.stream()
.filter(e -> e.getMember().getId().equals(member.getId()))
.findAny()
.orElseThrow(() -> new FriendoglyException("참여 중인 모임이 아닙니다."));
Comment on lines +191 to +195
Copy link
Contributor

Choose a reason for hiding this comment

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

public void removeClubMember(ClubMember clubMember) {
        if (!clubMembers.remove(clubMember)) {
            throw new FriendoglyException("참여 중인 모임이 아닙니다.");
        }
        // ...
}

ClubMember를 인자로 받으면 로직을 더 간결하게 표현할 수 있겠습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

엔티티에 동등성 부여하는게 가능하다면 저렇게 깔끔하게 해결할 수 있을 것 같아요.
저도 구현하면서 id 동등성이 엔티티에 있다면 정말 편하겠다는 생각은 했지만, 아직 다른 팀원들의 생각을 듣지 못해서 하진 않았어요

Copy link
Contributor

@ehtjsv2 ehtjsv2 Jul 29, 2024

Choose a reason for hiding this comment

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

나는 id에만 동등성 부여하는 것 동의합니다.
우리는 객체지향을 하고는 있지만, DB와 밀접하게 연결이 되어있기 때문에 DB와 같은 관점에서 바라봐야 인지비용이 줄어든다고 생각합니다~!

<1>
id: 1, name: 도선, gender: 남
id: 2, name: 도선, gender: 여

<2>
id: 1, name: 도선, gender: 남
id: 1, name: 도선, gender: 여

1번은 id값이 다르기에 충분히 다른 사람이지만,
2번은 id를 제외한 값은 충분히 가변적인 값이고 수정이 가능한 값이기에 다른 사람이라고 보기에는 적절하지 않다고 생각

그렇기 때문에 id에만 동등성에 부여하는 것을 동의합니다!

clubMembers.remove(target);
if (isOwner(target)) {
updateOwner();
Comment on lines +197 to +198
Copy link
Contributor

Choose a reason for hiding this comment

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

여기 가독성 훨씬 나아졌네요

}
target.updateClub(null);
removeClubPets(member);
}

private void updateOwner() {
if (!isEmpty()) {
this.owner = clubMembers.get(0).getMember();
}
}

private boolean isOwner(ClubMember target) {
return owner.getId().equals(target.getMember().getId());
}

private void removeClubPets(Member member) {
List<ClubPet> participatingMemberPets = clubPets.stream()
.filter(clubPet -> clubPet.getPet().getMember().getId().equals(member.getId()))
.toList();
clubPets.removeAll(participatingMemberPets);
participatingMemberPets.forEach(clubPet -> clubPet.updateClub(null));
Copy link
Contributor

Choose a reason for hiding this comment

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

clubPet.updateClub(null);
이렇게 하면 해당 row가 지워지나요? 서비스에서 해당 row에 대한 delete 쿼리를 날려줘야할 것 같습니다.

Copy link
Member Author

@jimi567 jimi567 Jul 25, 2024

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
Expand Up @@ -2,13 +2,15 @@

import com.woowacourse.friendogly.exception.FriendoglyException;
import com.woowacourse.friendogly.member.domain.Member;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import java.time.LocalDateTime;
import lombok.AccessLevel;
import lombok.Builder;
import lombok.Getter;
Expand All @@ -31,12 +33,25 @@ public class ClubMember {
@JoinColumn(name = "member_id", nullable = false)
private Member member;

@Column(name = "create_at", nullable = false)
private LocalDateTime createdAt;

@Builder
public ClubMember(Club club, Member member) {
public ClubMember(Club club, Member member, LocalDateTime createdAt) {
validateClub(club);
validateMember(member);
validateCreateAt(createdAt);
this.club = club;
this.member = member;
this.createdAt = createdAt;
}

public static ClubMember create(Club club, Member member) {
return ClubMember.builder()
.club(club)
.member(member)
.createdAt(LocalDateTime.now())
.build();
}
Comment on lines +49 to 55
Copy link
Contributor

Choose a reason for hiding this comment

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

정적 팩토리를 사용할 거라면 생성자를 private으로 막으면 어떨까요?

Copy link
Contributor

Choose a reason for hiding this comment

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

전 정적 팩토리 메서드 제거해도 괜찮을 것 같아요.


private void validateClub(Club club) {
Expand All @@ -45,9 +60,19 @@ private void validateClub(Club club) {
}
}

private void validateCreateAt(LocalDateTime createdAt) {
if (createdAt == null) {
throw new FriendoglyException("방에 참가한 시간은 필수입니다.");
}
}

private void validateMember(Member member) {
if (member == null) {
throw new FriendoglyException("모임에 참여하는 회원 정보는 필수입니다.");
}
}

public void updateClub(Club club) {
this.club = club;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,8 @@ private void validatePet(Pet pet) {
throw new FriendoglyException("모임에 참여하는 회원의 반려견 정보는 필수입니다.");
}
}

public void updateClub(Club club) {
this.club = club;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,8 @@ private void validate(int value) {
);
}
}

public boolean isCapacityReached(int memberCount) {
return memberCount >= value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.woowacourse.friendogly.club.dto.request;

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

public record SaveClubMemberRequest(
@NotEmpty(message = "모임에 참석 할 강아지를 1마리 이상 선택해주세요.")
List<Long> participatingPetsId
) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.woowacourse.friendogly.club.dto.request;

import com.woowacourse.friendogly.pet.domain.Gender;
import com.woowacourse.friendogly.pet.domain.SizeType;
import jakarta.validation.constraints.Max;
import jakarta.validation.constraints.Min;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.Size;
import java.util.List;
import java.util.Set;

public record SaveClubRequest(
@NotBlank(message = "제목을 작성해주세요.")
@Size(min = 1, max = 100, message = "제목은 1글자 100글자 사이입니다.")
String title,

@NotBlank(message = "본문을 작성해주세요.")
@Size(min = 1, max = 1000, message = "본문은 1글자 1000글자 사이입니다.")
String content,

@NotBlank(message = "모임 사진을 등록해주세요.")
String imageUrl,

@NotBlank(message = "주소를 읽을 수 없습니다. 다시 시도해주세요.")
String address,

@NotEmpty(message = "모임에 참여가능한 성별을 선택해주세요.")
Set<Gender> allowedGenders,

@NotEmpty(message = "모임에 참여가능한 댕댕이 사이즈를 선택해주세요.")
Set<SizeType> allowedSizes,

@Min(value = 1, message = "모임 최대 인원은 1명 이상입니다.")
@Max(value = 5, message = "모임 최대 인원은 5명 이하입니다.")
int memberCapacity,

@NotEmpty(message = "모임에 참여할 댕댕이를 선택해주세요.")
List<Long> participatingPetsId
) {

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,17 @@ public record FindSearchingClubResponse(
String address,
Status status,
LocalDateTime createdAt,
Set<SizeType> allowedSize,
Set<Gender> allowedGender,
Set<SizeType> allowedSize,
int memberCapacity,
int currentMemberCount,
String imageUrl,
List<String> petImageUrls

) {

public FindSearchingClubResponse(
Club club,
int currentMemberCount,
List<String> petImageUrls
) {
this(
Expand All @@ -37,12 +37,12 @@ public FindSearchingClubResponse(
club.getAddress().getValue(),
club.getStatus(),
club.getCreatedAt(),
club.getAllowedSizes(),
club.getAllowedGenders(),
club.getAllowedSizes(),
club.getMemberCapacity().getValue(),
currentMemberCount,
club.countClubMember(),
club.getImageUrl(),
petImageUrls
);
}

}
Loading
Loading