From becbc05b87af6d4f760b94bc397fc3e95389cd25 Mon Sep 17 00:00:00 2001 From: Chip Pate Date: Wed, 3 Jul 2024 11:33:50 -0400 Subject: [PATCH] IA-5009: resolve sonarcloud issues (#18) --- .../bio/terra/appmanager/model/Chart.java | 2 +- .../appmanager/api/AdminControllerTest.java | 14 +- .../terra/appmanager/dao/ChartDaoTest.java | 191 +++++++++--------- .../appmanager/service/ChartServiceTest.java | 93 +++++---- 4 files changed, 150 insertions(+), 150 deletions(-) diff --git a/service/src/main/java/bio/terra/appmanager/model/Chart.java b/service/src/main/java/bio/terra/appmanager/model/Chart.java index cddc596..52b70f6 100644 --- a/service/src/main/java/bio/terra/appmanager/model/Chart.java +++ b/service/src/main/java/bio/terra/appmanager/model/Chart.java @@ -37,7 +37,7 @@ public record Chart( // Must follow chart value conventions: // https://helm.sh/docs/chart_best_practices/conventions/#version-numbers // Semver conventions, with digits only - static final String chartValueRegex = "^([0-9]\\d*)\\.([0-9]\\d*)\\.([0-9]\\d*)$"; + static final String chartValueRegex = "^(\\d+)\\.(\\d+)\\.(\\d+)$"; static final Pattern chartVersionPattern = Pattern.compile(chartValueRegex); public static boolean isChartNameValid(String chartName) { diff --git a/service/src/test/java/bio/terra/appmanager/api/AdminControllerTest.java b/service/src/test/java/bio/terra/appmanager/api/AdminControllerTest.java index 2d55b6d..5d973dc 100644 --- a/service/src/test/java/bio/terra/appmanager/api/AdminControllerTest.java +++ b/service/src/test/java/bio/terra/appmanager/api/AdminControllerTest.java @@ -43,8 +43,8 @@ class AdminControllerTest { private MockMvc mockMvc; - @Captor ArgumentCaptor> capture_Charts; - @Captor ArgumentCaptor capture_chartName; + @Captor ArgumentCaptor> captureCharts; + @Captor ArgumentCaptor captureChartName; private AutoCloseable closeable; @@ -82,9 +82,9 @@ void testCreate_204() throws Exception { + "}]")) .andExpect(status().isNoContent()); - verify(serviceMock).createCharts(capture_Charts.capture()); - assert (capture_Charts.getValue().size() == 1); - verifyChart(capture_Charts.getValue().get(0), chartName, chartVersion, null, null, null); + verify(serviceMock).createCharts(captureCharts.capture()); + assert (captureCharts.getValue().size() == 1); + verifyChart(captureCharts.getValue().get(0), chartName, chartVersion, null, null, null); } @Test @@ -192,8 +192,8 @@ void testDelete_204() throws Exception { .perform(delete("/api/admin/v1/charts").queryParam("chartName", chartName)) .andExpect(status().isNoContent()); - verify(serviceMock).deleteVersion(capture_chartName.capture()); - assertEquals(capture_chartName.getValue(), chartName); + verify(serviceMock).deleteVersion(captureChartName.capture()); + assertEquals(captureChartName.getValue(), chartName); } @Test diff --git a/service/src/test/java/bio/terra/appmanager/dao/ChartDaoTest.java b/service/src/test/java/bio/terra/appmanager/dao/ChartDaoTest.java index 62c1364..b87dfb3 100644 --- a/service/src/test/java/bio/terra/appmanager/dao/ChartDaoTest.java +++ b/service/src/test/java/bio/terra/appmanager/dao/ChartDaoTest.java @@ -58,73 +58,73 @@ void testMultiVersionUpsert() { @Test void testMultiNameGet() { - String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - String chartVersion1_2 = ChartTestUtils.makeChartVersion(1); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); - Chart version1_2 = new Chart(chartName1, chartVersion1_2); - - String chartName2 = "chart-name-here-too"; - String chartVersion2_1 = ChartTestUtils.makeChartVersion(2); - String chartVersion2_2 = ChartTestUtils.makeChartVersion(3); - Chart version2_1 = new Chart(chartName2, chartVersion2_1); - Chart version2_2 = new Chart(chartName2, chartVersion2_2); - - String chartName3 = "chart-name"; - String chartVersion3_1 = ChartTestUtils.makeChartVersion(4); - String chartVersion3_2 = ChartTestUtils.makeChartVersion(5); - Chart version3_1 = new Chart(chartName3, chartVersion3_1); - Chart version3_2 = new Chart(chartName3, chartVersion3_2); - - chartDao.upsert(version1_1); - chartDao.upsert(version1_2); - chartDao.upsert(version2_1); - chartDao.upsert(version2_2); - chartDao.upsert(version3_1); - chartDao.upsert(version3_2); - - List storedVersions = chartDao.get((List.of(chartName1, chartName2))); + String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + String chart1Version2 = ChartTestUtils.makeChartVersion(1); + Chart oldChart1 = new Chart(chart1Name, chart1Version1); + Chart newChart1 = new Chart(chart1Name, chart1Version2); + + String chart2Name = "chart-name-here-too"; + String chart2Version1 = ChartTestUtils.makeChartVersion(2); + String chart2Version2 = ChartTestUtils.makeChartVersion(3); + Chart oldChart2 = new Chart(chart2Name, chart2Version1); + Chart newChart2 = new Chart(chart2Name, chart2Version2); + + String chart3Name = "chart-name"; + String chart3Version1 = ChartTestUtils.makeChartVersion(4); + String chart3Version2 = ChartTestUtils.makeChartVersion(5); + Chart oldChart3 = new Chart(chart3Name, chart3Version1); + Chart newChart3 = new Chart(chart3Name, chart3Version2); + + chartDao.upsert(oldChart1); + chartDao.upsert(newChart1); + chartDao.upsert(oldChart2); + chartDao.upsert(newChart2); + chartDao.upsert(oldChart3); + chartDao.upsert(newChart3); + + List storedVersions = chartDao.get((List.of(chart1Name, chart2Name))); assertEquals(2, storedVersions.size()); List targetCharts = - storedVersions.stream().filter(version -> chartName1.equals(version.name())).toList(); + storedVersions.stream().filter(version -> chart1Name.equals(version.name())).toList(); assertEquals(1, targetCharts.size()); Chart targetVersion = targetCharts.get(0); - assertEquals(chartVersion1_2, targetVersion.version()); + assertEquals(chart1Version2, targetVersion.version()); targetCharts = - storedVersions.stream().filter(version -> chartName2.equals(version.name())).toList(); + storedVersions.stream().filter(version -> chart2Name.equals(version.name())).toList(); assertEquals(1, targetCharts.size()); targetVersion = targetCharts.get(0); - assertEquals(chartVersion2_2, targetVersion.version()); + assertEquals(chart2Version2, targetVersion.version()); } @Test void testGetAll() { - String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - String chartVersion1_2 = ChartTestUtils.makeChartVersion(1); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); - Chart version1_2 = new Chart(chartName1, chartVersion1_2); - - String chartName2 = "chart-name-here-too"; - String chartVersion2_1 = ChartTestUtils.makeChartVersion(3); - String chartVersion2_2 = ChartTestUtils.makeChartVersion(4); - Chart version2_1 = new Chart(chartName2, chartVersion2_1); - Chart version2_2 = new Chart(chartName2, chartVersion2_2); - - String chartName3 = "chart-name-here-again"; - String chartVersion3_1 = ChartTestUtils.makeChartVersion(5); - String chartVersion3_2 = ChartTestUtils.makeChartVersion(6); - Chart version3_1 = new Chart(chartName3, chartVersion3_1); - Chart version3_2 = new Chart(chartName3, chartVersion3_2); - - chartDao.upsert(version1_1); - chartDao.upsert(version1_2); - chartDao.upsert(version2_1); - chartDao.upsert(version2_2); - chartDao.upsert(version3_1); - chartDao.upsert(version3_2); + String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + String chart1Version2 = ChartTestUtils.makeChartVersion(1); + Chart oldChart1 = new Chart(chart1Name, chart1Version1); + Chart newChart1 = new Chart(chart1Name, chart1Version2); + + String chart2Name = "chart-name-here-too"; + String chart2Version1 = ChartTestUtils.makeChartVersion(3); + String chart2Version2 = ChartTestUtils.makeChartVersion(4); + Chart oldChart2 = new Chart(chart2Name, chart2Version1); + Chart newChart2 = new Chart(chart2Name, chart2Version2); + + String chart3Name = "chart-name-here-again"; + String chart3Version1 = ChartTestUtils.makeChartVersion(5); + String chart3Version2 = ChartTestUtils.makeChartVersion(6); + Chart oldChart3 = new Chart(chart3Name, chart3Version1); + Chart newChart3 = new Chart(chart3Name, chart3Version2); + + chartDao.upsert(oldChart1); + chartDao.upsert(newChart1); + chartDao.upsert(oldChart2); + chartDao.upsert(newChart2); + chartDao.upsert(oldChart3); + chartDao.upsert(newChart3); List storedVersions = chartDao.get(true); assertEquals(6, storedVersions.size()); @@ -132,12 +132,12 @@ void testGetAll() { @Test void testDelete() { - String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); + String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version1); - chartDao.upsert(version1_1); - chartDao.delete(List.of(chartName1)); + chartDao.upsert(chart1); + chartDao.delete(List.of(chart1Name)); List deletedVersions = chartDao.get(true); assertEquals(1, deletedVersions.size()); @@ -147,61 +147,64 @@ void testDelete() { @Test void testDelete_noNames() { - final String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); + final String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version1); - chartDao.upsert(version1_1); + chartDao.upsert(chart1); chartDao.delete(List.of()); List versions = chartDao.get(); assertNotNull(versions); assertEquals(1, versions.size()); - assertEquals(chartName1, versions.get(0).name()); - assertEquals(chartVersion1_1, versions.get(0).version()); + assertEquals(chart1Name, versions.get(0).name()); + assertEquals(chart1Version1, versions.get(0).version()); } @Test void testMultiDelete() { - final String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - String chartVersion1_2 = ChartTestUtils.makeChartVersion(1); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); - Chart version1_2 = new Chart(chartName1, chartVersion1_2); - - final String chartName2 = "chart-name-here-too"; - String chartVersion2_1 = ChartTestUtils.makeChartVersion(2); - String chartVersion2_2 = ChartTestUtils.makeChartVersion(3); - Chart version2_1 = new Chart(chartName2, chartVersion2_1); - Chart version2_2 = new Chart(chartName2, chartVersion2_2); - - final String chartName3 = "chart-version-name-again"; - String chartVersion3_1 = ChartTestUtils.makeChartVersion(4); - String chartVersion3_2 = ChartTestUtils.makeChartVersion(5); - Chart version3_1 = new Chart(chartName3, chartVersion3_1); - Chart version3_2 = new Chart(chartName3, chartVersion3_2); - - chartDao.upsert(version1_1); - chartDao.upsert(version1_2); - chartDao.upsert(version2_1); - chartDao.upsert(version2_2); - chartDao.upsert(version3_1); - chartDao.upsert(version3_2); - - chartDao.delete(List.of(chartName1, chartName2)); + final String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + String chart1Version2 = ChartTestUtils.makeChartVersion(1); + Chart oldChart1 = new Chart(chart1Name, chart1Version1); + Chart newChart1 = new Chart(chart1Name, chart1Version2); + + final String chart2Name = "chart-name-here-too"; + String chart2Version1 = ChartTestUtils.makeChartVersion(2); + String chart2Version2 = ChartTestUtils.makeChartVersion(3); + Chart oldChart2 = new Chart(chart2Name, chart2Version1); + Chart newChart2 = new Chart(chart2Name, chart2Version2); + + final String chart3Name = "chart-version-name-again"; + String chart3Version1 = ChartTestUtils.makeChartVersion(4); + String chart3Version2 = ChartTestUtils.makeChartVersion(5); + Chart oldChart3 = new Chart(chart3Name, chart3Version1); + Chart newChart3 = new Chart(chart3Name, chart3Version2); + + chartDao.upsert(oldChart1); + chartDao.upsert(newChart1); + chartDao.upsert(oldChart2); + chartDao.upsert(newChart2); + chartDao.upsert(oldChart3); + chartDao.upsert(newChart3); + + chartDao.delete(List.of(chart1Name, chart2Name)); List allVersions = chartDao.get(true); for (Chart version : allVersions) { switch (version.name()) { - case chartName1, chartName2: + case chart1Name, chart2Name: assertNotNull(version.inactiveAt()); break; - case chartName3: - if (chartVersion3_2.equals(version.version())) { + case chart3Name: + if (chart3Version2.equals(version.version())) { assertNull(version.inactiveAt()); } else { assertNotNull(version.inactiveAt()); } + break; + default: + throw new IllegalStateException("unexpected chartName encountered"); } } } diff --git a/service/src/test/java/bio/terra/appmanager/service/ChartServiceTest.java b/service/src/test/java/bio/terra/appmanager/service/ChartServiceTest.java index 3a9e88f..79dfdd6 100644 --- a/service/src/test/java/bio/terra/appmanager/service/ChartServiceTest.java +++ b/service/src/test/java/bio/terra/appmanager/service/ChartServiceTest.java @@ -43,10 +43,10 @@ void testCreateChart_withEmptyList() { @Test void testCreateChart_unknownChartName() { - String chartName1 = "unknown-chart"; - String chartVersion1 = ChartTestUtils.makeChartVersion(0); - Chart version1 = new Chart(chartName1, chartVersion1); - List chartList = List.of(version1); + String chart1Name = "unknown-chart"; + String chart1Version = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version); + List chartList = List.of(chart1); Exception exception = assertThrows(IllegalArgumentException.class, () -> chartService.createCharts(chartList)); @@ -56,52 +56,52 @@ void testCreateChart_unknownChartName() { @Test void testCreateChart_singleElement() { - String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); + String chart1Name = "chart-name-here"; + String chart1Version = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version); ArgumentCaptor argument = ArgumentCaptor.forClass(Chart.class); - chartService.createCharts(List.of(version1_1)); + chartService.createCharts(List.of(chart1)); verify(chartDao, times(1)).upsert(argument.capture()); - assertEquals(version1_1.name(), argument.getValue().name()); - assertEquals(version1_1.version(), argument.getValue().version()); + assertEquals(chart1.name(), argument.getValue().name()); + assertEquals(chart1.version(), argument.getValue().version()); assertNull(argument.getValue().activeAt()); } @Test void testCreateChart_multipleElement() { - String chartName1 = "chart-name-here"; - String chartVersion1_1 = ChartTestUtils.makeChartVersion(0); - String chartVersion1_2 = ChartTestUtils.makeChartVersion(1); - Chart version1_1 = new Chart(chartName1, chartVersion1_1); - Chart version1_2 = new Chart(chartName1, chartVersion1_2); + String chart1Name = "chart-name-here"; + String chart1Version1 = ChartTestUtils.makeChartVersion(0); + String chart1Version2 = ChartTestUtils.makeChartVersion(1); + Chart oldChart1 = new Chart(chart1Name, chart1Version1); + Chart newChart1 = new Chart(chart1Name, chart1Version2); ArgumentCaptor argument = ArgumentCaptor.forClass(Chart.class); InOrder inOrder = inOrder(chartDao); - chartService.createCharts(List.of(version1_1, version1_2)); + chartService.createCharts(List.of(oldChart1, newChart1)); inOrder.verify(chartDao, calls(1)).upsert(argument.capture()); - assertEquals(version1_1.name(), argument.getValue().name()); - assertEquals(version1_1.version(), argument.getValue().version()); + assertEquals(oldChart1.name(), argument.getValue().name()); + assertEquals(oldChart1.version(), argument.getValue().version()); assertNull(argument.getValue().activeAt()); inOrder.verify(chartDao, calls(1)).upsert(argument.capture()); - assertEquals(version1_2.name(), argument.getValue().name()); - assertEquals(version1_2.version(), argument.getValue().version()); + assertEquals(newChart1.name(), argument.getValue().name()); + assertEquals(newChart1.version(), argument.getValue().version()); assertNull(argument.getValue().activeAt()); } @Test void testDeleteVersion() { - String chartName1 = "chart-name-here"; + String chart1Name = "chart-name-here"; ArgumentCaptor> argument = ArgumentCaptor.forClass(List.class); - chartService.deleteVersion(chartName1); + chartService.deleteVersion(chart1Name); verify(chartDao, times(1)).delete(argument.capture()); assertEquals(1, argument.getValue().size()); - assertEquals(chartName1, argument.getValue().get(0)); + assertEquals(chart1Name, argument.getValue().get(0)); } @Test @@ -109,9 +109,6 @@ void testGetVersions() { List chartNameList = List.of("chart-name-here"); Boolean includeAll = true; - ArgumentCaptor> argument1 = ArgumentCaptor.forClass(List.class); - ArgumentCaptor argument2 = ArgumentCaptor.forClass(Boolean.class); - InOrder inOrder = inOrder(chartDao); chartService.getCharts(chartNameList, includeAll); inOrder.verify(chartDao, calls(1)).get(chartNameList, includeAll); @@ -119,22 +116,22 @@ void testGetVersions() { @Test void testUpdateVersions() { - String chartName1 = "chart-name-here"; - String chartVersion = ChartTestUtils.makeChartVersion(0); - Chart chart = new Chart(chartName1, chartVersion); + String chart1Name = "chart-name-here"; + String chart1Version = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version); - when(chartDao.get(List.of(chartName1), true)).thenReturn(List.of(chart)); + when(chartDao.get(List.of(chart1Name), true)).thenReturn(List.of(chart1)); - chartService.updateVersions(List.of(chart)); - verify(chartDao, times(1)).upsert(chart); + chartService.updateVersions(List.of(chart1)); + verify(chartDao, times(1)).upsert(chart1); } @Test void testUpdateChart_unknownChartName() { - String chartName1 = "unknown-chart"; - String chartVersion1 = ChartTestUtils.makeChartVersion(0); - Chart version1 = new Chart(chartName1, chartVersion1); - List chartList = List.of(version1); + String chart1Name = "unknown-chart"; + String chart1Version = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version); + List chartList = List.of(chart1); Exception exception = assertThrows(IllegalArgumentException.class, () -> chartService.updateVersions(chartList)); @@ -144,27 +141,27 @@ void testUpdateChart_unknownChartName() { @Test void testUpdateVersions_chartNotFound() { - String chartName = "chart-name"; - String chartVersion = ChartTestUtils.makeChartVersion(0); - Chart chart = new Chart(chartName, chartVersion); - List charts = List.of(chart); + String chart1Name = "chart-name"; + String chart1Version = ChartTestUtils.makeChartVersion(0); + Chart chart1 = new Chart(chart1Name, chart1Version); + List charts = List.of(chart1); assertThrows(ChartNotFoundException.class, () -> chartService.updateVersions(charts)); } @Test void testUpdate_multipleChartNotFound() { - String chartName1 = "chart-name"; - String chartName2 = "chart-name2"; - String chartName3 = "chart-name3"; + String chart1Name = "chart-name"; + String chart2Name = "chart-name2"; + String chart3Name = "chart-name3"; String chartVersion = ChartTestUtils.makeChartVersion(0); - Chart chart1 = new Chart(chartName1, chartVersion); - Chart chart2 = new Chart(chartName2, chartVersion); - Chart chart3 = new Chart(chartName3, chartVersion); + Chart chart1 = new Chart(chart1Name, chartVersion); + Chart chart2 = new Chart(chart2Name, chartVersion); + Chart chart3 = new Chart(chart3Name, chartVersion); List charts = List.of(chart1, chart2, chart3); - List notPresentCharts = List.of(chartName2, chartName3); + List notPresentCharts = List.of(chart2Name, chart3Name); - when(chartDao.get(List.of(chartName1), true)).thenReturn(List.of(chart1)); + when(chartDao.get(List.of(chart1Name), true)).thenReturn(List.of(chart1)); ChartNotFoundException ex = assertThrows(ChartNotFoundException.class, () -> chartService.updateVersions(charts));