commit f0f16146066af3d2f9b5570f8f50b80aa4174706
author stefan-egli <stefanegli@apache.org> Mon Mar 20 09:56:05 2023 +0100
committer GitHub <noreply@github.com> Mon Mar 20 09:56:05 2023 +0100
tree 9cc785370cef8ef6973aee98ac0edcedad627480
parent d9788198588c1db60ada814ff92269ed5e369255

SLING-11805 : Don't stop slingId cleanup upon PROPERTIES_CHANGED (#14)

* SLING-11805 : added test to reproduce the current restricted behaviour

* SLING-11805 : PROPERTIES_CHANGED should not stop slingId cleanup

src/main/java/org/apache/sling/discovery/oak/SlingIdCleanupTask.java

diff --git a/src/main/java/org/apache/sling/discovery/oak/SlingIdCleanupTask.java b/src/main/java/org/apache/sling/discovery/oak/SlingIdCleanupTask.java
index 3316cbc..fb51070 100644
--- a/src/main/java/org/apache/sling/discovery/oak/SlingIdCleanupTask.java
+++ b/src/main/java/org/apache/sling/discovery/oak/SlingIdCleanupTask.java
@@ -273,12 +273,16 @@
             return;
         }
         final TopologyView newView = event.getNewView();
-        if (newView == null || event.getType() == Type.PROPERTIES_CHANGED) {
+        if (event.getType() == Type.PROPERTIES_CHANGED) {
+            // ignore those
+        } else if (newView == null) {
+            // that's a TOPOLOGY_CHANGING
             hasTopology = false; // stops potentially ongoing deletion
             currentView = null;
             // cancel cleanup schedule
             stop();
         } else {
+            // that's TOPOLOGY_INIT or TOPOLOGY_CHANGED
             hasTopology = true;
             currentView = newView;
             seenInstances.addAll(getActiveSlingIds(newView));
@@ -303,7 +307,11 @@
             return;
         }
         final boolean unscheduled = localScheduler.unschedule(SCHEDULE_NAME);
-        logger.debug("stop: unschedule result={}", unscheduled);
+        if (unscheduled) {
+            logger.info("stop: unscheduled");
+        } else {
+            logger.debug("stop: unschedule was not necessary");
+        }
     }
 
     /**
@@ -340,7 +348,7 @@
         }
         cal.add(Calendar.MILLISECOND, delayMillis);
         final Date scheduledDate = cal.getTime();
-        logger.debug(
+        logger.info(
                 "recreateSchedule: scheduling a cleanup in {} milliseconds from now, which is: {}",
                 delayMillis, scheduledDate);
         ScheduleOptions options = localScheduler.AT(scheduledDate);

src/test/java/org/apache/sling/discovery/oak/TestSlingIdCleanupTask.java

diff --git a/src/test/java/org/apache/sling/discovery/oak/TestSlingIdCleanupTask.java b/src/test/java/org/apache/sling/discovery/oak/TestSlingIdCleanupTask.java
index 01ed7b9..0a95df4 100644
--- a/src/test/java/org/apache/sling/discovery/oak/TestSlingIdCleanupTask.java
+++ b/src/test/java/org/apache/sling/discovery/oak/TestSlingIdCleanupTask.java
@@ -427,6 +427,27 @@
     }
 
     @Test
+    public void testTopologyThenPropertiesChanged() throws Exception {
+        createCleanupTask(1000, 86400000);
+        assertEquals(0, cleanupTask.getDeleteCount());
+        createSlingIds(5, 10, 0);
+
+        TopologyView view1 = newView();
+        cleanupTask.handleTopologyEvent(newInitEvent(view1));
+        cleanupTask.handleTopologyEvent(newChangingEvent(view1));
+        assertEquals(0, cleanupTask.getDeleteCount());
+
+        TopologyView view2 = newView();
+        cleanupTask.handleTopologyEvent(newChangedEvent(view1, view2));
+        // below properties changed event must not stop the cleanup
+        cleanupTask.handleTopologyEvent(newPropertiesChangedEvent(view1, view2));
+        Thread.sleep(500);
+        assertEquals(0, cleanupTask.getDeleteCount());
+        waitForRunCount(cleanupTask, 1, 5000);
+        assertEquals(10, cleanupTask.getDeleteCount());
+    }
+
+    @Test
     public void testRepetitionDelay() throws Exception {
         createCleanupTask(1000, 86400000);
         createSlingIds(5, 10, 0);