This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-9692_cleanup in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-feature-cpconverter.git
commit 2b2550f28847eb991a2e521214e219b7a67241e6 Author: angela <[email protected]> AuthorDate: Fri Jan 15 17:46:48 2021 +0100 minor improvements, cleanup, nullability-issues, ... --- .../accesscontrol/AccessControlEntry.java | 5 +- .../cpconverter/accesscontrol/AclManager.java | 6 +- .../accesscontrol/DefaultAclManager.java | 113 +++++++++++---------- .../cpconverter/accesscontrol/MixinParser.java | 4 +- .../accesscontrol/PrimaryTypeParser.java | 7 +- .../cpconverter/handlers/BundleEntryHandler.java | 28 +++-- .../handlers/DefaultEntryHandlersManager.java | 5 +- .../cpconverter/shared/AbstractJcrNodeParser.java | 4 +- .../vltpkg/RecollectorVaultPackageScanner.java | 2 +- .../cpconverter/accesscontrol/AclManagerTest.java | 29 +++--- .../accesscontrol/EnforcePrincipalBasedTest.java | 30 ++++-- .../handlers/PrivilegesHandlerTest.java | 20 ++-- .../handlers/RepPolicyEntryHandlerTest.java | 4 - .../RepPrincipalPolicyEntryHandlerTest.java | 4 +- .../handlers/RepRepoPolicyEntryHandlerTest.java | 3 - .../feature/cpconverter/handlers/TestUtils.java | 4 +- .../handlers/UsersEntryHandlerTest.java | 29 +++--- .../cpconverter/vltpkg/NodeTypesDetectionTest.java | 3 +- .../vltpkg/PackagesEventsEmitterTest.java | 31 ++---- 19 files changed, 160 insertions(+), 171 deletions(-) diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AccessControlEntry.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AccessControlEntry.java index 984e7f3..e1a8472 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AccessControlEntry.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AccessControlEntry.java @@ -18,7 +18,6 @@ package org.apache.sling.feature.cpconverter.accesscontrol; import org.apache.sling.feature.cpconverter.shared.RepoPath; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.util.LinkedList; import java.util.List; @@ -49,8 +48,8 @@ public final class AccessControlEntry { this.isPrincipalBased = isPrincipalBased; } - public void addRestriction(@Nullable String restriction) { - if (restriction != null && !restriction.isEmpty()) { + public void addRestriction(@NotNull String restriction) { + if (!restriction.isEmpty()) { restrictions.add(restriction); } } diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManager.java index 0aa5eae..8ca506d 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManager.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManager.java @@ -34,11 +34,11 @@ public interface AclManager { boolean addSystemUser(@NotNull SystemUser systemUser); - boolean addAcl(String systemUser, AccessControlEntry acl); + boolean addAcl(@NotNull String systemUser, @NotNull AccessControlEntry acl); - void addRepoinitExtension(List<VaultPackageAssembler> packageAssemblers, FeaturesManager featureManager); + void addRepoinitExtension(@NotNull List<VaultPackageAssembler> packageAssemblers, @NotNull FeaturesManager featureManager); - void addNodetypeRegistrationSentence(String nodetypeRegistrationSentence); + void addNodetypeRegistrationSentence(@NotNull String nodetypeRegistrationSentence); void addPrivilegeDefinitions(@NotNull PrivilegeDefinitions privilegeDefinitions); diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java index 728343b..f79c6d5 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/DefaultAclManager.java @@ -72,20 +72,22 @@ public class DefaultAclManager implements AclManager { this.supportedPrincipalBasedPath = (supportedPrincipalBasedPath == null) ? null : new RepoPath(supportedPrincipalBasedPath); } - @Override public boolean addUser(@NotNull User user) { return users.add(user); } + @Override public boolean addGroup(@NotNull Group group) { return groups.add(group); } + @Override public boolean addSystemUser(@NotNull SystemUser systemUser) { return systemUsers.add(systemUser); } + @Override public boolean addAcl(@NotNull String systemUser, @NotNull AccessControlEntry acl) { if (getSystemUser(systemUser).isPresent()) { acls.computeIfAbsent(systemUser, k -> new LinkedList<>()).add(acl); @@ -94,6 +96,7 @@ public class DefaultAclManager implements AclManager { return false; } + @Override public void addRepoinitExtension(@NotNull List<VaultPackageAssembler> packageAssemblers, @NotNull FeaturesManager featureManager) { try (Formatter formatter = new Formatter()) { @@ -105,52 +108,8 @@ public class DefaultAclManager implements AclManager { formatter.format("%s%n", nodetypeRegistrationSentence); } - // system users - - for (SystemUser systemUser : systemUsers) { - // make sure all users are created first - formatter.format("create service user %s with path %s%n", systemUser.getId(), calculateIntermediatePath(systemUser.getIntermediatePath())); - if (aclIsBelow(systemUser.getPath())) { - throw new IllegalStateException("Detected policy on subpath of system-user: " + systemUser); - } - } - - for (Group group : groups) { - if (aclStartsWith(group.getPath())) { - formatter.format("create group %s with path %s%n", group.getId(), group.getIntermediatePath()); - } - if (aclIsBelow(group.getPath())) { - throw new IllegalStateException("Detected policy on subpath of group: " + group); - } - } - - for (User user : users) { - if (aclStartsWith(user.getPath())) { - throw new IllegalStateException("Detected policy on user: " + user); - } - } - - if (!enforcePrincipalBased) { - Set<RepoPath> paths = acls.entrySet().stream() - .filter(entry -> getSystemUser(entry.getKey()).isPresent()) - .map(Entry::getValue) - .flatMap(Collection::stream) - // paths only should/need to be create with resource-based access control - .filter(((Predicate<AccessControlEntry>) AccessControlEntry::isPrincipalBased).negate()) - .map(AccessControlEntry::getRepositoryPath) - .collect(Collectors.toSet()); - - paths.stream() - .filter(path -> paths.stream().noneMatch(other -> !other.equals(path) && other.startsWith(path))) - .filter(((Predicate<RepoPath>)RepoPath::isRepositoryPath).negate()) - .filter(path -> Stream.of(systemUsers, users, groups).flatMap(Collection::stream) - .noneMatch(user -> user.getPath().startsWith(path))) - .map(path -> computePathWithTypes(path, packageAssemblers)) - .filter(Objects::nonNull) - .forEach( - path -> formatter.format("create path %s%n", path) - ); - } + addUsersAndGroups(formatter); + addPaths(formatter, packageAssemblers); // add the acls acls.forEach((systemUserID, authorizations) -> @@ -166,6 +125,31 @@ public class DefaultAclManager implements AclManager { } } + private void addUsersAndGroups(@NotNull Formatter formatter) { + for (SystemUser systemUser : systemUsers) { + // make sure all system users are created first + formatter.format("create service user %s with path %s%n", systemUser.getId(), calculateIntermediatePath(systemUser.getIntermediatePath())); + if (aclIsBelow(systemUser.getPath())) { + throw new IllegalStateException("Detected policy on subpath of system-user: " + systemUser); + } + } + + for (Group group : groups) { + if (aclStartsWith(group.getPath())) { + formatter.format("create group %s with path %s%n", group.getId(), group.getIntermediatePath()); + } + if (aclIsBelow(group.getPath())) { + throw new IllegalStateException("Detected policy on subpath of group: " + group); + } + } + + for (User user : users) { + if (aclStartsWith(user.getPath())) { + throw new IllegalStateException("Detected policy on user: " + user); + } + } + } + @NotNull private String calculateIntermediatePath(@NotNull RepoPath intermediatePath) { if (enforcePrincipalBased && supportedPrincipalBasedPath != null && !intermediatePath.startsWith(supportedPrincipalBasedPath)) { @@ -183,6 +167,30 @@ public class DefaultAclManager implements AclManager { } } + private void addPaths(@NotNull Formatter formatter, @NotNull List<VaultPackageAssembler> packageAssemblers) { + if (!enforcePrincipalBased) { + Set<RepoPath> paths = acls.entrySet().stream() + .filter(entry -> getSystemUser(entry.getKey()).isPresent()) + .map(Entry::getValue) + .flatMap(Collection::stream) + // paths only should/need to be create with resource-based access control + .filter(((Predicate<AccessControlEntry>) AccessControlEntry::isPrincipalBased).negate()) + .map(AccessControlEntry::getRepositoryPath) + .collect(Collectors.toSet()); + + paths.stream() + .filter(path -> paths.stream().noneMatch(other -> !other.equals(path) && other.startsWith(path))) + .filter(((Predicate<RepoPath>)RepoPath::isRepositoryPath).negate()) + .filter(path -> Stream.of(systemUsers, users, groups).flatMap(Collection::stream) + .noneMatch(user -> user.getPath().startsWith(path))) + .map(path -> computePathWithTypes(path, packageAssemblers)) + .filter(Objects::nonNull) + .forEach( + path -> formatter.format("create path %s%n", path) + ); + } + } + private boolean aclStartsWith(@NotNull RepoPath path) { return acls.values().stream().flatMap(List::stream).anyMatch(acl -> acl.getRepositoryPath().startsWith(path)); } @@ -194,10 +202,6 @@ public class DefaultAclManager implements AclManager { private void addStatements(@NotNull SystemUser systemUser, @NotNull List<AccessControlEntry> authorizations, @NotNull Formatter formatter) { - if (authorizations.isEmpty()) { - return; - } - Map<AccessControlEntry, String> resourceEntries = new LinkedHashMap<>(); Map<AccessControlEntry, String> principalEntries = new LinkedHashMap<>(); @@ -241,10 +245,8 @@ public class DefaultAclManager implements AclManager { } @Override - public void addNodetypeRegistrationSentence(@Nullable String nodetypeRegistrationSentence) { - if (nodetypeRegistrationSentence != null) { - nodetypeRegistrationSentences.add(nodetypeRegistrationSentence); - } + public void addNodetypeRegistrationSentence(@NotNull String nodetypeRegistrationSentence) { + nodetypeRegistrationSentences.add(nodetypeRegistrationSentence); } @Override @@ -252,6 +254,7 @@ public class DefaultAclManager implements AclManager { this.privilegeDefinitions = privilegeDefinitions; } + @Override public void reset() { systemUsers.clear(); acls.clear(); diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java index b447493..236e401 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/MixinParser.java @@ -19,7 +19,6 @@ package org.apache.sling.feature.cpconverter.accesscontrol; import org.apache.jackrabbit.JcrConstants; import org.apache.sling.feature.cpconverter.shared.AbstractJcrNodeParser; import org.xml.sax.Attributes; -import org.xml.sax.SAXException; final class MixinParser extends AbstractJcrNodeParser<String> { private String mixins; @@ -34,8 +33,7 @@ final class MixinParser extends AbstractJcrNodeParser<String> { } @Override - protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) - throws SAXException { + protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) { // not needed } diff --git a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java index ffed8fc..49799aa 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/accesscontrol/PrimaryTypeParser.java @@ -18,7 +18,6 @@ package org.apache.sling.feature.cpconverter.accesscontrol; import org.apache.sling.feature.cpconverter.shared.AbstractJcrNodeParser; import org.xml.sax.Attributes; -import org.xml.sax.SAXException; final class PrimaryTypeParser extends AbstractJcrNodeParser<String> { @@ -27,14 +26,12 @@ final class PrimaryTypeParser extends AbstractJcrNodeParser<String> { } @Override - protected void onJcrRootNode(String uri, String localName, String qName, Attributes attributes, String primaryType) - throws SAXException { + protected void onJcrRootNode(String uri, String localName, String qName, Attributes attributes, String primaryType) { detectedPrimaryType = primaryType; } @Override - protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) - throws SAXException { + protected void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) { // not needed } diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java index 13a8fa0..404d111 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/BundleEntryHandler.java @@ -16,8 +16,16 @@ */ package org.apache.sling.feature.cpconverter.handlers; -import static java.util.Objects.requireNonNull; -import static org.apache.jackrabbit.vault.packaging.PackageProperties.NAME_VERSION; +import org.apache.felix.utils.manifest.Clause; +import org.apache.felix.utils.manifest.Parser; +import org.apache.jackrabbit.vault.fs.io.Archive; +import org.apache.jackrabbit.vault.fs.io.Archive.Entry; +import org.apache.sling.feature.ArtifactId; +import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter; +import org.apache.sling.feature.cpconverter.artifacts.InputStreamArtifactWriter; +import org.codehaus.plexus.util.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.osgi.framework.Constants; import java.io.IOException; import java.io.InputStream; @@ -29,18 +37,8 @@ import java.util.jar.Manifest; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.apache.felix.utils.manifest.Clause; -import org.apache.felix.utils.manifest.Parser; -import org.apache.jackrabbit.vault.fs.io.Archive; -import org.apache.jackrabbit.vault.fs.io.Archive.Entry; -import org.apache.sling.feature.ArtifactId; -import org.apache.sling.feature.cpconverter.ContentPackage2FeatureModelConverter; -import org.apache.sling.feature.cpconverter.artifacts.InputStreamArtifactWriter; -import org.codehaus.plexus.util.StringUtils; -import org.jetbrains.annotations.NotNull; -import org.osgi.framework.Constants; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import static java.util.Objects.requireNonNull; +import static org.apache.jackrabbit.vault.packaging.PackageProperties.NAME_VERSION; public final class BundleEntryHandler extends AbstractRegexEntryHandler { @@ -58,8 +56,6 @@ public final class BundleEntryHandler extends AbstractRegexEntryHandler { private static final String JAR_TYPE = "jar"; - private final Logger logger = LoggerFactory.getLogger(getClass()); - private final Pattern pomPropertiesPattern = Pattern.compile("META-INF/maven/[^/]+/[^/]+/pom.properties"); public BundleEntryHandler() { diff --git a/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java b/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java index 9c20896..30bb854 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/handlers/DefaultEntryHandlersManager.java @@ -36,9 +36,7 @@ public class DefaultEntryHandlersManager implements EntryHandlersManager { @Override public void addEntryHandler(@NotNull EntryHandler handler) { - if (handler != null) { - entryHandlers.add(handler); - } + entryHandlers.add(handler); } @Override @@ -48,7 +46,6 @@ public class DefaultEntryHandlersManager implements EntryHandlersManager { return entryHandler; } } - return null; } diff --git a/src/main/java/org/apache/sling/feature/cpconverter/shared/AbstractJcrNodeParser.java b/src/main/java/org/apache/sling/feature/cpconverter/shared/AbstractJcrNodeParser.java index c265f5f..54d4d60 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/shared/AbstractJcrNodeParser.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/shared/AbstractJcrNodeParser.java @@ -58,14 +58,14 @@ public abstract class AbstractJcrNodeParser<O> extends DefaultHandler { } } - protected void onJcrRootNode(String uri, String localName, String qName, Attributes attributes, String primaryType) throws SAXException { + protected void onJcrRootNode(String uri, String localName, String qName, Attributes attributes, String primaryType) { if (this.primaryTypes.contains(primaryType)) { detectedPrimaryType = primaryType; onJcrRootElement(uri, localName, qName, attributes); } } - protected abstract void onJcrRootElement(String uri, String localName, String qName, Attributes attributes) throws SAXException; + protected abstract void onJcrRootElement(String uri, String localName, String qName, Attributes attributes); protected abstract O getParsingResult(); } diff --git a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java index ec25f3a..b8eff4d 100644 --- a/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java +++ b/src/main/java/org/apache/sling/feature/cpconverter/vltpkg/RecollectorVaultPackageScanner.java @@ -47,7 +47,7 @@ public final class RecollectorVaultPackageScanner extends BaseVaultPackageScanne } @Override - protected void onFile(String path, Archive archive, Entry entry) throws Exception { + protected void onFile(@NotNull String path, @NotNull Archive archive, @NotNull Entry entry) throws Exception { for (EntryHandler handler : handlers) { if (handler.matches(path)) { handler.handle(path, archive, entry, converter); diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java index 52b66fa..8e74167 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/AclManagerTest.java @@ -37,11 +37,14 @@ import java.io.File; import java.io.StringReader; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.List; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -83,7 +86,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); @@ -125,7 +128,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); @@ -147,7 +150,7 @@ public class AclManagerTest { } @Test - public void testAddACLforUnknownUser() throws RepoInitParsingException { + public void testAddACLforUnknownUser() { // we expect this acl to not show up because the user is unknown aclManager.addAcl("acs-commons-on-deploy-scripts-service", newAcl(true, "jcr:read,crx:replicate,jcr:removeNode", "/home/users/system")); @@ -158,7 +161,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); @@ -177,7 +180,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -210,7 +213,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -240,7 +243,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -268,7 +271,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); } @Test(expected = IllegalStateException.class) @@ -283,7 +286,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); } @Test @@ -299,7 +302,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -327,7 +330,7 @@ public class AclManagerTest { FeaturesManager fm = Mockito.spy(new DefaultFeaturesManager(tempDir.toFile())); when(fm.getTargetFeature()).thenReturn(feature); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); diff --git a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java index 51d9884..33c390f 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/accesscontrol/EnforcePrincipalBasedTest.java @@ -38,13 +38,14 @@ import java.io.File; import java.io.StringReader; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; +import java.util.Collections; import java.util.Comparator; import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -92,8 +93,21 @@ public class EnforcePrincipalBasedTest { RepoPath accessControlledPath = new RepoPath("/content/feature"); aclManager.addAcl(systemUser.getId(), new AccessControlEntry(true, "jcr:read", accessControlledPath , false)); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); + } + + @Test + public void testMissingSupportedPath() { + AclManager aclManager = new DefaultAclManager(true, null); + aclManager.addSystemUser(systemUser); + + RepoPath accessControlledPath = new RepoPath("/content/feature"); + aclManager.addAcl(systemUser.getId(), new AccessControlEntry(true, "jcr:read", accessControlledPath , false)); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); + String txt = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT).getText(); + assertFalse(txt.contains("create service user user1 with path /home/users/system/some/subtree/intermediate")); + assertTrue(txt.contains("create service user user1 with path " + systemUser.getIntermediatePath())); } @Test @@ -109,13 +123,13 @@ public class EnforcePrincipalBasedTest { RepoPath accessControlledPath = new RepoPath("/content/feature"); aclManager.addAcl(systemUser.getId(), new AccessControlEntry(true, "jcr:read", accessControlledPath , false)); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); String expected = - "create service user user1 with path /home/users/system/intermediate" + System.lineSeparator() + + "create service user user1 with path " + systemUser.getIntermediatePath() + System.lineSeparator() + "create path /content/feature(sling:Folder)" + System.lineSeparator() + "set ACL for user1" + System.lineSeparator() + "allow jcr:read on /content/feature" + System.lineSeparator() + @@ -134,7 +148,7 @@ public class EnforcePrincipalBasedTest { accessControlledPath = new RepoPath("/content/feature"); aclManager.addAcl(systemUser.getId(), new AccessControlEntry(true, "jcr:read", accessControlledPath , false)); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -160,7 +174,7 @@ public class EnforcePrincipalBasedTest { RepoPath accessControlledPath = new RepoPath("/content/feature"); aclManager.addAcl(systemUser.getId(), new AccessControlEntry(true, "jcr:read", accessControlledPath , false)); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -186,7 +200,7 @@ public class EnforcePrincipalBasedTest { RepoPath accessControlledPath = new RepoPath("/content/feature"); aclManager.addAcl("user1", new AccessControlEntry(true, "jcr:read", accessControlledPath, true)); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); @@ -213,7 +227,7 @@ public class EnforcePrincipalBasedTest { AccessControlEntry acl = new AccessControlEntry(true, "jcr:read", accessControlledPath, true); aclManager.addAcl("user1", acl); - aclManager.addRepoinitExtension(Arrays.asList(assembler), fm); + aclManager.addRepoinitExtension(Collections.singletonList(assembler), fm); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/PrivilegesHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/PrivilegesHandlerTest.java index bfe3fa3..06a8a74 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/PrivilegesHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/PrivilegesHandlerTest.java @@ -16,15 +16,6 @@ */ package org.apache.sling.feature.cpconverter.handlers; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; - -import java.util.Arrays; - import org.apache.jackrabbit.vault.fs.config.DefaultMetaInf; import org.apache.jackrabbit.vault.fs.io.Archive; import org.apache.jackrabbit.vault.fs.io.Archive.Entry; @@ -40,6 +31,15 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import java.util.Collections; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + public class PrivilegesHandlerTest { private PrivilegesHandler handler; @@ -87,7 +87,7 @@ public class PrivilegesHandlerTest { handler.handle(path, archive, entry, converter); - converter.getAclManager().addRepoinitExtension(Arrays.asList(packageAssembler), featuresManager); + converter.getAclManager().addRepoinitExtension(Collections.singletonList(packageAssembler), featuresManager); Extension repoinitExtension = feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); assertNotNull(repoinitExtension); diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java index e1c4b48..c70d39d 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPolicyEntryHandlerTest.java @@ -17,7 +17,6 @@ package org.apache.sling.feature.cpconverter.handlers; import org.apache.sling.feature.Extension; -import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager; import org.apache.sling.feature.cpconverter.accesscontrol.Group; @@ -38,11 +37,8 @@ import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.spy; public final class RepPolicyEntryHandlerTest { diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java index 9849b4f..fa4e04e 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepPrincipalPolicyEntryHandlerTest.java @@ -17,7 +17,6 @@ package org.apache.sling.feature.cpconverter.handlers; import org.apache.sling.feature.Extension; -import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager; import org.apache.sling.feature.cpconverter.accesscontrol.SystemUser; @@ -37,7 +36,6 @@ import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public final class RepPrincipalPolicyEntryHandlerTest { @@ -102,7 +100,7 @@ public final class RepPrincipalPolicyEntryHandlerTest { @Test(expected = IllegalStateException.class) public void parsePolicyInSubtree() throws Exception { - parseAndSetRepoinit("service3", "random3").getRepoinitExtension(); + parseAndSetRepoinit("service3", "random3"); } @Test diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java index cf65878..1d79ef1 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/RepRepoPolicyEntryHandlerTest.java @@ -17,7 +17,6 @@ package org.apache.sling.feature.cpconverter.handlers; import org.apache.sling.feature.Extension; -import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; import org.apache.sling.feature.cpconverter.accesscontrol.DefaultAclManager; import org.apache.sling.feature.cpconverter.accesscontrol.SystemUser; @@ -34,9 +33,7 @@ import java.io.OutputStream; import java.io.StringReader; import java.util.List; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public class RepRepoPolicyEntryHandlerTest { diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java index 396766a..a950260 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/TestUtils.java @@ -33,7 +33,7 @@ import org.slf4j.LoggerFactory; import java.io.File; import java.io.InputStream; import java.io.OutputStream; -import java.util.Arrays; +import java.util.Collections; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; @@ -71,7 +71,7 @@ class TestUtils { when(packageAssembler.getEntry(anyString())).thenReturn(new File("itdoesnotexist")); - converter.getAclManager().addRepoinitExtension(Arrays.asList(packageAssembler), featuresManager); + converter.getAclManager().addRepoinitExtension(Collections.singletonList(packageAssembler), featuresManager); return feature.getExtensions().getByName(Extension.EXTENSION_NAME_REPOINIT); } } \ No newline at end of file diff --git a/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java b/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java index 5ba09b5..914a51c 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/handlers/UsersEntryHandlerTest.java @@ -16,21 +16,6 @@ */ package org.apache.sling.feature.cpconverter.handlers; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -import java.io.StringReader; -import java.util.List; - import org.apache.sling.feature.Extension; import org.apache.sling.feature.ExtensionType; import org.apache.sling.feature.cpconverter.accesscontrol.AclManager; @@ -44,6 +29,20 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import java.io.StringReader; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + public class UsersEntryHandlerTest { private UsersEntryHandler usersEntryHandler; diff --git a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/NodeTypesDetectionTest.java b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/NodeTypesDetectionTest.java index ea80563..6d36490 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/NodeTypesDetectionTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/NodeTypesDetectionTest.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.regex.Pattern; import org.apache.jackrabbit.vault.fs.io.ImportOptions; +import org.jetbrains.annotations.NotNull; import org.junit.Before; import org.junit.Test; @@ -47,7 +48,7 @@ public class NodeTypesDetectionTest { new BaseVaultPackageScanner(true) { - protected void addCdnPattern(Pattern cndPattern) { + protected void addCdnPattern(@NotNull Pattern cndPattern) { detectedCndFiles.add(cndPattern.pattern()); } diff --git a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/PackagesEventsEmitterTest.java b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/PackagesEventsEmitterTest.java index 1dce955..0eee6bf 100644 --- a/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/PackagesEventsEmitterTest.java +++ b/src/test/java/org/apache/sling/feature/cpconverter/vltpkg/PackagesEventsEmitterTest.java @@ -16,33 +16,24 @@ */ package org.apache.sling.feature.cpconverter.vltpkg; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; - -import java.io.File; -import java.io.StringWriter; -import java.util.Calendar; - -import javax.jcr.RepositoryException; -import javax.jcr.Session; - -import org.apache.jackrabbit.vault.fs.config.MetaInf; -import org.apache.jackrabbit.vault.fs.io.AccessControlHandling; -import org.apache.jackrabbit.vault.fs.io.Archive; -import org.apache.jackrabbit.vault.fs.io.ImportOptions; import org.apache.jackrabbit.vault.packaging.Dependency; import org.apache.jackrabbit.vault.packaging.PackageException; import org.apache.jackrabbit.vault.packaging.PackageId; -import org.apache.jackrabbit.vault.packaging.PackageProperties; import org.apache.jackrabbit.vault.packaging.PackageType; -import org.apache.jackrabbit.vault.packaging.SubPackageHandling; import org.apache.jackrabbit.vault.packaging.VaultPackage; import org.junit.Test; +import javax.jcr.RepositoryException; +import java.io.File; +import java.io.StringWriter; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class PackagesEventsEmitterTest { private static final PackageId ID_NESTED_CHILD = new PackageId("apache/sling", "nested-child", "1.0.0");
