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 16:05:47 UTC
[sling-org-apache-sling-sitemap] branch issue/SLING-10819 created
(now a5fa6c2)
This is an automated email from the ASF dual-hosted git repository.
diru pushed a change to branch issue/SLING-10819
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-sitemap.git.
at a5fa6c2 SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps
This branch includes the following new commits:
new fae522e trivial: update content model example in README.md
new a5fa6c2 SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-sitemap] 02/02: SLING-10819: fix rendering
of sitemap-index for nested on-demand sitemaps
Posted by di...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
diru pushed a commit to branch issue/SLING-10819
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-sitemap.git
commit a5fa6c207cf8879865804245a653be7a80d6334b
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Thu Sep 16 18:05:20 2021 +0200
SLING-10819: fix rendering of sitemap-index for nested on-demand sitemaps
---
pom.xml | 1 -
.../java/org/apache/sling/sitemap/SitemapUtil.java | 5 +-
.../apache/sling/sitemap/impl/SitemapServlet.java | 21 ++++---
.../sling/sitemap/impl/helper/ChainedIterator.java | 68 ++++++++++++++++++++++
.../sling/sitemap/impl/SitemapServletTest.java | 52 ++++++++++++++---
.../sitemap/impl/helper/ChainedIteratorTest.java | 65 +++++++++++++++++++++
6 files changed, 194 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..54d2478
--- /dev/null
+++ b/src/main/java/org/apache/sling/sitemap/impl/helper/ChainedIterator.java
@@ -0,0 +1,68 @@
+/*
+ * 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;
+
+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() {
+ 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..a5fc62b
--- /dev/null
+++ b/src/test/java/org/apache/sling/sitemap/impl/helper/ChainedIteratorTest.java
@@ -0,0 +1,65 @@
+/*
+ * 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 org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+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 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());
+ }
+}
[sling-org-apache-sling-sitemap] 01/02: trivial: update content
model example in README.md
Posted by di...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
diru pushed a commit to branch issue/SLING-10819
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-sitemap.git
commit fae522e89b2367ffdc18be89a24c8b29261ccfa4
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jul 26 14:51:13 2021 +0200
trivial: update content model example in README.md
---
README.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/README.md b/README.md
index d2bac00..ad6adc9 100644
--- a/README.md
+++ b/README.md
@@ -63,6 +63,7 @@ sitemap root. The sitemaps will be served as following, assuming the appropriate
https://site.ch/de-ch.sitemap-index.xml
https://site.ch/de-ch.sitemap.xml
https://site.ch/de-ch.sitemap.products-sitemap.xml
+https://site.ch/fr-ch.sitemap-index.xml
https://site.ch/fr-ch.sitemap.xml
```