wcm-io · stefanseifert · Dec 5, 2024 · Dec 2, 2024 · Dec 4, 2024 · Dec 5, 2024
diff --git a/changes.xml b/changes.xml
@@ -23,6 +23,12 @@
     xsi:schemaLocation="http://maven.apache.org/changes/1.0.0 http://maven.apache.org/plugins/maven-changes-plugin/xsd/changes-1.0.0.xsd">
   <body>
 
+    <release version="1.9.8" date="not released">
+      <action type="fix" dev="srikanthgurram" issue="8">
+        Saving of Context-Aware configuration collections: Update last modified date of item pages only if the configuration of it has actually changed.
+      </action>
+    </release>
+
     <release version="1.9.6" date="2024-07-08">
       <action type="update" dev="cnagel" issue="4">
         Adds context path allow list to ToolsConfigPagePersistenceStrategy configuration.

diff --git a/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PagePersistenceStrategy.java b/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PagePersistenceStrategy.java
@@ -28,6 +28,7 @@
 import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.ensureContainingPage;
 import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.ensurePageIfNotContainingPage;
 import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.getOrCreateResource;
+import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.isItemModifiedOrNewlyAdded;
 import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.replaceProperties;
 import static io.wcm.caconfig.extensions.persistence.impl.PersistenceUtils.updatePageLastMod;
 
@@ -211,9 +212,11 @@ public boolean persistConfigurationCollection(@NotNull ResourceResolver resolver
     // create new or overwrite existing children
     for (ConfigurationPersistData item : data.getItems()) {
       String path = getCollectionItemResourcePath(parentPath + "/" + item.getCollectionItemName());
-      ensureContainingPage(resolver, path, resourceType, configurationManagementSettings);
-      getOrCreateResource(resolver, path, DEFAULT_CONFIG_NODE_TYPE, item.getProperties(), configurationManagementSettings);
-      updatePageLastMod(resolver, pageManager, path);
+      if (isItemModifiedOrNewlyAdded(resolver, path, item, configurationManagementSettings)) {
+        ensureContainingPage(resolver, path, resourceType, configurationManagementSettings);
+        getOrCreateResource(resolver, path, DEFAULT_CONFIG_NODE_TYPE, item.getProperties(), configurationManagementSettings);
+        updatePageLastMod(resolver, pageManager, path);
+      }
     }
 
     // if resource collection parent properties are given replace them as well

diff --git a/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PersistenceUtils.java b/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PersistenceUtils.java
@@ -281,6 +281,31 @@ public static void commit(ResourceResolver resourceResolver, String relatedResou
     }
   }
 
+  /**
+   * Checks if the given item is modified or newly added by comparing its properties with the current state of the resource.
+   *
+   * @param resolver     The ResourceResolver to access the resource.
+   * @param resourcePath The path of the resource to compare against.
+   * @param item         The ConfigurationPersistData item containing the properties to compare.
+   * @param settings     The ConfigurationManagementSettings to determine which properties to ignore.
+   * @return true if the resource does not exist or if any property value differs, false otherwise.
+   */
+  public static boolean isItemModifiedOrNewlyAdded(ResourceResolver resolver, String resourcePath, ConfigurationPersistData item, ConfigurationManagementSettings settings) {
+    Resource resource = resolver.getResource(resourcePath);
+    if (resource == null) {
+      return true; // Resource does not exist, so it is considered modified
+    }
+
+    Map<String, Object> currentProperties = new HashMap<>(resource.getValueMap());
+    Map<String, Object> newProperties = new HashMap<>(item.getProperties());
+
+    // Filter out ignored properties
+    PropertiesFilterUtil.removeIgnoredProperties(currentProperties, settings);
+    PropertiesFilterUtil.removeIgnoredProperties(newProperties, settings);
+
+    return !currentProperties.equals(newProperties);
+  }
+
   /**
    * If the given resource points to an AEM page, delete the page using PageManager.
    * Otherwise delete the resource using ResourceResolver.

diff --git a/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PropertiesFilterUtil.java b/src/main/java/io/wcm/caconfig/extensions/persistence/impl/PropertiesFilterUtil.java
@@ -19,6 +19,8 @@
  */
 package io.wcm.caconfig.extensions.persistence.impl;
 
+import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.sling.caconfig.management.ConfigurationManagementSettings;
@@ -28,13 +30,19 @@
  */
 final class PropertiesFilterUtil {
 
-    private PropertiesFilterUtil() {
-        // static methods only
-    }
+  private PropertiesFilterUtil() {
+    // static methods only
+  }
 
-    public static void removeIgnoredProperties(Set<String> propertyNames, ConfigurationManagementSettings settings) {
-        Set<String> ignoredProperties = settings.getIgnoredPropertyNames(propertyNames);
-        propertyNames.removeAll(ignoredProperties);
-    }
+  public static void removeIgnoredProperties(Set<String> propertyNames, ConfigurationManagementSettings settings) {
+    Set<String> ignoredProperties = settings.getIgnoredPropertyNames(propertyNames);
+    propertyNames.removeAll(ignoredProperties);
+  }
+
+  public static void removeIgnoredProperties(Map<String, Object> properties, ConfigurationManagementSettings settings) {
+    Set<String> propertyNames = new HashSet<>(properties.keySet());
+    removeIgnoredProperties(propertyNames, settings);
+    properties.keySet().retainAll(propertyNames);
+  }
 
 }
diff --git a/src/test/java/io/wcm/caconfig/extensions/persistence/impl/PagePersistenceStrategyTest.java b/src/test/java/io/wcm/caconfig/extensions/persistence/impl/PagePersistenceStrategyTest.java
@@ -19,16 +19,19 @@
  */
 package io.wcm.caconfig.extensions.persistence.impl;
 
+import static com.day.cq.wcm.api.NameConstants.PN_LAST_MOD;
 import static io.wcm.caconfig.extensions.persistence.testcontext.PersistenceTestUtils.writeConfiguration;
 import static io.wcm.caconfig.extensions.persistence.testcontext.PersistenceTestUtils.writeConfigurationCollection;
 import static org.apache.sling.api.resource.ResourceResolver.PROPERTY_RESOURCE_TYPE;
 import static org.apache.sling.testing.mock.caconfig.ContextPlugins.CACONFIG;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.util.Calendar;
 import java.util.List;
 
 import org.apache.sling.caconfig.ConfigurationBuilder;
@@ -162,6 +165,36 @@ void testListConfig() {
     assertEquals(234, config2.intParam());
   }
 
+  @Test
+  void testListConfig_updateLastModifiedIfPropertyRemoved() {
+    context.registerInjectActivateService(new PagePersistenceStrategy(), "enabled", true);
+
+    // write config
+    writeConfigurationCollection(context, contentPage.getPath(), ListConfig.class.getName(), List.of(
+            ImmutableValueMap.of("stringParam", "value1", "intParam", 123),
+            ImmutableValueMap.of("stringParam", "value2", "intParam", 234)
+    ));
+
+    // assert storage in page in /conf
+    Page parentPage = context.pageManager().getPage("/conf/test/site1/sling:configs/" + ListConfig.class.getName());
+    assertNotNull(parentPage);
+
+    Page configPage1 = context.pageManager().getPage("/conf/test/site1/sling:configs/" + ListConfig.class.getName() + "/item0");
+    assertThat(configPage1.getContentResource(), ResourceMatchers.props("stringParam", "value1", "intParam", 123));
+
+    Page configPage2 = context.pageManager().getPage("/conf/test/site1/sling:configs/" + ListConfig.class.getName() + "/item1");
+    assertThat(configPage2.getContentResource(), ResourceMatchers.props("stringParam", "value2", "intParam", 234));
+
+    writeConfigurationCollection(context, contentPage.getPath(), ListConfig.class.getName(), List.of(
+            ImmutableValueMap.of("stringParam", "value1", "intParam", 123),
+            ImmutableValueMap.of("stringParam", "value2")
+    ));
+    Calendar lastModifiedConfigPage2AfterUpdate = configPage2.getContentResource().getValueMap().get(PN_LAST_MOD, Calendar.class);
+    System.out.println(lastModifiedConfigPage2AfterUpdate);
+    //ConfigPage2 last modified date should be updated because it is updated
+      assertNotNull(lastModifiedConfigPage2AfterUpdate);
+  }
+
   @Test
   void testListConfig_Nested() {
     context.registerInjectActivateService(new PagePersistenceStrategy(), "enabled", true);
@@ -207,10 +240,14 @@ void testListConfig_Nested() {
     assertEquals(1, config2.subListConfig().length);
     assertEquals("value21", config2.subListConfig()[0].stringParam());
 
+
+    Calendar lastModifiedConfigPage1 = configPage1.getContentResource().getValueMap().get(PN_LAST_MOD, Calendar.class);
+    Calendar lastModifiedConfigPage2 = configPage2.getContentResource().getValueMap().get(PN_LAST_MOD, Calendar.class);
+
     // update config collection items
     writeConfigurationCollection(context, contentPage.getPath(), ListNestedConfig.class.getName(), List.of(
         ImmutableValueMap.of("stringParam", "value1-new", "intParam", 123),
-        ImmutableValueMap.of("stringParam", "value2-new", "intParam", 234),
+        ImmutableValueMap.of("stringParam", "value2", "intParam", 234),
         ImmutableValueMap.of("stringParam", "value3-new", "intParam", 345)));
 
     // read config
@@ -224,18 +261,34 @@ void testListConfig_Nested() {
     assertEquals(2, config1.subListConfig().length);
     assertEquals("value11", config1.subListConfig()[0].stringParam());
     assertEquals("value12", config1.subListConfig()[1].stringParam());
+    //ConfigPage1 last modified date should be updated because it is updated
+    assertTrue(configLastModifiedUpdated(configPage1, false, lastModifiedConfigPage1));
 
     config2 = configs.get(1);
-    assertEquals("value2-new", config2.stringParam());
+    assertEquals("value2", config2.stringParam());
     assertEquals(234, config2.intParam());
     assertEquals(1, config2.subListConfig().length);
     assertEquals("value21", config2.subListConfig()[0].stringParam());
+    //ConfigPage2 last modified date should not be updated because it is not updated
+    assertFalse(configLastModifiedUpdated(configPage2, false, lastModifiedConfigPage2));
 
     ListNestedConfig config3 = configs.get(2);
     assertEquals("value3-new", config3.stringParam());
     assertEquals(345, config3.intParam());
     assertEquals(0, config3.subListConfig().length);
+    Page configPage3 = context.pageManager().getPage("/conf/test/site1/sling:configs/" + ListNestedConfig.class.getName() + "/item2");
+    //ConfigPage3 last modified date should be added because it is newly created
+    assertTrue(configLastModifiedUpdated(configPage3, true, null));
+  }
 
+  private boolean configLastModifiedUpdated(Page configPage, boolean newlyCreatedConfig, Calendar lastModifiedBeforeUpdateOrCreate) {
+    Calendar lastModifiedAfterUpdateOrCreate = configPage.getContentResource().getValueMap().get(PN_LAST_MOD, Calendar.class);
+    if (newlyCreatedConfig) {
+      //If the config is newly created then last modified date should be updated
+      return lastModifiedAfterUpdateOrCreate != null;
+    }
+    //If the config is updated then last modified date should be updated
+    return lastModifiedAfterUpdateOrCreate != null && lastModifiedAfterUpdateOrCreate.after(lastModifiedBeforeUpdateOrCreate);
   }
 
   @Test

diff --git a/src/test/java/io/wcm/caconfig/extensions/persistence/impl/PropertiesFilterUtilTest.java b/src/test/java/io/wcm/caconfig/extensions/persistence/impl/PropertiesFilterUtilTest.java
@@ -0,0 +1,64 @@
+/*
+ * #%L
+ * wcm.io
+ * %%
+ * Copyright (C) 2024 wcm.io
+ * %%
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ * #L%
+ */
+package io.wcm.caconfig.extensions.persistence.impl;
+
+import static io.wcm.caconfig.extensions.persistence.impl.PropertiesFilterUtil.removeIgnoredProperties;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.sling.caconfig.management.ConfigurationManagementSettings;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+@ExtendWith(MockitoExtension.class)
+class PropertiesFilterUtilTest {
+
+  @Mock
+  private ConfigurationManagementSettings settings;
+
+  @BeforeEach
+  void setUp() {
+    when(settings.getIgnoredPropertyNames(any())).thenReturn(Set.of("prop1", "prop3"));
+  }
+
+  @Test
+  void testRemoveIgnoredPropertiesSet() {
+    Set<String> names = new HashSet<>(Set.of("prop1", "prop2", "prop3", "prop4"));
+    removeIgnoredProperties(names, settings);
+    assertEquals(Set.of("prop2", "prop4"), names);
+  }
+
+  @Test
+  void testRemoveIgnoredPropertiesMap() {
+    Map<String, Object> properties = new HashMap<>(Map.of("prop1", 1, "prop2", 2, "prop3", 3, "prop4", 4));
+    removeIgnoredProperties(properties, settings);
+    assertEquals(Map.of("prop2", 2, "prop4", 4), properties);
+  }
+
+}