Development: Fix an issue with duplicated notification recipients in …

…Bamboo (ls1intum#7231)

src/main/java/de/tum/in/www1/artemis/config/migration/MigrationRegistry.java

@@ -11,6 +11,7 @@
 import org.springframework.stereotype.Component;
 
 import de.tum.in.www1.artemis.config.migration.entries.MigrationEntry20230808_203400;
+import de.tum.in.www1.artemis.config.migration.entries.MigrationEntry20230920_181600;
 
 /**
  * This component allows registering certain entries containing functionality that gets executed on application startup. The entries must extend {@link MigrationEntry}.
@@ -29,6 +30,7 @@ public MigrationRegistry(MigrationService migrationService) {
         this.migrationService = migrationService;
 
         this.migrationEntryMap.put(1, MigrationEntry20230808_203400.class);
+        this.migrationEntryMap.put(2, MigrationEntry20230920_181600.class);
     }
 
     /**

src/main/java/de/tum/in/www1/artemis/config/migration/entries/BambooMigrationService.java

@@ -22,6 +22,7 @@
 import org.springframework.stereotype.Service;
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
+import org.springframework.web.client.RestClientException;
 import org.springframework.web.client.RestTemplate;
 import org.springframework.web.util.UriComponentsBuilder;
 
@@ -116,13 +117,29 @@ private static Optional<String> getRepositoryNameById(String html, Long id) {
 
     @Override
     public void overrideBuildPlanNotification(String projectKey, String buildPlanKey, VcsRepositoryUrl vcsRepositoryUrl) {
-        List<Long> notificationIds = getAllArtemisBuildPlanServerNotificationIds(buildPlanKey);
-
-        for (var notificationId : notificationIds) {
-            deleteBuildPlanServerNotificationId(buildPlanKey, notificationId);
+        Map<Long, String> notificationIds = getAllArtemisBuildPlanServerNotificationIds(buildPlanKey);
+        log.info("Found {} notifications for build plan {}", notificationIds.size(), buildPlanKey);
+
+        List<Long> idsWithValidUrl = notificationIds.entrySet().stream().filter(entry -> entry.getValue().equals(artemisServerUrl + NEW_RESULT_RESOURCE_API_PATH))
+                .map(Map.Entry::getKey).toList();
+        boolean hasValidUrl = !idsWithValidUrl.isEmpty();
+        if (hasValidUrl) {
+            log.info("Build plan {} already has a notification with the correct URL", buildPlanKey);
+            notificationIds.remove(idsWithValidUrl.get(0));
         }
 
-        createBuildPlanServerNotification(buildPlanKey, artemisServerUrl + NEW_RESULT_RESOURCE_API_PATH);
+        notificationIds.forEach((id, url) -> {
+            try {
+                deleteBuildPlanServerNotificationId(buildPlanKey, id);
+            }
+            catch (RestClientException e) {
+                log.error("Could not delete notification with id " + id + " for build plan " + buildPlanKey, e);
+            }
+        });
+
+        if (!hasValidUrl) {
+            createBuildPlanServerNotification(buildPlanKey, artemisServerUrl + NEW_RESULT_RESOURCE_API_PATH);
+        }
     }
 
     @Override
@@ -297,7 +314,7 @@ public boolean buildPlanExists(String projectKey, String buildPlanKey) {
      * @param buildPlanKey The key of the build plan, which is usually the name combined with the project, e.g. 'EIST16W1-GA56HUR'.
      * @return a list of all notification ids
      */
-    private List<Long> getAllArtemisBuildPlanServerNotificationIds(String buildPlanKey) {
+    private Map<Long, String> getAllArtemisBuildPlanServerNotificationIds(String buildPlanKey) {
         MultiValueMap<String, String> parameters = new LinkedMultiValueMap<>();
         parameters.add("buildKey", buildPlanKey);
         String requestUrl = bambooServerUrl + "/chain/admin/config/defaultChainNotification.action";
@@ -306,16 +323,16 @@ private List<Long> getAllArtemisBuildPlanServerNotificationIds(String buildPlanK
         var response = restTemplate.exchange(builder.build().toUri(), HttpMethod.GET, null, String.class);
         var html = response.getBody();
         if (html == null) {
-            return List.of();
+            return Map.of();
         }
         Element notificationTableBody = Jsoup.parse(html).selectFirst("table#notificationTable tbody");
         if (notificationTableBody == null) {
-            return List.of();
+            return Map.of();
         }
         // First column is the event, second column the recipient, third the actions
         // If there is a URL, the URL is the recipient. In that case we take the notification id from the edit button
         Elements entries = notificationTableBody.select("tr");
-        List<Long> notificationIds = new ArrayList<>();
+        Map<Long, String> notificationIdToRecipient = new HashMap<>();
         for (Element entry : entries) {
             Elements columns = entry.select("td");
             if (columns.size() != 3) {
@@ -324,16 +341,14 @@ private List<Long> getAllArtemisBuildPlanServerNotificationIds(String buildPlanK
             String recipient = columns.get(1).text();
             String actions = columns.get(2).toString();
             Pattern editNotificationIdPattern = Pattern.compile(".*?id=\"editNotification:(\\d+)\".*?");
-            if (recipient.trim().startsWith(artemisServerUrl)) {
-                Matcher matcher = editNotificationIdPattern.matcher(actions);
-                if (matcher.find()) {
-                    String notificationIdString = matcher.group(1);
-                    notificationIds.add(Long.parseLong(notificationIdString));
-                }
+            Matcher matcher = editNotificationIdPattern.matcher(actions);
+            if (matcher.find()) {
+                String notificationIdString = matcher.group(1);
+                notificationIdToRecipient.put(Long.parseLong(notificationIdString), recipient);
             }
         }
 
-        return notificationIds;
+        return notificationIdToRecipient;
     }
 
     /**

src/main/java/de/tum/in/www1/artemis/config/migration/entries/MigrationEntry20230808_203400.java

@@ -223,10 +223,10 @@ private void migrateSolutions(List<SolutionProgrammingExerciseParticipation> par
                 migrateSolutionBuildPlan(participation, auxiliaryRepositories);
 
                 migrateTestRepository(participation);
-                log.info("Migrated template build plan for exercise {} in {}ms", participation.getProgrammingExercise().getId(), System.currentTimeMillis() - startMs);
+                log.info("Migrated solution build plan for exercise {} in {}ms", participation.getProgrammingExercise().getId(), System.currentTimeMillis() - startMs);
             }
             catch (Exception e) {
-                log.warn("Failed to migrate template build plan for exercise {} with buildPlanId {}", participation.getProgrammingExercise().getId(),
+                log.warn("Failed to migrate solution build plan for exercise {} with buildPlanId {}", participation.getProgrammingExercise().getId(),
                         participation.getBuildPlanId(), e);
                 errorList.add(participation);
             }
@@ -319,8 +319,8 @@ private void migrateStudents(List<ProgrammingExerciseStudentParticipation> parti
                 log.info("Migrated student build plan for exercise {} in {}ms", participation.getProgrammingExercise().getId(), System.currentTimeMillis() - startMs);
             }
             catch (Exception e) {
-                log.warn("Failed to migrate template build plan for exercise {} with buildPlanId {}", participation.getProgrammingExercise().getId(),
-                        participation.getBuildPlanId(), e);
+                log.warn("Failed to migrate student build plan for exercise {} with buildPlanId {}", participation.getProgrammingExercise().getId(), participation.getBuildPlanId(),
+                        e);
                 errorList.add(participation);
             }
         }