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.

Sign up for GitHub

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

[Refactor] Anniversary 리팩터링 #86

Merged
merged 4 commits into from
Nov 26, 2023
Merged

[Refactor] Anniversary 리팩터링 #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5eb2f94
[Fix] ResponseStatus 어노테이션 삭제 - #67
Juser0 Nov 22, 2023
2515013
Merge branch 'main' of https://github.com/Favor-Gift-Reminder/Favor-S…
Juser0 Nov 24, 2023
968d79e
[Refactor] Anniversary Entity 리팩터링 - #67
Juser0 Nov 24, 2023
dec6a78
[Fix] stream 내부 생성자 -> 정적 팩토리 메소드로 수정
Juser0 Nov 24, 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
16 changes: 10 additions & 6 deletions favor/src/main/java/com/favor/favor/anniversary/Anniversary.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,27 @@ public class Anniversary extends TimeStamped {
private Long anniversaryNo;

private String anniversaryTitle;
public void setAnniversaryTitle(String anniversaryTitle){ this.anniversaryTitle = anniversaryTitle; }

private LocalDate anniversaryDate;
public void setAnniversaryDate(LocalDate anniversaryDate){ this.anniversaryDate = anniversaryDate; }

private Integer category;
public void setCategory(AnniversaryCategory anniversaryCategory){
this.category = anniversaryCategory.getType();
}

private Boolean isPinned;
public void setIsPinned(Boolean isPinned){ this.isPinned = isPinned; }

@ManyToOne(cascade = CascadeType.REFRESH, fetch = FetchType.EAGER)
@JoinColumn(name = "user_user_no")
private User user;

public void updateAnniversaryTitle(String anniversaryTitle){ this.anniversaryTitle = anniversaryTitle; }

public void updateAnniversaryDate(LocalDate anniversaryDate){ this.anniversaryDate = anniversaryDate; }

public void updateCategory(AnniversaryCategory anniversaryCategory){
this.category = anniversaryCategory.getType();
}

public void updateIsPinned(Boolean isPinned){ this.isPinned = isPinned; }

@Builder
public Anniversary(String anniversaryTitle, LocalDate anniversaryDate, Integer category, Boolean isPinned, User user) {
this.anniversaryTitle = anniversaryTitle;
Expand Down
53 changes: 11 additions & 42 deletions favor/src/main/java/com/favor/favor/anniversary/AnniversaryController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@

import java.util.List;

import static com.favor.favor.common.DefaultResponseDto.resWithData;
import static com.favor.favor.common.DefaultResponseDto.resWithoutData;

@Api(tags = "Anniversary")
@RestController
@RequestMapping("/anniversaries")
Expand All @@ -37,21 +34,17 @@ public class AnniversaryController {
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.CREATED)
@PostMapping
public ResponseEntity<DefaultResponseDto<Object>> createAnniversary(
@RequestBody AnniversaryRequestDto anniversaryRequestDto,
@AuthenticationPrincipal User loginUser){

Long userNo = loginUser.getUserNo();

anniversaryService.isExistingUserNo(userNo);

Anniversary anniversary = anniversaryService.createAnniversary(anniversaryRequestDto, userNo);
AnniversaryResponseDto dto = anniversaryService.returnDto(anniversary);
AnniversaryResponseDto anniversaryResponseDto = anniversaryService.createAnniversary(userNo, anniversaryRequestDto);

return ResponseEntity.status(201)
.body(resWithData("ANIVERSARY_CREATED", "기념일 생성 완료", dto));
return ResponseEntity.status(HttpStatus.CREATED)
.body(DefaultResponseDto.from("ANNIVERSARY_CREATED", "기념일 생성 완료", anniversaryResponseDto));
}

@ApiOperation("기념일 조회")
Expand All @@ -66,17 +59,13 @@ public ResponseEntity<DefaultResponseDto<Object>> createAnniversary(
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.OK)
@GetMapping("/{anniversaryNo}")
public ResponseEntity<DefaultResponseDto<Object>> readAnniversary(
@PathVariable Long anniversaryNo){

anniversaryService.isExistingAnniversaryNo(anniversaryNo);
Anniversary anniversary = anniversaryService.findAnniversaryByAnniversaryNo(anniversaryNo);
AnniversaryResponseDto dto = anniversaryService.returnDto(anniversary);
AnniversaryResponseDto anniversaryResponseDto = anniversaryService.readAnniversary(anniversaryNo);

return ResponseEntity.status(200)
.body(resWithData("ANNIVERSARY_FOUND", "기념일 조회 완료", dto));
return ResponseEntity.ok(DefaultResponseDto.from("ANNIVERSARY_FOUND", "기념일 조회 완료", anniversaryResponseDto));
}

@ApiOperation("기념일 수정")
Expand All @@ -91,20 +80,14 @@ public ResponseEntity<DefaultResponseDto<Object>> readAnniversary(
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.OK)
@PatchMapping("/{anniversaryNo}")
public ResponseEntity<DefaultResponseDto<Object>> updateAnniversary(
@RequestBody AnniversaryUpdateRequestDto anniversaryUpdateRequestDto,
@PathVariable Long anniversaryNo){

anniversaryService.isExistingAnniversaryNo(anniversaryNo);

Anniversary anniversary = anniversaryService.findAnniversaryByAnniversaryNo(anniversaryNo);
anniversaryService.updateAnniversary(anniversaryUpdateRequestDto, anniversary);
AnniversaryResponseDto dto = anniversaryService.returnDto(anniversary);
AnniversaryResponseDto anniversaryResponseDto = anniversaryService.updateAnniversary(anniversaryNo, anniversaryUpdateRequestDto);

return ResponseEntity.status(200)
.body(resWithData("ANNIVERSARY_UPDATED", "기념일 수정 완료", dto));
return ResponseEntity.ok(DefaultResponseDto.from("ANNIVERSARY_UPDATED", "기념일 수정 완료", anniversaryResponseDto));
}

@ApiOperation("기념일 핀 여부 수정")
Expand All @@ -119,19 +102,13 @@ public ResponseEntity<DefaultResponseDto<Object>> updateAnniversary(
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.OK)
@PatchMapping("/pin/{anniversaryNo}")
public ResponseEntity<DefaultResponseDto<Object>> updateIsPinned(
@PathVariable Long anniversaryNo){

anniversaryService.isExistingAnniversaryNo(anniversaryNo);
AnniversaryResponseDto anniversaryResponseDto = anniversaryService.updateIsPinned(anniversaryNo);

Anniversary anniversary = anniversaryService.findAnniversaryByAnniversaryNo(anniversaryNo);
anniversaryService.updateIsPinned(anniversary);
AnniversaryResponseDto dto = anniversaryService.returnDto(anniversary);

return ResponseEntity.status(200)
.body(resWithData("ANNIVERSARY_PIN_UPDATED", "기념일 핀 수정 완료", dto));
return ResponseEntity.ok(DefaultResponseDto.from("ANNIVERSARY_PIN_UPDATED", "기념일 핀 수정 완료", anniversaryResponseDto));
}

@ApiOperation("기념일 삭제")
Expand All @@ -146,19 +123,13 @@ public ResponseEntity<DefaultResponseDto<Object>> updateIsPinned(
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.OK)
@DeleteMapping("/{anniversaryNo}")
public ResponseEntity<DefaultResponseDto<Object>> deleteAnniversary(
@PathVariable Long anniversaryNo){

anniversaryService.isExistingAnniversaryNo(anniversaryNo);

Anniversary anniversary = anniversaryService.findAnniversaryByAnniversaryNo(anniversaryNo);

anniversaryService.deleteAnniversary(anniversaryNo);

return ResponseEntity.status(200)
.body(resWithoutData("ANNIVERSARY_DELETED", "기념일_삭제_완료"));
return ResponseEntity.ok(DefaultResponseDto.from("ANNIVERSARY_DELETED", "기념일_삭제_완료"));
}

@ApiOperation("전체 기념일 조회")
Expand All @@ -171,13 +142,11 @@ public ResponseEntity<DefaultResponseDto<Object>> deleteAnniversary(
@ApiResponse(code = 500,
message = "SERVER_ERROR")
})
@ResponseStatus(HttpStatus.OK)
@GetMapping("/admin")
public ResponseEntity<DefaultResponseDto<Object>> readAll(){

List<AnniversaryResponseDto> dto = anniversaryService.readAll();

return ResponseEntity.status(200)
.body(resWithData("ANIVERSARIES_FOUND", "전체 기념일 조회 완료", dto));
return ResponseEntity.ok(DefaultResponseDto.from("ANNIVERSARIES_FOUND", "전체 기념일 조회 완료", dto));
Copy link
Member

Choose a reason for hiding this comment

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

기능은 같은 것으로 알고 있는데
가독성 위한 리팩토링 맞을까?
다른 이점이 있다면 알려줘! AnniversaryController 외 다른 컨트롤러는 아직 status 를 사용하고 있어서 물어봤어

Copy link
Member Author

Choose a reason for hiding this comment

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

created도 메소드가 존재하는데
찾아보면 created(URI location)이라고 나와있어

이 부분은 created라는 메소드가 매개변수로 body를 요구하지 않는 걸 확인할 수 있어
이때 location은 header에 location=""에 해당해서 응답 DTO를 담아서 반환해주는 우리 코드에서는 적용하려면
ResponseEntity.created(null).body(~); 로 작성해야해

그래서 200 OK와 같은 경우에는 ok(T body)로 더 줄일 수 있어서 줄였고
201 CREATED는 그대로 놔둔거지

}
}
1 change: 0 additions & 1 deletion favor/src/main/java/com/favor/favor/anniversary/AnniversaryRequestDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

@Getter
@NoArgsConstructor
@AllArgsConstructor
public class AnniversaryRequestDto {
@ApiModelProperty(position = 1, required = true, value = "제목", example = "제목")
private String anniversaryTitle;
Expand Down
33 changes: 22 additions & 11 deletions favor/src/main/java/com/favor/favor/anniversary/AnniversaryResponseDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package com.favor.favor.anniversary;

import com.favor.favor.common.enums.AnniversaryCategory;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Getter;

import java.time.LocalDate;
import java.time.LocalDateTime;

@Getter
@AllArgsConstructor
Copy link
Member

Choose a reason for hiding this comment

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

@AllArgsContructor 를 지우고 밑에 static 으로 만들어 준 이유가 아래가 맞아? (정적 팩토리 메서드)

  1. from 정적 팩토리 메서드 사용을 통해서 DTO 변환 과정 중앙화
  2. AnniversaryResponseDto 가 Anniversary 에 덜 의존적이게 됨

Copy link
Member Author

Choose a reason for hiding this comment

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

@AllArgsConstructor가 있음에도 사용하지 않았기 때문에 어노테이션을 삭제한거고,
정적 팩토리 메소드를 사용한 이유는 인스턴스화에 있어서 형 말대로 덜 의존적이게 되도록 하는 의도지
(stream 등에서 활용 가능)

public class AnniversaryResponseDto {
private Long anniversaryNo;
private String anniversaryTitle;
Expand All @@ -21,14 +19,27 @@ public class AnniversaryResponseDto {
private AnniversaryCategory anniversaryCategory;

@Builder
Copy link
Member

Choose a reason for hiding this comment

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

리팩토링 이유 : 파라미터 1개인데 빌더 사용
이 맞아?
생성자 사용 안하는 것을 고려할 때 사용하는 경우 보여주려고 남겨 둔 것 같아서

Copy link
Member Author

Choose a reason for hiding this comment

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

파라미터가 1개인데 빌더 어노테이션을 사용해서 리팩토링을 진행한게 아니라,
생성자에 있어서는 매개변수가 다른 클래스에 의존적인 형태가 적절하지 못하다고 생각했어

그래서 private 접근 제한자로 모든 매개변수에 대한 생성자를 정의하고,
정적 팩토리 메소드로는 매개변수로 클래스를 받아서 각 매개변수에 해당하도록 작성한거지

public AnniversaryResponseDto(Anniversary anniversary){
this.anniversaryNo = anniversary.getAnniversaryNo();
this.anniversaryTitle = anniversary.getAnniversaryTitle();
this.anniversaryDate = anniversary.getAnniversaryDate();
this.isPinned = anniversary.getIsPinned();
this.userNo = anniversary.getUser().getUserNo();
this.createdAt = anniversary.getCreatedAt();
this.modifiedAt = anniversary.getModifiedAt();
this.anniversaryCategory = AnniversaryCategory.valueOf(anniversary.getCategory());
private AnniversaryResponseDto(Long anniversaryNo, String anniversaryTitle, LocalDate anniversaryDate, Boolean isPinned, Long userNo, LocalDateTime createdAt, LocalDateTime modifiedAt, AnniversaryCategory anniversaryCategory) {
this.anniversaryNo = anniversaryNo;
this.anniversaryTitle = anniversaryTitle;
this.anniversaryDate = anniversaryDate;
this.isPinned = isPinned;
this.userNo = userNo;
this.createdAt = createdAt;
this.modifiedAt = modifiedAt;
this.anniversaryCategory = anniversaryCategory;
}

public static AnniversaryResponseDto from(Anniversary anniversary) {
return AnniversaryResponseDto.builder()
.anniversaryNo(anniversary.getAnniversaryNo())
.anniversaryTitle(anniversary.getAnniversaryTitle())
.anniversaryDate(anniversary.getAnniversaryDate())
.isPinned(anniversary.getIsPinned())
.userNo(anniversary.getUser().getUserNo())
.createdAt(anniversary.getCreatedAt())
.modifiedAt(anniversary.getModifiedAt())
.anniversaryCategory(AnniversaryCategory.valueOf(anniversary.getCategory()))
.build();
}
}
96 changes: 61 additions & 35 deletions favor/src/main/java/com/favor/favor/anniversary/AnniversaryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.time.LocalDate;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

import static com.favor.favor.exception.ExceptionCode.*;

Expand All @@ -25,52 +26,80 @@ public class AnniversaryService {
private final FriendRepository friendRepository;

@Transactional
public Anniversary createAnniversary(AnniversaryRequestDto anniversaryRequestDto, Long userNo){
public AnniversaryResponseDto createAnniversary(Long userNo, AnniversaryRequestDto anniversaryRequestDto){
isExistingUserNo(userNo);
User user = findUserByUserNo(userNo);
Anniversary anniversary = anniversaryRepository.save(anniversaryRequestDto.toEntity(anniversaryRequestDto.getAnniversaryTitle(), user));
return anniversaryRepository.save(anniversary);

Anniversary anniversary = anniversaryRequestDto.toEntity(anniversaryRequestDto.getAnniversaryTitle(), user);
anniversaryRepository.save(anniversary);

return AnniversaryResponseDto.from(anniversary);
}

public AnniversaryResponseDto readAnniversary(Long anniversaryNo) {
isExistingAnniversaryNo(anniversaryNo);
Anniversary anniversary = findAnniversaryByAnniversaryNo(anniversaryNo);

return AnniversaryResponseDto.from(anniversary);
}

@Transactional
public void updateAnniversary(AnniversaryUpdateRequestDto dto, Anniversary anniversary){
anniversary.setAnniversaryTitle(dto.getAnniversaryTitle());
anniversary.setAnniversaryDate(LocalDate.parse(dto.getAnniversaryDate()));
anniversary.setCategory(dto.getAnniversaryCategory());
public AnniversaryResponseDto updateAnniversary(Long anniversaryNo, AnniversaryUpdateRequestDto anniversaryUpdateRequestDto){
isExistingAnniversaryNo(anniversaryNo);
Anniversary anniversary = findAnniversaryByAnniversaryNo(anniversaryNo);

anniversary.updateAnniversaryTitle(anniversaryUpdateRequestDto.getAnniversaryTitle());
anniversary.updateAnniversaryDate(LocalDate.parse(anniversaryUpdateRequestDto.getAnniversaryDate()));
anniversary.updateCategory(anniversaryUpdateRequestDto.getAnniversaryCategory());

anniversaryRepository.save(anniversary);

return AnniversaryResponseDto.from(anniversary);
}

@Transactional
public void updateIsPinned(Anniversary anniversary){
anniversary.setIsPinned(anniversary.getIsPinned() == true ? false : true);
public AnniversaryResponseDto updateIsPinned(Long anniversaryNo){
isExistingAnniversaryNo(anniversaryNo);
Anniversary anniversary = findAnniversaryByAnniversaryNo(anniversaryNo);

anniversary.updateIsPinned(anniversary.getIsPinned() == true ? false : true);
anniversaryRepository.save(anniversary);

return AnniversaryResponseDto.from(anniversary);
}

// @Transactional
// public void deleteAnniversary(Long anniversaryNo){
// List<Friend> friendList = findAnniversaryByAnniversaryNo(anniversaryNo).getUser().getFriendList();
// for(Friend friend : friendList){
// if(friend.getAnniversaryNoList().contains(anniversaryNo)){
// friend.getAnniversaryNoList().remove(anniversaryNo);
// friendRepository.save(friend);
// }
// }
//
// anniversaryRepository.deleteByAnniversaryNo(anniversaryNo);
// }

@Transactional
public void deleteAnniversary(Long anniversaryNo){
List<Friend> friendList = findAnniversaryByAnniversaryNo(anniversaryNo).getUser().getFriendList();
for(Friend friend : friendList){
if(friend.getAnniversaryNoList().contains(anniversaryNo)){
friend.getAnniversaryNoList().remove(anniversaryNo);
friendRepository.save(friend);
}
}
List<Friend> updateRequiredFriends = findAnniversaryByAnniversaryNo(anniversaryNo).getUser().getFriendList().stream()
.filter(friend -> friend.getAnniversaryNoList().contains(anniversaryNo))
.peek(friend -> friend.getAnniversaryNoList().remove(anniversaryNo))
.collect(Collectors.toList());

friendRepository.saveAll(updateRequiredFriends);
anniversaryRepository.deleteByAnniversaryNo(anniversaryNo);
}

public List<AnniversaryResponseDto> readAll(){
List<AnniversaryResponseDto> a_List = new ArrayList<>();
for(Anniversary a : anniversaryRepository.findAll()){
AnniversaryResponseDto dto = new AnniversaryResponseDto(a);
a_List.add(dto);
}
return a_List;
return anniversaryRepository.findAll().stream()
.map(AnniversaryResponseDto::from)
.collect(Collectors.toList());
}


public User findUserByUserNo(Long userNo){
User user = null;
private User findUserByUserNo(Long userNo){
User user;
try{
user = userRepository.findByUserNo(userNo).orElseThrow(
() -> new RuntimeException()
Expand All @@ -80,8 +109,9 @@ public User findUserByUserNo(Long userNo){
}
return user;
}
public Anniversary findAnniversaryByAnniversaryNo(Long anniversaryNo){
Anniversary anniversary = null;

private Anniversary findAnniversaryByAnniversaryNo(Long anniversaryNo){
Anniversary anniversary;
try{
anniversary = anniversaryRepository.findAnniversaryByAnniversaryNo(anniversaryNo).orElseThrow(
() -> new RuntimeException()
Expand All @@ -93,12 +123,8 @@ public Anniversary findAnniversaryByAnniversaryNo(Long anniversaryNo){
}


public AnniversaryResponseDto returnDto(Anniversary anniversary){
return new AnniversaryResponseDto(anniversary);
}

public void isExistingAnniversaryNo(Long anniversaryNo){
Boolean isExistingNo = null;
private void isExistingAnniversaryNo(Long anniversaryNo){
Boolean isExistingNo;
try{
isExistingNo = anniversaryRepository.existsByAnniversaryNo(anniversaryNo);
}catch(RuntimeException e){
Expand All @@ -109,8 +135,8 @@ public void isExistingAnniversaryNo(Long anniversaryNo){
}
}

public void isExistingUserNo (Long userNo){
Boolean isExistingNo = null;
private void isExistingUserNo (Long userNo){
Boolean isExistingNo;
try{
isExistingNo = userRepository.existsByUserNo(userNo);
} catch(RuntimeException e){
Expand Down
1 change: 0 additions & 1 deletion favor/src/main/java/com/favor/favor/anniversary/AnniversaryUpdateRequestDto.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

@Getter
@NoArgsConstructor
@AllArgsConstructor
public class AnniversaryUpdateRequestDto {

@ApiModelProperty(position = 1, required = true, value = "제목", example = "제목")
Expand Down
Loading