Save changed_by value to quays

Author: suvikankare

src/main/java/org/rutebanken/tiamat/versioning/save/StopPlaceVersionedSaverService.java

@@ -15,6 +15,7 @@
 
 package org.rutebanken.tiamat.versioning.save;
 
+import com.google.common.collect.Sets;
 import org.rutebanken.tiamat.auth.StopPlaceAuthorizationService;
 import org.rutebanken.tiamat.auth.UsernameFetcher;
 import org.rutebanken.tiamat.changelog.EntityChangedListener;
@@ -47,6 +48,7 @@
 
 import java.time.Instant;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -64,6 +66,17 @@ public class StopPlaceVersionedSaverService {
 
     public static final InterchangeWeightingEnumeration DEFAULT_WEIGHTING = InterchangeWeightingEnumeration.INTERCHANGE_ALLOWED;
 
+    private static final Set<String> QUAY_DIFF_IGNORE_FIELDS = Sets.newHashSet(
+        "id", "version", "changed", "changedBy", "status", "modification", "envelope", "polygon"
+    );
+
+    private static final Set<String> STOP_PLACE_DIFF_IGNORE_FIELDS = Sets.newHashSet(
+        "id", "version", "changed", "changedBy", "status", "modification", "envelope", "polygon", "quays", "children", "validBetween", "accessibilityAssessment"
+    );
+
+    private final org.rutebanken.tiamat.diff.generic.GenericDiffConfig quayDiffConfig;
+    private final org.rutebanken.tiamat.diff.generic.GenericDiffConfig stopPlaceDiffConfig;
+
     @Value("${tiamat.multimodal.allowSameNameForChild:false}")
     private boolean allowSameNameForChild;
 
@@ -115,9 +128,25 @@ public class StopPlaceVersionedSaverService {
     @Autowired
     private TiamatObjectDiffer tiamatObjectDiffer;
 
+    @Autowired
+    private org.rutebanken.tiamat.diff.generic.GenericObjectDiffer genericObjectDiffer;
+
     @Autowired
     private PrometheusMetricsService prometheusMetricsService;
 
+    public StopPlaceVersionedSaverService() {
+        this.quayDiffConfig = org.rutebanken.tiamat.diff.generic.GenericDiffConfig.builder()
+            .identifiers(Sets.newHashSet("netexId", "ref"))
+            .ignoreFields(QUAY_DIFF_IGNORE_FIELDS)
+            .onlyDoEqualsCheck(Sets.newHashSet(org.locationtech.jts.geom.Geometry.class))
+            .build();
+        this.stopPlaceDiffConfig = org.rutebanken.tiamat.diff.generic.GenericDiffConfig.builder()
+            .identifiers(Sets.newHashSet("netexId", "ref"))
+            .ignoreFields(STOP_PLACE_DIFF_IGNORE_FIELDS)
+            .onlyDoEqualsCheck(Sets.newHashSet(org.locationtech.jts.geom.Geometry.class))
+            .build();
+    }
+
     public StopPlace saveNewVersion(StopPlace existingVersion, StopPlace newVersion, Instant defaultValidFrom) {
         return saveNewVersion(existingVersion, newVersion, defaultValidFrom, new HashSet<>());
     }
@@ -133,7 +162,7 @@ public StopPlace saveNewVersion(StopPlace existingStopPlace, StopPlace newVersio
     public StopPlace saveNewVersion(StopPlace newVersion) {
         return saveNewVersion(null, newVersion);
     }
-
+    
     public StopPlace saveNewVersion(StopPlace existingVersion, StopPlace newVersion, Instant defaultValidFrom, Set<String> childStopsUpdated) {
 
         versionValidator.validate(existingVersion, newVersion);
@@ -158,11 +187,18 @@ public StopPlace saveNewVersion(StopPlace existingVersion, StopPlace newVersion,
         submodeValidator.validate(newVersion);
 
         Instant changed = Instant.now();
-        newVersion.setChanged(changed);
 
-        logger.debug("Rearrange accessibility assessments for: {}", newVersion);
         accessibilityAssessmentOptimizer.optimizeAccessibilityAssessments(newVersion);
 
+        // Clean up empty/invalid fields before comparison
+        cleanEmptyFields(newVersion);
+        if (existingVersion != null) {
+            cleanEmptyFields(existingVersion);
+        }
+
+        // Check if the stop place itself (excluding quays) has changed
+        boolean stopPlaceChanged = hasStopPlaceChanged(existingVersion, newVersion);
+
         Instant newVersionValidFrom = validityUpdater.updateValidBetween(existingVersion, newVersion, defaultValidFrom);
 
         if (existingVersion == null) {
@@ -177,9 +213,42 @@ public StopPlace saveNewVersion(StopPlace existingVersion, StopPlace newVersion,
             stopPlaceAuthorizationService.assertAuthorizedToEdit(existingVersionRefetched, newVersion, childStopsUpdated);
         }
 
+        // Identify which quays actually changed by comparing their content
+        final Set<String> modifiedQuayNetexIds = newVersion.getQuays() == null ? new HashSet<>() :
+            newVersion.getQuays().stream()
+                .filter(quay -> {
+                    if (existingVersion == null || existingVersion.getQuays() == null) {
+                        return true;
+                    }
+                    var existingQuay = existingVersion.getQuays().stream()
+                        .filter(eq -> eq.getNetexId() != null && eq.getNetexId().equals(quay.getNetexId()))
+                        .findFirst()
+                        .orElse(null);
+                    return hasQuayChanged(existingQuay, quay);
+                })
+                .map(quay -> quay.getNetexId())
+                .collect(Collectors.toSet());
+
         newVersion = versionIncrementor.initiateOrIncrementVersions(newVersion);
 
-        newVersion.setChangedBy(usernameFetcher.getUserNameForAuthenticatedUser());
+        String changedByUser = usernameFetcher.getUserNameForAuthenticatedUser();
+
+        // Only update stop place's changed/changedBy if stop place itself changed
+        if (stopPlaceChanged) {
+            newVersion.setChanged(changed);
+            newVersion.setChangedBy(changedByUser);
+        }
+
+        // Set changed and changedBy only on quays that were actually modified
+        if (newVersion.getQuays() != null) {
+            newVersion.getQuays().forEach(quay -> {
+                if (modifiedQuayNetexIds.contains(quay.getNetexId())) {
+                    quay.setChanged(changed);
+                    quay.setChangedBy(changedByUser);
+                }
+            });
+        }
+
         logger.info("StopPlace [{}], version {} changed by user [{}]. {}", newVersion.getNetexId(), newVersion.getVersion(), newVersion.getChangedBy(), newVersion.getValidBetween());
 
         if (newVersion.getWeighting() == null) {
@@ -194,6 +263,7 @@ public StopPlace saveNewVersion(StopPlace existingVersion, StopPlace newVersion,
         if (newVersion.getChildren() != null) {
             newVersion.getChildren().forEach(child -> {
                 child.setChanged(changed);
+                child.setChangedBy(changedByUser);
                 tariffZonesLookupService.populateTariffZone(child);
             });
 
@@ -307,4 +377,70 @@ private void updateParentSiteRefsForChildren(StopPlace parentStopPlace) {
         logger.info("Updated {} childs with parent site refs", count);
     }
 
+    /**
+     * Check if a quay has actual changes by comparing with its previous version.
+     * Ignores auto-generated fields like polygon, version, changed, etc.
+     * 
+     * @param existingQuay the previous version of the quay (optional)
+     * @param newQuay the new version of the quay
+     * @return true if the quay has actual changes or is new
+     */
+    private boolean hasQuayChanged(org.rutebanken.tiamat.model.Quay existingQuay, org.rutebanken.tiamat.model.Quay newQuay) {
+        if (existingQuay == null) {
+            return true;
+        }
+
+        try {
+            return !genericObjectDiffer.compareObjects(existingQuay, newQuay, quayDiffConfig).isEmpty();
+        } catch (IllegalAccessException e) {
+            logger.warn("Could not compare quay {}, marking as modified", newQuay.getNetexId(), e);
+            return true;
+        }
+    }
+
+    /**
+     * Check if a stop place has actual changes by comparing with its previous version.
+     * Excludes quays and children from comparison.
+     * 
+     * @param existingStopPlace the previous version of the stop place (optional)
+     * @param newStopPlace the new version of the stop place
+     * @return true if the stop place has actual changes or is new
+     */
+    private boolean hasStopPlaceChanged(StopPlace existingStopPlace, StopPlace newStopPlace) {
+        if (existingStopPlace == null) {
+            return true;
+        }
+
+        try {
+            return !genericObjectDiffer.compareObjects(existingStopPlace, newStopPlace, stopPlaceDiffConfig).isEmpty();
+        } catch (IllegalAccessException e) {
+            logger.warn("Could not compare stop place {}, marking as modified", newStopPlace.getNetexId(), e);
+            return true;
+        }
+    }
+
+    /**
+     * Remove empty or invalid fields that shouldn't count as actual changes.
+     */
+    private void cleanEmptyFields(StopPlace stopPlace) {
+        // Clean empty alternative names
+        if (stopPlace.getAlternativeNames() != null) {
+            stopPlace.getAlternativeNames().removeIf(altName ->
+                altName.getName() == null ||
+                altName.getName().getValue() == null ||
+                altName.getName().getValue().trim().isEmpty() ||
+                "null".equalsIgnoreCase(altName.getName().getValue().trim())
+            );
+        }
+
+        // Clean empty keyValues entries
+        if (stopPlace.getKeyValues() != null) {
+            stopPlace.getKeyValues().entrySet().removeIf(entry ->
+                entry.getValue() == null ||
+                entry.getValue().getItems() == null ||
+                entry.getValue().getItems().isEmpty()
+            );
+        }
+    }
+
 }