feature : vote, room 의존성 분리 및 vote repository 추가 #85

waterricecake · 2024-07-21T05:12:07Z

기존 vote의 구조를 말씀드렸던 것처럼 각각이 code, player, target을 가지고 있게 바꿨습니다.
이 후 Redis에 저장을 하도록하였습니다.

redis key의 경우는 redis의 다중키를 이용하여 vote:code:name 형식으로 저장하였습니다.
이는 Redis의 경우 쿼리처럼 value로 필터링하는 방법이 없어 키를 복합적으로 구현하고 search할때 "vote:code:*"이런식으로 검색해야한다고 하네요. 그래서 저렇게 구현하였습니다.

아직 Game 패키지를 구현하지 않았기 때문에 전원 투표시 status가 넘어가는 event는 구현하지 않았습니다.
이보다 더 진행하면 뭔가 복잡해질것 같아서 일단 여기까지 구현하고 pr 올려봅니다.

before : 대각선을 기준으로 vote와 room 사이 양방향 의존성 존재

after: room과 vote는 의존성이 끊김

ps. EventListener는 어디서 관리하면 좋을지 궁금합니다 .아이디어 부탁

kpeel5839

고생했어요 달리!

저는 EventListner를 지금처럼 두는 것이 맞다고 생각해요. 다만, 달리가 물어본 것이 애그리거트 패키지 내의 위치에 대해서 여쭤보신거라면, application내에 listener 패키지를 두어서 조금 보기 명확하게 하면 더 좋을 것 같다는 생각이 있습니다. application 하위에 있는 것은 맞는 것 같다고 생각이 들어요.

kpeel5839 · 2024-07-22T06:02:12Z

build.gradle

@@ -22,6 +22,9 @@ repositories {
 }

 dependencies {
+    implementation 'org.springframework.boot:spring-boot-starter-data-redis'
+    testImplementation 'org.testcontainers:testcontainers'


TestContainer.. 이제 내 컴퓨터에서 테스트를 돌릴 때 마다 1분씩 걸리겠군요 ㅠ

저도... 그게 걱정..... 테스트 너무 오래걸리는거 아닌가 싶기도 해서요

testcontainers는 springboot 자체에서 기본적으로 h2는 지원하는데 redis는 없어서 redis로 테스트 돌리기위해서 필요한건가?

예스 Redis를 돌리기위해 testContainers로 Redis 도커 띄워서 사용하고 있음

kpeel5839 · 2024-07-22T06:03:05Z

src/main/java/mafia/mafiatogether/game/application/GameEventListener.java

+    @EventListener
+    public void listenAllVotedEvent(final AllVotedEvent allVotedEvent){
+        // todo : 이후 status 변경 이벤트 발행
+    }


의존성 끊어내기 너어무우~ 좋아요

kpeel5839 · 2024-07-22T06:04:28Z

src/main/java/mafia/mafiatogether/room/domain/Room.java

+//    public void clearVote() {
+//        this.voteLegacy.clear();
+//    }


모듈 나누자니까아아

지금?? 일단 패키지 의존성 분리하고 생각하자는거 아니었슴까?

kpeel5839 · 2024-07-22T06:05:23Z

src/main/java/mafia/mafiatogether/vote/application/VoteService.java

+            if (maxCount == voteCount.getValue()) {
+                count++;
+            }
+            if (maxCount < voteCount.getValue()) {
+                maxCount = voteCount.getValue();
+                count = 1;
+                maxVotedPlayer = voteCount.getKey();
+            }


depth 2는 나는 못참아!

메서드로 빼도 괜찮을 것 같고, 두 번 나눠서 처리하는 것이 더 가독성이 좋을 것 같기도하고...

근데, 솔직히 지금도 괜찮기도 하고.. ㅋㅋㅋㅋ

count>1은 동점표 잖아요. 이런 것도 메서드로 분리할 수 있으면 좋을 것 같은데 이거는 일단, 임시로 해놓으신거죠?!

이거 아니면 다른 괜찮은 알고리즘 있으신가요? 투표 까는 뭔가 까리한 알고리즘

이렇게 하는게 최고 좋은 것 같은데, 의미만 명확히 하면 좋을 것 같아요.

그럼 네이밍과 함수분리를 좀 해볼게요!

나도 이부분 메서드로 분리하면 보기편할거같음 🧐

kpeel5839 · 2024-07-22T06:06:04Z

src/main/java/mafia/mafiatogether/vote/config/VoteRedisConfig.java

+        RedisTemplate<String, Vote> template = new RedisTemplate<>();
+        template.setConnectionFactory(redisConnectionFactory);
+
+        // Use String serializer for keys
+        template.setKeySerializer(new StringRedisSerializer());
+        template.setHashKeySerializer(new StringRedisSerializer());
+
+        // Use JSON serializer for values
+        template.setValueSerializer(new GenericJackson2JsonRedisSerializer());
+        template.setHashValueSerializer(new GenericJackson2JsonRedisSerializer());
+
+        template.setEnableDefaultSerializer(true);
+        template.setDefaultSerializer(new GenericJackson2JsonRedisSerializer());


달리의 고민의 흔적들이 보이는 설정들이네요 ㅋㅋㅋ

kpeel5839 · 2024-07-22T06:09:04Z

src/main/java/mafia/mafiatogether/vote/domain/Vote.java

-        int maxCount = 0;
-        int count = 0;
-        Player maxVotedPlayer = Player.NONE;
-        for (Entry<Player, Integer> voteCount : voteCounts.entrySet()) {
-            if (voteCount.getKey() == Player.NONE) {
-                continue;
-            }
-            if (maxCount == voteCount.getValue()) {
-                count++;
-            }
-            if (maxCount < voteCount.getValue()) {
-                maxCount = voteCount.getValue();
-                count = 1;
-                maxVotedPlayer = voteCount.getKey();
-            }
-        }
-        if (count > 1) {
-            return Player.NONE;
-        }
-        return maxVotedPlayer;
-    }


모야 임시로 해놓은 게 아니었네!
메서드 길이랑, 뎁스 안돼!요!

thdwoqor

고생했어

thdwoqor · 2024-07-23T02:29:29Z

src/main/java/mafia/mafiatogether/room/domain/Room.java

    private Status status;
    private final RoomInfo roomInfo;
    private final Chat chat;
    private final JobTarget jobTarget;
    private Player master;

+    public static Room create(final String code, final RoomInfo roomInfo, final Long now) {


이거 사용안하는데 왜있는거지?

아 이거 room Repository 생성할려고 적어둔거긴 한데 아직 Room 작업을 안해서 사용이 안됬네 나중에 RoomRepository 생성하면서 사용될듯

thdwoqor · 2024-07-23T02:35:19Z

src/main/java/mafia/mafiatogether/vote/application/VoteService.java

+            if (maxCount == voteCount.getValue()) {
+                count++;
+            }
+            if (maxCount < voteCount.getValue()) {
+                maxCount = voteCount.getValue();
+                count = 1;
+                maxVotedPlayer = voteCount.getKey();
+            }


나도 이부분 메서드로 분리하면 보기편할거같음 🧐

thdwoqor · 2024-07-23T02:40:33Z

src/main/java/mafia/mafiatogether/vote/domain/VoteRepositoryImpl.java

+
+@RequiredArgsConstructor
+@Repository
+public class VoteRepositoryImpl implements VoteRepository {


레디스는 구현체를 직접 만들어야해?

ㄴㄴ CrudRepository 사용하면 할수 있긴한데 그럴 경우
조회의 경우 findByID, findAll 밖에 안되고 조건 조회 같은게 불가능함
그래서 RedisTemplate으로 Secondary index 조회를 위해서 직접 구현해줘야함

thdwoqor · 2024-07-23T02:43:04Z

src/test/java/mafia/mafiatogether/redis/RedisTestConfig.java

@@ -0,0 +1,22 @@
+package mafia.mafiatogether.redis;


따로 패키지 분리안하고 코드단처럼 global에 있으면 보기편할거같아

thdwoqor · 2024-07-23T02:44:14Z

build.gradle

@@ -22,6 +22,9 @@ repositories {
 }

 dependencies {
+    implementation 'org.springframework.boot:spring-boot-starter-data-redis'
+    testImplementation 'org.testcontainers:testcontainers'


testcontainers는 springboot 자체에서 기본적으로 h2는 지원하는데 redis는 없어서 redis로 테스트 돌리기위해서 필요한건가?

kpeel5839 · 2024-08-01T10:16:36Z

충돌 폼 미쳐따이;;

waterricecake added the Backend label Jul 21, 2024

waterricecake requested review from thdwoqor and kpeel5839 July 21, 2024 05:12

waterricecake self-assigned this Jul 21, 2024

kpeel5839 approved these changes Jul 22, 2024

View reviewed changes

thdwoqor approved these changes Jul 23, 2024

View reviewed changes

waterricecake added 6 commits July 30, 2024 16:29

refactor : controllerTest 추상 메서드 구현

9e0bcdf

refactor : package 명 및 구조 수정

f23f5be

style: 불필요한 개행 제거

0b620a9

feat: redis dependency 추가 및 redis 테스트 구축

f359ff4

style : fixture static 변수 대문자로 변경

e0bbc80

feat : RedisConfig 추가 및 패키지 구조 변

aa66178

waterricecake force-pushed the feature/84 branch from c2af594 to 01c5e91 Compare August 1, 2024 09:19

waterricecake force-pushed the feature/84 branch from f2bb243 to aa66178 Compare August 1, 2024 14:24

waterricecake closed this Aug 1, 2024

waterricecake deleted the feature/84 branch August 1, 2024 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature : vote, room 의존성 분리 및 vote repository 추가 #85

feature : vote, room 의존성 분리 및 vote repository 추가 #85

waterricecake commented Jul 21, 2024

kpeel5839 left a comment

kpeel5839 Jul 22, 2024

waterricecake Jul 22, 2024

thdwoqor Jul 23, 2024

waterricecake Jul 23, 2024

kpeel5839 Jul 22, 2024

kpeel5839 Jul 22, 2024

waterricecake Jul 22, 2024

kpeel5839 Jul 22, 2024

kpeel5839 Jul 22, 2024

kpeel5839 Jul 22, 2024

waterricecake Jul 22, 2024

kpeel5839 Jul 22, 2024

waterricecake Jul 22, 2024

thdwoqor Jul 23, 2024

kpeel5839 Jul 22, 2024

kpeel5839 Jul 22, 2024

thdwoqor left a comment

thdwoqor Jul 23, 2024

waterricecake Jul 23, 2024

thdwoqor Jul 23, 2024

thdwoqor Jul 23, 2024

waterricecake Jul 23, 2024

thdwoqor Jul 23, 2024

thdwoqor Jul 23, 2024

kpeel5839 commented Aug 1, 2024

feature : vote, room 의존성 분리 및 vote repository 추가 #85

feature : vote, room 의존성 분리 및 vote repository 추가 #85

Conversation

waterricecake commented Jul 21, 2024

kpeel5839 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thdwoqor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpeel5839 commented Aug 1, 2024