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

2주차 리뷰 #173

Open
wants to merge 512 commits into
base: review-week2
Choose a base branch
from
Open

2주차 리뷰 #173

wants to merge 512 commits into from

Conversation

Account-4-Review
Copy link

🛠️ 과제 요약

  • 과제 요약 설명(ex 마이페이지 구현 완료)

📝 요구 사항과 구현 내용

  • 구현 내용 1
  • 구현 내용 2

✅ 피드백 반영사항

  • 피드백 받은 내용과 반영된 결과 적기

💬 PR 포인트 & 궁금한 점

  • 기능 관련 나누고 싶은 내용 or 리뷰 요구사항 작성

🔗 연관된 이슈

  • 열린 이슈를 닫고싶으면
  • close #이슈번호 [이슈제목]
  • 이슈를 닫지 않을 거라면 작성X

bung-dev and others added 15 commits December 5, 2024 16:59
…168-redis

♻️ refactor : 수정 사항 리팩토링
…168-redis

✨ feat : 결제 취소 반환값 추가 및 예약 수정사항 적용
# Conflicts:
#	src/main/java/roomit/main/domain/chat/chatmessage/service/ChatService.java
#	src/main/java/roomit/main/domain/chat/chatroom/controller/ChatRoomController.java
#	src/main/java/roomit/main/domain/chat/chatroom/dto/ChatRoomBusinessResponse.java
#	src/main/java/roomit/main/domain/chat/chatroom/dto/ChatRoomMemberResponse.java
#	src/main/java/roomit/main/domain/chat/chatroom/dto/response/ChatRoomResponse.java
#	src/main/java/roomit/main/domain/chat/chatroom/entity/ChatRoom.java
#	src/main/java/roomit/main/domain/chat/chatroom/repositoroy/ChatRoomRepository.java
#	src/main/java/roomit/main/domain/chat/chatroom/service/ChatRoomService.java
#	src/test/java/roomit/main/domain/chat/service/ChatServiceIntegrationTest.java
Copy link
Author

@Account-4-Review Account-4-Review left a comment

Choose a reason for hiding this comment

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

우선 러프하게 작성해봤습니다.
스타일이 너무 어긋난게 아니면 대부분 패스했고요, (시간 문제 때문에 ㅠㅠ) 러프하게 본지라 아직 로직을 구체적으로 살펴보진 않았습니다.
내일 조금 더 살펴보고 추가 리뷰 남길게요.

@PutMapping("api/v1/business")
public void businessModify(@RequestBody @Valid BusinessUpdateRequest businessUpdateRequest
,@AuthenticationPrincipal CustomBusinessDetails customBusinessDetails){
@PutMapping("")
Copy link
Author

Choose a reason for hiding this comment

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

사소한거긴 한데, 사실 이런 경우엔 그냥 @PutMapping 을 써도 됩니다.

@Query(value = "SELECT CASE WHEN COUNT(b) > 0 THEN true ELSE false END FROM Business b WHERE b.businessNum.value=:businessNum")
Boolean existsByBusinessNum(String businessNum);

@Query("""
Copy link
Author

Choose a reason for hiding this comment

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

존재 여부를 확인하는 건가요?
이 경우, EXIST 을 걸면 좋습니다.
한 개 검색이 되는 순간 쿼리를 종료시켜 버리거든요.

@Query("""
	    SELECT CASE WHEN EXISTS (
	        SELECT 1 
	        FROM Business b
	        WHERE b.businessId = :senderId AND b.businessName = :senderName
	    ) THEN TRUE ELSE FALSE END
	""")
boolean existsByIdAndBusinessName(Long senderId, String senderName);

Optional<Business> findByBusinessEmail(String businessEmail);

@Query(value = "SELECT CASE WHEN COUNT(b) > 0 THEN true ELSE false END FROM Business b WHERE b.businessName.value=:businessName")
boolean existsByBusinessName(String businessName);
Copy link
Author

Choose a reason for hiding this comment

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

이것만 primitive 네요.


import java.time.LocalDateTime;

@Builder
Copy link
Author

Choose a reason for hiding this comment

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

직접 Builder 를 쓸 일은 없겠죠?
(타 record 포함)

return tokenService.memberLogin(loginRequest,response);
}

@PostMapping("/login/business")
Copy link
Author

Choose a reason for hiding this comment

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

ResponseStatus 추가


@Scheduled(fixedRate = 60000) // 1분마다 실행
public void flushMessages() {
List<Long> roomIds = chatRoomRepository.findAllRoomIds(); // Room ID 동적 조회
Copy link
Author

Choose a reason for hiding this comment

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

roomId 의 수가 끝도 없이 많을 수 있지 않을까요?

redisPublisher.publish(topic, request);

// Redis에 저장
saveMessageToRedis(request);
Copy link
Author

Choose a reason for hiding this comment

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

해당 로직은, "Redis에 1분간 발생한 모든 채팅의 내용이 전부 적재될 수 있다" 라는 전제하에 수행되는 로직으로 보여요.

그렇지만... 여러분들 서버의 Redis 메모리는 안녕하실까요?

Copy link
Collaborator

@usingjun usingjun Dec 6, 2024

Choose a reason for hiding this comment

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

ttl에 따라 일정 시간이 지나면 삭제되도록 했는데 그래도 문제가 될까요?

image


ChatMessage message = new ChatMessage(room, request);

messageRepository.save(message);
Copy link
Author

Choose a reason for hiding this comment

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

결국 이것도 모든 데이터에 대해 일일히 INSERT를 날리는거 아닌가요?
묶어서 처리한다고 해도, 결국 성능을 별 차이 없을 것으로 보여요.

Copy link
Collaborator

Choose a reason for hiding this comment

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

인덱싱 처리하면 어떨까요?

END
)
FROM ChatRoom c
LEFT JOIN c.messages m
Copy link
Author

Choose a reason for hiding this comment

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

쿼리의 실행계획을 확인해 주세요. 쿼리 성능 병목으로 인해 발목잡힐 수 있을 것 같아보여요.

Copy link
Collaborator

@usingjun usingjun Dec 6, 2024

Choose a reason for hiding this comment

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

image

혹시 이렇게 쿼리를 바꿨는데 이렇게 하면 될까요?

"AND w.workplace_longitude BETWEEN :left AND :right " +
"ORDER BY distance ASC", nativeQuery = true)
@Query(value = """
SELECT w.workplace_id, w.workplace_name,
Copy link
Author

Choose a reason for hiding this comment

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

장소라면, 공간인덱스를 설정하고 관련 함수를 쓰는게 훨씬 성능이 좋고, 정확하지 않을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

공간인덱스 설정 후 관련함수 적용했습니다!

bung-dev and others added 29 commits December 9, 2024 11:26
♻️ Refactor : 리뷰 url 수정
Deploy Version 13버전 develop 브랜치로 clone
…tor/#199

Refactor/ 거리별 조회 최대 10개 , 리뷰 글자수 100
…tor/#199

♻️ Refactor : 특정 사업자 예약 확인 응답값 수정
Deploy Version 18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants