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/07 13:32:46 UTC

[sling-whiteboard] branch master updated (15f07b9 -> 1842433)

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 15f07b9  org.osgi.util.function 1.1.0.1 is not in Maven Central - all tests pass with this change
     new 17e264b  Added flag to sitemap finished event
     new cd90494  rename sitemap Externalizer
     new 6ca7662  add sitemap storage cleanup implementation
     new 528118c  add sling sitemap invetory plugin
     new 1842433  add API to schedule sitemap regeneration

The 5 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:
 sitemap/README.md                                  |   1 +
 sitemap/pom.xml                                    |  15 +-
 .../java/org/apache/sling/sitemap/SitemapInfo.java |  10 +
 .../org/apache/sling/sitemap/SitemapService.java   |  33 +++-
 ...ernalizer.java => SitemapLinkExternalizer.java} |   6 +-
 .../sling/sitemap/generator/SitemapGenerator.java  |   4 +
 .../sitemap/impl/SitemapGeneratorExecutor.java     |   5 +
 .../sling/sitemap/impl/SitemapScheduler.java       |  46 +++--
 .../sling/sitemap/impl/SitemapServiceImpl.java     | 112 +++++++++--
 .../apache/sling/sitemap/impl/SitemapServlet.java  |  14 +-
 .../apache/sling/sitemap/impl/SitemapStorage.java  | 190 +++++++++++++------
 .../org/apache/sling/sitemap/impl/SitemapUtil.java |  23 ++-
 .../impl/console/SitemapInventoryPlugin.java       | 205 +++++++++++++++++++++
 .../sitemap/impl/SitemapGeneratorExecutorTest.java |   8 +-
 .../impl/SitemapServiceImplSchedulingTest.java     | 135 ++++++++++++++
 .../sling/sitemap/impl/SitemapServletTest.java     |   5 +-
 .../sling/sitemap/impl/SitemapStorageTest.java     |  83 +++++++++
 17 files changed, 792 insertions(+), 103 deletions(-)
 rename sitemap/src/main/java/org/apache/sling/sitemap/common/{Externalizer.java => SitemapLinkExternalizer.java} (90%)
 create mode 100644 sitemap/src/main/java/org/apache/sling/sitemap/impl/console/SitemapInventoryPlugin.java
 create mode 100644 sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplSchedulingTest.java

[sling-whiteboard] 03/05: add sitemap storage cleanup implementation

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 6ca7662b3afe9f07ebc8bc5e86614360f71943a4
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jun 7 12:02:29 2021 +0200

    add sitemap storage cleanup implementation
---
 .../apache/sling/sitemap/impl/SitemapStorage.java  | 180 +++++++++++++++------
 .../sitemap/impl/SitemapGeneratorExecutorTest.java |   2 +-
 .../sling/sitemap/impl/SitemapServletTest.java     |   4 +-
 .../sling/sitemap/impl/SitemapStorageTest.java     |  83 ++++++++++
 4 files changed, 219 insertions(+), 50 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 b31786e..4ffa9c4 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
@@ -20,10 +20,13 @@ package org.apache.sling.sitemap.impl;
 
 import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.sling.api.SlingConstants;
 import org.apache.sling.api.resource.*;
 import org.apache.sling.api.wrappers.ValueMapDecorator;
+import org.apache.sling.commons.scheduler.Scheduler;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
@@ -39,13 +42,21 @@ import java.io.OutputStream;
 import java.util.*;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 
 import static org.apache.sling.sitemap.impl.SitemapUtil.*;
 
-@Component(service = SitemapStorage.class)
+@Component(
+        service = {SitemapStorage.class, Runnable.class},
+        property = {
+                Scheduler.PROPERTY_SCHEDULER_NAME + "=sitemap-storage-cleanup",
+                Scheduler.PROPERTY_SCHEDULER_CONCURRENT + "=false",
+                Scheduler.PROPERTY_SCHEDULER_RUN_ON + "=" + Scheduler.VALUE_RUN_ON_SINGLE
+        }
+)
 @Designate(ocd = SitemapStorage.Configuration.class)
-public class SitemapStorage {
+public class SitemapStorage implements Runnable {
 
     @ObjectClassDefinition(name = "Apache Sling Sitemap - Storage")
     @interface Configuration {
@@ -57,6 +68,10 @@ public class SitemapStorage {
         @AttributeDefinition(name = "Max State Age", description = "The number of milliseconds after which an " +
                 "intermediate state will deleted.")
         int stateMaxAge() default 60 * 30 * 1000;
+
+        @AttributeDefinition(name = "Cleanup Schedule", description = "A cron expression defining the schedule at " +
+                "which stale intermediate states and old sitemaps will be removed.")
+        String scheduler_expression() default "0 0 1 * * ?";
     }
 
     static final String PN_SITEMAP_ENTRIES = "entries";
@@ -67,11 +82,16 @@ public class SitemapStorage {
             "sitemap-writer");
     private static final String STATE_EXTENSION = ".part";
     private static final String XML_EXTENSION = ".xml";
+    private static final String RT_SITEMAP_PART = "sling/sitemap/part";
+    private static final String RT_SITEMAP_FILE = "sling/sitemap/file";
+    private static final String PN_RESOURCE_TYPE = SlingConstants.NAMESPACE_PREFIX + ':' + SlingConstants.PROPERTY_RESOURCE_TYPE;
 
     @Reference
     private ResourceResolverFactory resourceResolverFactory;
     @Reference(target = "(subServiceName=sitemap-writer)")
     private ServiceUserMapped serviceUserMapped;
+    @Reference
+    private SitemapGeneratorManager generatorManager;
 
     private String rootPath = "/var/sitemaps";
     private int maxStateAge = Integer.MAX_VALUE;
@@ -82,8 +102,30 @@ public class SitemapStorage {
         maxStateAge = configuration.stateMaxAge();
     }
 
+    @Override
+    public void run() {
+        try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
+            Iterator<Resource> descendants = traverse(resolver.getResource(rootPath)).iterator();
+            List<Resource> toDelete = new LinkedList<>();
+            while (descendants.hasNext()) {
+                Resource descendant = descendants.next();
+                if (descendant.isResourceType(RT_SITEMAP_PART) && isExpired(descendant)) {
+                    toDelete.add(descendant);
+                } else if (descendant.isResourceType(RT_SITEMAP_FILE) && !doesSitemapRootExist(descendant)) {
+                    toDelete.add(descendant);
+                }
+            }
+            for (Resource resource : toDelete) {
+                resolver.delete(resource);
+            }
+            resolver.commit();
+        } catch (LoginException | PersistenceException ex) {
+            LOG.warn("Failed to cleanup storage: {}", ex.getMessage(), ex);
+        }
+    }
+
     @NotNull
-    public ValueMap getState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
+    ValueMap getState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
         String statePath = getSitemapFilePath(sitemapRoot, name) + STATE_EXTENSION;
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
             Resource state = resolver.getResource(statePath);
@@ -91,19 +133,10 @@ public class SitemapStorage {
             if (state == null) {
                 return ValueMap.EMPTY;
             }
-            ValueMap stateProperties = state.getValueMap();
-            Calendar lastModified = stateProperties.get(JcrConstants.JCR_LASTMODIFIED, Calendar.class);
-            if (lastModified != null) {
-                // advance lastModified by maxStateAge to get the point in time the state would expire
-                lastModified.add(Calendar.MILLISECOND, maxStateAge);
-                // check if the expire time is in the future, if so return the state, if not fallback to an empty state
-                if (lastModified.after(Calendar.getInstance())) {
-                    return new ValueMapDecorator(new HashMap<>(state.getValueMap()));
-                } else if (LOG.isDebugEnabled()) {
-                    LOG.debug("State at {} expired at {}", statePath, lastModified.getTime().toGMTString());
-                }
-            }
-            return ValueMap.EMPTY;
+
+            return !isExpired(state)
+                    ? new ValueMapDecorator(new HashMap<>(state.getValueMap()))
+                    : ValueMap.EMPTY;
         } catch (LoginException ex) {
             throw new IOException("Cannot read state at " + statePath, ex);
         }
@@ -116,19 +149,20 @@ public class SitemapStorage {
             Resource folder = getOrCreateFolder(resolver, ResourceUtil.getParent(statePath));
             String stateName = ResourceUtil.getName(statePath);
             Resource stateResource = folder.getChild(stateName);
-            if (stateResource != null) {
+            if (stateResource == null) {
+                Map<String, Object> properties = new HashMap<>(state.size() + 1);
+                properties.putAll(state);
+                properties.put(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED);
+                properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
+                properties.put(PN_RESOURCE_TYPE, RT_SITEMAP_PART);
+                resolver.create(folder, stateName, properties);
+            } else {
                 ModifiableValueMap properties = stateResource.adaptTo(ModifiableValueMap.class);
                 if (properties == null) {
                     throw new IOException("Cannot modify properties of existing state: " + statePath);
                 }
                 properties.putAll(state);
                 properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
-            } else {
-                Map<String, Object> properties = new HashMap<>(state.size() + 1);
-                properties.putAll(state);
-                properties.put(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED);
-                properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
-                resolver.create(folder, stateName, properties);
             }
             resolver.commit();
         } catch (LoginException | PersistenceException ex) {
@@ -163,34 +197,21 @@ public class SitemapStorage {
             if (sitemapResource == null) {
                 Map<String, Object> properties = new HashMap<>(3);
                 properties.put(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_UNSTRUCTURED);
+                properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
+                properties.put(JcrConstants.JCR_DATA, data);
                 properties.put(PN_SITEMAP_ENTRIES, entries);
                 properties.put(PN_SITEMAP_SIZE, size);
-                sitemapResource = resolver.create(folder, sitemapFileName, properties);
+                properties.put(PN_RESOURCE_TYPE, RT_SITEMAP_FILE);
+                resolver.create(folder, sitemapFileName, properties);
             } else {
                 ModifiableValueMap properties = sitemapResource.adaptTo(ModifiableValueMap.class);
                 if (properties == null) {
                     throw new IOException("Cannot overwrite existing sitemap at: " + sitemapFilePath);
                 }
-                properties.put(PN_SITEMAP_ENTRIES, entries);
-                properties.put(PN_SITEMAP_SIZE, size);
-            }
-
-            Resource sitemapContent = sitemapResource.getChild(JcrConstants.JCR_CONTENT);
-            if (sitemapContent == null) {
-                Map<String, Object> properties = new HashMap<>();
-                properties.put(JcrConstants.JCR_PRIMARYTYPE, JcrConstants.NT_RESOURCE);
-                properties.put(JcrConstants.JCR_ENCODING, "UTF-8");
-                properties.put(JcrConstants.JCR_MIMETYPE, "application/xml");
-                properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
-                properties.put(JcrConstants.JCR_DATA, data);
-                resolver.create(sitemapResource, JcrConstants.JCR_CONTENT, properties);
-            } else {
-                ModifiableValueMap properties = sitemapContent.adaptTo(ModifiableValueMap.class);
-                if (properties == null) {
-                    throw new IOException("Cannot overwrite existing sitemap at: " + sitemapFilePath);
-                }
                 properties.put(JcrConstants.JCR_LASTMODIFIED, Calendar.getInstance());
                 properties.put(JcrConstants.JCR_DATA, data);
+                properties.put(PN_SITEMAP_ENTRIES, entries);
+                properties.put(PN_SITEMAP_SIZE, size);
             }
 
             Resource stateResource = resolver.getResource(statePath);
@@ -243,13 +264,11 @@ public class SitemapStorage {
             }
             return StreamSupport.stream(storageResource.getChildren().spliterator(), false)
                     .filter(child -> child.getName().endsWith(XML_EXTENSION))
+                    .filter(child -> child.isResourceType(RT_SITEMAP_FILE))
                     .map(child -> new SitemapStorageInfo(
                             child.getPath(),
                             child.getName().substring(0, child.getName().lastIndexOf('.')),
-                            Optional.ofNullable(child.getChild(JcrConstants.JCR_CONTENT))
-                                    .map(Resource::getValueMap)
-                                    .map(content -> content.get(JcrConstants.JCR_LASTMODIFIED, Calendar.class))
-                                    .orElse(null),
+                            child.getValueMap().get(JcrConstants.JCR_LASTMODIFIED, Calendar.class),
                             child.getValueMap().get(PN_SITEMAP_SIZE, 0),
                             child.getValueMap().get(PN_SITEMAP_ENTRIES, 0)))
                     .filter(filter)
@@ -267,7 +286,8 @@ public class SitemapStorage {
         String sitemapFilePath = rootPath + sitemapRoot.getPath() + '/' + sitemapSelector + XML_EXTENSION;
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
             InputStream data = Optional.ofNullable(resolver.getResource(sitemapFilePath))
-                    .map(r -> r.getChild(JcrConstants.JCR_CONTENT))
+                    .filter(r -> r.getName().endsWith(XML_EXTENSION))
+                    .filter(r -> r.isResourceType(RT_SITEMAP_FILE))
                     .map(r -> r.getValueMap().get(JcrConstants.JCR_DATA, InputStream.class))
                     .orElse(null);
 
@@ -284,6 +304,61 @@ public class SitemapStorage {
         }
     }
 
+    /**
+     * Returns true the sitemap root of the given file exists. This first checks if the file is in a top level sitemap's
+     * storage path and if so, checks if the selector resolves to at least one root.
+     *
+     * @param sitemapFile
+     * @return
+     */
+    private boolean doesSitemapRootExist(Resource sitemapFile) {
+        String path = sitemapFile.getPath();
+        String parentPath = ResourceUtil.getParent(path);
+        String sitemapRootPath = parentPath.substring(rootPath.length());
+        Resource sitemapRoot = sitemapFile.getResourceResolver().getResource(sitemapRootPath);
+
+        if (sitemapRoot == null || !SitemapUtil.isTopLevelSitemapRoot(sitemapRoot)) {
+            LOG.debug("Sitemap file's top level sitemap root does not exist: {}", sitemapRootPath);
+            return false;
+        }
+
+        String name = sitemapFile.getName();
+        int lastDot = name.lastIndexOf('.');
+
+        if (lastDot < 0) {
+            LOG.debug("Unexpected name, missing extension: {}", name);
+            return false;
+        }
+
+        Map<Resource, String> candidates = SitemapUtil.resolveSitemapRoots(sitemapRoot, name.substring(0, lastDot));
+        // check if for any of the candidate resource roots a generator with the name exists
+        return candidates.entrySet().stream()
+                .map(entry -> generatorManager.getApplicableNames(entry.getKey(), Collections.singleton(entry.getValue())))
+                .anyMatch(names -> names.size() > 0);
+    }
+
+    /**
+     * Returns true when the state expired according to the configured maximum age.
+     *
+     * @param state
+     * @return
+     */
+    private boolean isExpired(@NotNull Resource state) {
+        ValueMap stateProperties = state.getValueMap();
+        Calendar lastModified = stateProperties.get(JcrConstants.JCR_LASTMODIFIED, Calendar.class);
+        if (lastModified != null) {
+            // advance lastModified by maxStateAge to get the point in time the state would expire
+            lastModified.add(Calendar.MILLISECOND, maxStateAge);
+            // check if the expire time is in the future
+            if (lastModified.after(Calendar.getInstance())) {
+                return false;
+            } else if (LOG.isDebugEnabled()) {
+                LOG.debug("State at {} expired at {}", state.getPath(), lastModified.getTime().toGMTString());
+            }
+        }
+        return true;
+    }
+
     @NotNull
     private String getSitemapFilePath(@NotNull Resource sitemapRoot, @NotNull String name) {
         Resource topLevelSitemapRoot = getTopLevelSitemapRoot(sitemapRoot);
@@ -315,4 +390,15 @@ public class SitemapStorage {
         }
         return folder;
     }
+
+    private static Stream<Resource> traverse(@Nullable Resource resource) {
+        if (resource == null) {
+            return Stream.empty();
+        }
+
+        return Stream.concat(
+                Stream.of(resource),
+                StreamSupport.stream(resource.getChildren().spliterator(), false).flatMap(SitemapStorage::traverse)
+        );
+    }
 }
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
index 226fb3d..d000c35 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
@@ -88,8 +88,8 @@ public class SitemapGeneratorExecutorTest {
         context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
         context.registerService(SitemapGenerator.class, generator);
         context.registerService(JobManager.class, jobManager);
-        context.registerInjectActivateService(storage);
         context.registerInjectActivateService(generatorManager);
+        context.registerInjectActivateService(storage);
         context.registerInjectActivateService(extensionProviderManager);
         context.registerInjectActivateService(sitemapService);
 
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
index 02b4172..bc678ae 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
@@ -230,9 +230,9 @@ public class SitemapServletTest {
         byte[] expectedOutcomeBytes = expectedOutcome.getBytes(StandardCharsets.UTF_8);
         context.registerInjectActivateService(sitemapService, "onDemandNames", "news");
         context.registerInjectActivateService(subject);
-        storage.writeSitemap(root, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(expectedOutcomeBytes),
+        storage.writeSitemap(root, "foo", new ByteArrayInputStream(expectedOutcomeBytes),
                 expectedOutcomeBytes.length, 1);
-        MockSlingHttpServletRequest request = newSitemapReq("sitemap", root);
+        MockSlingHttpServletRequest request = newSitemapReq("foo-sitemap", root);
         MockSlingHttpServletResponse response = context.response();
 
         // when
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapStorageTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapStorageTest.java
index 55991b2..3603a69 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapStorageTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapStorageTest.java
@@ -20,6 +20,7 @@ package org.apache.sling.sitemap.impl;
 
 import com.google.common.collect.ImmutableMap;
 import org.apache.commons.io.IOUtils;
+import org.apache.sling.api.resource.ModifiableValueMap;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ValueMap;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
@@ -39,12 +40,15 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringWriter;
 import java.nio.charset.StandardCharsets;
+import java.util.Collections;
 import java.util.Set;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.hasItems;
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.when;
 
 @ExtendWith({SlingContextExtension.class, MockitoExtension.class})
 public class SitemapStorageTest {
@@ -52,16 +56,23 @@ public class SitemapStorageTest {
     public final SlingContext context = new SlingContext();
 
     private final SitemapStorage subject = new SitemapStorage();
+    private final SitemapGeneratorManager generatorManager = new SitemapGeneratorManager();
 
+    @Mock(lenient = true)
+    private SitemapGenerator generator;
     @Mock
     private ServiceUserMapped serviceUser;
 
     @BeforeEach
     public void setup() {
+        context.registerService(SitemapGenerator.class, generator);
         context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
+        context.registerInjectActivateService(generatorManager);
         context.registerInjectActivateService(subject,
                 "stateMaxAge", 100
         );
+
+        when(generator.getNames(any())).thenReturn(Collections.singleton(SitemapGenerator.DEFAULT_SITEMAP));
     }
 
     @Test
@@ -133,6 +144,78 @@ public class SitemapStorageTest {
         assertThat(sitemapNames, hasItems(storageInfo("foobar-sitemap"), storageInfo("sitemap")));
     }
 
+    @Test
+    public void testCleanupExpiredStates() throws Exception {
+        // given
+        Resource root = context.create().resource("/content/site/de", ImmutableMap.of(
+                "sitemapRoot", Boolean.TRUE
+        ));
+
+        // when
+        subject.writeState(root, SitemapGenerator.DEFAULT_SITEMAP, ImmutableMap.of("i", 1));
+
+        // then
+        assertNotNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/sitemap.part"));
+
+        // and when
+        Thread.sleep(100);
+        subject.run();
+
+        // then
+        assertNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/sitemap.part"));
+    }
+
+    @Test
+    public void testCleanupObsoleteSitemapsAfterTopLevelChanged() throws Exception {
+        // given
+        Resource newRoot = context.create().resource("/content/site/ch");
+        Resource initialRoot = context.create().resource("/content/site/ch/de-ch", ImmutableMap.of(
+                "sitemapRoot", Boolean.TRUE
+        ));
+
+        // when
+        subject.writeSitemap(initialRoot, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 0, 0);
+
+        // then
+        assertNotNull(context.resourceResolver().getResource("/var/sitemaps/content/site/ch/de-ch/sitemap.xml"));
+
+        // and when
+        newRoot.adaptTo(ModifiableValueMap.class).put("sitemapRoot", Boolean.TRUE);
+        context.resourceResolver().commit();
+        subject.run();
+
+        // then
+        assertNull(context.resourceResolver().getResource("/var/sitemaps/content/site/ch/de-ch/sitemap.xml"));
+    }
+
+    @Test
+    public void testCleanupObsoleteSitemapsAfterNestedSitemapRootChanged() throws Exception {
+        // given
+        Resource root = context.create().resource("/content/site/de", ImmutableMap.of(
+                "sitemapRoot", Boolean.TRUE
+        ));
+        Resource news = context.create().resource("/content/site/de/news", ImmutableMap.of(
+                "sitemapRoot", Boolean.TRUE
+        ));
+
+        // when
+        subject.writeSitemap(root, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 0, 0);
+        subject.writeSitemap(news, SitemapGenerator.DEFAULT_SITEMAP, new ByteArrayInputStream(new byte[0]), 0, 0);
+
+        // then
+        assertNotNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/sitemap.xml"));
+        assertNotNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/news-sitemap.xml"));
+
+        // and when
+        news.adaptTo(ModifiableValueMap.class).put("sitemapRoot", Boolean.FALSE);
+        context.resourceResolver().commit();
+        subject.run();
+
+        // then
+        assertNotNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/sitemap.xml"));
+        assertNull(context.resourceResolver().getResource("/var/sitemaps/content/site/de/news-sitemap.xml"));
+    }
+
     static void assertResourceDataEquals(String expectedValue, Resource resource) throws IOException {
         assertNotNull(resource);
         InputStream inputStream = resource.adaptTo(InputStream.class);

[sling-whiteboard] 04/05: add sling sitemap invetory plugin

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 528118cdfda86493addb4e19a28d3e1e53a8047a
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jun 7 14:44:31 2021 +0200

    add sling sitemap invetory plugin
---
 sitemap/pom.xml                                    |   6 +
 .../java/org/apache/sling/sitemap/SitemapInfo.java |  10 +
 .../sling/sitemap/impl/SitemapServiceImpl.java     |  33 ++--
 .../apache/sling/sitemap/impl/SitemapServlet.java  |  12 +-
 .../apache/sling/sitemap/impl/SitemapStorage.java  |  16 +-
 .../org/apache/sling/sitemap/impl/SitemapUtil.java |  23 ++-
 .../impl/console/SitemapInventoryPlugin.java       | 205 +++++++++++++++++++++
 7 files changed, 275 insertions(+), 30 deletions(-)

diff --git a/sitemap/pom.xml b/sitemap/pom.xml
index 711bb03..0302154 100644
--- a/sitemap/pom.xml
+++ b/sitemap/pom.xml
@@ -166,6 +166,12 @@
             <scope>provided</scope>
         </dependency>
         <dependency>
+            <groupId>org.apache.felix</groupId>
+            <artifactId>org.apache.felix.inventory</artifactId>
+            <version>1.0.6</version>
+            <scope>provided</scope>
+        </dependency>
+        <dependency>
             <groupId>org.slf4j</groupId>
             <artifactId>slf4j-api</artifactId>
             <scope>provided</scope>
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 da7caaa..6579b23 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapInfo.java
@@ -19,6 +19,7 @@
 package org.apache.sling.sitemap;
 
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.osgi.annotation.versioning.ProviderType;
 
 /**
@@ -28,6 +29,15 @@ import org.osgi.annotation.versioning.ProviderType;
 public interface SitemapInfo {
 
     /**
+     * Returns a resource path to the node the sitemap is stored. May return null if the sitemap or sitemap-index is
+     * served on-demand.
+     *
+     * @return
+     */
+    @Nullable
+    String getStoragePath();
+
+    /**
      * Returns the absolute, external url for the sitemap/sitemap-index.
      *
      * @return
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 22e6111..aa3703c 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
@@ -25,6 +25,7 @@ import org.apache.sling.sitemap.SitemapInfo;
 import org.apache.sling.sitemap.SitemapService;
 import org.apache.sling.sitemap.common.SitemapLinkExternalizer;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.osgi.service.component.annotations.*;
 import org.osgi.service.metatype.annotations.AttributeDefinition;
 import org.osgi.service.metatype.annotations.Designate;
@@ -58,7 +59,7 @@ public class SitemapServiceImpl implements SitemapService {
     private static final Logger LOG = LoggerFactory.getLogger(SitemapServiceImpl.class);
 
     @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
-    private SitemapLinkExternalizer externalizer = SitemapLinkExternalizer.DEFAULT;
+    private SitemapLinkExternalizer externalizer;
     @Reference
     private JobManager jobManager;
     @Reference
@@ -99,7 +100,7 @@ public class SitemapServiceImpl implements SitemapService {
             return getSitemapUrlsForNestedSitemapRoot(sitemapRoot);
         }
 
-        String url = externalizer.externalize(sitemapRoot);
+        String url = externalize(sitemapRoot);
         Collection<String> names = generatorManager.getGenerators(sitemapRoot).keySet();
 
         if (url == null) {
@@ -121,7 +122,7 @@ public class SitemapServiceImpl implements SitemapService {
             } else {
                 location += storageInfo.getSitemapSelector() + '.' + SitemapServlet.SITEMAP_EXTENSION;
             }
-            infos.add(newSitemapInfo(location, storageInfo.getSize(), storageInfo.getEntries()));
+            infos.add(newSitemapInfo(storageInfo.getPath(), location, storageInfo.getSize(), storageInfo.getEntries()));
         }
 
         return infos;
@@ -172,7 +173,7 @@ public class SitemapServiceImpl implements SitemapService {
     private Collection<SitemapInfo> getSitemapUrlsForNestedSitemapRoot(Resource sitemapRoot) {
         Collection<String> names = generatorManager.getGenerators(sitemapRoot).keySet();
         Resource topLevelSitemapRoot = getTopLevelSitemapRoot(sitemapRoot);
-        String topLevelSitemapRootUrl = externalizer.externalize(topLevelSitemapRoot);
+        String topLevelSitemapRootUrl = externalize(topLevelSitemapRoot);
 
         if (topLevelSitemapRootUrl == null || names.isEmpty()) {
             LOG.debug("Could not create absolute urls for nested sitemaps at: {}", sitemapRoot.getPath());
@@ -187,7 +188,7 @@ public class SitemapServiceImpl implements SitemapService {
             String selector = getSitemapSelector(sitemapRoot, topLevelSitemapRoot, name);
             String location = topLevelSitemapRootUrl + selector + '.' + SitemapServlet.SITEMAP_EXTENSION;
             if (onDemandNames.contains(name)) {
-                infos.add(newSitemapInfo(location, -1, -1));
+                infos.add(newSitemapInfo(null, location, -1, -1));
             } else {
                 if (storageInfos == null) {
                     storageInfos = storage.getSitemaps(sitemapRoot, names);
@@ -197,7 +198,7 @@ public class SitemapServiceImpl implements SitemapService {
                         .findFirst();
                 if (storageInfoOpt.isPresent()) {
                     SitemapStorageInfo storageInfo = storageInfoOpt.get();
-                    infos.add(newSitemapInfo(location, storageInfo.getSize(), storageInfo.getEntries()));
+                    infos.add(newSitemapInfo(storageInfo.getPath(), location, storageInfo.getSize(), storageInfo.getEntries()));
                 }
             }
         }
@@ -205,25 +206,29 @@ public class SitemapServiceImpl implements SitemapService {
         return infos;
     }
 
-
+    private String externalize(Resource resource) {
+        return (externalizer == null ? SitemapLinkExternalizer.DEFAULT : externalizer).externalize(resource);
+    }
 
     private SitemapInfo newSitemapIndexInfo(@NotNull String url) {
-        return new SitemapInfoImpl(url, -1, -1, true, true);
+        return new SitemapInfoImpl(null, url, -1, -1, true, true);
     }
 
-    private SitemapInfo newSitemapInfo(@NotNull String url, int size, int entries) {
-        return new SitemapInfoImpl(url, size, entries, false, isWithinLimits(size, entries));
+    private SitemapInfo newSitemapInfo(@Nullable String path, @NotNull String url, int size, int entries) {
+        return new SitemapInfoImpl(path, url, size, entries, false, isWithinLimits(size, entries));
     }
 
     private static class SitemapInfoImpl implements SitemapInfo {
 
         private final String url;
+        private final String path;
         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 isIndex, boolean withinLimits) {
+        private SitemapInfoImpl(@Nullable String path, @NotNull String url, int size, int entries, boolean isIndex, boolean withinLimits) {
+            this.path = path;
             this.url = url;
             this.size = size;
             this.entries = entries;
@@ -231,6 +236,12 @@ public class SitemapServiceImpl implements SitemapService {
             this.withinLimits = withinLimits;
         }
 
+        @Nullable
+        @Override
+        public String getStoragePath() {
+            return path;
+        }
+
         @NotNull
         @Override
         public String getUrl() {
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
index a254a10..a7828b1 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
@@ -79,7 +79,7 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
     };
 
     @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
-    private SitemapLinkExternalizer externalizer = SitemapLinkExternalizer.DEFAULT;
+    private SitemapLinkExternalizer externalizer;
     @Reference
     private SitemapGeneratorManager generatorManager;
     @Reference
@@ -134,8 +134,8 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
         // add any sitemap from the storage
         for (SitemapStorageInfo storageInfo : storage.getSitemaps(topLevelSitemapRoot)) {
             if (!addedSitemapSelectors.contains(storageInfo.getSitemapSelector())) {
-                String location = externalizer.externalize(request, getSitemapLink(topLevelSitemapRoot,
-                        storageInfo.getSitemapSelector()));
+                String location = externalize(request,
+                        getSitemapLink(topLevelSitemapRoot, storageInfo.getSitemapSelector()));
                 Calendar lastModified = storageInfo.getLastModified();
                 if (location != null && lastModified != null) {
                     sitemapIndex.addSitemap(location, lastModified.toInstant());
@@ -209,7 +209,7 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
             // 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, sitemapRoot, applicableName);
-                String location = externalizer.externalize(request, getSitemapLink(sitemapRoot, sitemapSelector));
+                String location = externalize(request, getSitemapLink(sitemapRoot, sitemapSelector));
                 if (location != null) {
                     index.addSitemap(location);
                     addedSitemapSelectors.add(sitemapSelector);
@@ -222,6 +222,10 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
         return addedSitemapSelectors;
     }
 
+    private String externalize(SlingHttpServletRequest request, String uri) {
+        return (externalizer == null ? SitemapLinkExternalizer.DEFAULT : externalizer).externalize(request, uri);
+    }
+
     private static String getSitemapLink(Resource sitemapRoot, String sitemapSelector) {
         String link = sitemapRoot.getPath() + '.' + SITEMAP_SELECTOR + '.';
         if (SITEMAP_SELECTOR.equals(sitemapSelector)) {
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 4ffa9c4..e6f41ea 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
@@ -51,7 +51,7 @@ import static org.apache.sling.sitemap.impl.SitemapUtil.*;
         service = {SitemapStorage.class, Runnable.class},
         property = {
                 Scheduler.PROPERTY_SCHEDULER_NAME + "=sitemap-storage-cleanup",
-                Scheduler.PROPERTY_SCHEDULER_CONCURRENT + "=false",
+                Scheduler.PROPERTY_SCHEDULER_CONCURRENT + ":Boolean=false",
                 Scheduler.PROPERTY_SCHEDULER_RUN_ON + "=" + Scheduler.VALUE_RUN_ON_SINGLE
         }
 )
@@ -125,7 +125,7 @@ public class SitemapStorage implements Runnable {
     }
 
     @NotNull
-    ValueMap getState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
+    public ValueMap getState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
         String statePath = getSitemapFilePath(sitemapRoot, name) + STATE_EXTENSION;
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
             Resource state = resolver.getResource(statePath);
@@ -142,7 +142,7 @@ public class SitemapStorage implements Runnable {
         }
     }
 
-    void writeState(@NotNull Resource sitemapRoot, @NotNull String name, @NotNull Map<String, Object> state)
+    public void writeState(@NotNull Resource sitemapRoot, @NotNull String name, @NotNull Map<String, Object> state)
             throws IOException {
         String statePath = getSitemapFilePath(sitemapRoot, name) + STATE_EXTENSION;
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
@@ -170,7 +170,7 @@ public class SitemapStorage implements Runnable {
         }
     }
 
-    void removeState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
+    public void removeState(@NotNull Resource sitemapRoot, @NotNull String name) throws IOException {
         String statePath = getSitemapFilePath(sitemapRoot, name) + STATE_EXTENSION;
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
             Resource stateResource = resolver.getResource(statePath);
@@ -183,7 +183,7 @@ public class SitemapStorage implements Runnable {
         }
     }
 
-    String writeSitemap(@NotNull Resource sitemapRoot, @NotNull String name, @NotNull InputStream data, int size,
+    public String writeSitemap(@NotNull Resource sitemapRoot, @NotNull String name, @NotNull InputStream data, int size,
                         int entries) throws IOException {
         String sitemapFilePath = getSitemapFilePath(sitemapRoot, name);
         String statePath = sitemapFilePath + STATE_EXTENSION;
@@ -227,7 +227,7 @@ public class SitemapStorage implements Runnable {
         return sitemapFilePath;
     }
 
-    Set<SitemapStorageInfo> getSitemaps(Resource sitemapRoot) {
+    public Set<SitemapStorageInfo> getSitemaps(Resource sitemapRoot) {
         return getSitemaps(sitemapRoot, Collections.emptySet());
     }
 
@@ -242,7 +242,7 @@ public class SitemapStorage implements Runnable {
      * @param names
      * @return
      */
-    Set<SitemapStorageInfo> getSitemaps(Resource sitemapRoot, Collection<String> names) {
+    public Set<SitemapStorageInfo> getSitemaps(Resource sitemapRoot, Collection<String> names) {
         Resource topLevelSitemapRoot = getTopLevelSitemapRoot(sitemapRoot);
         Predicate<SitemapStorageInfo> filter;
 
@@ -279,7 +279,7 @@ public class SitemapStorage implements Runnable {
         }
     }
 
-    boolean copySitemap(Resource sitemapRoot, String sitemapSelector, OutputStream output) throws IOException {
+    public boolean copySitemap(Resource sitemapRoot, String sitemapSelector, OutputStream output) throws IOException {
         if (!isTopLevelSitemapRoot(sitemapRoot)) {
             return false;
         }
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapUtil.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapUtil.java
index e5607bc..7ee438e 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapUtil.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapUtil.java
@@ -19,10 +19,11 @@
 package org.apache.sling.sitemap.impl;
 
 import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.util.ISO9075;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.sitemap.generator.SitemapGenerator;
 import org.apache.sling.sitemap.SitemapService;
+import org.apache.sling.sitemap.generator.SitemapGenerator;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -31,6 +32,8 @@ import java.util.*;
 
 public class SitemapUtil {
 
+    private static final String JCR_SYSTEM_PATH = "/" + JcrConstants.JCR_SYSTEM + "/";
+
     private SitemapUtil() {
         super();
     }
@@ -151,12 +154,16 @@ public class SitemapUtil {
      */
     @NotNull
     public static Iterator<Resource> findSitemapRoots(ResourceResolver resolver, String searchPath) {
-        return new Iterator<Resource>() {
+        String correctedSearchPath = searchPath == null ? "/" : searchPath;
+        StringBuilder query = new StringBuilder(correctedSearchPath.length() + 35);
+        query.append("/jcr:root").append(ISO9075.encodePath(correctedSearchPath));
+        if (!correctedSearchPath.endsWith("/")) {
+            query.append('/');
+        }
+        query.append("/*[@").append(SitemapService.PROPERTY_SITEMAP_ROOT).append('=').append(Boolean.TRUE).append(']');
 
-            private final Iterator<Resource> hits = resolver.findResources(
-                    "/jcr:root" + searchPath + "//*[@" + SitemapService.PROPERTY_SITEMAP_ROOT + "=true]",
-                    Query.XPATH
-            );
+        return new Iterator<Resource>() {
+            private final Iterator<Resource> hits = resolver.findResources(query.toString(), Query.XPATH);
             private Resource next = seek();
 
             private Resource seek() {
@@ -165,7 +172,9 @@ public class SitemapUtil {
                     // skip a hit on the given searchPath itself. This may be when a search is done for descendant
                     // sitemaps given the normalized sitemap root path and the sitemap root's jcr:content is in the
                     // result set.
-                    if (nextHit == null || nextHit.getPath().equals(searchPath)) {
+                    if (nextHit == null
+                            || nextHit.getPath().equals(correctedSearchPath)
+                            || nextHit.getPath().startsWith(JCR_SYSTEM_PATH)) {
                         continue;
                     }
                     return nextHit;
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/console/SitemapInventoryPlugin.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/console/SitemapInventoryPlugin.java
new file mode 100644
index 0000000..8feb7bd
--- /dev/null
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/console/SitemapInventoryPlugin.java
@@ -0,0 +1,205 @@
+/*
+ * 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.console;
+
+import org.apache.felix.inventory.Format;
+import org.apache.felix.inventory.InventoryPrinter;
+import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceResolverFactory;
+import org.apache.sling.commons.scheduler.Scheduler;
+import org.apache.sling.sitemap.SitemapInfo;
+import org.apache.sling.sitemap.SitemapService;
+import org.apache.sling.sitemap.impl.SitemapUtil;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceReference;
+import org.osgi.service.component.annotations.Activate;
+import org.osgi.service.component.annotations.Component;
+import org.osgi.service.component.annotations.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.PrintWriter;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.Map;
+
+@Component(
+        service = InventoryPrinter.class,
+        property = {
+                InventoryPrinter.NAME + "=slingsitemap",
+                InventoryPrinter.TITLE + "=Sling Sitemap",
+                InventoryPrinter.FORMAT + "=JSON",
+                InventoryPrinter.FORMAT + "=TEXT",
+                InventoryPrinter.WEBCONSOLE + "=true"
+
+        }
+)
+public class SitemapInventoryPlugin implements InventoryPrinter {
+
+    private static final Map<String, Object> AUTH = Collections.singletonMap(
+            ResourceResolverFactory.SUBSERVICE, "sitemap-reader");
+    private static final Logger LOG = LoggerFactory.getLogger(SitemapInventoryPlugin.class);
+
+    @Reference
+    private SitemapService sitemapService;
+    @Reference
+    private ResourceResolverFactory resourceResolverFactory;
+
+    private BundleContext bundleContext;
+
+    @Activate
+    protected void activate(BundleContext bundleContext) {
+        this.bundleContext = bundleContext;
+    }
+
+    @Override
+    public void print(PrintWriter printWriter, Format format, boolean isZip) {
+        if (Format.JSON.equals(format)) {
+            printJson(printWriter);
+        } else if (Format.TEXT.equals(format)) {
+            printText(printWriter);
+        }
+    }
+
+    private void printJson(PrintWriter pw) {
+        pw.print('{');
+        pw.print("\"schedulers\":[");
+        boolean hasScheduler = false;
+        for (ServiceReference<?> ref : bundleContext.getBundle().getRegisteredServices()) {
+            Object schedulerExp = ref.getProperty(Scheduler.PROPERTY_SCHEDULER_EXPRESSION);
+            Object schedulerName = ref.getProperty(Scheduler.PROPERTY_SCHEDULER_NAME);
+            if (schedulerExp instanceof String && schedulerName instanceof String) {
+                if (hasScheduler) {
+                    pw.print(',');
+                }
+                hasScheduler = true;
+                pw.print("{\"name\":\"");
+                pw.print(escapeDoubleQuotes((String) schedulerName));
+                pw.print("\",\"expression\":\"");
+                pw.print(escapeDoubleQuotes((String) schedulerExp));
+                pw.print("\"}");
+            }
+        }
+        pw.print("],");
+
+        pw.print("\"roots\":{");
+        try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
+            Iterator<Resource> roots = SitemapUtil.findSitemapRoots(resolver, "/");
+            while (roots.hasNext()) {
+                Resource root = roots.next();
+                pw.print('"');
+                pw.print(escapeDoubleQuotes(root.getPath()));
+                pw.print("\":[");
+                Iterator<SitemapInfo> infoIt = sitemapService.getSitemapInfo(root).iterator();
+                while (infoIt.hasNext()) {
+                    SitemapInfo info = infoIt.next();
+                    pw.print('{');
+                    pw.print("\"url\":\"");
+                    pw.print(escapeDoubleQuotes(info.getUrl()));
+                    pw.print('"');
+                    if (info.getStoragePath() != null) {
+                        pw.print(",\"path\":\"");
+                        pw.print(escapeDoubleQuotes(info.getStoragePath()));
+                        pw.print("\",\"size\":");
+                        pw.print(info.getSize());
+                        pw.print(",\"entries\":");
+                        pw.print(info.getEntries());
+                        pw.print(",\"inLimits\":");
+                        pw.print(info.isWithinLimits());
+                    }
+                    pw.print('}');
+                    if (infoIt.hasNext()) {
+                        pw.print(',');
+                    }
+                }
+                pw.print(']');
+                if (roots.hasNext()) {
+                    pw.print(',');
+                }
+            }
+        } catch (LoginException ex) {
+            pw.println("Failed to list sitemaps: " + ex.getMessage());
+            LOG.warn("Failed to get inventory of sitemaps: {}", ex.getMessage(), ex);
+        }
+        pw.print('}');
+        pw.print('}');
+    }
+
+    private void printText(PrintWriter pw) {
+        pw.println("Apache Sling Sitemap Schedulers");
+        pw.println("-------------------------------");
+
+        for (ServiceReference<?> ref : bundleContext.getBundle().getRegisteredServices()) {
+            Object schedulerExp = ref.getProperty(Scheduler.PROPERTY_SCHEDULER_EXPRESSION);
+            Object schedulerName = ref.getProperty(Scheduler.PROPERTY_SCHEDULER_NAME);
+            if (schedulerExp != null && schedulerName != null) {
+                pw.print(" - Name: ");
+                pw.print(schedulerName);
+                pw.println();
+                pw.print("   Expression: ");
+                pw.print(schedulerExp);
+                pw.println();
+            }
+        }
+
+        pw.println();
+        pw.println();
+        pw.println("Apache Sling Sitemap Roots");
+        pw.println("--------------------------");
+
+        try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
+            Iterator<Resource> roots = SitemapUtil.findSitemapRoots(resolver, "/");
+            while (roots.hasNext()) {
+                Resource root = roots.next();
+                pw.print(root.getPath());
+                pw.print(':');
+                pw.println();
+                for (SitemapInfo info : sitemapService.getSitemapInfo(root)) {
+                    pw.print(" - Url: ");
+                    pw.print(info.getUrl());
+                    pw.println();
+                    if (info.getStoragePath() != null) {
+                        pw.print("   Path: ");
+                        pw.print(info.getStoragePath());
+                        pw.println();
+                        pw.print("   Bytes: ");
+                        pw.print(info.getSize());
+                        pw.println();
+                        pw.print("   Urls: ");
+                        pw.print(info.getEntries());
+                        pw.println();
+                        pw.print("   Within Limits: ");
+                        pw.print(info.isWithinLimits() ? "yes": "no");
+                        pw.println();
+                    }
+                }
+            }
+        } catch (LoginException ex) {
+            pw.println("Failed to list sitemaps: " + ex.getMessage());
+            LOG.warn("Failed to get inventory of sitemaps: {}", ex.getMessage(), ex);
+        }
+    }
+
+    private static String escapeDoubleQuotes(String text) {
+        return text.replace("\"", "\\\"");
+    }
+}

[sling-whiteboard] 05/05: add API to schedule sitemap regeneration

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 184243311bb2b02021ce21e5adc88402266c01c6
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jun 7 15:32:34 2021 +0200

    add API to schedule sitemap regeneration
---
 sitemap/pom.xml                                    |   9 +-
 .../org/apache/sling/sitemap/SitemapService.java   |  29 +++++
 .../sling/sitemap/impl/SitemapScheduler.java       |  46 +++++--
 .../sling/sitemap/impl/SitemapServiceImpl.java     |  73 ++++++++++-
 .../impl/SitemapServiceImplSchedulingTest.java     | 135 +++++++++++++++++++++
 5 files changed, 273 insertions(+), 19 deletions(-)

diff --git a/sitemap/pom.xml b/sitemap/pom.xml
index 0302154..cea534d 100644
--- a/sitemap/pom.xml
+++ b/sitemap/pom.xml
@@ -137,6 +137,10 @@
             <artifactId>org.osgi.service.event</artifactId>
         </dependency>
         <dependency>
+            <groupId>org.osgi</groupId>
+            <artifactId>org.osgi.util.tracker</artifactId>
+        </dependency>
+        <dependency>
             <groupId>org.apache.jackrabbit</groupId>
             <artifactId>jackrabbit-jcr-commons</artifactId>
             <version>2.21.3</version>
@@ -221,11 +225,6 @@
         </dependency>
         <dependency>
             <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.util.tracker</artifactId>
-            <scope>test</scope>
-        </dependency>
-        <dependency>
-            <groupId>org.osgi</groupId>
             <artifactId>org.osgi.resource</artifactId>
             <scope>test</scope>
         </dependency>
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 190db7f..a5aa2b9 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
@@ -34,6 +34,35 @@ public interface SitemapService {
     String PROPERTY_SITEMAP_ROOT = "sitemapRoot";
 
     /**
+     * Calls all registered SitemapSchedulers to schedule (re)generation for all sitemap roots and names.
+     */
+    void scheduleGeneration();
+
+    /**
+     * Calls all registered SitemapSchedulers registered for the given name to schedule (re)generation.
+     *
+     * @param name
+     */
+    void scheduleGeneration(String name);
+
+    /**
+     * Calls all registered SitemapSchedulers with a search path containing the given resource to schedule
+     * (re)generation for all names.
+     *
+     * @param sitemapRoot
+     */
+    void scheduleGeneration(Resource sitemapRoot);
+
+    /**
+     * Calls all registered SitemapSchedulers with a search path containing the given resource and being registered for
+     * the given name to schedule (re)generation.
+     *
+     * @param sitemapRoot
+     * @param name
+     */
+    void scheduleGeneration(Resource sitemapRoot, String name);
+
+    /**
      * Returns the urls to the given {@link Resource}'s sitemaps, if any.
      * <p>
      * The returned urls may contain a sitemap index when there are multiple sitemaps generated for the given sitemap
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapScheduler.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapScheduler.java
index 078e938..0ce4615 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapScheduler.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapScheduler.java
@@ -42,10 +42,10 @@ import java.util.*;
 import static org.apache.sling.sitemap.impl.SitemapUtil.findSitemapRoots;
 
 @Component(
-        service = Runnable.class,
+        service = {SitemapScheduler.class, Runnable.class},
         configurationPolicy = ConfigurationPolicy.REQUIRE,
         property = {
-                Scheduler.PROPERTY_SCHEDULER_CONCURRENT + "=false",
+                Scheduler.PROPERTY_SCHEDULER_CONCURRENT + ":Boolean=false",
                 Scheduler.PROPERTY_SCHEDULER_RUN_ON + "=" + Scheduler.VALUE_RUN_ON_SINGLE
         }
 )
@@ -94,25 +94,45 @@ public class SitemapScheduler implements Runnable {
 
     @Override
     public void run() {
+        run(Arrays.asList(names));
+    }
+
+    public void run(String name) {
+        run(Collections.singleton(name));
+    }
+
+    public void run(Resource sitemapRoot) {
+        run(sitemapRoot, Arrays.asList(names));
+    }
+
+    public void run(Resource sitemapRoot, Collection<String> names) {
+        Set<String> applicableNames = generatorManager.getApplicableNames(sitemapRoot, names);
+        for (String applicableName : applicableNames) {
+            Map<String, Object> jobProperties = new HashMap<>();
+            jobProperties.put(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_NAME, applicableName);
+            jobProperties.put(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_ROOT, sitemapRoot.getPath());
+            Job job = jobManager.addJob(SitemapGeneratorExecutor.JOB_TOPIC, jobProperties);
+            LOG.debug("Added job {}", job.getId());
+        }
+    }
+
+    void run(ResourceResolver resolver) {
+        run(resolver, Arrays.asList(names));
+    }
+
+    private void run(Collection<String> names) {
         try (ResourceResolver resolver = resourceResolverFactory.getServiceResourceResolver(AUTH)) {
-            run(resolver);
+            run(resolver, names);
         } catch (LoginException ex) {
             LOG.warn("Failed start sitemap jobs: {}", ex.getMessage(), ex);
         }
     }
 
-    public void run(ResourceResolver resolver) {
+    private void run(ResourceResolver resolver, Collection<String> names) {
         Iterator<Resource> sitemapRoots = findSitemapRoots(resolver, searchPath);
         while (sitemapRoots.hasNext()) {
-            Resource sitemapRoot = sitemapRoots.next();
-            Set<String> applicableNames = generatorManager.getApplicableNames(sitemapRoot, Arrays.asList(names));
-            for (String applicableName : applicableNames) {
-                Map<String, Object> jobProperties = new HashMap<>();
-                jobProperties.put(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_NAME, applicableName);
-                jobProperties.put(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_ROOT, sitemapRoot.getPath());
-                Job job = jobManager.addJob(SitemapGeneratorExecutor.JOB_TOPIC, jobProperties);
-                LOG.debug("Added job {}", job.getId());
-            }
+            run(sitemapRoots.next(), names);
         }
     }
+
 }
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 aa3703c..b8925e9 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
@@ -26,10 +26,12 @@ import org.apache.sling.sitemap.SitemapService;
 import org.apache.sling.sitemap.common.SitemapLinkExternalizer;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.osgi.framework.*;
 import org.osgi.service.component.annotations.*;
 import org.osgi.service.metatype.annotations.AttributeDefinition;
 import org.osgi.service.metatype.annotations.Designate;
 import org.osgi.service.metatype.annotations.ObjectClassDefinition;
+import org.osgi.util.tracker.ServiceTracker;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -67,15 +69,23 @@ public class SitemapServiceImpl implements SitemapService {
     @Reference
     private SitemapStorage storage;
 
+    private ServiceTracker<SitemapScheduler, SitemapScheduler> schedulers;
     private Set<String> onDemandNames;
     private int maxSize;
     private int maxEntries;
 
     @Activate
-    protected void activate(Configuration configuration) {
+    protected void activate(Configuration configuration, BundleContext bundleContext) {
         onDemandNames = Arrays.stream(configuration.onDemandNames()).collect(Collectors.toSet());
         maxSize = configuration.maxSize();
         maxEntries = configuration.maxEntries();
+        schedulers = new ServiceTracker<>(bundleContext, SitemapScheduler.class, null);
+        schedulers.open();
+    }
+
+    @Deactivate
+    protected void deactivate() {
+        schedulers.close();
     }
 
     public Set<String> getSitemapNamesServedOnDemand() {
@@ -86,6 +96,67 @@ public class SitemapServiceImpl implements SitemapService {
         return size <= maxSize && entries <= maxEntries;
     }
 
+    @Override
+    public void scheduleGeneration() {
+        if (schedulers.getServiceReferences() == null) {
+            return;
+        }
+        for (ServiceReference<SitemapScheduler> scheduler : schedulers.getServiceReferences()) {
+            schedulers.getService(scheduler).run();
+        }
+    }
+
+    @Override
+    public void scheduleGeneration(String name) {
+        if (schedulers.getServiceReferences() == null) {
+            return;
+        }
+        try {
+            Filter filter = FrameworkUtil.createFilter("(names=" + name + ")");
+            for (ServiceReference<SitemapScheduler> scheduler : schedulers.getServiceReferences()) {
+                if (filter.match(scheduler)) {
+                    schedulers.getService(scheduler).run(name);
+                }
+            }
+        } catch (InvalidSyntaxException ex) {
+            LOG.warn("Failed to build filter for given argument: {}", name, ex);
+        }
+    }
+
+    @Override
+    public void scheduleGeneration(Resource sitemapRoot) {
+        if (schedulers.getServiceReferences() == null || !SitemapUtil.isSitemapRoot(sitemapRoot)) {
+            return;
+        }
+        for (ServiceReference<SitemapScheduler> scheduler : schedulers.getServiceReferences()) {
+            Object searchPath = scheduler.getProperty("searchPath");
+            if (searchPath instanceof String && sitemapRoot.getPath().startsWith(searchPath + "/")) {
+                schedulers.getService(scheduler).run(sitemapRoot);
+            }
+        }
+    }
+
+    @Override
+    public void scheduleGeneration(Resource sitemapRoot, String name) {
+        if (schedulers.getServiceReferences() == null || !SitemapUtil.isSitemapRoot(sitemapRoot)) {
+            return;
+        }
+        try {
+            Filter filter = FrameworkUtil.createFilter("(names=" + name + ")");
+            for (ServiceReference<SitemapScheduler> scheduler : schedulers.getServiceReferences()) {
+                if (!filter.match(scheduler)) {
+                    continue;
+                }
+                Object searchPath = scheduler.getProperty("searchPath");
+                if (searchPath instanceof String && sitemapRoot.getPath().startsWith(searchPath + "/")) {
+                    schedulers.getService(scheduler).run(sitemapRoot, Collections.singleton(name));
+                }
+            }
+        } catch (InvalidSyntaxException ex) {
+            LOG.warn("Failed to build filter for given argument: {}", name, ex);
+        }
+    }
+
     @NotNull
     @Override
     public Collection<SitemapInfo> getSitemapInfo(@NotNull Resource resource) {
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplSchedulingTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplSchedulingTest.java
new file mode 100644
index 0000000..c495344
--- /dev/null
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServiceImplSchedulingTest.java
@@ -0,0 +1,135 @@
+/*
+ * 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;
+
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.event.jobs.JobManager;
+import org.apache.sling.serviceusermapping.ServiceUserMapped;
+import org.apache.sling.sitemap.generator.SitemapGenerator;
+import org.apache.sling.testing.mock.sling.junit5.SlingContext;
+import org.apache.sling.testing.mock.sling.junit5.SlingContextExtension;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import java.util.Collections;
+
+import static org.mockito.Mockito.*;
+
+@ExtendWith({SlingContextExtension.class, MockitoExtension.class})
+public class SitemapServiceImplSchedulingTest {
+
+    public final SlingContext context = new SlingContext();
+
+    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;
+
+    private Resource root1;
+    private Resource root2;
+    private SitemapScheduler scheduler1;
+    private SitemapScheduler scheduler2;
+
+    @BeforeEach
+    public void setup() {
+        root1 = context.create().resource("/content/site/de", Collections.singletonMap(
+                "sitemapRoot", Boolean.TRUE
+        ));
+        root2 = context.create().resource("/content/microsite/de", Collections.singletonMap(
+                "sitemapRoot", Boolean.TRUE
+        ));
+
+        context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
+        context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-reader");
+        context.registerService(SitemapGenerator.class, generator);
+        context.registerService(JobManager.class, jobManager);
+        context.registerInjectActivateService(generatorManager);
+        context.registerInjectActivateService(storage);
+        context.registerInjectActivateService(subject);
+
+        scheduler1 = context.registerInjectActivateService(spy(new SitemapScheduler()),
+                "names", "<default>",
+                "searchPath", "/content/site"
+        );
+        scheduler2 = context.registerInjectActivateService(spy(new SitemapScheduler()),
+                "names", new String[]{"<default>", "foo"},
+                "searchPath", "/content/microsite"
+        );
+    }
+
+    @Test
+    public void testAllSchedulersCalled() {
+        // when
+        subject.scheduleGeneration();
+
+        // then
+        verify(scheduler1, times(1)).run();
+        verify(scheduler2, times(1)).run();
+    }
+
+    @Test
+    public void testAllSchedulersCalledForName() {
+        // when
+        subject.scheduleGeneration("<default>");
+
+        // then
+        verify(scheduler1, times(1)).run("<default>");
+        verify(scheduler2, times(1)).run("<default>");
+    }
+
+    @Test
+    public void testSchedulersCalledForName() {
+        // when
+        subject.scheduleGeneration("foo");
+
+        // then
+        verify(scheduler1, never()).run();
+        verify(scheduler2, times(1)).run("foo");
+    }
+
+    @Test
+    public void testSchedulersCalledForPath() {
+        // when
+        subject.scheduleGeneration(root1);
+
+        // then
+        verify(scheduler1, times(1)).run(root1);
+        verify(scheduler2, never()).run();
+    }
+
+    @Test
+    public void testSchedulersCalledForPathAndName() {
+        // when
+        subject.scheduleGeneration(root1, "foo");
+        subject.scheduleGeneration(root2, "foo");
+
+        // then
+        verify(scheduler1, never()).run(eq(root1), argThat(collection -> collection.contains("foo")));
+        verify(scheduler2, times(1)).run(eq(root2), argThat(collection -> collection.contains("foo")));
+    }
+}

[sling-whiteboard] 01/05: Added flag to sitemap finished event

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 17e264b66ea919e26cd416f78b1691fa44772ad6
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jun 7 10:27:40 2021 +0200

    Added flag to sitemap finished event
    
    Added a boolean flag to the event fired when a sitemap finished generation in the
    background to indicate if it exceeds the configured limits
---
 sitemap/README.md                                                 | 1 +
 .../java/org/apache/sling/sitemap/generator/SitemapGenerator.java | 4 ++++
 .../org/apache/sling/sitemap/impl/SitemapGeneratorExecutor.java   | 5 +++++
 .../java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java    | 8 +++++++-
 .../apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java   | 6 ++++++
 5 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/sitemap/README.md b/sitemap/README.md
index 1bb35cd..2b48881 100644
--- a/sitemap/README.md
+++ b/sitemap/README.md
@@ -16,6 +16,7 @@ background to even sites that collect 3rd party data to include dynamically rend
 
 * Housekeeping of SitemapStorage (used for background generation), esp. when sitemap roots change
 * More general approach for creating absolute urls.
+* Implement Google specific sitemap extensions (image/video/news)
 
 ## Getting Started
 
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/generator/SitemapGenerator.java b/sitemap/src/main/java/org/apache/sling/sitemap/generator/SitemapGenerator.java
index 062cd6e..f6e099c 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/generator/SitemapGenerator.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/generator/SitemapGenerator.java
@@ -62,6 +62,10 @@ public interface SitemapGenerator {
      */
     String EVENT_PROPERTY_SITEMAP_URLS = "sitemap.urls";
     /**
+     * The event property storing a flag whether the generated sitemap exceeds the configured limits.
+     */
+    String EVENT_PROPERTY_SITEMAP_EXCEEDS_LIMITS = "sitemap.exceedsLimits";
+    /**
      * The event property storing the generated sitemap's storage path.
      */
     String EVENT_PROPERTY_SITEMAP_STORAGE_PATH = "sitemap.storagePath";
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutor.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutor.java
index b108e2c..2c31f74 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutor.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutor.java
@@ -93,6 +93,8 @@ public class SitemapGeneratorExecutor implements JobExecutor {
     private ServiceUserMapped serviceUserMapped;
     @Reference
     private EventAdmin eventAdmin;
+    @Reference
+    private SitemapServiceImpl sitemapService;
 
     private int chunkSize = 10;
 
@@ -173,6 +175,9 @@ public class SitemapGeneratorExecutor implements JobExecutor {
             eventProperties.put(SitemapGenerator.EVENT_PROPERTY_SITEMAP_URLS, sitemap.getUrlCount());
             eventProperties.put(SitemapGenerator.EVENT_PROPERTY_SITEMAP_STORAGE_PATH, storagePath);
             eventProperties.put(SitemapGenerator.EVENT_PROPERTY_SITEMAP_STORAGE_SIZE, buffer.size());
+            eventProperties.put(SitemapGenerator.EVENT_PROPERTY_SITEMAP_EXCEEDS_LIMITS,
+                    !sitemapService.isWithinLimits(buffer.size(), sitemap.getUrlCount()));
+            
             eventAdmin.sendEvent(new Event(SitemapGenerator.EVENT_TOPIC_SITEMAP_UPDATED, new EventProperties(eventProperties)));
         } catch (JobStoppedException ex) {
             LOG.debug("Job stopped, removing state", ex);
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 f77a56b..6cbee90 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
@@ -81,6 +81,10 @@ public class SitemapServiceImpl implements SitemapService {
         return Collections.unmodifiableSet(onDemandNames);
     }
 
+    public boolean isWithinLimits(int size, int entries) {
+        return size <= maxSize && entries <= maxEntries;
+    }
+
     @NotNull
     @Override
     public Collection<SitemapInfo> getSitemapInfo(@NotNull Resource resource) {
@@ -201,12 +205,14 @@ 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, false, size <= maxSize && entries <= maxEntries);
+        return new SitemapInfoImpl(url, size, entries, false, isWithinLimits(size, entries));
     }
 
     private static class SitemapInfoImpl implements SitemapInfo {
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
index 4db5c41..226fb3d 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapGeneratorExecutorTest.java
@@ -20,6 +20,7 @@ package org.apache.sling.sitemap.impl;
 
 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.event.jobs.consumer.JobExecutionContext;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.apache.sling.sitemap.SitemapException;
@@ -60,8 +61,11 @@ public class SitemapGeneratorExecutorTest {
     private final SitemapGeneratorManager generatorManager = new SitemapGeneratorManager();
     private final ExtensionProviderManager extensionProviderManager = new ExtensionProviderManager();
     private final SitemapStorage storage = spy(new SitemapStorage());
+    private final SitemapServiceImpl sitemapService = new SitemapServiceImpl();
 
     @Mock
+    private JobManager jobManager;
+    @Mock
     private ServiceUserMapped serviceUser;
     @Mock
     private Job job;
@@ -83,9 +87,11 @@ public class SitemapGeneratorExecutorTest {
         context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-reader");
         context.registerService(ServiceUserMapped.class, serviceUser, "subServiceName", "sitemap-writer");
         context.registerService(SitemapGenerator.class, generator);
+        context.registerService(JobManager.class, jobManager);
         context.registerInjectActivateService(storage);
         context.registerInjectActivateService(generatorManager);
         context.registerInjectActivateService(extensionProviderManager);
+        context.registerInjectActivateService(sitemapService);
 
         when(job.getProperty(SitemapGeneratorExecutor.JOB_PROPERTY_SITEMAP_NAME, SitemapGenerator.DEFAULT_SITEMAP))
                 .thenReturn(SitemapGenerator.DEFAULT_SITEMAP);

[sling-whiteboard] 02/05: rename sitemap Externalizer

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 cd904940290e93ef78d5a5f5da665e6a4e84d515
Author: Dirk Rudolph <di...@apache.org>
AuthorDate: Mon Jun 7 10:29:45 2021 +0200

    rename sitemap Externalizer
    
    In order to better reflect its purpose, rename the Externalizer interface
    to SitemapLinkExternalizer.
---
 sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java  | 4 ++--
 .../common/{Externalizer.java => SitemapLinkExternalizer.java}      | 6 +++---
 .../main/java/org/apache/sling/sitemap/impl/SitemapServiceImpl.java | 4 ++--
 .../src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java | 4 ++--
 .../test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java | 1 -
 5 files changed, 9 insertions(+), 10 deletions(-)

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 10cf7f7..190db7f 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/SitemapService.java
@@ -19,7 +19,7 @@
 package org.apache.sling.sitemap;
 
 import org.apache.sling.api.resource.Resource;
-import org.apache.sling.sitemap.common.Externalizer;
+import org.apache.sling.sitemap.common.SitemapLinkExternalizer;
 import org.jetbrains.annotations.NotNull;
 import org.osgi.annotation.versioning.ProviderType;
 
@@ -43,7 +43,7 @@ public interface SitemapService {
      * Numbers for size and entries can only be provided for sitemaps served from storage. For sitemap index or
      * on-demand sitemaps {@code -1} will be returned.
      * <p>
-     * The default implementation uses {@link Externalizer#externalize(Resource)} to create absolute urls.
+     * The default implementation uses {@link SitemapLinkExternalizer#externalize(Resource)} to create absolute urls.
      *
      * @param sitemapRoot a {@link Resource} having {@link SitemapService#PROPERTY_SITEMAP_ROOT} set to true
      * @return the url, or null
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/common/Externalizer.java b/sitemap/src/main/java/org/apache/sling/sitemap/common/SitemapLinkExternalizer.java
similarity index 90%
rename from sitemap/src/main/java/org/apache/sling/sitemap/common/Externalizer.java
rename to sitemap/src/main/java/org/apache/sling/sitemap/common/SitemapLinkExternalizer.java
index 74e3c0f..0354d9f 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/common/Externalizer.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/common/SitemapLinkExternalizer.java
@@ -27,12 +27,12 @@ import org.osgi.annotation.versioning.ConsumerType;
  * Consumers may implement this interface to override the default externalisation behaviour.
  */
 @ConsumerType
-public interface Externalizer {
+public interface SitemapLinkExternalizer {
 
     /**
-     * A default implementation of the {@link Externalizer} which may be used as fallback.
+     * A default implementation of the {@link SitemapLinkExternalizer} which may be used as fallback.
      */
-    Externalizer DEFAULT = new Externalizer() {
+    SitemapLinkExternalizer DEFAULT = new SitemapLinkExternalizer() {
         @Override
         public @Nullable String externalize(SlingHttpServletRequest context, String uri) {
             return context.getResourceResolver().map(context, uri);
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 6cbee90..22e6111 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
@@ -23,7 +23,7 @@ import org.apache.sling.event.jobs.Job;
 import org.apache.sling.event.jobs.JobManager;
 import org.apache.sling.sitemap.SitemapInfo;
 import org.apache.sling.sitemap.SitemapService;
-import org.apache.sling.sitemap.common.Externalizer;
+import org.apache.sling.sitemap.common.SitemapLinkExternalizer;
 import org.jetbrains.annotations.NotNull;
 import org.osgi.service.component.annotations.*;
 import org.osgi.service.metatype.annotations.AttributeDefinition;
@@ -58,7 +58,7 @@ public class SitemapServiceImpl implements SitemapService {
     private static final Logger LOG = LoggerFactory.getLogger(SitemapServiceImpl.class);
 
     @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
-    private Externalizer externalizer = Externalizer.DEFAULT;
+    private SitemapLinkExternalizer externalizer = SitemapLinkExternalizer.DEFAULT;
     @Reference
     private JobManager jobManager;
     @Reference
diff --git a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
index 15ff464..a254a10 100644
--- a/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
+++ b/sitemap/src/main/java/org/apache/sling/sitemap/impl/SitemapServlet.java
@@ -23,7 +23,7 @@ import org.apache.sling.api.SlingHttpServletResponse;
 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.common.Externalizer;
+import org.apache.sling.sitemap.common.SitemapLinkExternalizer;
 import org.apache.sling.sitemap.SitemapException;
 import org.apache.sling.sitemap.generator.SitemapGenerator;
 import org.apache.sling.sitemap.impl.builder.extensions.ExtensionProviderManager;
@@ -79,7 +79,7 @@ public class SitemapServlet extends SlingSafeMethodsServlet {
     };
 
     @Reference(cardinality = ReferenceCardinality.OPTIONAL, policyOption = ReferencePolicyOption.GREEDY)
-    private Externalizer externalizer = Externalizer.DEFAULT;
+    private SitemapLinkExternalizer externalizer = SitemapLinkExternalizer.DEFAULT;
     @Reference
     private SitemapGeneratorManager generatorManager;
     @Reference
diff --git a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
index b2e5b7f..02b4172 100644
--- a/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
+++ b/sitemap/src/test/java/org/apache/sling/sitemap/impl/SitemapServletTest.java
@@ -23,7 +23,6 @@ import org.apache.sling.api.resource.Resource;
 import org.apache.sling.event.jobs.JobManager;
 import org.apache.sling.serviceusermapping.ServiceUserMapped;
 import org.apache.sling.sitemap.TestResourceTreeSitemapGenerator;
-import org.apache.sling.sitemap.common.Externalizer;
 import org.apache.sling.sitemap.generator.SitemapGenerator;
 import org.apache.sling.sitemap.impl.builder.SitemapImplTest;
 import org.apache.sling.sitemap.impl.builder.extensions.ExtensionProviderManager;