Provide a way to access a history item by id

Use it within the Touch UI for stable URLs to logs
Improve exception handling

This closes #761

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/AcHistoryService.java

@@ -15,6 +15,8 @@
 
 import java.util.List;
 
+import javax.jcr.RepositoryException;
+
 import org.osgi.annotation.versioning.ProviderType;
 
 import biz.netcentric.cq.tools.actool.history.impl.PersistableInstallationLogger;
@@ -28,13 +30,25 @@ public interface AcHistoryService {
 
     /** Returns history items of previous runs
      * 
-     * @return Set of AcToolExecutions */
-    public List<AcToolExecution> getAcToolExecutions();
+     * @return Set of AcToolExecutions 
+     * @throws RepositoryException */
+    public List<AcToolExecution> getAcToolExecutions() throws RepositoryException;
 
     public String getLastInstallationHistory();
 
+    /**
+     * Exposes the log contents of a specific run of the AC Tool. The given index is volatile (due to history order changes), 
+     * so consider using {@link #getLogFromHistory(String, boolean, boolean)} instead.
+     * @param n the index of the child node below the history root node which should be returned
+     * @param inHtmlFormat
+     * @param includeVerbose
+     * @return the log's content
+     * @see #getLogFromHistory(String, boolean, boolean)
+     */
     public String getLogFromHistory(int n, boolean inHtmlFormat, boolean includeVerbose);
 
+    public String getLogFromHistory(String id, boolean inHtmlFormat, boolean includeVerbose) throws RepositoryException;
+
     public boolean wasLastPersistHistoryCallSuccessful();
 
 }

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/AcToolExecution.java

@@ -22,7 +22,8 @@
  */
 @ProviderType
 public interface AcToolExecution {
-
+    public String getId();
+
     public String getLogsPath();
 
     public Date getInstallationDate();

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/impl/AcHistoryServiceImpl.java

@@ -109,17 +109,13 @@ public void persistHistory(PersistableInstallationLogger installLog) {
     }
 
     @Override
-    public List<AcToolExecution> getAcToolExecutions() {
+    public List<AcToolExecution> getAcToolExecutions() throws RepositoryException {
 
         Session session = null;
         try {
             session = repository.loginService(null, null);
             List<AcToolExecution> historyItems = HistoryUtils.getAcToolExecutions(session);
             return historyItems;
-
-        } catch (RepositoryException e) {
-            LOG.error("Could not get history items: "+e, e);
-            return Collections.<AcToolExecution>emptyList();
         } finally {
             if (session != null) {
                 session.logout();
@@ -216,6 +212,22 @@ public String getLogFromHistory(int n, boolean inHtmlFormat, boolean includeVerb
         return history;
     }
 
+    @Override
+    public String getLogFromHistory(String id, boolean inHtmlFormat, boolean includeVerbose) throws RepositoryException {
+        Session session = null;
+        try {
+            session = repository.loginService(null, null);
+            // construct path from id
+            Node acHistoryRootNode = HistoryUtils.getAcHistoryRootNode(session);
+            String path = HistoryUtils.getPathFromId(id, acHistoryRootNode.getPath());
+            return inHtmlFormat ? getLogHtml(session, path, includeVerbose) : getLogTxt(session, path, includeVerbose);
+        } finally {
+            if (session != null) {
+                session.logout();
+            }
+        }
+    }
+
     @Override
     public void persistAcePurgeHistory(PersistableInstallationLogger installLog) {
 

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/impl/AcToolExecutionImpl.java

@@ -22,6 +22,7 @@
 public class AcToolExecutionImpl implements AcToolExecution, Comparable<AcToolExecution> {
     static final String TRIGGER_SEPARATOR_IN_NODE_NAME = "_via_";
 
+    private final String id;
     private final String path;
     private final Date installationDate;
     private final boolean isSuccess;
@@ -30,8 +31,9 @@ public class AcToolExecutionImpl implements AcToolExecution, Comparable<AcToolEx
     private final int authorizableChanges;
     private final int aclChanges;
 
-    public AcToolExecutionImpl(String path, Date installationDate, boolean isSuccess, String configurationRootPath, int authorizableChanges, int aclChanges) {
+    public AcToolExecutionImpl(String id, String path, Date installationDate, boolean isSuccess, String configurationRootPath, int authorizableChanges, int aclChanges) {
         super();
+        this.id = id;
         this.path = path;
         this.installationDate = installationDate;
         this.isSuccess = isSuccess;
@@ -47,6 +49,11 @@ public String toString() {
         return path + " (" + installationDate.toString() + ")(" + successStatusString  + ")";
     }
 
+    @Override
+    public String getId() {
+        return id;
+    }
+
     public String getLogsPath() {
         return path;
     }

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/history/impl/HistoryUtils.java

@@ -39,6 +39,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.day.text.Text;
+
 import biz.netcentric.cq.tools.actool.api.InstallationResult;
 import biz.netcentric.cq.tools.actool.comparators.TimestampPropertyComparator;
 import biz.netcentric.cq.tools.actool.configuploadlistener.impl.UploadListenerServiceImpl.AcToolConfigUpdateListener;
@@ -284,7 +286,8 @@ static List<AcToolExecution> getAcToolExecutions(final Session session)
                 int authorizableChanges = node.hasProperty(PROPERTY_AUTHORIZABLES_CHANGES) ? (int) node.getProperty(PROPERTY_AUTHORIZABLES_CHANGES).getLong() : -1;
                 int aclChanges = node.hasProperty(PROPERTY_ACL_CHANGES) ? (int) node.getProperty(PROPERTY_ACL_CHANGES).getLong() : -1;
 
-                historyInfos.add(new AcToolExecutionImpl(node.getPath(), 
+                historyInfos.add(new AcToolExecutionImpl(getIdFromPath(node.getPath()),
+                        node.getPath(), 
                         new Date(node.getProperty(PROPERTY_TIMESTAMP).getLong()), 
                         node.getProperty(PROPERTY_SUCCESS).getBoolean(),
                         configRoot, authorizableChanges, aclChanges));
@@ -294,6 +297,14 @@ static List<AcToolExecution> getAcToolExecutions(final Session session)
         return new ArrayList<>(historyInfos);
     }
 
+    static String getIdFromPath(String path) {
+        return StringUtils.removeStart(Text.getName(path), HISTORY_NODE_NAME_PREFIX);
+    }
+
+    static String getPathFromId(String id, String rootPath) {
+        return rootPath + "/" + HISTORY_NODE_NAME_PREFIX + id;
+    }
+
     public static String getLogTxt(final Session session, final String path, boolean includeVerbose) {
         return getLog(session, path, "\n", includeVerbose).toString();
     }

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/jmx/AceServiceMBean.java

@@ -77,7 +77,7 @@ public String applyRestrictedToPaths(@Name("configurationRootPath") @Description
     @Description("Returns installation log for the given ordinal")
     public String showInstallationLog(
             @Name("installationLogNumber") @Description("Ordinal of the installation log to be shown") final String historyLogNumber, 
-            @Name("includeVerbose") @Description("Include verbose messages") boolean verbose);
+            @Name("includeVerbose") @Description("Include verbose messages") boolean verbose) throws RepositoryException;
 
     @Description("Purges authorizable(s) and respective ACEs from the system.")
     public String purgeAuthorizables(

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/jmx/AceServiceMBeanImpl.java

@@ -16,6 +16,7 @@
 import java.util.List;
 import java.util.Set;
 
+import javax.jcr.RepositoryException;
 import javax.management.NotCompliantMBeanException;
 
 import org.apache.commons.lang3.StringUtils;
@@ -110,7 +111,7 @@ public String[] getConfigurationFiles() {
     }
 
     @Override
-    public String[] getSavedLogs() {
+    public String[] getSavedLogs() throws RepositoryException {
         List<AcToolExecution> executions = acHistoryService.getAcToolExecutions();
         if (executions.isEmpty()) {
             return new String[] { "no executions found" };
@@ -139,7 +140,7 @@ public String groupBasedDump() {
     }
 
     @Override
-    public String showInstallationLog(final String n, boolean verbose) {
+    public String showInstallationLog(final String n, boolean verbose) throws RepositoryException {
         int i;
         String[] logs = getSavedLogs();
         if (logs.length == 0) {

accesscontroltool-bundle/src/main/java/biz/netcentric/cq/tools/actool/ui/AcToolUiService.java

@@ -24,11 +24,13 @@
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.function.Consumer;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 import javax.jcr.RepositoryException;
@@ -69,7 +71,7 @@ public class AcToolUiService {
     public static final String PARAM_CONFIGURATION_ROOT_PATH = "configurationRootPath";
     public static final String PARAM_APPLY_ONLY_IF_CHANGED = "applyOnlyIfChanged";
     public static final String PARAM_BASE_PATHS = "basePaths";
-    public static final String PARAM_SHOW_LOG_NO = "showLogNo";
+    public static final String PARAM_SHOW_LOG_ID = "showLogId";
     public static final String PARAM_SHOW_LOG_VERBOSE = "showLogVerbose";
 
     public static final String PAGE_NAME = "actool";
@@ -200,7 +202,7 @@ public String getWebConsoleRoot(HttpServletRequest req) {
         return (String) req.getAttribute(WebConsoleConstants.ATTR_APP_ROOT);
     }
 
-    private void renderUi(HttpServletRequest req, HttpServletResponse resp, String path, boolean isTouchUi) throws IOException {
+    private void renderUi(HttpServletRequest req, HttpServletResponse resp, String path, boolean isTouchUi) throws ServletException, IOException {
         RequestParameters reqParams = RequestParameters.fromRequest(req, acInstallationService);
 
         final PrintWriter out = resp.getWriter();
@@ -211,7 +213,11 @@ private void renderUi(HttpServletRequest req, HttpServletResponse resp, String p
         printImportSection(writer, reqParams, path, isTouchUi, getWebConsoleRoot(req));
         printExportSection(writer, reqParams, path, isTouchUi, getWebConsoleRoot(req));
 
-        printInstallationLogsSection(writer, reqParams, isTouchUi);
+        try {
+            printInstallationLogsSection(writer, reqParams, isTouchUi);
+        } catch (RepositoryException e) {
+            throw new ServletException("Could not read log from repository", e);
+        }
 
         if(!isTouchUi) {
             String jmxUrl = getWebConsoleRoot(req) + "/jmx/"
@@ -322,9 +328,17 @@ private void printVersion(HtmlWriter writer) {
         writer.closeTable();
     }
 
-    private void printInstallationLogsSection(HtmlWriter writer, RequestParameters reqParams, boolean isTouchUi) {
+    private void printInstallationLogsSection(HtmlWriter writer, RequestParameters reqParams, boolean isTouchUi) throws RepositoryException {
 
-        List<AcToolExecution> acToolExecutions = acHistoryService.getAcToolExecutions();
+        // generate an ordered map of all executions (key = id, value = execution)
+        Map<String, AcToolExecution> acToolExecutions = acHistoryService.getAcToolExecutions().stream().collect(
+                Collectors.toMap(
+                        AcToolExecution::getId, 
+                        Function.identity(),
+                        (u, v) -> {
+                            throw new IllegalStateException(String.format("Duplicate key %s", u));
+                        },
+                        LinkedHashMap::new));
 
         writer.openTable("previousLogs");
         writer.tableHeader("Previous Logs", 5);
@@ -337,9 +351,8 @@ private void printInstallationLogsSection(HtmlWriter writer, RequestParameters r
             return;
         }
 
-        for (int i = 1; i <= acToolExecutions.size(); i++) {
-            AcToolExecution acToolExecution = acToolExecutions.get(i - 1);
-            String linkToLog = PAGE_NAME + "?showLogNo=" + i;
+        for (AcToolExecution acToolExecution : acToolExecutions.values()) {
+            String linkToLog = PAGE_NAME + "?" + PARAM_SHOW_LOG_ID + "=" + acToolExecution.getId();
             writer.tr();
             writer.openTd();
             writer.println(getExecutionDateStr(acToolExecution));
@@ -360,20 +373,25 @@ private void printInstallationLogsSection(HtmlWriter writer, RequestParameters r
         }
         writer.closeTable();
 
-        if (reqParams.showLogNo > 0 && reqParams.showLogNo <= acToolExecutions.size()) {
-
-            AcToolExecution acToolExecution = acToolExecutions.get(reqParams.showLogNo - 1);
-            String logLabel = "Previous Log " + reqParams.showLogNo + ": " + getExecutionLabel(acToolExecution);
-            String logHtml = acHistoryService.getLogFromHistory(reqParams.showLogNo, true, reqParams.showLogVerbose);
+        if (StringUtils.isNotBlank(reqParams.showLogId)) {
 
-            writer.openTable("logTable");
-            writer.tableHeader(logLabel, 1, false);
-            writer.tr();
-            writer.openTd();
-            writer.println(logHtml);
-            writer.closeTd();
-            writer.closeTr();
-            writer.closeTable();
+            AcToolExecution acToolExecution = acToolExecutions.get(reqParams.showLogId);
+            if (acToolExecution == null) {
+                writer.println("No log found for id " + reqParams.showLogId);
+                return;
+            } else {
+                String logLabel = "Previous Log " + reqParams.showLogId + ": " + getExecutionLabel(acToolExecution);
+                String logHtml = acHistoryService.getLogFromHistory(reqParams.showLogId, true, reqParams.showLogVerbose);
+
+                writer.openTable("logTable");
+                writer.tableHeader(logLabel, 1, false);
+                writer.tr();
+                writer.openTd();
+                writer.println(logHtml);
+                writer.closeTd();
+                writer.closeTr();
+                writer.closeTable();
+            }
         }
 
     }
@@ -453,8 +471,8 @@ private void printImportSection(final HtmlWriter writer, RequestParameters reqPa
 
         writer.tr();
         writer.openTd();
-        String onClick = "var as=$('#applySpinner');as.show(); var b=$('#applyButton');b.prop('disabled', true); oldL = b.text();b.text(' Applying AC Tool Configuration... ');var f=$('#acForm');var fd=f.serialize();$.post(f.attr('action'), fd).done(function(text){alert(text)}).fail(function(xhr){alert(xhr.status===403?'Permission Denied':'Config could not be applied - check log for errors')}).always(function(text) { var ll=text&amp;&amp;text.indexOf&amp;&amp;text.indexOf('identical to last execution')===-1?'"
-                + PARAM_SHOW_LOG_NO + "=1&':'';as.hide();b.text(oldL);b.prop('disabled', false);location.href='" + PAGE_NAME + "?'+ll+fd; });return false";
+        String onClick = "var as=$('#applySpinner');as.show(); var b=$('#applyButton');b.prop('disabled', true); oldL = b.text();b.text(' Applying AC Tool Configuration... ');var f=$('#acForm');var fd=f.serialize();$.post(f.attr('action'), fd).done(function(text){alert(text)}).fail(function(xhr){alert(xhr.status===403?'Permission Denied':'Config could not be applied - check log for errors')}).always(function(text) { "
+                + "as.hide();b.text(oldL);b.prop('disabled', false);location.href='" + PAGE_NAME + "?'+fd; });return false";
         writer.println("<button " + getCoralButtonAtts(isTouchUI) + " id='applyButton' onclick=\"" + onClick + "\"> Apply AC Tool Configuration </button>");
         writer.closeTd();
         writer.openTd();
@@ -524,23 +542,23 @@ static RequestParameters fromRequest(HttpServletRequest req, AcInstallationServi
             return new RequestParameters(
                     configRootPath,
                     StringUtils.isNotBlank(basePathsParam) ? Arrays.asList(basePathsParam.split(" *, *")) : null,
-                    Integer.parseInt(getParam(req, AcToolUiService.PARAM_SHOW_LOG_NO, "0")),
+                    getParam(req, AcToolUiService.PARAM_SHOW_LOG_ID, null),
                     Boolean.valueOf(req.getParameter(AcToolUiService.PARAM_SHOW_LOG_VERBOSE)),
                     Boolean.valueOf(req.getParameter(AcToolUiService.PARAM_APPLY_ONLY_IF_CHANGED)));
         }
 
         final String configurationRootPath;
         final List<String> basePaths;
-        final int showLogNo;
+        final String showLogId;
         final boolean showLogVerbose;
         final boolean applyOnlyIfChanged;
 
-        public RequestParameters(String configurationRootPath, List<String> basePaths, int showLogNo, boolean showLogVerbose,
+        public RequestParameters(String configurationRootPath, List<String> basePaths, String showLogId, boolean showLogVerbose,
                 boolean applyOnlyIfChanged) {
             super();
             this.configurationRootPath = configurationRootPath;
             this.basePaths = basePaths;
-            this.showLogNo = showLogNo;
+            this.showLogId = showLogId;
             this.showLogVerbose = showLogVerbose;
             this.applyOnlyIfChanged = applyOnlyIfChanged;
         }

accesscontroltool-bundle/src/test/java/biz/netcentric/cq/tools/actool/history/impl/HistoryUtilsTest.java

@@ -0,0 +1,33 @@
+package biz.netcentric.cq.tools.actool.history.impl;
+
+/*-
+ * #%L
+ * Access Control Tool Bundle
+ * %%
+ * Copyright (C) 2015 - 2024 Cognizant Netcentric
+ * %%
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ * #L%
+ */
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.api.Test;
+
+class HistoryUtilsTest {
+
+    @Test
+    void testGetIdFromPath() {
+        assertEquals("test", HistoryUtils.getIdFromPath("/base/history_test"));
+        assertEquals("test", HistoryUtils.getIdFromPath("/some/deeply/nested/history_test"));
+    }
+
+    @Test
+    void testGetPathFromId() {
+        assertEquals("/base/history_test", HistoryUtils.getPathFromId("test", "/base"));
+        assertEquals("/deeply/nested/base/history_test", HistoryUtils.getPathFromId("test", "/deeply/nested/base"));
+    }
+}

accesscontroltool-package/pom.xml

@@ -142,6 +142,10 @@
                     <name>env.CI</name>
                 </property>
             </activation>
+            <properties>
+                <!-- install package with "cloud" classifier to AEM -->
+                <contentPackageFile>${project.build.directory}/${project.build.finalName}-cloud.zip</contentPackageFile>
+            </properties>
             <build>
                 <plugins>
                      <!-- for "cloud" package use repoinit instead of actool-content-package -->

pom.xml

@@ -781,10 +781,6 @@
                     <name>env.CI</name>
                 </property>
             </activation>
-            <properties>
-                <!-- install package with "cloud" classifier to AEM -->
-                <contentPackageFile>${project.build.directory}/${project.build.finalName}-cloud.zip</contentPackageFile>
-            </properties>
         </profile>
     </profiles>