You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by di...@apache.org on 2021/09/16 17:02:19 UTC
[sling-org-apache-sling-sitemap] branch master updated:
SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps
(#3)
This is an automated email from the ASF dual-hosted git repository.
diru pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-sitemap.git
The following commit(s) were added to refs/heads/master by this push:
new b17051a SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps (#3)
b17051a is described below
commit b17051a86d786b5ab0842dd00a771eadecad3c87
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Thu Sep 16 19:02:14 2021 +0200
SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps (#3)
---
pom.xml | 1 -
.../java/org/apache/sling/sitemap/SitemapUtil.java | 5 +-
.../apache/sling/sitemap/impl/SitemapServlet.java | 21 ++++---
.../sling/sitemap/impl/helper/ChainedIterator.java | 72 ++++++++++++++++++++++
.../sling/sitemap/impl/SitemapServletTest.java | 52 +++++++++++++---
.../sitemap/impl/helper/ChainedIteratorTest.java | 72 ++++++++++++++++++++++
6 files changed, 205 insertions(+), 18 deletions(-)
diff --git a/pom.xml b/pom.xml
index 0c9fbd6..afb95c9 100644
--- a/pom.xml
+++ b/pom.xml
@@ -46,7 +46,6 @@
<build>
<plugins>
-
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-baseline-maven-plugin</artifactId>
diff --git a/src/main/java/org/apache/sling/sitemap/SitemapUtil.java b/src/main/java/org/apache/sling/sitemap/SitemapUtil.java
index 250d823..ee933ae 100644
--- a/src/main/java/org/apache/sling/sitemap/SitemapUtil.java
+++ b/src/main/java/org/apache/sling/sitemap/SitemapUtil.java
@@ -196,7 +196,10 @@ public final class SitemapUtil {
}
/**
- * Returns all sitemap root {@link Resource}s within the given search path.
+ * Returns all sitemap root {@link Resource}s within the given search path, excluding the search path itself even if it is a sitemap
+ * root.
+ * <p>
+ * The {@link Resource}s returned are normalized using {@link SitemapUtil#normalizeSitemapRoot(Resource)}.
*
* @param resolver
* @param searchPath
diff --git a/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java b/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
index 8b622f6..d9d0ec4 100644
--- a/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
+++ b/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
@@ -24,6 +24,7 @@ import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.servlets.ServletResolverConstants;
import org.apache.sling.api.servlets.SlingSafeMethodsServlet;
import org.apache.sling.sitemap.SitemapException;
+import org.apache.sling.sitemap.impl.helper.ChainedIterator;
import org.apache.sling.sitemap.spi.common.SitemapLinkExternalizer;
import org.apache.sling.sitemap.spi.generator.SitemapGenerator;
import org.apache.sling.sitemap.SitemapGeneratorManager;
@@ -76,7 +77,7 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
@Override
public void setProperty(@NotNull String name, @Nullable Object data) {
- // thei implementation of SitemapGenerator.Context doesn't track any state
+ // the implementation of SitemapGenerator.Context doesn't track any state
}
};
@@ -184,26 +185,32 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
* Adds all on-demand sitemaps to the index within the given sitemap root.
*
* @param request
- * @param parentSitemapRoot
+ * @param topLevelSitemapRoot
* @param index
* @return
* @throws SitemapException
*/
- private Set<String> addOnDemandSitemapsToIndex(SlingHttpServletRequest request, Resource parentSitemapRoot,
+ private Set<String> addOnDemandSitemapsToIndex(SlingHttpServletRequest request, Resource topLevelSitemapRoot,
SitemapIndexImpl index) throws SitemapException {
Set<String> addedSitemapSelectors = new HashSet<>();
- Iterator<Resource> sitemapRoots = findSitemapRoots(request.getResourceResolver(), parentSitemapRoot.getPath());
+ Iterator<Resource> sitemapRoots = findSitemapRoots(request.getResourceResolver(), topLevelSitemapRoot.getPath());
if (!sitemapRoots.hasNext()) {
// serve at least the top level sitemap
- sitemapRoots = Collections.singleton(parentSitemapRoot).iterator();
+ sitemapRoots = Collections.singleton(topLevelSitemapRoot).iterator();
+ } else {
+ // chain with the given sitemap root as it is not part of the result set
+ sitemapRoots = new ChainedIterator<>(
+ sitemapRoots,
+ Collections.singleton(topLevelSitemapRoot).iterator()
+ );
}
while (sitemapRoots.hasNext()) {
Resource sitemapRoot = sitemapRoots.next();
Set<String> applicableNames = generatorManager.getOnDemandNames(sitemapRoot);
// applicable names we may serve directly, not applicable names, if any, we have to serve from storage
for (String applicableName : applicableNames) {
- String sitemapSelector = getSitemapSelector(sitemapRoot, parentSitemapRoot, applicableName);
- String location = externalize(request, getSitemapLink(sitemapRoot, sitemapSelector));
+ String sitemapSelector = getSitemapSelector(sitemapRoot, topLevelSitemapRoot, applicableName);
+ String location = externalize(request, getSitemapLink(topLevelSitemapRoot, sitemapSelector));
if (location != null) {
index.addSitemap(location);
addedSitemapSelectors.add(sitemapSelector);
diff --git a/src/main/java/org/apache/sling/sitemap/impl/helper/ChainedIterator.java b/src/main/java/org/apache/sling/sitemap/impl/helper/ChainedIterator.java
new file mode 100644
index 0000000..9aa27a4
--- /dev/null
+++ b/src/main/java/org/apache/sling/sitemap/impl/helper/ChainedIterator.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.sling.sitemap.impl.helper;
+
+import java.util.Arrays;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+public class ChainedIterator<T> implements Iterator<T> {
+
+ private final Iterator<Iterator<T>> iterators;
+ private Iterator<T> currentIterator;
+ private T currentItem;
+
+ public ChainedIterator(Iterator<T>... iterators) {
+ this(Arrays.asList(iterators));
+ }
+
+ public ChainedIterator(Iterator<T> iterator1, Iterator<T> iterator2) {
+ this(Arrays.asList(iterator1, iterator2));
+ }
+
+ public ChainedIterator(Iterable<Iterator<T>> iterators) {
+ this.iterators = iterators.iterator();
+ seek();
+ }
+
+ private void seek() {
+ while (currentItem == null) {
+ if (currentIterator != null && currentIterator.hasNext()) {
+ currentItem = currentIterator.next();
+ } else {
+ if (iterators.hasNext()) {
+ currentIterator = iterators.next();
+ } else {
+ break;
+ }
+ }
+ }
+ }
+
+ @Override
+ public boolean hasNext() {
+ return currentItem != null;
+ }
+
+ @Override public T next() {
+ if (!hasNext()) {
+ throw new NoSuchElementException();
+ }
+ T item = currentItem;
+ currentItem = null;
+ seek();
+ return item;
+ }
+}
diff --git a/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java b/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
index 0d879a0..ad7178d 100644
--- a/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
+++ b/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
@@ -42,6 +42,7 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import javax.jcr.Node;
+import javax.jcr.RepositoryException;
import javax.jcr.Session;
import javax.servlet.ServletException;
import java.io.ByteArrayInputStream;
@@ -66,16 +67,16 @@ class SitemapServletTest {
private final SitemapGeneratorManagerImpl generatorManager = new SitemapGeneratorManagerImpl();
private final ExtensionProviderManager extensionProviderManager = new ExtensionProviderManager();
- private TestResourceTreeSitemapGenerator generator = new TestResourceTreeSitemapGenerator() {
+ private final TestResourceTreeSitemapGenerator generator = new TestResourceTreeSitemapGenerator() {
@Override
public @NotNull Set<String> getNames(@NotNull Resource sitemapRoot) {
return ImmutableSet.of(SitemapService.DEFAULT_SITEMAP_NAME);
}
};
- private TestResourceTreeSitemapGenerator newsGenerator = new TestResourceTreeSitemapGenerator() {
+ private final TestResourceTreeSitemapGenerator newsGenerator = new TestResourceTreeSitemapGenerator() {
@Override
public @NotNull Set<String> getNames(@NotNull Resource sitemapRoot) {
- return ImmutableSet.of("news");
+ return sitemapRoot.getPath().equals(root.getPath()) ? ImmutableSet.of("news") : Collections.emptySet();
}
};
@@ -96,7 +97,7 @@ class SitemapServletTest {
@BeforeEach
void setup() {
root = context.create().resource("/content/site/de");
- context.create().resource("/content/site/de/jcr:content", Collections.singletonMap(
+ context.create().resource(root.getPath() + "/jcr:content", Collections.singletonMap(
SitemapService.PROPERTY_SITEMAP_ROOT, Boolean.TRUE));
context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
@@ -118,11 +119,6 @@ class SitemapServletTest {
})
.collect(Collectors.toCollection(LinkedHashSet::new))
).when(storage).getSitemaps(any());
-
- MockJcr.setQueryResult(
- context.resourceResolver().adaptTo(Session.class),
- Collections.singletonList(root.adaptTo(Node.class))
- );
}
@Test
@@ -234,6 +230,44 @@ class SitemapServletTest {
}
@Test
+ void testSitemapIndexContainsNestedOnDemandSitemaps() throws ServletException, IOException, RepositoryException {
+ // given
+ Resource products = context.create().resource("/content/site/de/products");
+ context.create().resource(products.getPath() + "/jcr:content", Collections.singletonMap(
+ SitemapService.PROPERTY_SITEMAP_ROOT, Boolean.TRUE));
+ Resource categories = context.create().resource("/content/site/de/categories");
+ context.create().resource(categories.getPath() + "/jcr:content", Collections.singletonMap(
+ SitemapService.PROPERTY_SITEMAP_ROOT, Boolean.TRUE));
+
+ MockJcr.setQueryResult(
+ context.resourceResolver().adaptTo(Session.class).getWorkspace().getQueryManager(),
+ Arrays.asList(products.adaptTo(Node.class), categories.adaptTo(Node.class))
+ );
+
+ MockSlingHttpServletRequest request = newSitemapIndexReq(root);
+ MockSlingHttpServletResponse response = context.response();
+ generator.setServeOnDemand(true);
+ newsGenerator.setServeOnDemand(true);
+
+ // when
+ subject.doGet(request, response);
+
+ // then
+ assertEquals(200, response.getStatus());
+ assertEquals("application/xml;charset=utf-8", response.getContentType());
+ assertEquals("utf-8", response.getCharacterEncoding());
+ assertEquals(
+ AbstractBuilderTest.XML_HEADER + "<sitemapindex xmlns=\"http://www.sitemaps.org/schemas/sitemap/0.9\">"
+ + "<sitemap><loc>/site/de.sitemap.products-sitemap.xml</loc></sitemap>"
+ + "<sitemap><loc>/site/de.sitemap.categories-sitemap.xml</loc></sitemap>"
+ + "<sitemap><loc>/site/de.sitemap.news-sitemap.xml</loc></sitemap>"
+ + "<sitemap><loc>/site/de.sitemap.xml</loc></sitemap>"
+ + "</sitemapindex>",
+ response.getOutputAsString()
+ );
+ }
+
+ @Test
void testSitemapServedOnDemand() throws ServletException, IOException {
// given
MockSlingHttpServletRequest request = newSitemapReq("news-sitemap", root);
diff --git a/src/test/java/org/apache/sling/sitemap/impl/helper/ChainedIteratorTest.java b/src/test/java/org/apache/sling/sitemap/impl/helper/ChainedIteratorTest.java
new file mode 100644
index 0000000..39332be
--- /dev/null
+++ b/src/test/java/org/apache/sling/sitemap/impl/helper/ChainedIteratorTest.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.
+ */
+package org.apache.sling.sitemap.impl.helper;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+public class ChainedIteratorTest {
+
+ @Test
+ public void testEmptyNoIterator() {
+ assertFalse(new ChainedIterator<>().hasNext());
+ }
+
+ @Test
+ public void testEmptyOneEmptyIterator() {
+ assertFalse(new ChainedIterator<>(Collections.emptyIterator()).hasNext());
+ }
+
+ @Test
+ public void testEmptyMutlipleEmptyIterators() {
+ assertFalse(new ChainedIterator<>(Collections.emptyIterator(), Collections.emptyIterator(), Collections.emptyIterator()).hasNext());
+ }
+
+ @Test
+ public void testThorwsNoSuchElementException() {
+ assertThrows(NoSuchElementException.class, () -> new ChainedIterator<>().next());
+ }
+
+ @Test
+ public void testContainsAll() {
+ Iterator<String> it = new ChainedIterator<>(
+ Arrays.asList("a", "b").iterator(),
+ Arrays.asList("c", "d").iterator()
+ );
+
+ assertTrue(it.hasNext());
+ assertEquals("a", it.next());
+ assertTrue(it.hasNext());
+ assertEquals("b", it.next());
+ assertTrue(it.hasNext());
+ assertEquals("c", it.next());
+ assertTrue(it.hasNext());
+ assertEquals("d", it.next());
+ assertFalse(it.hasNext());
+ }
+}