Improve PipelineRun update methods when Pipeline is renamed or deleted

epam · Dec 26, 2023 · 62303e3 · 62303e3
1 parent 4df7a4a
commit 62303e3
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 34 deletions.
diff --git a/api/src/main/java/com/epam/pipeline/dao/pipeline/PipelineRunDao.java b/api/src/main/java/com/epam/pipeline/dao/pipeline/PipelineRunDao.java
@@ -125,6 +125,8 @@ public class PipelineRunDao extends NamedParameterJdbcDaoSupport {
     private String updateLastNotificationQuery;
     private String updateProlongedAtTimeAndLastIdleNotificationTimeQuery;
     private String updateRunQuery;
+    private String updatePipelineNameForRunsQuery;
+    private String clearPipelineIdForRunsQuery;
     private String loadRunByPrettyUrlQuery;
     private String updateTagsQuery;
     private String loadAllRunsPossiblyActiveInPeriodQuery;
@@ -405,6 +407,21 @@ public void updateRuns(final Collection<PipelineRun> runs) {
         getNamedParameterJdbcTemplate().batchUpdate(updateRunQuery, getParamsForBatchUpdate(runs));
     }
 
+    @Transactional(propagation = Propagation.MANDATORY)
+    public void updatePipelineNameForRuns(final String pipelineName, final Long pipelineId) {
+        final MapSqlParameterSource params = new MapSqlParameterSource();
+        params.addValue(PipelineRunParameters.PIPELINE_NAME.name(), pipelineName);
+        params.addValue(PipelineRunParameters.PIPELINE_ID.name(), pipelineId);
+        getNamedParameterJdbcTemplate().update(updatePipelineNameForRunsQuery, params);
+    }
+
+    @Transactional(propagation = Propagation.MANDATORY)
+    public void clearPipelineIdForRuns(final Long pipelineId) {
+        final MapSqlParameterSource params = new MapSqlParameterSource();
+        params.addValue(PipelineRunParameters.PIPELINE_ID.name(), pipelineId);
+        getNamedParameterJdbcTemplate().update(clearPipelineIdForRunsQuery, params);
+    }
+
     public int countFilteredPipelineRuns(PipelineRunFilterVO filter, PipelineRunFilterVO.ProjectFilter projectFilter) {
         MapSqlParameterSource params = new MapSqlParameterSource();
         String query = wherePattern.matcher(countFilteredPipelineRunsBaseQuery).replaceFirst(makeFilterCondition(filter,
@@ -1428,4 +1445,15 @@ public void setLoadRunsByParentRunsIdsQuery(final String loadRunsByParentRunsIds
     public void setLoadRunsByPoolIdQuery(final String loadRunsByPoolIdQuery) {
         this.loadRunsByPoolIdQuery = loadRunsByPoolIdQuery;
     }
+
+    @Required
+    public void setUpdatePipelineNameForRunsQuery(final String updatePipelineNameForRunsQuery) {
+        this.updatePipelineNameForRunsQuery = updatePipelineNameForRunsQuery;
+    }
+
+    @Required
+
+    public void setClearPipelineIdForRunsQuery(final String clearPipelineIdForRunsQuery) {
+        this.clearPipelineIdForRunsQuery = clearPipelineIdForRunsQuery;
+    }
 }
diff --git a/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineManager.java b/api/src/main/java/com/epam/pipeline/manager/pipeline/PipelineManager.java
@@ -30,7 +30,6 @@
 import com.epam.pipeline.entity.git.GitProject;
 import com.epam.pipeline.entity.pipeline.Folder;
 import com.epam.pipeline.entity.pipeline.Pipeline;
-import com.epam.pipeline.entity.pipeline.PipelineRun;
 import com.epam.pipeline.entity.pipeline.PipelineType;
 import com.epam.pipeline.entity.pipeline.RepositoryType;
 import com.epam.pipeline.entity.pipeline.Revision;
@@ -57,10 +56,8 @@
 import org.springframework.web.client.HttpClientErrorException;
 
 import java.util.List;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
 
 @Service
 @AclSync
@@ -186,6 +183,7 @@ public Pipeline update(final PipelineVO pipelineVO) {
         Assert.isTrue(GitUtils.checkGitNaming(pipelineVOName),
                 messageHelper.getMessage(MessageConstants.ERROR_INVALID_PIPELINE_NAME, pipelineVOName));
         Pipeline dbPipeline = load(pipelineVO.getId());
+        String previousName = dbPipeline.getName();
         final String currentProjectPath = dbPipeline.getRepository();
         final String currentProjectName = GitUtils.convertPipeNameToProject(dbPipeline.getName());
         final String newProjectName = GitUtils.convertPipeNameToProject(pipelineVOName);
@@ -208,8 +206,9 @@ public Pipeline update(final PipelineVO pipelineVO) {
         dbPipeline.setDocsPath(pipelineVO.getDocsPath());
         dbPipeline.setConfigurationPath(StringUtils.strip(pipelineVO.getConfigurationPath(), Constants.PATH_DELIMITER));
         pipelineDao.updatePipeline(dbPipeline);
-
-        updatePipelineNameForRuns(pipelineVO, pipelineVOName);
+        if (!previousName.equals(pipelineVOName)) {
+            updatePipelineNameForRuns(pipelineVO.getId(), pipelineVOName);
+        }
 
         if (projectNameUpdated) {
             pipelineRepositoryService.updateRepositoryName(dbPipeline, currentProjectPath, newProjectName);
@@ -434,32 +433,12 @@ private void copyStorageRules(final Long sourcePipelineId, final Long newPipelin
         });
     }
 
-    private void updatePipelineNameForRuns(final PipelineVO pipelineVO, final String pipelineVOName) {
-        final List<PipelineRun> runsToUpdate = ListUtils.emptyIfNull(
-                pipelineRunDao.loadAllRunsForPipeline(pipelineVO.getId())).stream()
-                .map(run -> updatePipelineNameForRun(pipelineVOName, run))
-                .filter(Objects::nonNull)
-                .collect(Collectors.toList());
-        pipelineRunDao.updateRuns(runsToUpdate);
-    }
-
-    private PipelineRun updatePipelineNameForRun(final String pipelineName, final PipelineRun run) {
-        if (Objects.equals(run.getPipelineName(), pipelineName)) {
-            return null;
-        }
-        run.setPipelineName(pipelineName);
-        return run;
+    private void updatePipelineNameForRuns(final Long pipelineId, final String pipelineVOName) {
+        pipelineRunDao.updatePipelineNameForRuns(pipelineVOName, pipelineId);
     }
 
     private void resetPipelineIdForRuns(final Long id) {
-        pipelineRunDao.updateRuns(ListUtils.emptyIfNull(pipelineRunDao.loadAllRunsForPipeline(id)).stream()
-                .map(this::resetPipelineIdForRun)
-                .collect(Collectors.toSet()));
-    }
-
-    private PipelineRun resetPipelineIdForRun(final PipelineRun run) {
-        run.setPipelineId(null);
-        return run;
+        pipelineRunDao.clearPipelineIdForRuns(id);
     }
 
     private void checkBranchExists(final PipelineVO pipelineVO) {

diff --git a/api/src/main/resources/dao/pipeline-run-dao.xml b/api/src/main/resources/dao/pipeline-run-dao.xml
@@ -474,6 +474,26 @@
                 ]]>
             </value>
         </property>
+        <property name="updatePipelineNameForRunsQuery">
+            <value>
+                <![CDATA[
+                    UPDATE pipeline.pipeline_run SET
+                        pipeline_name = :PIPELINE_NAME
+                    WHERE
+                        pipeline_id = :PIPELINE_ID
+                ]]>
+            </value>
+        </property>
+        <property name="clearPipelineIdForRunsQuery">
+            <value>
+                <![CDATA[
+                    UPDATE pipeline.pipeline_run SET
+                        pipeline_id = NULL
+                    WHERE
+                        pipeline_id = :PIPELINE_ID
+                ]]>
+            </value>
+        </property>
         <property name="updateRunQuery">
             <value>
                 <![CDATA[

diff --git a/api/src/test/java/com/epam/pipeline/dao/pipeline/PipelineRunDaoTest.java b/api/src/test/java/com/epam/pipeline/dao/pipeline/PipelineRunDaoTest.java
@@ -76,6 +76,7 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.hasItems;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -1111,6 +1112,40 @@ public void shouldLoadRunsByPoolId() {
         assertThat(loaded.get(0).getId(), is(run1.getId()));
     }
 
+    @Test
+    public void shouldUpdatePipelineNameForRuns() {
+        final Pipeline pipeline = getPipeline();
+        final PipelineRun run1 = buildPipelineRun(pipeline.getId());
+        run1.setPipelineName(TEST_PIPELINE_NAME);
+        pipelineRunDao.createPipelineRun(run1);
+
+        final PipelineRun run2 = buildPipelineRun(pipeline.getId());
+        run2.setPipelineName(TEST_PIPELINE_NAME);
+        pipelineRunDao.createPipelineRun(run2);
+
+        final String newName = "New";
+        pipelineRunDao.updatePipelineNameForRuns(newName, pipeline.getId());
+
+        final List<PipelineRun> runs = pipelineRunDao.loadAllRunsForPipeline(pipeline.getId());
+        runs.forEach(run -> assertThat(run.getPipelineName(), is(newName)));
+    }
+
+    @Test
+    public void shouldClearPipelineIdForRuns() {
+        final Pipeline pipeline = getPipeline();
+        final PipelineRun run1 = buildPipelineRun(pipeline.getId());
+        run1.setPipelineName(TEST_PIPELINE_NAME);
+        pipelineRunDao.createPipelineRun(run1);
+
+        final PipelineRun run2 = buildPipelineRun(pipeline.getId());
+        run2.setPipelineName(TEST_PIPELINE_NAME);
+        pipelineRunDao.createPipelineRun(run2);
+
+        pipelineRunDao.clearPipelineIdForRuns(pipeline.getId());
+        assertThat(pipelineRunDao.loadAllRunsForPipeline(pipeline.getId()).isEmpty(), is(true));
+        assertThat(pipelineRunDao.loadPipelineRun(run1.getId()).getPipelineId(), is(nullValue()));
+    }
+
     private PipelineRun createTestPipelineRun() {
         return createTestPipelineRun(testPipeline.getId());
     }

diff --git a/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineManagerTest.java b/api/src/test/java/com/epam/pipeline/manager/pipeline/PipelineManagerTest.java
@@ -41,7 +41,6 @@
 import org.mockito.MockitoAnnotations;
 
 import java.util.Collections;
-import java.util.List;
 
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
@@ -285,11 +284,7 @@ public void shouldUpdatePipelineNameForPipelineRuns() {
 
         pipelineManager.update(pipelineVO);
 
-        final ArgumentCaptor<List<PipelineRun>> runsCaptor = ArgumentCaptor.forClass((Class) List.class);
-        verify(pipelineRunDao).updateRuns(runsCaptor.capture());
-        assertThat(runsCaptor.getValue().size(), is(1));
-        assertThat(runsCaptor.getValue().get(0).getPipelineName(), is(NEW_REPOSITORY_NAME));
-        verify(pipelineRunDao).loadAllRunsForPipeline(ID);
+        verify(pipelineRunDao).updatePipelineNameForRuns(eq(NEW_REPOSITORY_NAME), eq(ID));
     }
 
     private DataStorageRule buildStorageRule(final Long pipelineId, final String mask) {