[BE] Refactor/#430 핀 복사 시 JdbcTemplate을 이용하여 bulk insert하도록 변경 (#617)

* feat: batch update 적용 중 * chore: batch update 적용 중 * feat: pin image를 고려한 batch update 적용 - datasource url에 &rewriteBatchedStatements=true&profileSQL=true&logger=Slf4JLogger&maxQuerySizeToLog=999999 추가할 것 * feat: batch update 시 pinImage N+1 문제 해결을 위한 fetch join * refactor: PinBatchRepository 중복 제거 및 메서드 분리 * refactor: jdbcTemplate 사용하는 리포지토리를 사용자 정의 리포지토리로 추상화 - 추상화 후 실패하는 테스트의 영속성 컨텍스트 문제 해결 * refactor: PinBatchRepository 인덴트 개선 * refactor: 테스트 컨테이너 jdbcURl에 batch update를 위한 설정 추가 - 실제 서브모듈에도 반영할것 * refactor: saveAllToTopic 불필요한 인자 제거로 인한 시그니처 변경 * refactor: 사용자 정의 리포지토리의 saveAllToTopic 테스트 작성 * refactor: jdbcTemplate, hibernate 중복 로그 하는 대신 커스텀 로그 설정 * test: 테스트 내 불필요한 출력 삭제 * test: JPA 쿼리 호출과 JdbcTemplate 쿼리 호출 분리 * chore: batch update를 위한 jdbc url 설정 추가 * chore: 로컬 환경 batch update 설정 추가 * refactor: 불필요한 flush 삭제 * refactor: 개행 추가 * chore: 커스텀 리포지토리 패키지 컨텍스트 별로 변경 * refactor: 메서드명 수정 및 가독성 개선 * refactor: 불필요한 flush 삭제 * refactor: rowCount 상태코드 상수화 * refactor: sql 문자열 개행 방식 수정 * refactor: DB용 Dto record로 변경 * refactor: 사용하지 않는 메서드 삭제 * refactor: 불필요한 update 호출 삭제
woowacourse-teams · Nov 6, 2023 · 9a26db2 · 9a26db2
1 parent 3c8cd1a
commit 9a26db2
Show file tree

Hide file tree

Showing 14 changed files with 245 additions and 95 deletions.
diff --git a/backend/src/main/java/com/mapbefine/mapbefine/pin/domain/Pin.java b/backend/src/main/java/com/mapbefine/mapbefine/pin/domain/Pin.java
@@ -108,24 +108,6 @@ public void decreaseTopicPinCount() {
         topic.decreasePinCount();
     }
 
-    public void copyToTopic(Member creator, Topic topic) {
-        Pin copiedPin = Pin.createPinAssociatedWithLocationAndTopicAndMember(
-                pinInfo.getName(),
-                pinInfo.getDescription(),
-                location,
-                topic,
-                creator
-        );
-
-        copyPinImages(copiedPin);
-    }
-
-    private void copyPinImages(Pin pin) {
-        for (PinImage pinImage : pinImages) {
-            PinImage.createPinImageAssociatedWithPin(pinImage.getImageUrl(), pin);
-        }
-    }
-
     public void addPinImage(PinImage pinImage) {
         pinImages.add(pinImage);
     }
@@ -138,6 +120,10 @@ public double getLongitude() {
         return location.getLongitude();
     }
 
+    public String getName() {
+        return pinInfo.getName();
+    }
+
     public String getDescription() {
         return pinInfo.getDescription();
     }

diff --git a/backend/src/main/java/com/mapbefine/mapbefine/pin/domain/PinRepository.java b/backend/src/main/java/com/mapbefine/mapbefine/pin/domain/PinRepository.java
@@ -1,5 +1,6 @@
 package com.mapbefine.mapbefine.pin.domain;
 
+import com.mapbefine.mapbefine.pin.infrastructure.PinBatchRepositoryCustom;
 import java.util.List;
 import org.springframework.data.jpa.repository.EntityGraph;
 import org.springframework.data.jpa.repository.JpaRepository;
@@ -9,11 +10,14 @@
 import org.springframework.stereotype.Repository;
 
 @Repository
-public interface PinRepository extends JpaRepository<Pin, Long> {
+public interface PinRepository extends JpaRepository<Pin, Long>, PinBatchRepositoryCustom {
 
     @EntityGraph(attributePaths = {"location", "topic", "creator", "pinImages"})
     List<Pin> findAll();
 
+    @EntityGraph(attributePaths = {"location", "topic", "creator", "pinImages"})
+    List<Pin> findAllByIdIn(List<Long> pinIds);
+
     @EntityGraph(attributePaths = {"location", "topic", "creator", "pinImages"})
     List<Pin> findAllByTopicId(Long topicId);
 

diff --git a/...nd/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustom.java b/...nd/src/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustom.java
@@ -0,0 +1,11 @@
+package com.mapbefine.mapbefine.pin.infrastructure;
+
+import com.mapbefine.mapbefine.pin.domain.Pin;
+import com.mapbefine.mapbefine.topic.domain.Topic;
+import java.util.List;
+
+public interface PinBatchRepositoryCustom {
+
+    int[] saveAllToTopic(Topic topicForCopy, List<Pin> originalPins);
+
+}
diff --git a/...rc/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java b/...rc/main/java/com/mapbefine/mapbefine/pin/infrastructure/PinBatchRepositoryCustomImpl.java
@@ -0,0 +1,142 @@
+package com.mapbefine.mapbefine.pin.infrastructure;
+
+import static java.sql.Statement.EXECUTE_FAILED;
+
+import com.mapbefine.mapbefine.pin.domain.Pin;
+import com.mapbefine.mapbefine.pin.domain.PinImage;
+import com.mapbefine.mapbefine.topic.domain.Topic;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.time.LocalDateTime;
+import java.util.Collection;
+import java.util.List;
+import java.util.Objects;
+import java.util.stream.IntStream;
+import lombok.extern.slf4j.Slf4j;
+import org.springframework.jdbc.core.BatchPreparedStatementSetter;
+import org.springframework.jdbc.core.JdbcTemplate;
+import org.springframework.stereotype.Repository;
+
+@Slf4j
+@Repository
+public class PinBatchRepositoryCustomImpl implements PinBatchRepositoryCustom {
+
+    private final JdbcTemplate jdbcTemplate;
+
+    public PinBatchRepositoryCustomImpl(JdbcTemplate jdbcTemplate) {
+        this.jdbcTemplate = jdbcTemplate;
+    }
+
+    public int[] saveAllToTopic(Topic topicForCopy, List<Pin> originalPins) {
+        int[] rowCount = bulkInsertPins(topicForCopy, originalPins);
+        List<PinImageInsertDto> pinImageInsertDtos = createPinImageInsertDtos(originalPins, rowCount);
+
+        if (pinImageInsertDtos.isEmpty()) {
+            return rowCount;
+        }
+        return bulkInsertPinImages(pinImageInsertDtos);
+    }
+
+    private int[] bulkInsertPins(Topic topicForCopy, List<Pin> originalPins) {
+        String bulkInsertSql = "INSERT INTO pin "
+                + "(name, description, member_id, topic_id, location_id, "
+                + "created_at, updated_at) "
+                + "VALUES "
+                + "(?, ?, ?, ?, ?, "
+                + "?, ?)";
+        LocalDateTime createdAt = topicForCopy.getLastPinUpdatedAt();
+        Long topicId = topicForCopy.getId();
+        Long creatorId = topicForCopy.getCreator().getId();
+        log.debug("[Query] bulk insert size {} : {}", originalPins.size(), bulkInsertSql);
+
+        return jdbcTemplate.batchUpdate(bulkInsertSql, new BatchPreparedStatementSetter() {
+            @Override
+            public void setValues(PreparedStatement ps, int i) throws SQLException {
+                Pin pin = originalPins.get(i);
+                ps.setString(1, pin.getName());
+                ps.setString(2, pin.getDescription());
+                ps.setLong(3, creatorId);
+                ps.setLong(4, topicId);
+                ps.setLong(5, pin.getLocation().getId());
+                ps.setTimestamp(6, Timestamp.valueOf(createdAt));
+                ps.setTimestamp(7, Timestamp.valueOf(createdAt));
+                log.trace("[Parameter Binding] {} : "
+                                + "name={}, description={}, member_id={}, topic_id={}, location_id={}, "
+                                + "created_at={}, updated_at={}",
+                        i, pin.getName(), pin.getDescription(), creatorId, topicId, pin.getLocation().getId(),
+                        createdAt, createdAt);
+            }
+
+            @Override
+            public int getBatchSize() {
+                return originalPins.size();
+            }
+        });
+    }
+
+    private List<PinImageInsertDto> createPinImageInsertDtos(List<Pin> originalPins, int[] rowCount) {
+        Long firstIdFromBatch = jdbcTemplate.queryForObject("SELECT last_insert_id()", Long.class);
+        validateId(firstIdFromBatch);
+
+        return IntStream.range(0, originalPins.size())
+                .filter(index -> rowCount[index] != EXECUTE_FAILED)
+                .mapToObj(index -> {
+                    Pin pin = originalPins.get(index);
+                    return PinImageInsertDto.of(pin.getPinImages(), firstIdFromBatch + index);
+                }).flatMap(Collection::stream)
+                .toList();
+    }
+
+    private void validateId(Long firstIdFromBatch) {
+        if (Objects.isNull(firstIdFromBatch)) {
+            throw new IllegalStateException("fail to batch update pins");
+        }
+    }
+
+    private int[] bulkInsertPinImages(List<PinImageInsertDto> pinImages) {
+        String bulkInsertSql = "INSERT INTO pin_image "
+                + "(image_url, pin_id) "
+                + "VALUES "
+                + "(?, ?)";
+        log.debug("[Query] bulk insert size {} : {}", pinImages.size(), bulkInsertSql);
+
+        return jdbcTemplate.batchUpdate(bulkInsertSql, new BatchPreparedStatementSetter() {
+            @Override
+            public void setValues(PreparedStatement ps, int i) throws SQLException {
+                PinImageInsertDto pinImage = pinImages.get(i);
+                ps.setString(1, pinImage.imageUrl);
+                ps.setLong(2, pinImage.pinId);
+                log.trace("[Parameter Binding] {} : imageUrl={}, pinImage={} ",
+                        i, pinImage.imageUrl, pinImage.pinId);
+            }
+
+            @Override
+            public int getBatchSize() {
+                return pinImages.size();
+            }
+        });
+    }
+
+    private record PinImageInsertDto(
+            String imageUrl,
+            Long pinId,
+            boolean isDeleted
+    ) {
+
+        public static PinImageInsertDto of(PinImage pinImage, Long pinId) {
+            return new PinImageInsertDto(
+                    pinImage.getImageUrl(),
+                    pinId,
+                    pinImage.isDeleted()
+            );
+        }
+
+        private static List<PinImageInsertDto> of(List<PinImage> pinImages, Long pinId) {
+            return pinImages.stream()
+                    .map(pinImage -> PinImageInsertDto.of(pinImage, pinId))
+                    .toList();
+        }
+    }
+
+}
diff --git a/backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java b/backend/src/main/java/com/mapbefine/mapbefine/topic/application/TopicCommandService.java
@@ -55,12 +55,11 @@ public Long saveTopic(AuthMember member, TopicCreateRequest request) {
         Topic topic = convertToTopic(member, request);
         List<Long> pinIds = request.pins();
 
+        topicRepository.save(topic);
         if (!pinIds.isEmpty()) {
             copyPinsToTopic(member, topic, pinIds);
         }
 
-        topicRepository.save(topic);
-
         return topic.getId();
     }
 
@@ -105,14 +104,14 @@ private void copyPinsToTopic(
     ) {
         List<Pin> originalPins = findAllPins(pinIds);
         validateCopyablePins(member, originalPins);
+        topic.increasePinCount(pinIds.size());
+        pinRepository.flush();
 
-        Member creator = findCreatorByAuthMember(member);
-
-        originalPins.forEach(pin -> pin.copyToTopic(creator, topic));
+        pinRepository.saveAllToTopic(topic, originalPins);
     }
 
     private List<Pin> findAllPins(List<Long> pinIds) {
-        List<Pin> findPins = pinRepository.findAllById(pinIds);
+        List<Pin> findPins = pinRepository.findAllByIdIn(pinIds);
 
         if (pinIds.size() != findPins.size()) {
             throw new PinBadRequestException(ILLEGAL_PIN_ID);
@@ -134,15 +133,13 @@ private void validateCopyablePins(AuthMember member, List<Pin> originalPins) {
     public Long merge(AuthMember member, TopicMergeRequest request) {
         Topic topic = convertToTopic(member, request);
         List<Topic> originalTopics = findAllTopics(request.topics());
-
         validateCopyableTopics(member, originalTopics);
-
-        Member creator = findCreatorByAuthMember(member);
         List<Pin> originalPins = getAllPinsFromTopics(originalTopics);
-        originalPins.forEach(pin -> pin.copyToTopic(creator, topic));
 
-        topicRepository.save(topic);
+        topic.increasePinCount(originalPins.size());
 
+        topicRepository.save(topic);
+        pinRepository.saveAllToTopic(topic, originalPins);
         return topic.getId();
     }
 

diff --git a/backend/src/main/java/com/mapbefine/mapbefine/topic/domain/Topic.java b/backend/src/main/java/com/mapbefine/mapbefine/topic/domain/Topic.java
@@ -152,6 +152,10 @@ public void removeImage() {
         this.topicInfo = topicInfo.removeImage();
     }
 
+    public void increasePinCount(int count) {
+        pinCount += count;
+    }
+
     public void decreasePinCount() {
         pinCount--;
     }

diff --git a/backend/src/main/resources/config b/backend/src/main/resources/config
diff --git a/backend/src/test/java/com/mapbefine/mapbefine/TestDatabaseContainer.java b/backend/src/test/java/com/mapbefine/mapbefine/TestDatabaseContainer.java
@@ -15,11 +15,15 @@ public abstract class TestDatabaseContainer {
 
     @DynamicPropertySource
     public static void overrideProps(DynamicPropertyRegistry registry) {
-        registry.add("spring.datasource.url", mySQLContainer::getJdbcUrl);
+        registry.add("spring.datasource.url", TestDatabaseContainer::getJdbcUrlWithQueryStrings);
         registry.add("spring.datasource.username", mySQLContainer::getUsername);
         registry.add("spring.datasource.password", mySQLContainer::getPassword);
         registry.add("spring.datasource.driver-class-name", mySQLContainer::getDriverClassName);
     }
 
+    private static String getJdbcUrlWithQueryStrings() {
+        return mySQLContainer.getJdbcUrl() + "?rewriteBatchedStatements=true";
+    }
+
 }
 
diff --git a/backend/src/test/java/com/mapbefine/mapbefine/common/config/MockBeansConfig.java b/backend/src/test/java/com/mapbefine/mapbefine/common/config/MockBeansConfig.java
diff --git a/backend/src/test/java/com/mapbefine/mapbefine/pin/PinImageFixture.java b/backend/src/test/java/com/mapbefine/mapbefine/pin/PinImageFixture.java
@@ -5,8 +5,10 @@
 
 public class PinImageFixture {
 
+    public static String IMAGE_URL = "https://example.com/image.jpg";
+
     public static PinImage create(Pin pin) {
-        return PinImage.createPinImageAssociatedWithPin("https://example.com/image.jpg", pin);
+        return PinImage.createPinImageAssociatedWithPin(IMAGE_URL, pin);
     }
 
 }
diff --git a/backend/src/test/java/com/mapbefine/mapbefine/pin/domain/PinRepositoryTest.java b/backend/src/test/java/com/mapbefine/mapbefine/pin/domain/PinRepositoryTest.java
@@ -1,27 +1,27 @@
 package com.mapbefine.mapbefine.pin.domain;
 
+import static com.mapbefine.mapbefine.member.domain.Role.USER;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.SoftAssertions.assertSoftly;
 
 import com.mapbefine.mapbefine.TestDatabaseContainer;
 import com.mapbefine.mapbefine.common.annotation.RepositoryTest;
-import com.mapbefine.mapbefine.common.config.JpaConfig;
 import com.mapbefine.mapbefine.location.LocationFixture;
 import com.mapbefine.mapbefine.location.domain.Location;
 import com.mapbefine.mapbefine.location.domain.LocationRepository;
 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.pin.PinFixture;
+import com.mapbefine.mapbefine.pin.PinImageFixture;
 import com.mapbefine.mapbefine.topic.TopicFixture;
 import com.mapbefine.mapbefine.topic.domain.Topic;
 import com.mapbefine.mapbefine.topic.domain.TopicRepository;
+import java.util.List;
 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.boot.test.autoconfigure.orm.jpa.DataJpaTest;
-import org.springframework.context.annotation.Import;
 
 @RepositoryTest
 class PinRepositoryTest extends TestDatabaseContainer {
@@ -31,6 +31,8 @@ class PinRepositoryTest extends TestDatabaseContainer {
     @Autowired
     private PinRepository pinRepository;
     @Autowired
+    private PinImageRepository pinImageRepository;
+    @Autowired
     private LocationRepository locationRepository;
     @Autowired
     private MemberRepository memberRepository;
@@ -41,7 +43,7 @@ class PinRepositoryTest extends TestDatabaseContainer {
 
     @BeforeEach
     void setUp() {
-        member = memberRepository.save(MemberFixture.create("member", "[email protected]", Role.USER));
+        member = memberRepository.save(MemberFixture.create("member", "[email protected]", USER));
         topic = topicRepository.save(TopicFixture.createByName("topic", member));
         location = locationRepository.save(LocationFixture.create());
     }
@@ -123,4 +125,39 @@ void deleteAllByMemberIdInOtherTopics_Success() {
         assertThat(pinRepository.findAllByCreatorId(MemberId)).isEmpty();
     }
 
+    @Test
+    @DisplayName("기존에 존재하는 핀들을 토픽에 한 번에 복사할 수 있다. (bulk insert)")
+    void saveAllToTopic() {
+        // given
+        for (int i = 0; i < 10; i++) {
+            Pin pin = pinRepository.save(PinFixture.create(location, topic, member));
+            pinRepository.flush();
+            pinImageRepository.save(PinImageFixture.create(pin));
+        }
+        Member copier = memberRepository.save(MemberFixture.create("copier", "[email protected]", USER));
+        Topic topicForCopy = topicRepository.save(TopicFixture.createByName("otherTopic", copier));
+
+        // when
+        List<Pin> originalPins = topic.getPins();
+        pinRepository.saveAllToTopic(topicForCopy, originalPins);
+
+        // then
+        List<Pin> copiedPins = pinRepository.findAllByTopicId(topicForCopy.getId());
+        List<PinInfo> originalPinInfos = originalPins.stream()
+                .map(Pin::getPinInfo)
+                .toList();
+
+        assertSoftly(softly -> {
+            softly.assertThat(copiedPins).extracting("pinInfo")
+                    .usingRecursiveComparison()
+                    .isEqualTo(originalPinInfos);
+            softly.assertThat(copiedPins.get(0).getCreator())
+                    .isEqualTo(copier);
+            softly.assertThat(copiedPins).hasSize(originalPins.size())
+                    .flatMap(Pin::getPinImages)
+                    .allSatisfy(pinImage -> {
+                        assertThat(pinImage.getImageUrl()).isEqualTo(PinImageFixture.IMAGE_URL);
+                    });
+        });
+    }
 }