GT-2961 - CategoryPath - Changes from first review

This commit is contained in:
ghizard 2019-07-09 17:43:06 -04:00
parent ea88558241
commit f62b220e09
2 changed files with 75 additions and 94 deletions

View file

@ -17,6 +17,8 @@ package ghidra.program.model.data;
import java.util.*; import java.util.*;
import org.apache.commons.collections4.CollectionUtils;
/** /**
* A category path is the full path to a particular data type * A category path is the full path to a particular data type
*/ */
@ -31,8 +33,8 @@ public class CategoryPath implements Comparable<CategoryPath> {
private static final String ILLEGAL_STRING = DELIMITER_STRING + DELIMITER_STRING; private static final String ILLEGAL_STRING = DELIMITER_STRING + DELIMITER_STRING;
private static final int DIFF = ESCAPED_DELIMITER_STRING.length() - DELIMITER_STRING.length(); private static final int DIFF = ESCAPED_DELIMITER_STRING.length() - DELIMITER_STRING.length();
// parentPath can only be null for ROOT // parent can only be null for ROOT
private final CategoryPath parentPath; private final CategoryPath parent;
private final String name; private final String name;
/** /**
@ -63,8 +65,8 @@ public class CategoryPath implements Comparable<CategoryPath> {
* Constructor for internal creation of ROOT. * Constructor for internal creation of ROOT.
*/ */
private CategoryPath() { private CategoryPath() {
// parentPath can only be null for ROOT // parent can only be null for ROOT
parentPath = null; parent = null;
name = ""; name = "";
} }
@ -72,41 +74,41 @@ public class CategoryPath implements Comparable<CategoryPath> {
* Construct a CategoryPath from a parent and a hierarchical array of strings where each * Construct a CategoryPath from a parent and a hierarchical array of strings where each
* string is the name of a category in the category path. * string is the name of a category in the category path.
* *
* @param parentPathIn the parent CategoryPath. Choose {@code ROOT} if needed. * @param parent the parent CategoryPath. Choose {@code ROOT} if needed.
* @param categoryPath the array of names of categories. * @param subPathElements the array of names of sub-categories of the parent.
* @throws IllegalArgumentException if the given array is null or empty. * @throws IllegalArgumentException if the given array is null or empty.
*/ */
public CategoryPath(CategoryPath parentPathIn, String... categoryPath) { public CategoryPath(CategoryPath parent, String... subPathElements) {
this(parentPathIn, Arrays.asList(categoryPath)); this(parent, Arrays.asList(subPathElements));
} }
/** /**
* Construct a CategoryPath from a parent and a hierarchical list of strings where each * Construct a CategoryPath from a parent and a hierarchical list of strings where each
* string is the name of a category in the category path. * string is the name of a category in the category path.
* *
* @param parentPathIn the parent CategoryPath. Choose {@code ROOT} if needed. * @param parent the parent CategoryPath. Choose {@code ROOT} if needed.
* @param categoryList the hierarchical array of categories to place after parentPath. * @param subPathElements the hierarchical array of sub-categories of the parent.
* @throws IllegalArgumentException if the given list is null or empty. * @throws IllegalArgumentException if the given list is null or empty.
*/ */
public CategoryPath(CategoryPath parentPathIn, List<String> categoryList) { public CategoryPath(CategoryPath parent, List<String> subPathElements) {
Objects.requireNonNull(parentPathIn); Objects.requireNonNull(parent);
if (categoryList == null || categoryList.isEmpty()) { if (CollectionUtils.isEmpty(subPathElements)) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
"Category list must contain at least one string name!"); "Category list must contain at least one string name!");
} }
name = categoryList.get(categoryList.size() - 1); name = subPathElements.get(subPathElements.size() - 1);
if (categoryList.size() == 1) { if (subPathElements.size() == 1) {
parentPath = parentPathIn; this.parent = parent;
} }
else { else {
parentPath = this.parent =
new CategoryPath(parentPathIn, categoryList.subList(0, categoryList.size() - 1)); new CategoryPath(parent, subPathElements.subList(0, subPathElements.size() - 1));
} }
} }
/** /**
* Creates a category path given a forward-slash-delimited string (e.g., {@code "/aa/bb"}). * Creates a category path given a forward-slash-delimited string (e.g., {@code "/aa/bb"}).
* If an individual component has one or more '/' characters in it, then it can be * If an individual path component has one or more '/' characters in it, then it can be
* <I><B>escaped</B></I> using the {@link #escapeString(String)} utility method. The * <I><B>escaped</B></I> using the {@link #escapeString(String)} utility method. The
* {@link #unescapeString(String)} method can be used to unescape an individual component. * {@link #unescapeString(String)} method can be used to unescape an individual component.
* <P> * <P>
@ -115,39 +117,50 @@ public class CategoryPath implements Comparable<CategoryPath> {
* is OK is in simple cases where a literal is passed in, such as in testing methods or in * is OK is in simple cases where a literal is passed in, such as in testing methods or in
* scripts. * scripts.
* @param path category path string, delimited with '/' characters where individual components * @param path category path string, delimited with '/' characters where individual components
* may have '/' characters escaped. * may have '/' characters escaped. Must start with the '/' character.
*/ */
// NOTE: We purposefully did not create a constructor that takes varags only, as that
// constructor, called with a single argument that would not be escaped, would conflict with
// this constructor, which requires an escaped argument.
public CategoryPath(String path) { public CategoryPath(String path) {
if (path == null || path.length() == 0 || path.equals(DELIMITER_STRING)) { if (path == null || path.length() == 0 || path.equals(DELIMITER_STRING)) {
// parentPath can only be null for ROOT // parent can only be null for ROOT
parentPath = null; parent = null;
name = ""; name = "";
return; return;
} }
else if (path.charAt(0) != DELIMITER_CHAR) { else if (path.charAt(0) != DELIMITER_CHAR) {
throw new IllegalArgumentException("Paths must start with " + DELIMITER_STRING); throw new IllegalArgumentException("Paths must start with " + DELIMITER_STRING);
} }
else if (path.charAt(path.length() - 1) == DELIMITER_CHAR && else if (endsWithNonEscapedDelimiter(path)) {
path.lastIndexOf(ESCAPED_DELIMITER_STRING) != path.length() -
ESCAPED_DELIMITER_STRING.length()) {
throw new IllegalArgumentException("Paths must not end with " + DELIMITER_STRING); throw new IllegalArgumentException("Paths must not end with " + DELIMITER_STRING);
} }
else if (path.indexOf(ILLEGAL_STRING) >= 0) { else if (path.indexOf(ILLEGAL_STRING) >= 0) {
throw new IllegalArgumentException("Paths must have non-empty elements"); throw new IllegalArgumentException("Paths must have non-empty elements");
} }
else {
int escapedIndex = path.length(); int delimiterIndex = findIndexOfLastNonEscapedDelimiter(path);
int delimiterIndex = path.length(); this.parent = new CategoryPath(path.substring(0, delimiterIndex));
this.name = unescapeString(path.substring(delimiterIndex + 1));
}
private boolean endsWithNonEscapedDelimiter(String string) {
return (string.charAt(string.length() - 1) == DELIMITER_CHAR &&
string.lastIndexOf(ESCAPED_DELIMITER_STRING) != string.length() -
ESCAPED_DELIMITER_STRING.length());
}
private int findIndexOfLastNonEscapedDelimiter(String string) {
int escapedIndex = string.length();
int delimiterIndex = escapedIndex;
while (delimiterIndex > 0) { while (delimiterIndex > 0) {
escapedIndex = path.lastIndexOf(ESCAPED_DELIMITER_STRING, escapedIndex - 1); escapedIndex = string.lastIndexOf(ESCAPED_DELIMITER_STRING, escapedIndex - 1);
delimiterIndex = path.lastIndexOf(DELIMITER_CHAR, delimiterIndex - 1); delimiterIndex = string.lastIndexOf(DELIMITER_CHAR, delimiterIndex - 1);
if (delimiterIndex != escapedIndex + DIFF) { if (delimiterIndex != escapedIndex + DIFF) {
break; break;
} }
} }
this.parentPath = new CategoryPath(path.substring(0, delimiterIndex)); return delimiterIndex;
this.name = unescapeString(path.substring(delimiterIndex + 1));
}
} }
/** /**
@ -155,8 +168,8 @@ public class CategoryPath implements Comparable<CategoryPath> {
* @return true if this is a root category path * @return true if this is a root category path
*/ */
public boolean isRoot() { public boolean isRoot() {
// parentPath can only be null for ROOT // parent can only be null for ROOT
return parentPath == null; return parent == null;
} }
/** /**
@ -164,7 +177,7 @@ public class CategoryPath implements Comparable<CategoryPath> {
* @return the parent * @return the parent
*/ */
public CategoryPath getParent() { public CategoryPath getParent() {
return parentPath; return parent;
} }
/** /**
@ -185,10 +198,10 @@ public class CategoryPath implements Comparable<CategoryPath> {
if (isRoot()) { if (isRoot()) {
return DELIMITER_STRING; return DELIMITER_STRING;
} }
if (parentPath.isRoot()) { if (parent.isRoot()) {
return DELIMITER_CHAR + escapeString(name); return DELIMITER_CHAR + escapeString(name);
} }
return parentPath.getPath() + DELIMITER_CHAR + escapeString(name); return parent.getPath() + DELIMITER_CHAR + escapeString(name);
} }
@Override @Override
@ -211,12 +224,12 @@ public class CategoryPath implements Comparable<CategoryPath> {
else if (!name.equals(other.name)) { else if (!name.equals(other.name)) {
return false; return false;
} }
if (parentPath == null) { if (parent == null) {
if (other.parentPath != null) { if (other.parent != null) {
return false; return false;
} }
} }
else if (!parentPath.equals(other.parentPath)) { else if (!parent.equals(other.parent)) {
return false; return false;
} }
return true; return true;
@ -227,16 +240,16 @@ public class CategoryPath implements Comparable<CategoryPath> {
final int prime = 31; final int prime = 31;
int result = 1; int result = 1;
result = prime * result + ((name == null) ? 0 : name.hashCode()); result = prime * result + ((name == null) ? 0 : name.hashCode());
result = prime * result + ((parentPath == null) ? 0 : parentPath.hashCode()); result = prime * result + ((parent == null) ? 0 : parent.hashCode());
return result; return result;
} }
/** /**
* Tests if the specified categoryPath is the same as, or an ancestor of, this category path. * Tests if the specified categoryPath is the same as, or an ancestor of, this category path.
* @param categoryPath the category path to be checked. * @param candidateAncestorPath the category path to be checked.
* @return true if the given path is the same as, or an ancestor of, this category path. * @return true if the given path is the same as, or an ancestor of, this category path.
*/ */
public boolean isAncestorOrSelf(CategoryPath categoryPath) { public boolean isAncestorOrSelf(CategoryPath candidateAncestorPath) {
// Result categoryPath This // Result categoryPath This
// ------ --------------------- ------------------------ // ------ --------------------- ------------------------
@ -248,13 +261,13 @@ public class CategoryPath implements Comparable<CategoryPath> {
// False /app /apple // False /app /apple
// False /pear /apple // False /pear /apple
if (categoryPath.isRoot()) { if (candidateAncestorPath.isRoot()) {
return true; return true;
} }
CategoryPath path = this; CategoryPath path = this;
while (!path.isRoot()) { while (!path.isRoot()) {
if (categoryPath.equals(path)) { if (candidateAncestorPath.equals(path)) {
return true; return true;
} }
path = path.getParent(); path = path.getParent();
@ -275,19 +288,18 @@ public class CategoryPath implements Comparable<CategoryPath> {
*/ */
@Override @Override
public int compareTo(CategoryPath other) { public int compareTo(CategoryPath other) {
CategoryPath otherParentPath = other.getParent(); CategoryPath otherParent = other.getParent();
int result = 0; if (parent == null) {
if (parentPath == null) { if (otherParent != null) {
if (otherParentPath != null) {
return -1; return -1;
} }
} }
else if (otherParentPath == null) { else if (otherParent == null) {
return 1; return 1;
} }
else {
result = parentPath.compareTo(otherParentPath); int result = parent.compareTo(otherParent);
}
if (result == 0) { if (result == 0) {
result = name.compareTo(other.getName()); result = name.compareTo(other.getName());
} }
@ -309,8 +321,11 @@ public class CategoryPath implements Comparable<CategoryPath> {
* @return a hierarchical list of names of the category in the category path. * @return a hierarchical list of names of the category in the category path.
*/ */
public List<String> asList() { public List<String> asList() {
List<String> list = new ArrayList<>(); if (isRoot()) {
addToList(list); return new ArrayList<>();
}
List<String> list = parent.asList();
list.add(name);
return list; return list;
} }
@ -321,32 +336,8 @@ public class CategoryPath implements Comparable<CategoryPath> {
* @return a hierarchical array of names of the categories in the category path. * @return a hierarchical array of names of the categories in the category path.
*/ */
public String[] asArray() { public String[] asArray() {
List<String> list = new ArrayList<>(); List<String> list = asList();
addToList(list);
return list.toArray(new String[list.size()]); return list.toArray(new String[list.size()]);
} }
private void addToList(List<String> list) {
if (!parentPath.isRoot()) {
parentPath.addToList(list);
}
list.add(name);
}
/**
* This constructor is purposefully private and asserting. We do not want anyone to implement
* this constructor, as it would confuse the notion of the constructor that takes a single
* {@link String} that has escaped delimiters vs. what this constructor would have to require,
* which is non-escaped {@link String Strings}. There would be no way for the two Constructors
* to be distinguished from each other when passing a single argument.
*
* @param categoryPath varargs hierarchical list of names of categories.
*/
@SuppressWarnings("unused") // for categoryPath varargs
private CategoryPath(String... categoryPath) {
assert false;
parentPath = null;
name = "";
}
} }

View file

@ -277,16 +277,6 @@ public class CategoryPathTest extends AbstractGenericTest {
@Test @Test
public void testConstructorDelimeterEscape4() { public void testConstructorDelimeterEscape4() {
CategoryPath path = new CategoryPath("/\\/aaa/bbb/bob");
List<String> names = path.asList();
assertEquals("/aaa", names.get(0));
assertEquals("bbb", names.get(1));
assertEquals("bob", names.get(2));
assertEquals("/\\/aaa/bbb/bob", path.getPath());
}
@Test
public void testConstructorDelimeterEscape5() {
CategoryPath path = new CategoryPath("/\\/\\/aaa/bbb/bob"); CategoryPath path = new CategoryPath("/\\/\\/aaa/bbb/bob");
List<String> names = path.asList(); List<String> names = path.asList();
assertEquals("//aaa", names.get(0)); assertEquals("//aaa", names.get(0));