You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2022/02/11 14:06:53 UTC
[sling-org-apache-sling-jcr-repoinit] branch improvement/10625-avoid-RTE created (now ab1da04)
This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a change to branch improvement/10625-avoid-RTE
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git.
at ab1da04 SLING-10625 convert RuntimeExceptions into RepositoryExceptions
This branch includes the following new commits:
new ab1da04 SLING-10625 convert RuntimeExceptions into RepositoryExceptions
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[sling-org-apache-sling-jcr-repoinit] 01/01: SLING-10625 convert RuntimeExceptions into RepositoryExceptions
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch improvement/10625-avoid-RTE
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit ab1da044580f860a446bd7c25dcc41941f7ea36c
Author: Jörg Hoh <jo...@joerghoh.de>
AuthorDate: Fri Feb 11 15:06:37 2022 +0100
SLING-10625 convert RuntimeExceptions into RepositoryExceptions
---
pom.xml | 6 +++
.../apache/sling/jcr/repoinit/impl/AclVisitor.java | 20 ++++----
.../sling/jcr/repoinit/impl/DoNothingVisitor.java | 6 ++-
.../jcr/repoinit/impl/GroupMembershipVisitor.java | 4 +-
.../jcr/repoinit/impl/NodePropertiesVisitor.java | 2 +-
.../sling/jcr/repoinit/impl/RepoInitException.java | 34 +++++++++++++
.../impl/RepositoryInitializerFactory.java | 10 +++-
.../sling/jcr/repoinit/impl/UserVisitor.java | 4 +-
.../impl/RepositoryInitializerFactoryTest.java | 59 ++++++++++++++++++++++
9 files changed, 128 insertions(+), 17 deletions(-)
diff --git a/pom.xml b/pom.xml
index f909c5f..1b7ea45 100644
--- a/pom.xml
+++ b/pom.xml
@@ -169,6 +169,12 @@
<scope>test</scope>
</dependency>
<dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <version>4.3.1</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
<groupId>org.osgi</groupId>
<artifactId>osgi.core</artifactId>
<scope>provided</scope>
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
index 9e363ef..25e24b4 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclVisitor.java
@@ -72,7 +72,7 @@ class AclVisitor extends DoNothingVisitor {
private void setAcl(AclLine line, Session s, List<String> principals, List<String> paths, List<String> privileges, AclLine.Action action) {
try {
if (action == AclLine.Action.REMOVE) {
- throw new RuntimeException("remove not supported");
+ report("remove not supported");
} else if (action == AclLine.Action.REMOVE_ALL) {
AclUtil.removeEntries(s, principals, paths);
} else {
@@ -82,14 +82,14 @@ class AclVisitor extends DoNothingVisitor {
AclUtil.setAcl(s, principals, paths, privileges, isAllow, restrictions);
}
} catch (Exception e) {
- throw new RuntimeException("Failed to set ACL (" + e.toString() + ") " + line, e);
+ report(e,"Failed to set ACL (" + e.toString() + ") " + line);
}
}
private void setRepositoryAcl(AclLine line, Session s, List<String> principals, List<String> privileges, AclLine.Action action) {
try {
if (action == AclLine.Action.REMOVE) {
- throw new RuntimeException("remove not supported");
+ report("remove not supported");
} else if (action == AclLine.Action.REMOVE_ALL) {
AclUtil.removeEntries(s, principals, Collections.singletonList(null));
} else {
@@ -99,7 +99,7 @@ class AclVisitor extends DoNothingVisitor {
AclUtil.setRepositoryAcl(s, principals, privileges, isAllow, restrictions);
}
} catch (Exception e) {
- throw new RuntimeException("Failed to set repository level ACL (" + e.toString() + ") " + line, e);
+ report(e, "Failed to set repository level ACL (" + e.toString() + ") " + line);
}
}
@@ -131,7 +131,7 @@ class AclVisitor extends DoNothingVisitor {
log.info("Adding principal-based access control entry for {}", principalName);
AclUtil.setPrincipalAcl(session, principalName, s.getLines());
} catch (Exception e) {
- throw new RuntimeException("Failed to set principal-based ACL (" + e.getMessage() + ")", e);
+ report(e, "Failed to set principal-based ACL (" + e.getMessage() + ")");
}
}
}
@@ -157,14 +157,14 @@ class AclVisitor extends DoNothingVisitor {
}
}
} catch (Exception e) {
- throw new RuntimeException("CreatePath execution failed at " + psd + ": " + e, e);
+ report(e, "CreatePath execution failed at " + psd + ": " + e);
}
parentPath += "/" + psd.getSegment();
}
try {
session.save();
} catch (Exception e) {
- throw new RuntimeException("Session.save failed: " + e, e);
+ report(e, "Session.save failed: " + e);
}
}
@@ -191,7 +191,7 @@ class AclVisitor extends DoNothingVisitor {
log.info("Removing access control policy for {}", principalName);
AclUtil.removePolicy(session, principalName);
} catch (RepositoryException e) {
- throw new RuntimeException("Failed to remove ACL (" + e.getMessage() + ")");
+ report(e, "Failed to remove ACL (" + e.getMessage() + ")");
}
}
}
@@ -201,7 +201,7 @@ class AclVisitor extends DoNothingVisitor {
try {
AclUtil.removePolicies(session, s.getPaths());
} catch (RepositoryException e) {
- throw new RuntimeException("Failed to remove ACL (" + e.getMessage() + ")");
+ report(e, "Failed to remove ACL (" + e.getMessage() + ")");
}
}
@@ -212,7 +212,7 @@ class AclVisitor extends DoNothingVisitor {
log.info("Removing principal-based access control policy for {}", principalName);
AclUtil.removePrincipalPolicy(session, principalName);
} catch (RepositoryException e) {
- throw new RuntimeException("Failed to remove principal-based ACL (" + e.getMessage() + ")");
+ report(e, "Failed to remove principal-based ACL (" + e.getMessage() + ")");
}
}
}
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
index edf159f..8e43e20 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/DoNothingVisitor.java
@@ -60,7 +60,11 @@ class DoNothingVisitor implements OperationVisitor {
}
protected void report(Exception e, String message) {
- throw new RuntimeException(message, e);
+ throw new RepoInitException(message, e);
+ }
+
+ protected void report(String message) {
+ throw new RepoInitException(message);
}
protected static String excerpt(String s, int maxLength) {
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/GroupMembershipVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/GroupMembershipVisitor.java
index 3918a40..9e01dcd 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/GroupMembershipVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/GroupMembershipVisitor.java
@@ -49,7 +49,7 @@ class GroupMembershipVisitor extends DoNothingVisitor {
try {
group = UserUtil.getAuthorizable(session, groupname);
if (group == null || !group.isGroup()) {
- throw new RuntimeException(groupname + " is not a group");
+ report(groupname + " is not a group");
}
((Group) group).addMembers(members.toArray(new String[0]));
} catch (RepositoryException e) {
@@ -66,7 +66,7 @@ class GroupMembershipVisitor extends DoNothingVisitor {
try {
group = UserUtil.getAuthorizable(session, groupname);
if (group == null || !group.isGroup()) {
- throw new RuntimeException(groupname + " is not a group");
+ report(groupname + " is not a group");
}
((Group) group).removeMembers(members.toArray(new String[0]));
} catch (RepositoryException e) {
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
index 3483d69..465685b 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/NodePropertiesVisitor.java
@@ -241,7 +241,7 @@ class NodePropertiesVisitor extends DoNothingVisitor {
} else if (value instanceof Calendar) {
convertedValue = new DateValue((Calendar) value);
} else {
- throw new RuntimeException("Unable to convert " + value + " to jcr Value");
+ report("Unable to convert " + value + " to jcr Value");
}
return convertedValue;
}
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/RepoInitException.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/RepoInitException.java
new file mode 100644
index 0000000..dd958f4
--- /dev/null
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/RepoInitException.java
@@ -0,0 +1,34 @@
+/*
+ * 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.jcr.repoinit.impl;
+
+/**
+ * A marker exception which is only thrown by these repoinit operations, so it is
+ * possible to handle them explicitly.
+ *
+ */
+public class RepoInitException extends RuntimeException {
+
+ public RepoInitException(String msg, Exception e) {
+ super(msg, e);
+ }
+
+ public RepoInitException(String msg) {
+ super(msg);
+ }
+
+}
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
index ac21355..39844b3 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
@@ -143,7 +143,7 @@ public class RepositoryInitializerFactory implements SlingRepositoryInitializer
* @param logMessage the messages to print when retry
* @throws Exception if the application fails despite the retry
*/
- private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+ protected void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
RetryableOperation.RetryableOperationResult result = retry.apply(() -> {
@@ -168,6 +168,14 @@ public class RepositoryInitializerFactory implements SlingRepositoryInitializer
// ignore
}
return new RetryableOperation.RetryableOperationResult(false,false,re);
+ } catch (RepoInitException rie) {
+ // treat them as permanent exceptions, where retry is not useful
+ try {
+ session.refresh(false); // discard all pending changes
+ } catch (RepositoryException re) {
+ // ignore
+ }
+ return new RetryableOperation.RetryableOperationResult(false,false,rie);
}
}, logMessage);
if (!result.isSuccessful()) {
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/UserVisitor.java b/src/main/java/org/apache/sling/jcr/repoinit/impl/UserVisitor.java
index c11f1d2..48a5e0f 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/UserVisitor.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/UserVisitor.java
@@ -163,10 +163,10 @@ class UserVisitor extends DoNothingVisitor {
}
}
- private static void checkUserType(@NotNull String id, @Nullable User user, boolean expectedSystemUser) {
+ private void checkUserType(@NotNull String id, @Nullable User user, boolean expectedSystemUser) {
if (user != null && user.isSystemUser() != expectedSystemUser) {
String msg = (expectedSystemUser) ? "Existing user %s is not a service user." : "Existing user %s is a service user.";
- throw new RuntimeException(String.format(msg, id));
+ report(String.format(msg, id));
}
}
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactoryTest.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactoryTest.java
new file mode 100644
index 0000000..ac69d7c
--- /dev/null
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactoryTest.java
@@ -0,0 +1,59 @@
+/*
+ * 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.jcr.repoinit.impl;
+
+import org.apache.sling.jcr.repoinit.JcrRepoInitOpsProcessor;
+import org.apache.sling.repoinit.parser.RepoInitParser;
+import org.apache.sling.testing.mock.sling.junit.SlingContext;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.mockito.ArgumentMatchers;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+public class RepositoryInitializerFactoryTest {
+
+ @Rule
+ public SlingContext context = new SlingContext();
+
+ RepositoryInitializerFactory sut;
+ JcrRepoInitOpsProcessor processor;
+
+ @Before
+ public void setup() {
+ RepoInitParser parser = mock(RepoInitParser.class);
+ context.registerService(RepoInitParser.class, parser);
+ processor = mock(JcrRepoInitOpsProcessor.class);
+ context.registerService(JcrRepoInitOpsProcessor.class, processor);
+
+ sut = new RepositoryInitializerFactory();
+ context.registerInjectActivateService(sut);
+ }
+
+ @Test(expected = RepositoryException.class)
+ public void handleUncheckedErrorsInOperations() throws RepositoryException {
+ doThrow(new RepoInitException("some op failed", new Exception("root cause")))
+ .when(processor).apply(ArgumentMatchers.any(), ArgumentMatchers.any());
+ sut.applyOperations(mock(Session.class), null, null);
+ }
+}