Skip to content

Commit

Permalink
Issue #3514: Rename PkgControl to ImportControl
Browse files Browse the repository at this point in the history
  • Loading branch information
jochenvdv authored and romani committed Oct 26, 2016
1 parent e9b3b9c commit 5530d4c
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 97 deletions.
2 changes: 1 addition & 1 deletion config/pmd.xml
Expand Up @@ -152,7 +152,7 @@
<rule ref="rulesets/java/design.xml/ConfusingTernary">
<properties>
<!-- false positives on IF_ELSE-IF-ELSE-IF -->
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='XMLLogger']//MethodDeclarator[@Image='isReference'] | //ClassOrInterfaceDeclaration[@Image='DetailAST']//MethodDeclarator[@Image='addPreviousSibling'] | //ClassOrInterfaceDeclaration[@Image='AnnotationLocationCheck']//MethodDeclarator[@Image='checkAnnotations'] | //ClassOrInterfaceDeclaration[@Image='PkgControl']//MethodDeclarator[@Image='checkAccess'] | //ClassOrInterfaceDeclaration[@Image='HandlerFactory']//MethodDeclarator[@Image='getHandler']"/>
<property name="violationSuppressXPath" value="//ClassOrInterfaceDeclaration[@Image='XMLLogger']//MethodDeclarator[@Image='isReference'] | //ClassOrInterfaceDeclaration[@Image='DetailAST']//MethodDeclarator[@Image='addPreviousSibling'] | //ClassOrInterfaceDeclaration[@Image='AnnotationLocationCheck']//MethodDeclarator[@Image='checkAnnotations'] | //ClassOrInterfaceDeclaration[@Image='ImportControl']//MethodDeclarator[@Image='checkAccess'] | //ClassOrInterfaceDeclaration[@Image='HandlerFactory']//MethodDeclarator[@Image='getHandler']"/>
</properties>
</rule>
<rule ref="rulesets/java/design.xml/AccessorClassGeneration">
Expand Down
Expand Up @@ -26,13 +26,13 @@
import java.util.regex.Pattern;

/**
* Represents the a tree of guards for controlling whether packages are allowed
* to be used. Each instance must have a single parent or be the root node.
* Each instance may have zero or more children.
* Represents a tree of import rules for controlling whether packages or
* classes are allowed to be used. Each instance must have a single parent or
* be the root node. Each instance may have zero or more children.
*
* @author Oliver Burn
*/
class PkgControl {
class ImportControl {
/** The package separator: "." */
private static final String DOT = ".";
/** A pattern matching the package separator: "." */
Expand All @@ -41,10 +41,10 @@ class PkgControl {
private static final String DOT_REGEX = "\\.";
/** List of {@link AbstractImportRule} objects to check. */
private final Deque<AbstractImportRule> rules = new LinkedList<>();
/** List of children {@link PkgControl} objects. */
private final List<PkgControl> children = new ArrayList<>();
/** List of children {@link ImportControl} objects. */
private final List<ImportControl> children = new ArrayList<>();
/** The parent. Null indicates we are the root node. */
private final PkgControl parent;
private final ImportControl parent;
/** The full package name for the node. */
private final String fullPackage;
/**
Expand All @@ -62,7 +62,7 @@ class PkgControl {
* @param pkgName the name of the package.
* @param regex flags interpretation of pkgName as regex pattern.
*/
PkgControl(final String pkgName, final boolean regex) {
ImportControl(final String pkgName, final boolean regex) {
parent = null;
this.regex = regex;
if (regex) {
Expand All @@ -85,7 +85,7 @@ class PkgControl {
* @param subPkg the sub package name.
* @param regex flags interpretation of subPkg as regex pattern.
*/
PkgControl(final PkgControl parent, final String subPkg, final boolean regex) {
ImportControl(final ImportControl parent, final String subPkg, final boolean regex) {
this.parent = parent;
if (regex || parent.regex) {
// regex gets inherited
Expand Down Expand Up @@ -180,7 +180,7 @@ private static Pattern createPatternForExactMatch(String expression) {
}

/**
* Adds a {@link AbstractImportRule} to the node.
* Adds an {@link AbstractImportRule} to the node.
* @param rule the rule to be added.
*/
protected void addImportRule(final AbstractImportRule rule) {
Expand All @@ -192,15 +192,15 @@ protected void addImportRule(final AbstractImportRule rule) {
* @param forPkg the package to search for.
* @return the finest match, or null if no match at all.
*/
public PkgControl locateFinest(final String forPkg) {
PkgControl finestMatch = null;
public ImportControl locateFinest(final String forPkg) {
ImportControl finestMatch = null;
// Check if we are a match.
if (matchesAtFront(forPkg)) {
// If there won't be match so I am the best there is.
finestMatch = this;
// Check if any of the children match.
for (PkgControl child : children) {
final PkgControl match = child.locateFinest(forPkg);
for (ImportControl child : children) {
final ImportControl match = child.locateFinest(forPkg);
if (match != null) {
finestMatch = match;
break;
Expand Down Expand Up @@ -239,12 +239,12 @@ private boolean matchesAtFrontNoRegex(final String pkg) {
}

/**
* Returns whether a package is allowed to be used. The algorithm checks
* with the current node for a result, and if none is found then calls
* its parent looking for a match. This will recurse looking for match.
* If there is no clear result then {@link AccessResult#UNKNOWN} is
* returned.
* @param forImport the package to check on.
* Returns whether a package or class is allowed to be imported.
* The algorithm checks with the current node for a result, and if none is
* found then calls its parent looking for a match. This will recurse
* looking for match. If there is no clear result then
* {@link AccessResult#UNKNOWN} is returned.
* @param forImport the import to check on.
* @param inPkg the package doing the import.
* @return an {@link AccessResult}.
*/
Expand Down Expand Up @@ -274,7 +274,7 @@ else if (parent == null) {
private AccessResult localCheckAccess(final String forImport,
final String inPkg) {
for (AbstractImportRule r : rules) {
// Check if a PkgImportRule is only meant to be applied locally.
// Check if an import rule is only meant to be applied locally.
if (r.isLocalOnly() && !matchesExactly(inPkg)) {
continue;
}
Expand Down
Expand Up @@ -76,15 +76,15 @@ public class ImportControlCheck extends AbstractCheck implements ExternalResourc
private String fileLocation;

/** The root package controller. */
private PkgControl root;
private ImportControl root;
/** The package doing the import. */
private String inPkg;

/**
* The package controller for the current file. Used for performance
* optimisation.
*/
private PkgControl currentLeaf;
private ImportControl currentLeaf;

@Override
public int[] getDefaultTokens() {
Expand Down
Expand Up @@ -70,8 +70,8 @@ final class ImportControlLoader extends AbstractLoader {
/** Qualified name for element 'allow'. */
private static final String ALLOW_ELEMENT_NAME = "allow";

/** Used to hold the {@link PkgControl} objects. */
private final Deque<PkgControl> stack = new ArrayDeque<>();
/** Used to hold the {@link ImportControl} objects. */
private final Deque<ImportControl> stack = new ArrayDeque<>();

static {
DTD_RESOURCE_BY_ID.put(DTD_PUBLIC_ID_1_0, DTD_RESOURCE_NAME_1_0);
Expand All @@ -97,12 +97,12 @@ public void startElement(final String namespaceUri,
if ("import-control".equals(qName)) {
final String pkg = safeGet(attributes, PKG_ATTRIBUTE_NAME);
final boolean regex = containsRegexAttribute(attributes);
stack.push(new PkgControl(pkg, regex));
stack.push(new ImportControl(pkg, regex));
}
else if (SUBPACKAGE_ELEMENT_NAME.equals(qName)) {
final String name = safeGet(attributes, "name");
final boolean regex = containsRegexAttribute(attributes);
stack.push(new PkgControl(stack.peek(), name, regex));
stack.push(new ImportControl(stack.peek(), name, regex));
}
else if (ALLOW_ELEMENT_NAME.equals(qName) || "disallow".equals(qName)) {
// Need to handle either "pkg" or "class" attribute.
Expand Down Expand Up @@ -148,10 +148,10 @@ public void endElement(final String namespaceUri, final String localName,
/**
* Loads the import control file from a file.
* @param uri the uri of the file to load.
* @return the root {@link PkgControl} object.
* @return the root {@link ImportControl} object.
* @throws CheckstyleException if an error occurs.
*/
public static PkgControl load(final URI uri) throws CheckstyleException {
public static ImportControl load(final URI uri) throws CheckstyleException {
final InputStream inputStream;
try {
inputStream = uri.toURL().openStream();
Expand All @@ -170,10 +170,10 @@ public static PkgControl load(final URI uri) throws CheckstyleException {
* Loads the import control file from a {@link InputSource}.
* @param source the source to load from.
* @param uri uri of the source being loaded.
* @return the root {@link PkgControl} object.
* @return the root {@link ImportControl} object.
* @throws CheckstyleException if an error occurs.
*/
private static PkgControl load(final InputSource source,
private static ImportControl load(final InputSource source,
final URI uri) throws CheckstyleException {
try {
final ImportControlLoader loader = new ImportControlLoader();
Expand All @@ -190,9 +190,9 @@ private static PkgControl load(final InputSource source,
}

/**
* @return the root {@link PkgControl} object loaded.
* @return the root {@link ImportControl} object loaded.
*/
private PkgControl getRoot() {
private ImportControl getRoot() {
return stack.peek();
}

Expand Down
Expand Up @@ -40,22 +40,22 @@ private static String getPath(String filename) {

@Test
public void testLoad() throws CheckstyleException {
final PkgControl root =
final ImportControl root =
ImportControlLoader.load(new File(getPath("import-control_complete.xml")).toURI());
assertNotNull(root);
}

@Test(expected = CheckstyleException.class)
public void testWrongFormatUri() throws Exception {
final PkgControl root =
final ImportControl root =
ImportControlLoader.load(new URI("aaa://"
+ getPath("import-control_complete.xml")));
assertNotNull(root);
}

@Test
public void testExtraElementInConfig() throws Exception {
final PkgControl root =
final ImportControl root =
ImportControlLoader.load(
new File(getPath("import-control_WithNewElement.xml")).toURI());
assertNotNull(root);
Expand Down
Expand Up @@ -24,40 +24,40 @@

import org.junit.Test;

public class PkgControlRegExpInPkgTest {
private final PkgControl pcRoot = new PkgControl("com\\.[^.]+\\.courtlink", true);
private final PkgControl pcCommon = new PkgControl(pcRoot, "com+on", true);
public class ImportControlRegExpInPkgTest {
private final ImportControl icRoot = new ImportControl("com\\.[^.]+\\.courtlink", true);
private final ImportControl icCommon = new ImportControl(icRoot, "com+on", true);

@Test
public void testRegExpInRootIsConsidered() {
assertNull(pcRoot.locateFinest("com"));
assertNull(pcRoot.locateFinest("com/hurz/courtlink"));
assertNull(pcRoot.locateFinest("com.hurz.hurz.courtlink"));
assertEquals(pcRoot, pcRoot
assertNull(icRoot.locateFinest("com"));
assertNull(icRoot.locateFinest("com/hurz/courtlink"));
assertNull(icRoot.locateFinest("com.hurz.hurz.courtlink"));
assertEquals(icRoot, icRoot
.locateFinest("com.hurz.courtlink.domain"));
assertEquals(pcRoot, pcRoot
assertEquals(icRoot, icRoot
.locateFinest("com.kazgroup.courtlink.domain"));
}

@Test
public void testRegExpInSubpackageIsConsidered() {
assertEquals(pcCommon, pcRoot
assertEquals(icCommon, icRoot
.locateFinest("com.kazgroup.courtlink.common.api"));
assertEquals(pcCommon, pcRoot
assertEquals(icCommon, icRoot
.locateFinest("com.kazgroup.courtlink.comon.api"));
}

@Test
public void testEnsureTrailingDot() {
assertNull(pcRoot.locateFinest("com.kazgroup.courtlinkkk"));
assertNull(pcRoot.locateFinest("com.kazgroup.courtlink/common.api"));
assertNull(icRoot.locateFinest("com.kazgroup.courtlinkkk"));
assertNull(icRoot.locateFinest("com.kazgroup.courtlink/common.api"));
}

@Test
public void testAlternationInParentIsHandledCorrectly() {
// the regular expression has to be adjusted to (com\.foo|com\.bar)
final PkgControl root = new PkgControl("com\\.foo|com\\.bar", true);
final PkgControl common = new PkgControl(root, "common", false);
final ImportControl root = new ImportControl("com\\.foo|com\\.bar", true);
final ImportControl common = new ImportControl(root, "common", false);
assertEquals(root, root.locateFinest("com.foo"));
assertEquals(common, root.locateFinest("com.foo.common"));
assertEquals(root, root.locateFinest("com.bar"));
Expand All @@ -67,8 +67,8 @@ public void testAlternationInParentIsHandledCorrectly() {
@Test
public void testAlternationInParentIfUserCaresForIt() {
// the regular expression has to be adjusted to (com\.foo|com\.bar)
final PkgControl root = new PkgControl("(com\\.foo|com\\.bar)", true);
final PkgControl common = new PkgControl(root, "common", false);
final ImportControl root = new ImportControl("(com\\.foo|com\\.bar)", true);
final ImportControl common = new ImportControl(root, "common", false);
assertEquals(root, root.locateFinest("com.foo"));
assertEquals(common, root.locateFinest("com.foo.common"));
assertEquals(root, root.locateFinest("com.bar"));
Expand All @@ -77,16 +77,16 @@ public void testAlternationInParentIfUserCaresForIt() {

@Test
public void testAlternationInSubpackageIsHandledCorrectly() {
final PkgControl root = new PkgControl("org.somewhere", false);
final ImportControl root = new ImportControl("org.somewhere", false);
// the regular expression has to be adjusted to (foo|bar)
final PkgControl subpackages = new PkgControl(root, "foo|bar", true);
final ImportControl subpackages = new ImportControl(root, "foo|bar", true);
assertEquals(root, root.locateFinest("org.somewhere"));
assertEquals(subpackages, root.locateFinest("org.somewhere.foo"));
assertEquals(subpackages, root.locateFinest("org.somewhere.bar"));
}

@Test
public void testUnknownPkg() {
assertNull(pcRoot.locateFinest("net.another"));
assertNull(icRoot.locateFinest("net.another"));
}
}
Expand Up @@ -25,66 +25,66 @@
import org.junit.Before;
import org.junit.Test;

public class PkgControlRegExpTest {
private final PkgControl pcRoot = new PkgControl("com.kazgroup.courtlink", false);
private final PkgControl pcCommon = new PkgControl(pcRoot, "common", false);
public class ImportControlRegExpTest {
private final ImportControl icRoot = new ImportControl("com.kazgroup.courtlink", false);
private final ImportControl icCommon = new ImportControl(icRoot, "common", false);

@Before
public void setUp() {
pcRoot.addImportRule(
icRoot.addImportRule(
new PkgImportRule(false, false, ".*\\.(spring|lui)framework", false, true));
pcRoot.addImportRule(
icRoot.addImportRule(
new PkgImportRule(false, false, "org\\.hibernate", false, true));
pcRoot.addImportRule(
icRoot.addImportRule(
new PkgImportRule(true, false, "org\\.(apache|lui)\\.commons", false, true));

pcCommon.addImportRule(
icCommon.addImportRule(
new PkgImportRule(true, false, "org\\.h.*", false, true));
}

@Test
public void testLocateFinest() {
assertEquals(pcRoot, pcRoot
assertEquals(icRoot, icRoot
.locateFinest("com.kazgroup.courtlink.domain"));
assertEquals(pcCommon, pcRoot
assertEquals(icCommon, icRoot
.locateFinest("com.kazgroup.courtlink.common.api"));
assertNull(pcRoot.locateFinest("com"));
assertNull(icRoot.locateFinest("com"));
}

@Test
public void testCheckAccess() {
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"org.springframework.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"org.luiframework.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"de.springframework.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"de.luiframework.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.ALLOWED, pcCommon
assertEquals(AccessResult.ALLOWED, icCommon
.checkAccess("org.apache.commons.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.ALLOWED, pcCommon
assertEquals(AccessResult.ALLOWED, icCommon
.checkAccess("org.lui.commons.something",
"com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"org.apache.commons", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"org.lui.commons", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.ALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.ALLOWED, icCommon.checkAccess(
"org.hibernate.something", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcCommon.checkAccess(
assertEquals(AccessResult.DISALLOWED, icCommon.checkAccess(
"com.badpackage.something", "com.kazgroup.courtlink.common"));
assertEquals(AccessResult.DISALLOWED, pcRoot.checkAccess(
assertEquals(AccessResult.DISALLOWED, icRoot.checkAccess(
"org.hibernate.something", "com.kazgroup.courtlink"));
}

@Test
public void testUnknownPkg() {
assertNull(pcRoot.locateFinest("net.another"));
assertNull(icRoot.locateFinest("net.another"));
}
}

0 comments on commit 5530d4c

Please sign in to comment.