Skip to content

Commit 831829e

Browse files
authored
Merge pull request #5046 from evolvedbinary/6.x.x/hotfix/nodepath-allocation-strategy
[6.x.x] Fix a bug in Node Path equality
2 parents e6cd0a3 + 23827b9 commit 831829e

File tree

6 files changed

+364
-38
lines changed

6 files changed

+364
-38
lines changed

exist-core/src/main/java/org/exist/dom/QName.java

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ public class QName implements Comparable<QName> {
4141

4242
public static final String WILDCARD = "*";
4343
private static final char COLON = ':';
44+
private static final char LEFT_BRACE = '{';
45+
private static final char RIGHT_BRACE = '}';
4446

4547
public static final QName EMPTY_QNAME = new QName("", XMLConstants.NULL_NS_URI);
4648
public static final QName DOCUMENT_QNAME = EMPTY_QNAME;
@@ -126,32 +128,44 @@ public byte getNameType() {
126128
return nameType;
127129
}
128130

131+
/**
132+
* Get a string representation of this qualified name.
133+
*
134+
* Will either be of the format `local-name` or `prefix:local-name`.
135+
*
136+
* @return the string representation of this qualified name.
137+
* */
129138
public String getStringValue() {
130-
if (prefix != null && prefix.length() > 0) {
131-
return prefix + COLON + localPart;
132-
}
133-
return localPart;
139+
return getStringRepresentation(false);
134140
}
135141

136142
/**
137-
* @deprecated Use for debugging purpose only,
138-
* use {@link #getStringValue()} for production
143+
* Get a string representation of this qualified name.
144+
*
145+
* Will either be of the format `local-name`, `prefix:local-name`, or `{namespace}local-name`.
146+
*
147+
* @return the string representation of this qualified name.
139148
*/
140149
@Override
141150
public String toString() {
142-
//TODO : remove this copy of getStringValue()
143-
return getStringValue();
144-
//TODO : replace by something like this
145-
/*
146-
if (prefix != null && prefix.length() > 0)
151+
return getStringRepresentation(true);
152+
}
153+
154+
/**
155+
* Get a string representation of this qualified name.
156+
*
157+
* @param showNsWithoutPrefix true if the namespace should be shown even when there is no prefix, false otherwise.
158+
* When shown, it will be output using Clark notation, e.g. `{http://namespace}local-name`.
159+
*
160+
* @return the string representation of this qualified name.
161+
*/
162+
private String getStringRepresentation(final boolean showNsWithoutPrefix) {
163+
if (prefix != null && !prefix.isEmpty()) {
147164
return prefix + COLON + localPart;
148-
if (hasNamespace()) {
149-
if (prefix != null && prefix.length() > 0)
150-
return "{" + namespaceURI + "}" + prefix + COLON + localPart;
151-
return "{" + namespaceURI + "}" + localPart;
152-
} else
153-
return localPart;
154-
*/
165+
} else if (showNsWithoutPrefix && namespaceURI != null && !XMLConstants.NULL_NS_URI.equals(namespaceURI)) {
166+
return LEFT_BRACE + namespaceURI + RIGHT_BRACE + localPart;
167+
}
168+
return localPart;
155169
}
156170

157171
/**

exist-core/src/main/java/org/exist/storage/NodePath.java

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,22 @@
3333

3434

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

41-
private static final int DEFAULT_NODE_PATH_SIZE = 5;
50+
static final int DEFAULT_NODE_PATH_SIZE = 4;
51+
static final int MAX_OVER_ALLOCATION_FACTOR = 2;
4252

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

@@ -89,21 +99,33 @@ public boolean includeDescendants() {
8999
}
90100

91101
public void append(final NodePath other) {
92-
// expand the array
93-
final int newLength = pos + other.length();
94-
this.components = Arrays.copyOf(components, newLength);
95-
System.arraycopy(other.components, 0, components, pos, other.length());
96-
this.pos = newLength;
102+
// do we have enough space to accommodate the components from `other`
103+
final int numOtherComponentsToAppend = other.length();
104+
allocateIfNeeded(numOtherComponentsToAppend);
105+
106+
// at this point we have enough space, append the components from `other`
107+
System.arraycopy(other.components, 0, components, pos, numOtherComponentsToAppend);
108+
this.pos += numOtherComponentsToAppend;
97109
}
98110

99111
public void addComponent(final QName component) {
100-
if (pos == components.length) {
101-
// extend the array
102-
this.components = Arrays.copyOf(components, pos + 1);
103-
}
112+
// do we have enough space to add the component
113+
allocateIfNeeded(1);
114+
115+
// at this point we have enough space, add the component
104116
components[pos++] = component;
105117
}
106118

119+
private void allocateIfNeeded(final int numOtherComponentsToAppend) {
120+
final int available = components.length - pos;
121+
if (available < numOtherComponentsToAppend) {
122+
// we need more space, allocate a multiple of DEFAULT_NODE_PATH_SIZE
123+
final int allocationFactor = (int) Math.ceil((numOtherComponentsToAppend - available + components.length) / ((float) DEFAULT_NODE_PATH_SIZE));
124+
final int newSize = allocationFactor * DEFAULT_NODE_PATH_SIZE;
125+
this.components = Arrays.copyOf(components, newSize);
126+
}
127+
}
128+
107129
/**
108130
* Remove the Last Component from the NodePath.
109131
*
@@ -117,8 +139,8 @@ public void removeLastComponent() {
117139
}
118140

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

155+
/**
156+
* Return the size of the components array.
157+
*
158+
* This function is intentionally marked as package-private
159+
* so that it may only be called for testing purposes!
160+
*
161+
* @return the size of the components array.
162+
*/
163+
int componentsSize() {
164+
return components.length;
165+
}
166+
133167
protected void reverseComponents() {
134168
for (int i = 0; i < pos / 2; ++i) {
135169
QName tmp = components[i];
@@ -290,10 +324,24 @@ private void init(@Nullable final Map<String, String> namespaces, final String p
290324

291325
@Override
292326
public boolean equals(final Object obj) {
293-
if (obj != null && obj instanceof NodePath) {
327+
if (this == obj) {
328+
return true;
329+
}
330+
331+
if (obj instanceof NodePath) {
294332
final NodePath otherNodePath = (NodePath) obj;
295-
return Arrays.equals(components, otherNodePath.components);
333+
334+
// NOTE(AR) we cannot use Array.equals on the components of the NodePaths as they may be over-allocated!
335+
if (pos == otherNodePath.pos) {
336+
for (int i = 0; i < pos; i++) {
337+
if (!components[i].equals(otherNodePath.components[i])) {
338+
return false;
339+
}
340+
}
341+
return true;
342+
}
296343
}
344+
297345
return false;
298346
}
299347

exist-core/src/main/java/org/exist/xquery/functions/inspect/InspectFunctionHelper.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ public static int generateDocs(final FunctionSignature sig, final UserDefinedFun
6363
XQDocHelper.parse(sig);
6464

6565
final AttributesImpl attribs = new AttributesImpl();
66-
attribs.addAttribute("", "name", "name", "CDATA", sig.getName().toString());
66+
attribs.addAttribute("", "name", "name", "CDATA", sig.getName().getStringValue());
6767
attribs.addAttribute("", "module", "module", "CDATA", sig.getName().getNamespaceURI());
6868
final int nodeNr = builder.startElement(FUNCTION_QNAME, attribs);
6969
writeParameters(sig, builder);
@@ -130,7 +130,7 @@ private static void writeAnnotations(final FunctionSignature signature, final Me
130130
if (annots != null) {
131131
for (final Annotation annot : annots) {
132132
attribs.clear();
133-
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().toString());
133+
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().getStringValue());
134134
attribs.addAttribute(null, "namespace", "namespace", "CDATA", annot.getName().getNamespaceURI());
135135
builder.startElement(ANNOTATION_QNAME, attribs);
136136
final LiteralValue[] value = annot.getValue();
@@ -164,7 +164,7 @@ private static void generateDependencies(final UserDefinedFunction function, fin
164164
final AttributesImpl attribs = new AttributesImpl();
165165
for (final FunctionSignature signature : signatures) {
166166
attribs.clear();
167-
attribs.addAttribute(null, "name", "name", "CDATA", signature.getName().toString());
167+
attribs.addAttribute(null, "name", "name", "CDATA", signature.getName().getStringValue());
168168
attribs.addAttribute("", "module", "module", "CDATA", signature.getName().getNamespaceURI());
169169
attribs.addAttribute("", "arity", "arity", "CDATA", Integer.toString(signature.getArgumentCount()));
170170

exist-core/src/main/java/org/exist/xquery/functions/inspect/InspectModule.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
117117
// variables
118118
for (final VariableDeclaration var : externalModule.getVariableDeclarations()) {
119119
attribs.clear();
120-
attribs.addAttribute("", "name", "name", "CDATA", var.getName().toString());
120+
attribs.addAttribute("", "name", "name", "CDATA", var.getName().getStringValue());
121121
final SequenceType type = var.getSequenceType();
122122
if (type != null) {
123123
attribs.addAttribute("", "type", "type", "CDATA", Type.getTypeName(type.getPrimaryType()));

exist-core/src/main/java/org/exist/xquery/functions/util/DescribeFunction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ private void writeAnnotations(FunctionSignature signature, MemTreeBuilder builde
199199
if (annots != null) {
200200
for (final Annotation annot : annots) {
201201
attribs.clear();
202-
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().toString());
202+
attribs.addAttribute(null, "name", "name", "CDATA", annot.getName().getStringValue());
203203
attribs.addAttribute(null, "namespace", "namespace", "CDATA", annot.getName().getNamespaceURI());
204204
builder.startElement(ANNOTATION_QNAME, attribs);
205205
final LiteralValue[] value = annot.getValue();

0 commit comments

Comments
 (0)