Skip to content

Commit

Permalink
Upgrade to oakpal-1.1.13 for more maintainable unit tests. (#1650)
Browse files Browse the repository at this point in the history 
* Upgrade to oakpal-1.1.13 for more maintainable unit tests.

* Improvements to code coverage.
  • Loading branch information
@adamcin @justinedelson
adamcin authored and justinedelson committed Feb 6, 2019
1 parent 8b477d1 commit d87bef3
Show file tree
Hide file tree
Showing 15 changed files with 178 additions and 123 deletions.
5 changes: 4 additions & 1 deletion bundle/src/main/java/com/adobe/acs/commons/rewriter/impl/XMLParserGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ public final class XMLParserGenerator implements Generator {
private ContentHandler contentHandler;

public XMLParserGenerator() throws ParserConfigurationException, SAXException {
SAXParserFactory factory = SAXParserFactory.newInstance();
this(SAXParserFactory.newInstance());
}

public XMLParserGenerator(final SAXParserFactory factory) throws ParserConfigurationException, SAXException {
factory.setNamespaceAware(true);

saxParser = factory.newSAXParser();
Expand Down
13 changes: 11 additions & 2 deletions bundle/src/main/java/com/adobe/acs/commons/rewriter/impl/XMLParserGeneratorFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
*/
package com.adobe.acs.commons.rewriter.impl;

import javax.xml.parsers.SAXParserFactory;

import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Property;
import org.apache.felix.scr.annotations.Service;
Expand All @@ -36,12 +38,19 @@ public final class XMLParserGeneratorFactory implements GeneratorFactory {

@Override
public Generator createGenerator() {
return createGenerator(null);
}

Generator createGenerator(final SAXParserFactory saxParserFactory) {
try {
return new XMLParserGenerator();
if (saxParserFactory == null) {
return new XMLParserGenerator();
} else {
return new XMLParserGenerator(saxParserFactory);
}
} catch (Exception e) {
log.error("Unable to create parser", e);
return null;
}
}

}
44 changes: 43 additions & 1 deletion bundle/src/test/java/com/adobe/acs/commons/rewriter/impl/XMLParserGeneratorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@

import java.io.PrintWriter;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
Expand All @@ -32,6 +37,9 @@
import org.xml.sax.Attributes;
import org.xml.sax.ContentHandler;
import org.xml.sax.Locator;
import org.xml.sax.SAXException;
import org.xml.sax.SAXNotRecognizedException;
import org.xml.sax.SAXNotSupportedException;

@RunWith(MockitoJUnitRunner.class)
public class XMLParserGeneratorTest {
Expand All @@ -46,13 +54,15 @@ public void test() throws Exception {

XMLParserGeneratorFactory factory = new XMLParserGeneratorFactory();
XMLParserGenerator generator = (XMLParserGenerator) factory.createGenerator();
// nothing to do, nothing to mock...
generator.init(null, null);
generator.setContentHandler(contentHandler);

PrintWriter printWriter = generator.getWriter();
printWriter.println("<?xml version=\"1.0\" encoding=\"utf-8\"?>");
printWriter.println("<fo:root xmlns:fo=\"http://www.w3.org/1999/XSL/Format\"></fo:root>");

generator.finished();
generator.dispose();

verify(contentHandler).startDocument();
verify(contentHandler).setDocumentLocator((Locator) anyObject());
Expand All @@ -66,4 +76,36 @@ public void test() throws Exception {
verify(contentHandler).endDocument();
verifyNoMoreInteractions(contentHandler);
}

@Test
public void testParserException() {
SAXParserFactory fakeParserFactory = new SAXParserFactory() {
@Override
public SAXParser newSAXParser() throws ParserConfigurationException, SAXException {
throw new ParserConfigurationException("failers gonna fail.");
}

@Override
public void setFeature(final String name, final boolean value)
throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException {
}

@Override
public boolean getFeature(final String name)
throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException {
return false;
}
};

XMLParserGeneratorFactory factory = new XMLParserGeneratorFactory();
XMLParserGenerator generator = (XMLParserGenerator) factory.createGenerator(fakeParserFactory);
Assert.assertNull("generator is null when parser factory throws config exception.", generator);
}

@Test
public void testOwnParser() {
XMLParserGeneratorFactory factory = new XMLParserGeneratorFactory();
XMLParserGenerator generator = (XMLParserGenerator) factory.createGenerator(SAXParserFactory.newInstance());
Assert.assertNotNull("generator is not null with own default sax parser.", generator);
}
}
44 changes: 18 additions & 26 deletions ...in/java/com/adobe/acs/commons/oakpal/checks/AcsCommonsAuthorizableCompatibilityCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import net.adamcin.oakpal.core.ProgressCheck;
import net.adamcin.oakpal.core.ProgressCheckFactory;
import net.adamcin.oakpal.core.SimpleProgressCheck;
import net.adamcin.oakpal.core.SimpleViolation;
import net.adamcin.oakpal.core.Violation;
import net.adamcin.oakpal.core.checks.Rule;
import org.apache.jackrabbit.api.JackrabbitSession;
Expand All @@ -45,54 +44,47 @@
* compatibility check.</dd>
* </dl>
*/
public class AcsCommonsAuthorizableCompatibilityCheck implements ProgressCheckFactory {
public final class AcsCommonsAuthorizableCompatibilityCheck implements ProgressCheckFactory {
public static final String NT_REP_AUTHORIZABLE = "rep:Authorizable";
public static final String CONFIG_SCOPE_IDS = "scopeIds";

class Check extends SimpleProgressCheck {
static final class Check extends SimpleProgressCheck {
private final List<Rule> scopeIds;

public Check(final List<Rule> scopeIds) {
Check(final List<Rule> scopeIds) {
this.scopeIds = scopeIds;
}

@Override
public String getCheckName() {
return AcsCommonsAuthorizableCompatibilityCheck.this.getClass().getSimpleName();
return AcsCommonsAuthorizableCompatibilityCheck.class.getSimpleName();
}

@Override
public void importedPath(final PackageId packageId, final String path, final Node node)
throws RepositoryException {
// fast check for authorizables
if (node.isNodeType(NT_REP_AUTHORIZABLE)) {
UserManager userManager = ((JackrabbitSession) node.getSession()).getUserManager();
Authorizable authz = userManager.getAuthorizableByPath(path);
final UserManager userManager = ((JackrabbitSession) node.getSession()).getUserManager();
final Authorizable authz = userManager.getAuthorizableByPath(path);

// if an authorizable is not loaded from the path, short circuit.
if (authz == null) {
return;
}
if (authz != null) {
final String id = authz.getID();

final String id = authz.getID();
// check for inclusion based on authorizableId
Rule lastMatched = Rule.lastMatch(scopeIds, id);

// check for inclusion based on authorizableId
Rule lastMatched = Rule.fuzzyDefaultAllow(scopeIds);
for (Rule scopeId : scopeIds) {
if (scopeId.matches(id)) {
lastMatched = scopeId;
// if id is excluded, short circuit
if (lastMatched.isExclude()) {
return;
}
}

// if id is excluded, short circuit
if (lastMatched.isDeny()) {
return;
}

if (authz.getID().startsWith("acs-commons")) {
reportViolation(new SimpleViolation(Violation.Severity.MAJOR,
String.format("%s: reserved ID prefix [%s]", path, authz.getID()),
packageId));
if (authz.getID().startsWith("acs-commons")) {
reportViolation(Violation.Severity.MAJOR,
String.format("%s: reserved ID prefix [%s]", path, authz.getID()),
packageId);
}
}
}
}
Expand Down
53 changes: 22 additions & 31 deletions oakpal-checks/src/main/java/com/adobe/acs/commons/oakpal/checks/ContentClassifications.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import net.adamcin.oakpal.core.ProgressCheck;
import net.adamcin.oakpal.core.ProgressCheckFactory;
import net.adamcin.oakpal.core.SimpleProgressCheck;
import net.adamcin.oakpal.core.SimpleViolation;
import net.adamcin.oakpal.core.Violation;
import net.adamcin.oakpal.core.checks.Rule;
import org.apache.jackrabbit.vault.packaging.PackageId;
Expand Down Expand Up @@ -94,14 +93,14 @@ public ProgressCheck newInstance(final JSONObject jsonObject) {
return new Check(libsPathPrefix, severity, scopePaths, searchPaths);
}

class Check extends SimpleProgressCheck {
static final class Check extends SimpleProgressCheck {
final String libsPathPrefix;
final Violation.Severity severity;
final List<Rule> scopePaths;
final List<String> searchPaths;

public Check(final String libsPathPrefix, final Violation.Severity severity,
final List<Rule> scopePaths, final List<String> searchPaths) {
Check(final String libsPathPrefix, final Violation.Severity severity,
final List<Rule> scopePaths, final List<String> searchPaths) {
this.libsPathPrefix = libsPathPrefix;
this.severity = severity;
this.scopePaths = scopePaths;
Expand All @@ -110,7 +109,7 @@ public Check(final String libsPathPrefix, final Violation.Severity severity,

@Override
public String getCheckName() {
return ContentClassifications.this.getClass().getSimpleName();
return ContentClassifications.class.getSimpleName();
}

@Override
Expand All @@ -125,15 +124,10 @@ public void importedPath(final PackageId packageId, final String path, final Nod

// default to ALLOW ALL
// if first rule is allow, change default to DENY ALL
Rule lastMatched = Rule.fuzzyDefaultAllow(scopePaths);
for (Rule rule : scopePaths) {
if (rule.matches(path)) {
lastMatched = rule;
}
}
Rule lastMatched = Rule.lastMatch(scopePaths, path);

// if path is denied from scope, short circuit.
if (lastMatched.isDeny()) {
if (lastMatched.isExclude()) {
return;
}

Expand All @@ -160,8 +154,8 @@ void checkOverlay(final PackageId packageId, final String path, final Node node)
final String libsRt = libsPathPrefix + relPath;

assertClassifications(node.getSession(), libsRt, AreaType.ALLOWED_FOR_RESOURCE_SUPER_TYPE)
.ifPresent(message -> reportViolation(new SimpleViolation(severity,
String.format("%s [restricted overlay]: %s", path, message), packageId)));
.ifPresent(message -> reportViolation(severity,
String.format("%s [restricted overlay]: %s", path, message), packageId));

}

Expand All @@ -178,9 +172,9 @@ void checkResourceType(final PackageId packageId, final String path, final Node
}

assertClassifications(node.getSession(), libsRt, AreaType.ALLOWED_FOR_RESOURCE_TYPE)
.ifPresent(message -> reportViolation(new SimpleViolation(severity,
.ifPresent(message -> reportViolation(severity,
String.format("%s [restricted resource type]: %s", path, message),
packageId)));
packageId));
}
}
}
Expand All @@ -198,17 +192,17 @@ void checkResourceSuperType(final PackageId packageId, final String path, final
}

assertClassifications(node.getSession(), libsRst, AreaType.ALLOWED_FOR_RESOURCE_SUPER_TYPE)
.ifPresent(message -> reportViolation(new SimpleViolation(severity,
.ifPresent(message -> reportViolation(severity,
String.format("%s [restricted super type]: %s", path, message),
packageId)));
packageId));
}
}
}

Optional<String> assertClassifications(final Session session, final String libsPath,
final Set<AreaType> allowedAreas) throws RepositoryException {
final Node leaf = getLibsLeaf(session, libsPath);
if (leaf != null) {
if (libsPath.startsWith(libsPathPrefix)) {
final Node leaf = getLeafNode(session, libsPath);
final AreaType leafArea = AreaType.fromNode(leaf);
if (leafArea == AreaType.FINAL && libsPath.startsWith(leaf.getPath() + "/")) {
return Optional.of(String.format("%s is implicitly marked %s", libsPath, AreaType.INTERNAL));
Expand All @@ -221,22 +215,19 @@ Optional<String> assertClassifications(final Session session, final String libsP
return Optional.empty();
}

Node getLibsLeaf(final Session session, final String absPath) throws RepositoryException {
Node getLeafNode(final Session session, final String absPath) throws RepositoryException {
if ("/".equals(absPath)) {
return session.getRootNode();
} else if (absPath.startsWith(libsPathPrefix)) {
} else {
final String parentPath = Text.getRelativeParent(absPath, 1, true);
Node parent = getLibsLeaf(session, parentPath);
if (parent != null) {
final String name = Text.getName(absPath, true);
if (parent.getPath().equals(parentPath) && parent.hasNode(name)) {
return parent.getNode(name);
} else {
return parent;
}
final Node parent = getLeafNode(session, parentPath);
final String name = Text.getName(absPath, true);
if (parent.getPath().equals(parentPath) && parent.hasNode(name)) {
return parent.getNode(name);
} else {
return parent;
}
}
return null;
}
}

Expand Down
Loading

0 comments on commit d87bef3

Please sign in to comment.