Skip to content

Commit

Permalink
Merge pull request #5048 from evolvedbinary/5.x.x/hotfix/nodepath-all…
Browse files Browse the repository at this point in the history
…ocation-strategy

[5.x.x] Fix a bug in Node Path equality
  • Loading branch information
reinhapa authored Sep 12, 2023
2 parents 887e458 + d50703a commit fadddc3
Show file tree
Hide file tree
Showing 6 changed files with 364 additions and 38 deletions.
50 changes: 32 additions & 18 deletions exist-core/src/main/java/org/exist/dom/QName.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class QName implements Comparable<QName> {

public static final String WILDCARD = "*";
private static final char COLON = ':';
private static final char LEFT_BRACE = '{';
private static final char RIGHT_BRACE = '}';

public static final QName EMPTY_QNAME = new QName("", XMLConstants.NULL_NS_URI);
public static final QName DOCUMENT_QNAME = EMPTY_QNAME;
Expand Down Expand Up @@ -126,32 +128,44 @@ public byte getNameType() {
return nameType;
}

/**
* Get a string representation of this qualified name.
*
* Will either be of the format `local-name` or `prefix:local-name`.
*
* @return the string representation of this qualified name.
* */
public String getStringValue() {
if (prefix != null && prefix.length() > 0) {
return prefix + COLON + localPart;
}
return localPart;
return getStringRepresentation(false);
}

/**
* @deprecated Use for debugging purpose only,
* use {@link #getStringValue()} for production
* Get a string representation of this qualified name.
*
* Will either be of the format `local-name`, `prefix:local-name`, or `{namespace}local-name`.
*
* @return the string representation of this qualified name.
*/
@Override
public String toString() {
//TODO : remove this copy of getStringValue()
return getStringValue();
//TODO : replace by something like this
/*
if (prefix != null && prefix.length() > 0)
return getStringRepresentation(true);
}

/**
* Get a string representation of this qualified name.
*
* @param showNsWithoutPrefix true if the namespace should be shown even when there is no prefix, false otherwise.
* When shown, it will be output using Clark notation, e.g. `{http://namespace}local-name`.
*
* @return the string representation of this qualified name.
*/
private String getStringRepresentation(final boolean showNsWithoutPrefix) {
if (prefix != null && !prefix.isEmpty()) {
return prefix + COLON + localPart;
if (hasNamespace()) {
if (prefix != null && prefix.length() > 0)
return "{" + namespaceURI + "}" + prefix + COLON + localPart;
return "{" + namespaceURI + "}" + localPart;
} else
return localPart;
*/
} else if (showNsWithoutPrefix && namespaceURI != null && !XMLConstants.NULL_NS_URI.equals(namespaceURI)) {
return LEFT_BRACE + namespaceURI + RIGHT_BRACE + localPart;
}
return localPart;
}

/**
Expand Down
78 changes: 63 additions & 15 deletions exist-core/src/main/java/org/exist/storage/NodePath.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,22 @@


/**
* Represents a Node Path.
*
* Internally the node path is held as an array of {@link QName} components.
* Upon construction the array of `components` is sized such that it will be exactly
* what is required to represent the Node Path.
* Mutation operations however will over-allocate the array as an optmisation, so that
* we need not allocate/free memory on every mutation operation, but only
* every {@link #DEFAULT_NODE_PATH_SIZE} operations.
*
* @author <a href="mailto:[email protected]">Adam Retter</a>
* @author wolf
* @author Adam Retter
*/
public class NodePath implements Comparable<NodePath> {

private static final int DEFAULT_NODE_PATH_SIZE = 5;
static final int DEFAULT_NODE_PATH_SIZE = 4;
static final int MAX_OVER_ALLOCATION_FACTOR = 2;

private static final Logger LOG = LogManager.getLogger(NodePath.class);

Expand Down Expand Up @@ -89,21 +99,33 @@ public boolean includeDescendants() {
}

public void append(final NodePath other) {
// expand the array
final int newLength = pos + other.length();
this.components = Arrays.copyOf(components, newLength);
System.arraycopy(other.components, 0, components, pos, other.length());
this.pos = newLength;
// do we have enough space to accommodate the components from `other`
final int numOtherComponentsToAppend = other.length();
allocateIfNeeded(numOtherComponentsToAppend);

// at this point we have enough space, append the components from `other`
System.arraycopy(other.components, 0, components, pos, numOtherComponentsToAppend);
this.pos += numOtherComponentsToAppend;
}

public void addComponent(final QName component) {
if (pos == components.length) {
// extend the array
this.components = Arrays.copyOf(components, pos + 1);
}
// do we have enough space to add the component
allocateIfNeeded(1);

// at this point we have enough space, add the component
components[pos++] = component;
}

private void allocateIfNeeded(final int numOtherComponentsToAppend) {
final int available = components.length - pos;
if (available < numOtherComponentsToAppend) {
// we need more space, allocate a multiple of DEFAULT_NODE_PATH_SIZE
final int allocationFactor = (int) Math.ceil((numOtherComponentsToAppend - available + components.length) / ((float) DEFAULT_NODE_PATH_SIZE));
final int newSize = allocationFactor * DEFAULT_NODE_PATH_SIZE;
this.components = Arrays.copyOf(components, newSize);
}
}

/**
* Remove the Last Component from the NodePath.
*
Expand All @@ -117,8 +139,8 @@ public void removeLastComponent() {
}

public void reset() {
// when resetting if this object has twice the capacity of a new object, then set it back to the default capacity
if (pos > DEFAULT_NODE_PATH_SIZE * 2) {
// when resetting if this object has more than twice the capacity of a new object, then set it back to the default capacity
if (pos > DEFAULT_NODE_PATH_SIZE * MAX_OVER_ALLOCATION_FACTOR) {
components = new QName[DEFAULT_NODE_PATH_SIZE];
} else {
Arrays.fill(components, null);
Expand All @@ -130,6 +152,18 @@ public int length() {
return pos;
}

/**
* Return the size of the components array.
*
* This function is intentionally marked as package-private
* so that it may only be called for testing purposes!
*
* @return the size of the components array.
*/
int componentsSize() {
return components.length;
}

protected void reverseComponents() {
for (int i = 0; i < pos / 2; ++i) {
QName tmp = components[i];
Expand Down Expand Up @@ -290,10 +324,24 @@ private void init(@Nullable final Map<String, String> namespaces, final String p

@Override
public boolean equals(final Object obj) {
if (obj != null && obj instanceof NodePath) {
if (this == obj) {
return true;
}

if (obj instanceof NodePath) {
final NodePath otherNodePath = (NodePath) obj;
return Arrays.equals(components, otherNodePath.components);

// NOTE(AR) we cannot use Array.equals on the components of the NodePaths as they may be over-allocated!
if (pos == otherNodePath.pos) {
for (int i = 0; i < pos; i++) {
if (!components[i].equals(otherNodePath.components[i])) {
return false;
}
}
return true;
}
}

return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public static int generateDocs(final FunctionSignature sig, final UserDefinedFun
XQDocHelper.parse(sig);

final AttributesImpl attribs = new AttributesImpl();
attribs.addAttribute("", "name", "name", "CDATA", sig.getName().toString());
attribs.addAttribute("", "name", "name", "CDATA", sig.getName().getStringValue());
attribs.addAttribute("", "module", "module", "CDATA", sig.getName().getNamespaceURI());
final int nodeNr = builder.startElement(FUNCTION_QNAME, attribs);
writeParameters(sig, builder);
Expand Down Expand Up @@ -130,7 +130,7 @@ private static void writeAnnotations(final FunctionSignature signature, final Me
if (annots != null) {
for (final Annotation annot : annots) {
attribs.clear();
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().toString());
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().getStringValue());
attribs.addAttribute(null, "namespace", "namespace", "CDATA", annot.getName().getNamespaceURI());
builder.startElement(ANNOTATION_QNAME, attribs);
final LiteralValue[] value = annot.getValue();
Expand Down Expand Up @@ -164,7 +164,7 @@ private static void generateDependencies(final UserDefinedFunction function, fin
final AttributesImpl attribs = new AttributesImpl();
for (final FunctionSignature signature : signatures) {
attribs.clear();
attribs.addAttribute(null, "name", "name", "CDATA", signature.getName().toString());
attribs.addAttribute(null, "name", "name", "CDATA", signature.getName().getStringValue());
attribs.addAttribute("", "module", "module", "CDATA", signature.getName().getNamespaceURI());
attribs.addAttribute("", "arity", "arity", "CDATA", Integer.toString(signature.getArgumentCount()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
// variables
for (final VariableDeclaration var : externalModule.getVariableDeclarations()) {
attribs.clear();
attribs.addAttribute("", "name", "name", "CDATA", var.getName().toString());
attribs.addAttribute("", "name", "name", "CDATA", var.getName().getStringValue());
final SequenceType type = var.getSequenceType();
if (type != null) {
attribs.addAttribute("", "type", "type", "CDATA", Type.getTypeName(type.getPrimaryType()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ private void writeAnnotations(FunctionSignature signature, MemTreeBuilder builde
if (annots != null) {
for (final Annotation annot : annots) {
attribs.clear();
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().toString());
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().getStringValue());
attribs.addAttribute(null, "namespace", "namespace", "CDATA", annot.getName().getNamespaceURI());
builder.startElement(ANNOTATION_QNAME, attribs);
final LiteralValue[] value = annot.getValue();
Expand Down
Loading

0 comments on commit fadddc3

Please sign in to comment.