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/06/04 15:29:10 UTC
[sling-whiteboard] branch master updated (80b1a95 -> 3aa38e2)
This is an automated email from the ASF dual-hosted git repository.
diru pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/sling-whiteboard.git.
from 80b1a95 Implemented sling sitemap module
new 7e5e68e fix servlet serving sitemaps from storage
new 3aa38e2 always return all sitemap infos
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.
Summary of changes:
.../java/org/apache/sling/sitemap/SitemapInfo.java | 7 +++
.../org/apache/sling/sitemap/SitemapService.java | 4 +-
.../sling/sitemap/impl/SitemapServiceImpl.java | 54 ++++++++++---------
.../apache/sling/sitemap/impl/SitemapStorage.java | 6 ++-
.../sling/sitemap/impl/SitemapServiceImplTest.java | 60 ++++++++++++++--------
5 files changed, 82 insertions(+), 49 deletions(-)
[sling-whiteboard] 02/02: always return all sitemap infos
Posted by di...@apache.org.
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-whiteboard.git
commit 3aa38e2fc322b87e38d41cf9130909a6e1d51b0a
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Fri Jun 4 17:28:17 2021 +0200
always return all sitemap infos
It is in the responsibility of the user level to filter sitemap infos further.
On a fragmework level all should be returned, if any at all.
---
.../java/org/apache/sling/sitemap/SitemapInfo.java | 7 +++
.../org/apache/sling/sitemap/SitemapService.java | 4 +-
.../sling/sitemap/impl/SitemapServiceImpl.java | 54 ++++++++++---------
.../sling/sitemap/impl/SitemapServiceImplTest.java | 60 ++++++++++++++--------
4 files changed, 78 insertions(+), 47 deletions(-)
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java
index ce3067b..da7caaa 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java
@@ -50,6 +50,13 @@ public interface SitemapInfo {
int getEntries();
/**
+ * Returns true when the url is pointing to a sitemap-index.
+ *
+ * @return
+ */
+ boolean isIndex();
+
+ /**
* Returns true if both size and entries are within the configured limits.
*
* @return
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
index ef22d1d..10cf7f7 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
@@ -36,8 +36,8 @@ public interface SitemapService {
/**
* Returns the urls to the given {@link Resource}'s sitemaps, if any.
* <p>
- * The returned urls may point to a sitemap index when there are multiple sitemaps generated for the given sitemap
- * root {@link Resource}. Or it may point to another sitemap root, if the sitemap is nested below a top level
+ * The returned urls may contain a sitemap index when there are multiple sitemaps generated for the given sitemap
+ * root {@link Resource}. Or it may contain urls to another sitemap root, if the sitemap is nested below a top level
* sitemap root.
* <p>
* Numbers for size and entries can only be provided for sitemaps served from storage. For sitemap index or
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java
index cf93af1..f77a56b 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java
@@ -98,32 +98,29 @@ public class SitemapServiceImpl implements SitemapService {
String url = externalizer.externalize(sitemapRoot);
Collection<String> names = generatorManager.getGenerators(sitemapRoot).keySet();
- if (url == null || names.isEmpty()) {
+ if (url == null) {
LOG.debug("Could not get absolute url to sitemap: {}", resource.getPath());
return Collections.emptySet();
}
- if (names.size() > 1 || requiresSitemapIndex(sitemapRoot)) {
- return Collections.singletonList(newSitemapInfo(
- url + '.' + SitemapServlet.SITEMAP_INDEX_SELECTOR + '.' + SitemapServlet.SITEMAP_EXTENSION,
- -1,
- -1
- ));
- } else {
- String sitemapSelector = getSitemapSelector(sitemapRoot, getTopLevelSitemapRoot(sitemapRoot),
- names.iterator().next());
- return storage.getSitemaps(sitemapRoot, names).stream()
- .filter(info -> info.getSitemapSelector().equals(sitemapSelector))
- .findFirst()
- .map(storageInfo -> Collections.<SitemapInfo>singleton(newSitemapInfo(
- sitemapSelector.equals(SitemapServlet.SITEMAP_SELECTOR)
- ? url + '.' + SitemapServlet.SITEMAP_SELECTOR + '.' + SitemapServlet.SITEMAP_EXTENSION
- : url + '.' + SitemapServlet.SITEMAP_SELECTOR + '.' + sitemapSelector + '.' + SitemapServlet.SITEMAP_EXTENSION,
- storageInfo.getSize(),
- storageInfo.getEntries()
- )))
- .orElseGet(Collections::emptySet);
+ Collection<SitemapInfo> infos = new ArrayList<>(names.size() + 1);
+
+ if (requiresSitemapIndex(sitemapRoot)) {
+ infos.add(newSitemapIndexInfo(
+ url + '.' + SitemapServlet.SITEMAP_INDEX_SELECTOR + '.' + SitemapServlet.SITEMAP_EXTENSION));
+ }
+
+ for (SitemapStorageInfo storageInfo : storage.getSitemaps(sitemapRoot, names)) {
+ String location = url + '.' + SitemapServlet.SITEMAP_SELECTOR + '.';
+ if (storageInfo.getSitemapSelector().equals(SitemapServlet.SITEMAP_SELECTOR)) {
+ location += SitemapServlet.SITEMAP_EXTENSION;
+ } else {
+ location += storageInfo.getSitemapSelector() + '.' + SitemapServlet.SITEMAP_EXTENSION;
+ }
+ infos.add(newSitemapInfo(location, storageInfo.getSize(), storageInfo.getEntries()));
}
+
+ return infos;
}
@Override
@@ -204,8 +201,12 @@ public class SitemapServiceImpl implements SitemapService {
return infos;
}
+ private SitemapInfo newSitemapIndexInfo(@NotNull String url) {
+ return new SitemapInfoImpl(url, -1, -1, true, true);
+ }
+
private SitemapInfo newSitemapInfo(@NotNull String url, int size, int entries) {
- return new SitemapInfoImpl(url, size, entries, size <= maxSize && entries <= maxEntries);
+ return new SitemapInfoImpl(url, size, entries, false, size <= maxSize && entries <= maxEntries);
}
private static class SitemapInfoImpl implements SitemapInfo {
@@ -213,12 +214,14 @@ public class SitemapServiceImpl implements SitemapService {
private final String url;
private final int size;
private final int entries;
+ private final boolean isIndex;
private final boolean withinLimits;
- private SitemapInfoImpl(@NotNull String url, int size, int entries, boolean withinLimits) {
+ private SitemapInfoImpl(@NotNull String url, int size, int entries, boolean isIndex, boolean withinLimits) {
this.url = url;
this.size = size;
this.entries = entries;
+ this.isIndex = isIndex;
this.withinLimits = withinLimits;
}
@@ -239,6 +242,11 @@ public class SitemapServiceImpl implements SitemapService {
}
@Override
+ public boolean isIndex() {
+ return isIndex;
+ }
+
+ @Override
public boolean isWithinLimits() {
return withinLimits;
}
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplTest.java
index 07b9fed..3b188f4 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplTest.java
@@ -22,8 +22,8 @@ import com.google.common.collect.ImmutableSet;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.event.jobs.Job;
import org.apache.sling.event.jobs.JobManager;
+import org.apache.sling.serviceusermapping.ServiceUserMapped;
import org.apache.sling.sitemap.SitemapInfo;
-import org.apache.sling.sitemap.common.Externalizer;
import org.apache.sling.sitemap.generator.SitemapGenerator;
import org.apache.sling.testing.mock.jcr.MockJcr;
import org.apache.sling.testing.mock.sling.ResourceResolverType;
@@ -41,6 +41,8 @@ import org.mockito.junit.jupiter.MockitoExtension;
import javax.jcr.Node;
import javax.jcr.Session;
import javax.jcr.query.Query;
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -62,14 +64,15 @@ public class SitemapServiceImplTest {
public final SlingContext context = new SlingContext(ResourceResolverType.JCR_MOCK);
private final SitemapServiceImpl subject = new SitemapServiceImpl();
+ private final SitemapStorage storage = new SitemapStorage();
private final SitemapGeneratorManager generatorManager = new SitemapGeneratorManager();
@Mock
+ private ServiceUserMapped serviceUser;
+ @Mock
private JobManager jobManager;
@Mock
private SitemapGenerator generator;
- @Mock
- private SitemapStorage storage;
private Resource deRoot;
private Resource enRoot;
@@ -97,16 +100,19 @@ public class SitemapServiceImplTest {
));
noRoot = context.create().resource("/content/site/nothing");
- context.registerService(SitemapStorage.class, storage);
+ context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
context.registerService(SitemapGenerator.class, generator);
context.registerService(JobManager.class, jobManager);
context.registerInjectActivateService(generatorManager);
+ context.registerInjectActivateService(storage);
context.registerInjectActivateService(subject, "onDemandNames", "news");
}
@Test
- public void testIsPendingReturnsTrue() {
+ public void testIsPendingReturnsTrue() throws IOException {
// given
+ storage.writeSitemap(deRoot, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 0, 0);
+
when(generator.getNames(any())).thenReturn(Collections.singleton(SitemapGenerator.DEFAULT_SITEMAP));
when(jobManager.findJobs(
eq(JobManager.QueryType.ALL),
@@ -116,7 +122,6 @@ public class SitemapServiceImplTest {
&& arg.containsKey(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_ROOT)
&& arg.containsValue("/content/site/de"))))
.thenReturn(Collections.singleton(mock(Job.class)));
- when(storage.getSitemaps(eq(deRoot), any())).thenReturn(Collections.singleton(mock(SitemapStorageInfo.class)));
// when, then
// one in storage, one job
@@ -124,8 +129,10 @@ public class SitemapServiceImplTest {
}
@Test
- public void testIsPendingReturnsFalse() {
+ public void testIsPendingReturnsFalse() throws IOException {
// given
+ storage.writeSitemap(deRoot, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 0, 0);
+
when(generator.getNames(any())).thenReturn(ImmutableSet.of(SitemapGenerator.DEFAULT_SITEMAP));
when(generator.getNames(frRoot)).thenReturn(ImmutableSet.of("a", "b"));
when(generator.getNames(enNews)).thenReturn(ImmutableSet.of("news"));
@@ -137,7 +144,6 @@ public class SitemapServiceImplTest {
&& arg.containsKey(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_ROOT)
&& arg.containsValue("/content/site/de"))))
.thenReturn(Collections.emptyList());
- when(storage.getSitemaps(eq(deRoot), any())).thenReturn(Collections.singleton(mock(SitemapStorageInfo.class)));
MockJcr.setQueryResult(
context.resourceResolver().adaptTo(Session.class),
@@ -200,27 +206,31 @@ public class SitemapServiceImplTest {
}
@Test
- public void testSitemapUrlReturnsProperSelectors() {
+ public void testSitemapUrlReturnsProperSelectors() throws IOException {
// given
- SitemapStorageInfo storageInfoSitemap =
- new SitemapStorageInfo("", "sitemap", null, 100, 1);
- SitemapStorageInfo storageInfoFoobarSitemap =
- new SitemapStorageInfo("", "foobar-sitemap", null, 100, 1);
- SitemapStorageInfo storageInfoNewsFooSitemap =
- new SitemapStorageInfo("", "news-foo-sitemap", null, 100, 1);
- SitemapStorageInfo storageInfoNewsBarSitemap =
- new SitemapStorageInfo("", "news-bar-sitemap", null, 100, 1);
+ storage.writeSitemap(deRoot, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 100, 1);
+ storage.writeSitemap(frRoot, "foobar", new ByteArrayInputStream(new byte[0]), 100, 1);
+ storage.writeSitemap(enRoot, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 100, 1);
+ storage.writeSitemap(enNews, "foo", new ByteArrayInputStream(new byte[0]), 100, 1);
+ storage.writeSitemap(enNews, "bar", new ByteArrayInputStream(new byte[0]), 100, 1);
when(generator.getNames(deRoot)).thenReturn(ImmutableSet.of(SitemapGenerator.DEFAULT_SITEMAP));
when(generator.getNames(frRoot)).thenReturn(ImmutableSet.of("foobar"));
when(generator.getNames(enNews)).thenReturn(ImmutableSet.of("foo", "bar", "news"));
- when(storage.getSitemaps(any(), any())).thenReturn(ImmutableSet.of(
- storageInfoSitemap, storageInfoFoobarSitemap, storageInfoNewsFooSitemap, storageInfoNewsBarSitemap));
+ when(generator.getNames(enRoot)).thenReturn(ImmutableSet.of(SitemapGenerator.DEFAULT_SITEMAP));
+
+ MockJcr.setQueryResult(
+ context.resourceResolver().adaptTo(Session.class),
+ "/jcr:root/content/site/en//*[@sitemapRoot=true]",
+ Query.XPATH,
+ Arrays.asList(enRoot.adaptTo(Node.class), enNews.adaptTo(Node.class))
+ );
// when
Collection<SitemapInfo> deInfos = subject.getSitemapInfo(deRoot);
Collection<SitemapInfo> frInfos = subject.getSitemapInfo(frRoot);
- Collection<SitemapInfo> enInfos = subject.getSitemapInfo(enNews);
+ Collection<SitemapInfo> enInfos = subject.getSitemapInfo(enRoot);
+ Collection<SitemapInfo> enNestedInfos = subject.getSitemapInfo(enNews);
// no name
assertThat(deInfos, hasSize(1));
@@ -229,13 +239,19 @@ public class SitemapServiceImplTest {
assertThat(frInfos, hasSize(1));
assertThat(frInfos, hasItems(eqSitemapInfo("/site/fr.sitemap.foobar-sitemap.xml", 100, 1)));
// nested with multiple names
- assertThat(enInfos, hasSize(3));
- assertThat(enInfos, hasItems(
+ assertThat(enNestedInfos, hasSize(3));
+ assertThat(enNestedInfos, hasItems(
eqSitemapInfo("/site/en.sitemap.news-foo-sitemap.xml", 100, 1),
eqSitemapInfo("/site/en.sitemap.news-bar-sitemap.xml", 100, 1),
// on-demand
eqSitemapInfo("/site/en.sitemap.news-news-sitemap.xml", -1, -1)
));
+ // nested => index and default self
+ assertThat(enInfos, hasSize(2));
+ assertThat(enInfos, hasItems(
+ eqSitemapInfo("/site/en.sitemap-index.xml", -1, -1),
+ eqSitemapInfo("/site/en.sitemap.xml", 100, 1)
+ ));
}
private static Matcher<SitemapInfo> eqSitemapInfo(String url, int size, int entries) {
[sling-whiteboard] 01/02: fix servlet serving sitemaps from storage
Posted by di...@apache.org.
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-whiteboard.git
commit 7e5e68e72b40c70224657456237a7b08f03a2b21
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Fri Jun 4 17:25:21 2021 +0200
fix servlet serving sitemaps from storage
after changing from nt:file to nt:unstructured as node type for the
sitemap nodes, resource.adaptTo(InputStream.class) does not work
anymore. The jcr:data attribute needs to be accessed directly.
---
.../src/main/java/org/apache/sling/sitemap/impl/SitemapStorage.java | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapStorage.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapStorage.java
index 4fb3494..b31786e 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapStorage.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapStorage.java
@@ -266,8 +266,10 @@ public class SitemapStorage {
}
String sitemapFilePath = rootPath + sitemapRoot.getPath() + '/' + sitemapSelector + XML_EXTENSION;
try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
- Resource sitemap = resolver.getResource(sitemapFilePath);
- InputStream data = sitemap != null ? sitemap.adaptTo(InputStream.class) : null;
+ InputStream data = Optional.ofNullable(resolver.getResource(sitemapFilePath))
+ .map(r -> r.getChild(JcrConstants.JCR_CONTENT))
+ .map(r -> r.getValueMap().get(JcrConstants.JCR_DATA, InputStream.class))
+ .orElse(null);
if (data != null) {
IOUtils.copyLarge(data, output);