diff --git a/build.xml b/build.xml index 66587bb65..e2ac90804 100644 --- a/build.xml +++ b/build.xml @@ -21,7 +21,7 @@ - + diff --git a/gradle.properties b/gradle.properties index 1ff4209cf..03001ee23 100644 --- a/gradle.properties +++ b/gradle.properties @@ -14,8 +14,8 @@ # comment-out, because of naming problems with the distribution plugin -#project.version=5.3.1-SNAPSHOT -XMLBeansVersion=5.3.1-SNAPSHOT +#project.version=5.4.0-SNAPSHOT +XMLBeansVersion=5.4.0-SNAPSHOT # Specifies the JVM arguments used for the daemon process. # The setting is particularly useful for tweaking memory settings. diff --git a/src/main/java/org/apache/xmlbeans/impl/store/Cur.java b/src/main/java/org/apache/xmlbeans/impl/store/Cur.java index 241508f6f..5b7d12eb4 100755 --- a/src/main/java/org/apache/xmlbeans/impl/store/Cur.java +++ b/src/main/java/org/apache/xmlbeans/impl/store/Cur.java @@ -343,7 +343,7 @@ static Xobj createElementXobj(Locale l, QName name, QName parentName) { private void createHelper(Xobj x) { assert x._locale == _locale; - // insert the new Xobj into an exisiting tree. + // insert the new Xobj into an existing tree. if (isPositioned()) { Cur from = tempCur(x, 0); diff --git a/src/main/java/org/apache/xmlbeans/impl/store/Xobj.java b/src/main/java/org/apache/xmlbeans/impl/store/Xobj.java index 55391fe7d..7e42ebf50 100644 --- a/src/main/java/org/apache/xmlbeans/impl/store/Xobj.java +++ b/src/main/java/org/apache/xmlbeans/impl/store/Xobj.java @@ -1992,6 +1992,19 @@ public void find_all_element_users(QName name, List fil } } + @SuppressWarnings("unchecked") + @Override + public void find_multiple_element_users(final QName name, final List fillMeUp, + final int maxCount) { + int count = 0; + for (Xobj x = _firstChild; x != null && count < maxCount; x = x._nextSibling) { + if (x.isElem() && x._name.equals(name)) { + fillMeUp.add((T) x.getUser()); + count++; + } + } + } + @SuppressWarnings("unchecked") @Override public void find_all_element_users(QNameSet names, List fillMeUp) { @@ -2002,6 +2015,19 @@ public void find_all_element_users(QNameSet names, List } } + @SuppressWarnings("unchecked") + @Override + public void find_multiple_element_users(final QNameSet names, final List fillMeUp, + final int maxCount) { + int count = 0; + for (Xobj x = _firstChild; x != null && count < maxCount; x = x._nextSibling) { + if (x.isElem() && names.contains(x._name)) { + fillMeUp.add((T) x.getUser()); + count++; + } + } + } + private static TypeStoreUser insertElement(QName name, Xobj x, int pos) { x._locale.enter(); @@ -2017,6 +2043,26 @@ private static TypeStoreUser insertElement(QName name, Xobj x, int pos) { } } + private static TypeStoreUser[] insertElements(final QName name, final Xobj x, + final int pos, final int count) { + x._locale.enter(); + + TypeStoreUser[] users = new TypeStoreUser[count]; + try { + Cur c = x._locale.tempCur(); + c.moveTo(x, pos); + for (int i = count - 1; i >= 0; i--) { + c.createElement(name); + users[i] = c.getUser(); + } + c.release(); + } finally { + x._locale.exit(); + } + return users; + } + + @Override public TypeStoreUser insert_element_user(QName name, int i) { if (i < 0) { throw new IndexOutOfBoundsException(); @@ -2039,6 +2085,35 @@ public TypeStoreUser insert_element_user(QName name, int i) { return insertElement(name, x, 0); } + @Override + public TypeStoreUser[] insert_elements_users(final QName name, final int i, + final int count) { + if (i < 0) { + throw new IndexOutOfBoundsException(); + } + + if (!isContainer()) { + throw new IllegalStateException(); + } + + if (count <= 0) { + return new TypeStoreUser[0]; + } + + Xobj x = _locale.findNthChildElem(this, name, null, i); + + if (x == null) { + if (i > _locale.count(this, name, null) + 1) { + throw new IndexOutOfBoundsException(); + } + + return add_elements_users(name, count); + } + + return insertElements(name, x, 0, count); + } + + @Override public TypeStoreUser insert_element_user(QNameSet names, QName name, int i) { if (i < 0) { throw new IndexOutOfBoundsException(); @@ -2061,6 +2136,35 @@ public TypeStoreUser insert_element_user(QNameSet names, QName name, int i) { return insertElement(name, x, 0); } + @Override + public TypeStoreUser[] insert_elements_users(final QNameSet names, final QName name, + final int i, final int count) { + if (i < 0) { + throw new IndexOutOfBoundsException(); + } + + if (!isContainer()) { + throw new IllegalStateException(); + } + + if (count <= 0) { + return new TypeStoreUser[0]; + } + + Xobj x = _locale.findNthChildElem(this, null, names, i); + + if (x == null) { + if (i > _locale.count(this, null, names) + 1) { + throw new IndexOutOfBoundsException(); + } + + return add_elements_users(name, count); + } + + return insertElements(name, x, 0, count); + } + + @Override public TypeStoreUser add_element_user(QName name) { if (!isContainer()) { throw new IllegalStateException(); @@ -2094,6 +2198,51 @@ public TypeStoreUser add_element_user(QName name) { : insertElement(name, candidate, 0); } + @Override + public TypeStoreUser[] add_elements_users(final QName name, final int count) { + if (!isContainer()) { + throw new IllegalStateException(); + } + + if (count <= 0) { + return new TypeStoreUser[0]; + } + + QNameSet endSet = null; + boolean gotEndSet = false; + + Xobj candidate = null; + + for (Xobj x = _lastChild; x != null; x = x._prevSibling) { + if (x.isContainer()) { + if (x._name.equals(name)) { + break; + } + + if (!gotEndSet) { + endSet = _user.get_element_ending_delimiters(name); + gotEndSet = true; + } + + if (endSet == null || endSet.contains(x._name)) { + candidate = x; + } + } + } + + final TypeStoreUser[] users; + if (candidate == null) { + // If there is no candidate, then I need to insert at the end of this container + // and create a new element for each of the count + users = insertElements(name, this, END_POS, count); + } else { + // If I have a candidate, then I need to insert at the candidate and create + // a new element for each of the count + users = insertElements(name, candidate, 0, count); + } + return users; + } + private static void removeElement(Xobj x) { if (x == null) { throw new IndexOutOfBoundsException(); @@ -2110,6 +2259,7 @@ private static void removeElement(Xobj x) { } } + @Override public void remove_element(QName name, int i) { if (i < 0) { throw new IndexOutOfBoundsException(); @@ -2130,6 +2280,30 @@ public void remove_element(QName name, int i) { removeElement(x); } + @Override + public void remove_elements_after(QName name, int i) { + if (i < 0) { + throw new IndexOutOfBoundsException(); + } + + if (!isContainer()) { + throw new IllegalStateException(); + } + + ArrayList toRemove = new ArrayList<>(); + Xobj x; + for (x = _firstChild; x != null; x = x._nextSibling) { + if (x.isElem() && x._name.equals(name) && --i < 0) { + toRemove.add(x); + } + } + final int size = toRemove.size(); + for (int j = size - 1; j >= 0; j--) { + removeElement(toRemove.get(j)); + } + } + + @Override public void remove_element(QNameSet names, int i) { if (i < 0) { throw new IndexOutOfBoundsException(); @@ -2150,6 +2324,25 @@ public void remove_element(QNameSet names, int i) { removeElement(x); } + @Override + public void remove_elements_after(QNameSet names, int i) { + if (i < 0) { + throw new IndexOutOfBoundsException(); + } + + if (!isContainer()) { + throw new IllegalStateException(); + } + + Xobj x; + + for (x = _firstChild; x != null; x = x._nextSibling) { + if (x.isElem() && names.contains(x._name) && --i < 0) { + removeElement(x); + } + } + } + public TypeStoreUser find_attribute_user(QName name) { Xobj a = getAttr(name); @@ -2294,7 +2487,7 @@ public void array_setter(XmlObject[] sources, QName elementName) { try { // TODO - this is the quick and dirty implementation, make this faster - int m = sources.length; + final int m = sources.length; List copies = new ArrayList<>(); List types = new ArrayList<>(); @@ -2326,18 +2519,14 @@ public void array_setter(XmlObject[] sources, QName elementName) { } } - int n = count_elements(elementName); + final int n = count_elements(elementName); - for (; n > m; n--) { - remove_element(elementName, m); + if (n > m) { + remove_elements_after(elementName, m); + } else if (n < m) { + add_elements_users(elementName, m - n); } - for (; m > n; n++) { - add_element_user(elementName); - } - - assert m == n; - List elementsUser = new ArrayList<>(); find_all_element_users(elementName, elementsUser); @@ -2348,8 +2537,6 @@ public void array_setter(XmlObject[] sources, QName elementName) { .map(x -> (Xobj) x) .collect(Collectors.toList()); - assert elements.size() == n; - Cur c = tempCur(); for (int i = 0; i < n; i++) { diff --git a/src/main/java/org/apache/xmlbeans/impl/values/TypeStore.java b/src/main/java/org/apache/xmlbeans/impl/values/TypeStore.java index 876fbf7c8..5e404bf14 100644 --- a/src/main/java/org/apache/xmlbeans/impl/values/TypeStore.java +++ b/src/main/java/org/apache/xmlbeans/impl/values/TypeStore.java @@ -183,7 +183,19 @@ public interface TypeStore extends NamespaceManager * Returns all TypeStoreUsers corresponding to elements with one * of the names is the QNameSet. */ - void find_all_element_users(QNameSet name, List fillMeUp); + void find_all_element_users(QNameSet names, List fillMeUp); + + /** + * @since 5.4.0 + */ + void find_multiple_element_users(QName name, List fillMeUp, + int maxCount); + + /** + * @since 5.4.0 + */ + void find_multiple_element_users(QNameSet names, List fillMeUp, + int maxCount); /** * Inserts a new element at the position that will make it @@ -202,11 +214,35 @@ public interface TypeStore extends NamespaceManager TypeStoreUser insert_element_user(QName name, int i); /** - * Like the above method, except that it inserts an element named + * Inserts new elements at the position that will make the first one + * the ith element with the given name owned by this textstore, + * and returns a TypeStoreUser array with all new elements. + * + * Note that if there are no existing elements of the given + * name, you may need to call back to discover the proper + * ordering to use to insert the first one. Otherwise, + * it should be inserted adjacent to existing elements with + * the same name. + * + * Should throw an IndexOutOfBoundsException if i < 0 + * or if i > # of elts + * @since 5.4.0 + */ + TypeStoreUser[] insert_elements_users(QName name, int i, int count); + + /** + * Like the {@link #insert_element_user(QName, int)} method, except that it inserts an element named * name, after the ith member of set. */ TypeStoreUser insert_element_user(QNameSet set, QName name, int i); + /** + * Like the {@link #insert_elements_users(QName, int, int)} method, except that it inserts elements named + * name, after the ith member of set. + * @since 5.4.0 + */ + TypeStoreUser[] insert_elements_users(QNameSet set, QName name, int i, int count); + /** * Adds a new element at the last position adjacent to existing * elements of the same name. @@ -216,6 +252,16 @@ public interface TypeStore extends NamespaceManager */ TypeStoreUser add_element_user(QName name); + /** + * Adds elements at the last position adjacent to existing + * elements of the same name. + * + * Note that if there are no existing elements of the given + * name, the same comment applies as with insert_element_user. + * @since 5.4.0 + */ + TypeStoreUser[] add_elements_users(QName name, int count); + /** * Removes the ith element with the given name. * @@ -229,6 +275,17 @@ public interface TypeStore extends NamespaceManager */ void remove_element(QNameSet names, int i); + /** + * Removes all elements after the i-th element with the given name. + * @since 5.4.0 + */ + void remove_elements_after(QName name, int i); + + /** + * Removes all elements after the i-th element with the given name. + * @since 5.4.0 + */ + void remove_elements_after(QNameSet name, int i); /** * Returns the TypeStoreUser underneath the attribute with the given diff --git a/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java b/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java index 2407c785e..b4ad6a119 100644 --- a/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java +++ b/src/main/java/org/apache/xmlbeans/impl/values/XmlComplexContentImpl.java @@ -22,6 +22,7 @@ import javax.xml.namespace.QName; import java.math.BigDecimal; import java.math.BigInteger; +import java.util.ArrayList; import java.util.Calendar; import java.util.Date; import java.util.List; @@ -315,13 +316,10 @@ protected void arraySetterHelper(XmlObject[] sources, QName elemName, QNameSet s TypeStore store = get_store(); if (sources == null || sources.length == 0) { - int m = (set == null) ? store.count_elements(elemName) : store.count_elements(set); - for (; m > 0; m--) { - if (set == null) { - store.remove_element(elemName, 0); - } else { - store.remove_element(set, 0); - } + if (set == null) { + store.remove_elements_after(elemName, 0); + } else { + store.remove_elements_after(set, 0); } return; } @@ -428,6 +426,140 @@ protected void arraySetterHelper(XmlObject[] sources, QName elemName, QNameSet s // get_store().array_setter( sources, elemName ); } + protected void arraySetterHelper2(XmlObject[] sources, QName elemName, QNameSet set) { + TypeStore store = get_store(); + + if (sources == null || sources.length == 0) { + if (set == null) { + store.remove_elements_after(elemName, 0); + } else { + store.remove_elements_after(set, 0); + } + return; + } + + // Verify if the sources contain children of this node + int i; + // how many elements in the original array + int m = (set == null) ? store.count_elements(elemName) : store.count_elements(set); + int startSrc = 0, startDest = 0; + for (i = 0; i < sources.length; i++) { + if (sources[i].isImmutable()) { + continue; + } + try (XmlCursor c = sources[i].newCursor()) { + if (c.toParent() && c.getObject() == this) { + break; + } + } + } + if (i < sources.length) { + TypeStoreUser current = (set == null) ? store.find_element_user(elemName, 0) : store.find_element_user(set, 0); + if (current == sources[i]) { + // The new object matches what already exists in the array + // Heuristic: we optimize for the case where the new elements + // in the array are the same as the existing elements with + // potentially new elements inserted + + // First insert the new element in the array at position 0 + int j = 0; + if (i > 0) { + TypeStoreUser[] users; + if (set == null) { + users = store.insert_elements_users(elemName, 0, i); + } else { + users = store.insert_elements_users(set, elemName, 0, i); + } + for (TypeStoreUser user : users) { + ((XmlObjectBase) user).set(sources[j++]); + } + } + final ArrayList users = new ArrayList<>(sources.length); + if (set == null) { + store.find_all_element_users(elemName, users); + } else { + store.find_all_element_users(set, users); + } + final int usersLen = users.size(); + int inserted = 0; + for (i++, j++; i < sources.length; i++, j++) { + // Cursor is implicitly closed + XmlCursor c = sources[i].isImmutable() ? null : sources[i].newCursor(); + if (c != null && c.toParent() && c.getObject() == this) { + c.close(); + final int pos = j + inserted; + current = pos < usersLen ? users.get(pos) : null; + if (current != sources[i]) { + // Fall back to the general case + break; + } + } else { + if (c != null) { + c.close(); + } + // Insert before the current element + TypeStoreUser user = (set == null) ? store.insert_element_user(elemName, j) : store.insert_element_user(set, elemName, j); + ((XmlObjectBase) user).set(sources[i]); + inserted++; + } + } + startDest = j; + startSrc = i; + m = store.count_elements(elemName); + } + // Fall through + } else { + // All of the elements in the existing array are to + // be deleted and replaced with elements from the + // sources array + } + + // The general case: we assume that some of the elements + // in the new array already exist, but at different indexes + + // Starting with position i in the sources array, copy the remaining elements + // to the end of the original array... + if (i < sources.length) { + TypeStoreUser[] users = store.add_elements_users(elemName, sources.length - i); + int j = i; + for (TypeStoreUser user : users) { + ((XmlObjectBase) user).set(sources[j++]); + } + } + + // ... then come back and insert the elements starting with startSource + // up to i from the sources array into the current array, starting with + // startDest + final int n = i; + + if (set == null) { + store.remove_elements_after(elemName, n - startSrc + startDest); + } else { + store.remove_elements_after(set, n - startSrc + startDest); + } + + + final int size = Math.min(m - startDest, sources.length - startSrc); + ArrayList users = new ArrayList<>(size); + if (set == null) { + store.find_multiple_element_users(elemName, users, size); + } else { + store.find_multiple_element_users(set, users, size); + } + i = startSrc; + for (XmlObjectBase user : users) { + user.set(sources[i++]); + } + + for (TypeStoreUser u : store.add_elements_users(elemName, n - i)) { + ((XmlObjectBase) u).set(sources[i++]); + } + + // We can't just delegate to array_setter because we need + // synchronization on the sources (potentially each element + // in the array on a different lock) + // get_store().array_setter( sources, elemName ); + } private void commonSetterHelper(QName elemName, QNameSet set, T[] sources, BiConsumer fun) { commonSetterHelper(elemName, set, (sources == null) ? 0 : sources.length, fun); @@ -438,54 +570,68 @@ private void commonSetterHelper(QName elemName, QNameSet set, int n, BiConsumer< int m = (set == null) ? store.count_elements(elemName) : store.count_elements(set); - for (; m > n; m--) { + if (m > n) { if (set == null) { - store.remove_element(elemName, m - 1); + store.remove_elements_after(elemName, n); } else { - store.remove_element(set, m - 1); + store.remove_elements_after(set, n); } + m = n; } - for (int i = 0; i < n; i++) { - TypeStoreUser user; + ArrayList users = new ArrayList<>(m); + if (set == null) { + store.find_all_element_users(elemName, users); + } else { + store.find_all_element_users(set, users); + } + int i = 0; + for (XmlObjectBase user : users) { + fun.accept(user, i++); + } - if (i >= m) { - user = store.add_element_user(elemName); - } else if (set == null) { - user = store.find_element_user(elemName, i); - } else { - user = store.find_element_user(set, i); - } - fun.accept((XmlObjectBase) user, i); + for (TypeStoreUser u : store.add_elements_users(elemName, n - m)) { + fun.accept((XmlObjectBase) u, i++); } } private void commonSetterHelper2(QName elemName, QNameSet set, T[] sources, BiConsumer c) { - int n = (sources == null) ? 0 : sources.length; + final int n = (sources == null) ? 0 : sources.length; TypeStore store = get_store(); int m = (set == null) ? store.count_elements(elemName) : store.count_elements(set); - for (; m > n; m--) { + if (m > n) { + // If the existing array is longer than the new one, we need to remove + // the extra elements if (set == null) { - store.remove_element(elemName, m - 1); + store.remove_elements_after(elemName, n); } else { - store.remove_element(set, m - 1); + store.remove_elements_after(set, n); } + m = n; } - for (int i = 0; i < n; i++) { - TypeStoreUser user; + ArrayList users = new ArrayList<>(m); + if (set == null) { + store.find_all_element_users(elemName, users); + } else { + store.find_all_element_users(set, users); + } + int i = 0; + for (XmlObjectBase user : users) { + if (i >= n) { + break; + } + c.accept(user, sources[i++]); + } - if (i >= m) { - user = store.add_element_user(elemName); - } else if (set == null) { - user = store.find_element_user(elemName, i); - } else { - user = store.find_element_user(set, i); + for (TypeStoreUser u : store.add_elements_users(elemName, n - m)) { + if (i >= n) { + break; } - c.accept((XmlObjectBase) user, sources[i]); + c.accept((XmlObjectBase) u, sources[i++]); } } } diff --git a/src/test/java/common/Common.java b/src/test/java/common/Common.java index 8569433fa..e34bb3081 100644 --- a/src/test/java/common/Common.java +++ b/src/test/java/common/Common.java @@ -119,6 +119,22 @@ public static boolean hasSevereError(List errors) { return errFound; } + public static String collateSevereErrors(List errors) { + boolean errFound = errors.stream().anyMatch(e -> e.getSeverity() == XmlError.SEVERITY_ERROR); + String errorTxt = ""; + if (errFound) { + StringBuilder errorBuilder = new StringBuilder("Errors found:\n"); + for (XmlError xmlError : errors) { + if (xmlError.getSeverity() == XmlError.SEVERITY_ERROR) { + errorBuilder.append(xmlError).append("\n"); + } + } + errorTxt = errorBuilder.toString(); + } + errors.clear(); + return errorTxt; + } + /** * Validate schemas to instance based on the docType */ diff --git a/src/test/java/compile/scomp/detailed/SchemaCompilerTests.java b/src/test/java/compile/scomp/detailed/SchemaCompilerTests.java index f5d13bbea..a825103d8 100644 --- a/src/test/java/compile/scomp/detailed/SchemaCompilerTests.java +++ b/src/test/java/compile/scomp/detailed/SchemaCompilerTests.java @@ -24,6 +24,7 @@ import java.util.List; import static common.Common.*; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; /** @@ -144,7 +145,7 @@ void testNoExt() { params.setNoExt(true); SchemaCompiler.compile(params); - assertFalse(hasSevereError(errors), "testNoExt(): failure when executing scomp"); + assertEquals("", collateSevereErrors(errors)); } @Test