Merge pull request #2592 from mercedes-benz/feature-2075-auto-cleanup…

…-shall-drop-old-scan-results

Enable auto cleanup for old scan reports #2075

sechub-integrationtest/src/test/java/com/mercedesbenz/sechub/integrationtest/scenario1/AutoCleanupScenario1IntTest.java

@@ -54,8 +54,9 @@ public void auto_cleanup_executed_in_every_domain_when_admin_configures_cleanupd
             /* schedule domain */
             addExpectedDeleteInspection("sechub-jobs","com.mercedesbenz.sechub.domain.schedule.autocleanup.ScheduleAutoCleanupService",0).
             addExpectedDeleteInspection("product-results","com.mercedesbenz.sechub.domain.scan.autocleanup.ScanAutoCleanupService",0).
+            addExpectedDeleteInspection("scan-reports","com.mercedesbenz.sechub.domain.scan.autocleanup.ScanAutoCleanupService",0).
 
-            addExpectedDifferentKindOfDeleteInspections(4).
+            addExpectedDifferentKindOfDeleteInspections(5).
 
             assertAsExpectedWithTimeOut(15);
 

sechub-integrationtest/src/test/java/com/mercedesbenz/sechub/integrationtest/scenario3/FalsePositivesScenario3IntTest.java

@@ -59,6 +59,15 @@ public void with_sechubclient_mark_falsepositives_of_only_existing_medium_will_r
             finding().id(1).name("Absolute Path Traversal").isNotContained().
             hasTrafficLight(TrafficLight.GREEN);
 
+        /* execute 2 - duplicate call to mark false positives*/
+        as(USER_1).withSecHubClient().startFalsePositiveDefinition(project,location).add(1, jobUUID).markAsFalsePositive();
+
+        /* test 2 - false positive works also after second call*/
+        ExecutionResult result3 = as(USER_1).withSecHubClient().startSynchronScanFor(project, location);
+        assertReportUnordered(result3).
+            finding().id(1).name("Absolute Path Traversal").isNotContained().
+            hasTrafficLight(TrafficLight.GREEN);
+
         /* @formatter:on */
     }
 

sechub-scan/src/main/java/com/mercedesbenz/sechub/domain/scan/autocleanup/ScanAutoCleanupService.java

@@ -41,8 +41,6 @@ public class ScanAutoCleanupService {
     @Autowired
     AutoCleanupResultInspector inspector;
 
-    private static boolean statistic_feature_1010_implemented = false;
-
     @UseCaseScanAutoCleanExecution(@Step(number = 2, name = "Delete old data", description = "deletes old job information"))
     public void cleanup() {
         /* calculate */
@@ -75,9 +73,6 @@ private void deleteScanLogs(long days, LocalDateTime cleanTimeStamp) {
 
     private void deleteScanResults(long days, LocalDateTime cleanTimeStamp) {
         /* @formatter:off */
-        if (! statistic_feature_1010_implemented) {
-            return;
-        }
         int amount = scanReportRepository.deleteReportsOlderThan(cleanTimeStamp);
         inspector.inspect(AutoCleanupResult.builder().
                 autoCleanup("scan-reports",getClass()).

sechub-scan/src/main/java/com/mercedesbenz/sechub/domain/scan/project/FalsePositiveJobDataConfigMerger.java

@@ -58,6 +58,10 @@ public void removeJobDataWithMetaDataFromConfig(FalsePositiveProjectConfiguratio
         config.getFalsePositives().remove(entry);
     }
 
+    public boolean isFalsePositiveEntryAlreadyExisting(FalsePositiveProjectConfiguration config, FalsePositiveJobData falsePositiveJobData) {
+        return findExistingFalsePositiveEntryInConfig(config, falsePositiveJobData) != null;
+    }
+
     private FalsePositiveEntry findExistingFalsePositiveEntryInConfig(FalsePositiveProjectConfiguration config, FalsePositiveJobData falsePositiveJobData) {
         for (FalsePositiveEntry existingFPEntry : config.getFalsePositives()) {
             FalsePositiveJobData jobData = existingFPEntry.getJobData();

sechub-scan/src/main/java/com/mercedesbenz/sechub/domain/scan/project/FalsePositiveJobDataService.java

@@ -109,25 +109,41 @@ private void validateProjectIdAndProjectAccess(String projectId) {
     private void addJobDataListToConfiguration(FalsePositiveProjectConfiguration config, FalsePositiveJobDataList jobDataList) {
         List<FalsePositiveJobData> list = jobDataList.getJobData();
 
-        /* we want to load reports only one time, so sort by report job UUID... */
+        /*
+         * Reason for sorting: we want to load reports only one time, so sort by report
+         * job UUID is necessary for method "fetchReportIfNotAlreadyLoaded"
+         */
         list.sort(Comparator.comparing(FalsePositiveJobData::getJobUUID));
 
-        ScanSecHubReport report = null;
+        ScanSecHubReport currentReport = null;
+
         for (FalsePositiveJobData data : list) {
+            if (merger.isFalsePositiveEntryAlreadyExisting(config, data)) {
+                LOG.debug("Skip processing because FP already defined: {}", data);
+                continue;
+            }
+
             UUID jobUUID = data.getJobUUID();
+            currentReport = fetchReportIfNotAlreadyLoaded(jobUUID, currentReport);
 
-            if (report == null || !jobUUID.equals(report.getJobUUID())) {
-                ScanReport scanReport = scanReportRepository.findBySecHubJobUUID(jobUUID);
-                if (scanReport == null) {
-                    throw new NotFoundException("No report found for job " + jobUUID);
-                }
-                report = new ScanSecHubReport(scanReport);
-            }
-            merger.addJobDataWithMetaDataToConfig(report, config, data, userContextService.getUserId());
+            merger.addJobDataWithMetaDataToConfig(currentReport, config, data, userContextService.getUserId());
         }
 
     }
 
+    private ScanSecHubReport fetchReportIfNotAlreadyLoaded(UUID jobUUID, ScanSecHubReport currentReport) {
+
+        /* load report if it is not the current report */
+        if (currentReport == null || !jobUUID.equals(currentReport.getJobUUID())) {
+            ScanReport scanReport = scanReportRepository.findBySecHubJobUUID(jobUUID);
+            if (scanReport == null) {
+                throw new NotFoundException("No report found for job " + jobUUID);
+            }
+            currentReport = new ScanSecHubReport(scanReport);
+        }
+        return currentReport;
+    }
+
     private FalsePositiveProjectConfiguration fetchOrCreateConfiguration(String projectId) {
         ScanProjectConfig projectConfig = configService.getOrCreate(projectId, CONFIG_ID, false, "{}"); // access check unnecessary, already done
 

sechub-scan/src/test/java/com/mercedesbenz/sechub/domain/scan/autocleanup/ScanAutoCleanupServiceTest.java

@@ -90,13 +90,11 @@ void cleanup_executes_delete_job_information_for_30_days() {
         verify(timeCalculationService).calculateNowMinusDays(eq(days));
         verify(productResultRepository, times(1)).deleteResultsOlderThan(cleanTime);
         verify(projectScanLogRepository, times(1)).deleteLogsOlderThan(cleanTime);
-        // as long as issue https://github.com/mercedes-benz/sechub/issues/1010 is not
-        // implemented we keep the old data for statistics, so never called:
-        verify(scanReportRepository, never()).deleteReportsOlderThan(cleanTime);
+        verify(scanReportRepository, times(1)).deleteReportsOlderThan(cleanTime);
 
         // check inspection as expected
         ArgumentCaptor<AutoCleanupResult> captor = ArgumentCaptor.forClass(AutoCleanupResult.class);
-        verify(inspector, times(2)).inspect(captor.capture());
+        verify(inspector, times(3)).inspect(captor.capture());
 
         List<AutoCleanupResult> values = captor.getAllValues();
         for (AutoCleanupResult result : values) {
@@ -112,6 +110,9 @@ void cleanup_executes_delete_job_information_for_30_days() {
             case "product-results":
                 assertEquals(20, result.getDeletedEntries());
                 break;
+            case "scan-reports":
+                assertEquals(30, result.getDeletedEntries());
+                break;
             default:
                 fail("unexpected variant:" + variant);
             }