You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by re...@apache.org on 2024/09/09 12:04:32 UTC
(sling-org-apache-sling-resourceresolver) branch master updated: SLING-12400: ResourceResolver: add test coverage for aliases on jcr:content nodes and NPEs on getParent, refactor associated code (#124)
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
The following commit(s) were added to refs/heads/master by this push:
new 53daef2 SLING-12400: ResourceResolver: add test coverage for aliases on jcr:content nodes and NPEs on getParent, refactor associated code (#124)
53daef2 is described below
commit 53daef232eef3b1329934f395ffe7989cd40c282
Author: Julian Reschke <ju...@gmx.de>
AuthorDate: Mon Sep 9 14:04:27 2024 +0200
SLING-12400: ResourceResolver: add test coverage for aliases on jcr:content nodes and NPEs on getParent, refactor associated code (#124)
* SLING-12400: ResourceResolver: add test coverage for aliases on jcr:content nodes
* SLING-12400: ResourceResolver: add test coverage for aliases on jcr:content nodes -- add test coverage for alias/jcr:content getParent()==null, slightly refactor the associated code
---
.../resourceresolver/impl/mapping/MapEntries.java | 43 +++++++++-------
.../impl/mapping/MapEntriesTest.java | 59 +++++++++++++++++-----
2 files changed, 73 insertions(+), 29 deletions(-)
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index 0ed8a25..44b83df 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -35,6 +35,7 @@ import org.apache.sling.resourceresolver.impl.ResourceResolverImpl;
import org.apache.sling.resourceresolver.impl.ResourceResolverMetrics;
import org.apache.sling.resourceresolver.impl.mapping.MapConfigurationProvider.VanityPathConfig;
import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
import org.osgi.framework.BundleContext;
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceRegistration;
@@ -82,9 +83,9 @@ public class MapEntries implements
private static final String JCR_CONTENT = "jcr:content";
- private static final String JCR_CONTENT_PREFIX = "jcr:content/";
+ private static final String JCR_CONTENT_PREFIX = JCR_CONTENT + "/";
- private static final String JCR_CONTENT_SUFFIX = "/jcr:content";
+ private static final String JCR_CONTENT_SUFFIX = "/" + JCR_CONTENT;
private static final String PROP_REG_EXP = "sling:match";
@@ -588,12 +589,9 @@ public class MapEntries implements
* @return {@code true} if any change
*/
private boolean doUpdateAlias(final Resource resource) {
- final Resource containingResource;
- if (JCR_CONTENT.equals(resource.getName())) {
- containingResource = resource.getParent();
- } else {
- containingResource = resource;
- }
+
+ // resource containing the alias
+ final Resource containingResource = getResourceToBeAliased(resource);
if ( containingResource != null ) {
final String containingResourceName = containingResource.getName();
@@ -618,7 +616,10 @@ public class MapEntries implements
}
return changed;
+ } else {
+ log.warn("containingResource is null for alias on {}, skipping.", resource.getPath());
}
+
return false;
}
@@ -1233,16 +1234,11 @@ public class MapEntries implements
private boolean loadAlias(final Resource resource, Map<String, Map<String, Collection<String>>> map) {
// resource containing the alias
- final Resource containingResource;
+ final Resource containingResource = getResourceToBeAliased(resource);
- if (JCR_CONTENT.equals(resource.getName())) {
- containingResource = resource.getParent();
- if (containingResource == null) {
- log.warn("containingResource is null for alias on {}, skipping.", resource.getPath());
- return false;
- }
- } else {
- containingResource = resource;
+ if (containingResource == null) {
+ log.warn("containingResource is null for alias on {}, skipping.", resource.getPath());
+ return false;
}
final Resource parent = containingResource.getParent();
@@ -1294,6 +1290,19 @@ public class MapEntries implements
}
}
+ /**
+ * Given a resource, check whether the name is "jcr:content", in which case return the parent resource
+ * @param resource resource to check
+ * @return parent of jcr:content resource (may be null), otherwise the resource itself
+ */
+ @Nullable private Resource getResourceToBeAliased(Resource resource) {
+ if (JCR_CONTENT.equals(resource.getName())) {
+ return resource.getParent();
+ } else {
+ return resource;
+ }
+ }
+
/**
* Check alias syntax
*/
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
index 4335efe..4402741 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
@@ -121,7 +121,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
prevPageSize = Integer.getInteger("sling.vanityPath.pageSize", 2000);
System.setProperty("sling.vanityPath.pageSize", Integer.toString(pageSize));
- MockitoAnnotations.initMocks(this);
+ MockitoAnnotations.openMocks(this);
final List<VanityPathConfig> configs = new ArrayList<>();
configs.add(new VanityPathConfig("/libs/", false));
@@ -175,31 +175,58 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
mapEntries.dispose();
}
+ private void internal_test_simple_alias_support(boolean onJcrContent) {
+ prepareMapEntriesForAlias(onJcrContent, false, "alias");
+ mapEntries.doInit();
+ Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
+ assertNotNull(aliasMap);
+ assertTrue(aliasMap.containsKey("child"));
+ assertEquals(List.of("alias"), aliasMap.get("child"));
+ }
@Test
public void test_simple_alias_support() {
- prepareMapEntriesForAlias("alias");
+ internal_test_simple_alias_support(false);
+ }
+
+ @Test
+ public void test_simple_alias_support_on_jcr_content() {
+ internal_test_simple_alias_support(true);
+ }
+
+ private void internal_test_simple_multi_alias_support(boolean onJcrContent) {
+ prepareMapEntriesForAlias(onJcrContent, false, "foo", "bar");
mapEntries.doInit();
Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
assertNotNull(aliasMap);
assertTrue(aliasMap.containsKey("child"));
- assertEquals(List.of("alias"), aliasMap.get("child"));
+ assertEquals(List.of("foo", "bar"), aliasMap.get("child"));
}
@Test
public void test_simple_multi_alias_support() {
- prepareMapEntriesForAlias("foo", "bar");
+ internal_test_simple_multi_alias_support(false);
+ }
+
+ @Test
+ public void test_simple_multi_alias_support_on_jcr_content() {
+ internal_test_simple_multi_alias_support(true);
+ }
+
+ @Test
+ public void test_simple_multi_alias_support_with_null_parent() {
+ // see SLING-12383
+ prepareMapEntriesForAlias(true, true, "foo", "bar");
mapEntries.doInit();
Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
assertNotNull(aliasMap);
- assertTrue(aliasMap.containsKey("child"));
- assertEquals(List.of("foo", "bar"), aliasMap.get("child"));
+ assertFalse(aliasMap.containsKey("child"));
}
@Test
public void test_simple_multi_alias_support_with_blank_and_invalid() {
// invalid aliases filtered out
- prepareMapEntriesForAlias("", "foo", ".", "bar", "x/y", "qux", " ");
+ prepareMapEntriesForAlias(false, false, "", "foo", ".", "bar", "x/y", "qux", " ");
mapEntries.doInit();
Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
assertNotNull(aliasMap);
@@ -210,29 +237,37 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@Test
public void test_alias_support_invalid() {
for (String invalidAlias : List.of(".", "..", "foo/bar", "# foo", "")) {
- prepareMapEntriesForAlias(invalidAlias);
+ prepareMapEntriesForAlias(false, false, invalidAlias);
mapEntries.doInit();
Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent");
assertEquals(Collections.emptyMap(), aliasMap);
}
}
- private void prepareMapEntriesForAlias(String... alias) {
+ private void prepareMapEntriesForAlias(boolean onJcrContent, boolean withNullParent, String... alias) {
Resource parent = mock(Resource.class);
when(parent.getPath()).thenReturn("/parent");
final Resource result = mock(Resource.class);
- when(result.getParent()).thenReturn(parent);
+ final Resource content = mock(Resource.class);
+ final Resource aliasResource = onJcrContent ? content : result;
+
+ when(result.getParent()).thenReturn(withNullParent && !onJcrContent ? null : parent);
when(result.getPath()).thenReturn("/parent/child");
when(result.getName()).thenReturn("child");
- when(result.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, alias));
+
+ when(content.getParent()).thenReturn(withNullParent && onJcrContent ? null : result);
+ when(content.getPath()).thenReturn("/parent/child/jcr:content");
+ when(content.getName()).thenReturn("jcr:content");
+
+ when(aliasResource.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, alias));
when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))).thenAnswer(new Answer<Iterator<Resource>>() {
@Override
public Iterator<Resource> answer(InvocationOnMock invocation) throws Throwable {
if (invocation.getArguments()[0].toString().contains(ResourceResolverImpl.PROP_ALIAS)) {
- return Collections.singleton(result).iterator();
+ return Collections.singleton(aliasResource).iterator();
} else {
return Collections.<Resource> emptySet().iterator();
}