Skip to content

Commit

Permalink
Updated Review comments - covered edge case with deletion of property
Browse files Browse the repository at this point in the history
  • Loading branch information
srikanthgurram-17 committed Dec 4, 2024
1 parent c0b0825 commit 24203b6
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public boolean persistConfigurationCollection(@NotNull ResourceResolver resolver
// create new or overwrite existing children
for (ConfigurationPersistData item : data.getItems()) {
String path = getCollectionItemResourcePath(parentPath + "/" + item.getCollectionItemName());
if (isItemModifiedOrNewlyAdded(resolver, path, item)) {
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,29 +285,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 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 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) {
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
}
ValueMap valueMap = resource.getValueMap();
Map<String, Object> currentProperties = new HashMap<>(valueMap);
Map<String, Object> newProperties = new HashMap<>(item.getProperties());

ValueMap currentProperties = resource.getValueMap();
Map<String, Object> itemProperties = item.getProperties();
// Filter out ignored properties
Set<String> currentPropertyNames = new HashSet<>(currentProperties.keySet());
PropertiesFilterUtil.removeIgnoredProperties(currentPropertyNames, settings);
currentProperties.keySet().retainAll(currentPropertyNames);

for (Map.Entry<String, Object> entry : itemProperties.entrySet()) {
String key = entry.getKey();
Object value = entry.getValue();
if (!value.equals(currentProperties.get(key))) {
return true; // Property value differs
}
}
Set<String> newPropertyNames = new HashSet<>(newProperties.keySet());
PropertiesFilterUtil.removeIgnoredProperties(newPropertyNames, settings);
newProperties.keySet().retainAll(newPropertyNames);

return false; // No differences found
return !currentProperties.equals(newProperties);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@
import java.util.Calendar;
import java.util.List;

import org.apache.sling.api.resource.PersistenceException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.caconfig.ConfigurationBuilder;
import org.apache.sling.caconfig.management.ConfigurationManager;
import org.apache.sling.hamcrest.ResourceMatchers;
Expand Down Expand Up @@ -161,6 +164,38 @@ void testListConfig() {
assertEquals(234, config2.intParam());
}

@Test
void testListConfig_updateLastModifiedIfPropertyRemoved() throws PersistenceException {
context.registerInjectActivateService(new PagePersistenceStrategy(), "enabled", true);
ResourceResolver resourceResolver = context.resourceResolver();


// 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);
Expand Down

0 comments on commit 24203b6

Please sign in to comment.