fixes:

- boolean config validator
- set last alerted for all profiles when enabling alerter
- skips events that are too old (based on property alerter.aggregation-time-multiplier:50 * aggregationTime)
- updates profile lastAlertedAt even if profile has no recipients
- changes "errori" into "eventi" in the mail body
- fixes a nullpointerexception when getting user from context
- alert for changed configuration/repository values
- disable autorestart of docker containers (for local dev)
- default profiles are created (if needed) with lastAlertedAt using current timestamp
- disallow deletion of default profiles
- disallow updating the name of default profiles

docker-compose.yaml

@@ -25,7 +25,7 @@ services:
   mysql:
     image: mysql
     command: --mysql-native-password=ON
-    restart: always
+    restart: no
     ports:
       - 3306:3306
     environment:
@@ -34,7 +34,7 @@ services:
 
   backend:
     image: gcr.io/distroless/java:11-debug-nonroot
-    restart: on-failure
+    restart: no
     depends_on:
       - virtuoso
       - mysql

src/main/java/it/gov/innovazione/ndc/alerter/AlerterService.java

@@ -4,10 +4,13 @@
 import it.gov.innovazione.ndc.alerter.dto.EventDto;
 import it.gov.innovazione.ndc.alerter.event.AlertableEvent;
 import lombok.RequiredArgsConstructor;
+import org.springframework.security.core.context.SecurityContext;
 import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.stereotype.Service;
 
+import java.security.Principal;
 import java.time.Instant;
+import java.util.Optional;
 
 import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;
 
@@ -30,8 +33,10 @@ public void alert(AlertableEvent alertableEvent) {
     }
 
     private String getUser() {
-        return defaultIfNull(SecurityContextHolder.getContext().getAuthentication().getName(), "system");
-
+        return Optional.of(SecurityContextHolder.getContext())
+                .map(SecurityContext::getAuthentication)
+                .map(Principal::getName)
+                .orElse("system");
     }
 
 }

src/main/java/it/gov/innovazione/ndc/alerter/data/EntityService.java

@@ -58,9 +58,14 @@ public D update(D dto) {
             throw new ConflictingOperationException(getEntityName() + " id must not be null for update operation");
         }
         assertExistsById(dto.getId());
+        beforeUpdate(dto);
         return toDto(getRepository().save(getEntityMapper().toEntity(dto)));
     }
 
+    protected void beforeUpdate(D dto) {
+
+    }
+
     private void assertExistsById(String id) {
         if (!getRepository().existsById(id)) {
             throw entityDoesNotExistsException();
@@ -69,11 +74,16 @@ private void assertExistsById(String id) {
 
     public D delete(String id) {
         T entity = getEntityById(id);
+        beforeDelete(entity);
         D dto = toDto(entity);
         getRepository().delete(entity);
         return dto;
     }
 
+    protected void beforeDelete(T entity) {
+
+    }
+
     public D getById(String id) {
         return toDto(getEntityById(id));
     }

src/main/java/it/gov/innovazione/ndc/alerter/data/ProfileInitializer.java

@@ -8,6 +8,7 @@
 import org.apache.commons.lang3.tuple.Pair;
 import org.springframework.stereotype.Service;
 
+import java.time.Instant;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -38,11 +39,21 @@ public void init() {
                         .eventCategories(pair.getRight())
                         .minSeverity(Severity.INFO)
                         .aggregationTime(60L)
+                        .lastAlertedAt(Instant.now())
                         .build())
                 .forEach(p -> {
                     log.info("Creating default profile: {}", p.getName());
                     repository.save(p);
                 });
+    }
+
+    private List<String> getUnmodifiableProfileNames() {
+        return DEFAULT_PROFILES.stream()
+                .map(Pair::getLeft)
+                .collect(Collectors.toList());
+    }
 
+    public boolean isProfileNameUnmodifiable(String name) {
+        return getUnmodifiableProfileNames().contains(name);
     }
 }

src/main/java/it/gov/innovazione/ndc/alerter/data/ProfileService.java

@@ -22,6 +22,8 @@ public class ProfileService extends EntityService<Profile, ProfileDto> {
     @Getter(AccessLevel.PROTECTED)
     private final ProfileRepository repository;
 
+    private final ProfileInitializer profileInitializer;
+
     @Getter(AccessLevel.PROTECTED)
     private final ProfileMapper entityMapper;
 
@@ -31,17 +33,35 @@ public class ProfileService extends EntityService<Profile, ProfileDto> {
     @Getter(AccessLevel.PROTECTED)
     private final Sort defaultSorting = Sort.by("name").ascending();
 
-    public void setLastUpdated(String id) {
+    public void setLastAlertedAt(String id) {
         Profile profile = repository.findById(id)
                 .orElseThrow(() -> new IllegalStateException("Profile not found: " + id));
         profile.setLastAlertedAt(Instant.now());
         repository.save(profile);
     }
 
-    public void setAllLastUpdated(Duration backoff) {
+    public void setAllLastAlertedAt(Duration backoff) {
         repository.findAll().forEach(profile -> {
-            profile.setLastAlertedAt(Instant.now().minus(backoff));
+            Instant lastAlertedAt = Instant.now().minus(backoff);
+            log.info("Setting last updated {} for profile {}", lastAlertedAt, profile.getName());
+            profile.setLastAlertedAt(lastAlertedAt);
             repository.save(profile);
         });
     }
+
+    @Override
+    public void beforeDelete(Profile profile) {
+        if (profileInitializer.isProfileNameUnmodifiable(profile.getName())) {
+            throw new ConflictingOperationException("Profile " + profile.getName() + " cannot be deleted");
+        }
+    }
+
+    @Override
+    public void beforeUpdate(ProfileDto dto) {
+        Profile profile = repository.findById(dto.getId())
+                .orElseThrow(() -> new IllegalStateException("Profile not found: " + dto.getId()));
+        if (profileInitializer.isProfileNameUnmodifiable(dto.getName()) && !profile.getName().equals(dto.getName())) {
+            throw new ConflictingOperationException("Profile " + profile.getName() + " cannot be renamed");
+        }
+    }
 }

src/main/java/it/gov/innovazione/ndc/controller/ConfigurationController.java

@@ -1,12 +1,8 @@
 package it.gov.innovazione.ndc.controller;
 
-import it.gov.innovazione.ndc.alerter.entities.EventCategory;
-import it.gov.innovazione.ndc.alerter.entities.Severity;
-import it.gov.innovazione.ndc.alerter.event.AlertableEvent;
 import it.gov.innovazione.ndc.eventhandler.NdcEventPublisher;
 import it.gov.innovazione.ndc.eventhandler.event.ConfigService;
 import it.gov.innovazione.ndc.harvester.service.ActualConfigService;
-import lombok.Builder;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.web.bind.annotation.DeleteMapping;
@@ -22,7 +18,6 @@
 
 import java.security.Principal;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static org.springframework.http.HttpStatus.ACCEPTED;
 import static org.springframework.http.HttpStatus.CREATED;
@@ -56,10 +51,6 @@ public void setConfig(
             return;
         }
         configService.setConfig(config, principal.getName(), repoId);
-        eventPublisher.publishAlertableEvent("Configuration", WebConfigAlertableEvent.builder()
-                .repoId(repoId)
-                .config(config)
-                .build());
     }
 
     @PutMapping("/{configKey}")
@@ -74,11 +65,6 @@ public void updateRepository(
             return;
         }
         configService.writeConfigKey(configKey, principal.getName(), value, repoId);
-        eventPublisher.publishAlertableEvent("Configuration",
-                WebConfigAlertableEvent.builder()
-                        .repoId(repoId)
-                        .config(Map.of(configKey, value))
-                        .build());
     }
 
     @DeleteMapping("/{configKey}")
@@ -92,43 +78,5 @@ public void deleteRepository(
             return;
         }
         configService.removeConfigKey(configKey, principal.getName(), repoId);
-        eventPublisher.publishAlertableEvent("Configuration",
-                WebConfigAlertableEvent.builder()
-                        .repoId(repoId)
-                        .config(Map.of(configKey, "removed"))
-                        .build());
-    }
-
-    @Builder
-    @RequiredArgsConstructor
-    private static class WebConfigAlertableEvent implements AlertableEvent {
-        private final String repoId;
-        private final Map<ActualConfigService.ConfigKey, Object> config;
-
-        @Override
-        public String getName() {
-            return "Configuration updated";
-        }
-
-        @Override
-        public String getDescription() {
-            return "Configuration updated for repository " + repoId;
-        }
-
-        @Override
-        public EventCategory getCategory() {
-            return EventCategory.APPLICATION;
-        }
-
-        @Override
-        public Severity getSeverity() {
-            return Severity.INFO;
-        }
-
-        @Override
-        public Map<String, Object> getContext() {
-            return config.entrySet().stream()
-                    .collect(Collectors.toMap(e -> e.getKey().name(), Map.Entry::getValue));
-        }
     }
 }

src/main/java/it/gov/innovazione/ndc/eventhandler/event/ConfigService.java

@@ -1,5 +1,8 @@
 package it.gov.innovazione.ndc.eventhandler.event;
 
+import it.gov.innovazione.ndc.alerter.entities.EventCategory;
+import it.gov.innovazione.ndc.alerter.entities.Severity;
+import it.gov.innovazione.ndc.alerter.event.AlertableEvent;
 import it.gov.innovazione.ndc.harvester.service.ActualConfigService;
 import lombok.Builder;
 import lombok.Data;
@@ -11,6 +14,9 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.stream.Collectors;
+
+import static org.eclipse.jgit.util.StringUtils.equalsIgnoreCase;
 
 @Slf4j
 public abstract class ConfigService {
@@ -66,7 +72,7 @@ public <T> T getFromRepoOrGlobalOrDefault(
     @SuppressWarnings("unchecked")
     <T> T safelyParse(Object value, ActualConfigService.ConfigKey key, Class<T> clazz) {
         try {
-            return (T) key.getParser().getParser().apply(value.toString());
+            return (T) key.getParser().getParsingFunction().apply(value.toString());
         } catch (Exception e) {
             log.warn("Error parsing value for key {} with value \"{}\"", key, value, e);
             return null;
@@ -109,22 +115,77 @@ public static class ConfigEntry {
 
     @Builder
     @Getter
-    public static class ConfigEvent {
+    public static class ConfigEvent implements AlertableEvent {
         private final Map<ActualConfigService.ConfigKey, ConfigChange> changes;
         private final String destination;
         private final Exception error;
 
-        public boolean isChange(ActualConfigService.ConfigKey key) {
+        public boolean isChangeKey(ActualConfigService.ConfigKey key) {
             return Objects.nonNull(changes) && changes.containsKey(key);
         }
 
-        public boolean isChange(ActualConfigService.ConfigKey key, Object newValue) {
+        public boolean isChange(ActualConfigService.ConfigKey key, Object newValue, Object oldValue) {
             try {
-                return isChange(key) && changes.get(key).getNewValue().getValue().equals(newValue);
+                return isChangeKey(key) && isNewValue(key, newValue) && isOldValue(key, oldValue);
             } catch (Exception e) {
                 return false;
             }
         }
+
+        private boolean isNewValue(ActualConfigService.ConfigKey key, Object newValue) {
+            return isValue(key, newValue, ConfigChange::getNewValue);
+        }
+
+        private Boolean isOldValue(ActualConfigService.ConfigKey key, Object oldValue) {
+            return isValue(key, oldValue, ConfigChange::getOldValue);
+        }
+
+        private Boolean isValue(ActualConfigService.ConfigKey key,
+                                Object value,
+                                java.util.function.Function<ConfigChange, ConfigEntry> valueGetter) {
+            return Optional.ofNullable(changes)
+                    .map(c -> c.get(key))
+                    .map(valueGetter)
+                    .map(ConfigEntry::getValue)
+                    .map(Object::toString)
+                    .map(v -> equalsIgnoreCase(v, value.toString()))
+                    .orElse(false);
+        }
+
+        @Override
+        public String getName() {
+            if (Objects.nonNull(error)) {
+                return "Configuration update error: " + error.getMessage();
+            }
+            return "Configuration updated";
+        }
+
+        @Override
+        public String getDescription() {
+            if (Objects.nonNull(error)) {
+                return "Error updating configuration for " + destination + ", cause: " + error.getMessage();
+            }
+            return "Configuration updated for " + destination;
+        }
+
+        @Override
+        public EventCategory getCategory() {
+            return EventCategory.APPLICATION;
+        }
+
+        @Override
+        public Severity getSeverity() {
+            if (Objects.nonNull(error)) {
+                return Severity.ERROR;
+            }
+            return Severity.INFO;
+        }
+
+        @Override
+        public Map<String, Object> getContext() {
+            return changes.entrySet().stream()
+                    .collect(Collectors.toMap(e -> e.getKey().name(), Map.Entry::getValue));
+        }
     }
 
     @Builder

src/main/java/it/gov/innovazione/ndc/eventhandler/event/HarvesterFinishedEvent.java

@@ -9,6 +9,7 @@
 import lombok.Data;
 
 import java.util.Map;
+import java.util.Optional;
 
 @Builder
 @Data
@@ -36,7 +37,7 @@ public EventCategory getCategory() {
 
     @Override
     public Severity getSeverity() {
-        if (status == HarvesterRun.Status.ALREADY_RUNNING) {
+        if (status == HarvesterRun.Status.ALREADY_RUNNING || status == HarvesterRun.Status.NDC_ISSUES_PRESENT) {
             return Severity.WARNING;
         } else if (status == HarvesterRun.Status.SUCCESS || status == HarvesterRun.Status.UNCHANGED) {
             return Severity.INFO;
@@ -53,7 +54,7 @@ public Map<String, Object> getContext() {
                 "repository", repository,
                 "revision", revision,
                 "status", status,
-                "exception", exception
+                "exception", Optional.ofNullable(exception).map(Exception::getMessage).orElse("")
         );
     }
 }

src/main/java/it/gov/innovazione/ndc/eventhandler/handler/AlerterEnabledEventListener.java

@@ -39,9 +39,9 @@ public void handle(NdcEventWrapper<?> event) {
         if (Objects.isNull(payload) || Objects.isNull(payload.getChanges())) {
             return;
         }
-        if (payload.isChange(ConfigKey.ALERTER_ENABLED, Boolean.TRUE)) {
+        if (payload.isChange(ConfigKey.ALERTER_ENABLED, Boolean.TRUE, Boolean.FALSE)) {
             log.info("Alerter enabled, setting all profiles last updated");
-            profileService.setAllLastUpdated(backoff);
+            profileService.setAllLastAlertedAt(backoff);
         }
     }
 }