diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConcurrentXMLHandler.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConcurrentXMLHandler.java index 281a29e83..7d691b5e4 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConcurrentXMLHandler.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConcurrentXMLHandler.java @@ -30,6 +30,7 @@ import java.util.Collection; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; +import org.forgerock.openicf.connectors.xml.query.abstracts.Query; import org.identityconnectors.common.security.GuardedString; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.ConnectorObject; @@ -107,6 +108,19 @@ public Collection search(String query, ObjectClass objectClass) return result; } + @Override + public Collection search(Query query, ObjectClass objectClass) { + Collection result = null; + lock.readLock().lock(); + try { + result = proxy.search(query, objectClass); + } + finally { + lock.readLock().unlock(); + } + return result; + } + @Override public Uid authenticate(String username, GuardedString password) { Uid result = null; diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/EntryIndex.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/EntryIndex.java new file mode 100644 index 000000000..fd6e13943 --- /dev/null +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/EntryIndex.java @@ -0,0 +1,281 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openicf.connectors.xml; + +import static org.forgerock.openicf.connectors.xml.util.XmlHandlerUtil.following; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.TreeMap; + +import org.identityconnectors.framework.common.objects.Name; +import org.identityconnectors.framework.common.objects.Uid; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; + +/** + * Finds entries by {@code __UID__} and {@code __NAME__} without walking the document. + *

+ * Gives the same entries, in the same order, as {@code /icf:OpenICFContainer//ri:CLASS} filtered by + * {@code $x/icf:FIELD = 'VALUE'}: entries at any depth below the container, an entry under + * every value of its identifier child elements, values compared as they are, results in document + * order, so the first of several duplicates comes first. + *

+ * Every change to the entries must be reported through {@link #added}, {@link #removed} and + * {@link #rekeyed}. Nothing is computed on read, so concurrent readers are safe while nobody + * changes the index; callers change it under the handler's write lock only. + *

+ * An entry inside another entry's identifier element is part of that identifier's value, but + * changing or removing it re-keys only the inner entry: the outer one keeps its old key until the + * next {@link #rebuild}. The connector never writes that shape; only a hand-edited file has it. + */ +final class EntryIndex { + + private final String icfNamespace; + private final String riNamespace; + private final Set objectClasses; + + private final Map entries = new IdentityHashMap(); + private final Map> byClass = new HashMap>(); + private final Map> byIdentifier = new HashMap>(); + private long nextOrder; + /** Whether the last rebuilt document has the container as its root; otherwise nothing is indexed. */ + private boolean containerRoot; + + EntryIndex(String icfNamespace, String riNamespace, Set objectClasses) { + this.icfNamespace = icfNamespace; + this.riNamespace = riNamespace; + this.objectClasses = new HashSet(objectClasses); + } + + /** Replaces the content of the index with the entries of {@code document}. */ + void rebuild(Document document) { + entries.clear(); + byClass.clear(); + byIdentifier.clear(); + nextOrder = 0; + containerRoot = false; + Element root = document.getDocumentElement(); + if (root == null || !icfNamespace.equals(root.getNamespaceURI()) + || !XMLHandlerImpl.ICF_CONTAINER_TAG.equals(root.getLocalName())) { + return; + } + containerRoot = true; + addAll(root); + } + + Element first(String objectClass, String field, String value) { + List list = byIdentifier.get(new Key(objectClass, field, value)); + return list == null ? null : list.get(0).element; + } + + List find(String objectClass, String field, String value) { + List list = byIdentifier.get(new Key(objectClass, field, value)); + if (list == null) { + return Collections.emptyList(); + } + List result = new ArrayList(list.size()); + for (Indexed indexed : list) { + result.add(indexed.element); + } + return result; + } + + List all(String objectClass) { + TreeMap ordered = byClass.get(objectClass); + return ordered == null ? Collections.emptyList() : new ArrayList(ordered.values()); + } + + /** Records an entry just appended as the last child of the container, and every entry inside it. */ + void added(Element entry) { + if (containerRoot) { + addAll(entry); + } + } + + /** Forgets {@code subtree} if it is an entry, and every entry inside it. */ + void removed(Node subtree) { + for (Element entry : entriesIn(subtree)) { + Indexed indexed = entries.remove(entry); + if (indexed != null) { + byClass.get(indexed.objectClass).remove(indexed.order); + unlink(indexed); + } + } + } + + /** + * Reads the identifiers of an entry whose children have changed; its place in document order stays. + * A new entry inside it, such as a value element named like an object class, has no place in + * document order yet, so the whole index is rebuilt then. + */ + void rekeyed(Element entry) { + Indexed indexed = entries.get(entry); + if (indexed == null) { + return; + } + if (holdsUnindexedEntry(entry)) { + rebuild(entry.getOwnerDocument()); + return; + } + unlink(indexed); + indexed.keys = keysOf(entry, indexed.objectClass); + link(indexed); + } + + private boolean isEntry(Node node) { + return riNamespace.equals(node.getNamespaceURI()) && objectClasses.contains(node.getLocalName()); + } + + /** Indexes {@code subtree} if it is an entry, and every entry inside it, in document order. */ + private void addAll(Node subtree) { + for (Element entry : entriesIn(subtree)) { + add(entry); + } + } + + private boolean holdsUnindexedEntry(Node subtree) { + for (Element entry : entriesIn(subtree)) { + if (!entries.containsKey(entry)) { + return true; + } + } + return false; + } + + /** {@code subtree} if it is an entry, and every entry inside it, in document order. */ + private List entriesIn(Node subtree) { + List found = new ArrayList(); + Node node = subtree; + while (node != null) { + Node child = null; + if (node.getNodeType() == Node.ELEMENT_NODE) { + if (isEntry(node)) { + found.add((Element) node); + } + child = node.getFirstChild(); + } + node = child != null ? child : following(node, subtree); + } + return found; + } + + private void add(Element entry) { + String objectClass = entry.getLocalName(); + Indexed indexed = new Indexed(entry, objectClass, nextOrder++, keysOf(entry, objectClass)); + entries.put(entry, indexed); + TreeMap ordered = byClass.get(objectClass); + if (ordered == null) { + ordered = new TreeMap(); + byClass.put(objectClass, ordered); + } + ordered.put(indexed.order, entry); + link(indexed); + } + + /** + * Identifier child elements only: {@code $x/icf:FIELD} selects children, not descendants. Each key + * once; an array, because an entry usually has one or two. + */ + private Key[] keysOf(Element entry, String objectClass) { + List keys = new ArrayList(2); + for (Node child = entry.getFirstChild(); child != null; child = child.getNextSibling()) { + if (child.getNodeType() == Node.ELEMENT_NODE && icfNamespace.equals(child.getNamespaceURI())) { + String field = child.getLocalName(); + if (Uid.NAME.equals(field) || Name.NAME.equals(field)) { + Key key = new Key(objectClass, field, child.getTextContent()); + if (!keys.contains(key)) { + keys.add(key); + } + } + } + } + return keys.toArray(new Key[keys.size()]); + } + + private void link(Indexed indexed) { + for (Key key : indexed.keys) { + List list = byIdentifier.get(key); + if (list == null) { + list = new ArrayList(1); + byIdentifier.put(key, list); + } + int i = list.size(); + while (i > 0 && list.get(i - 1).order > indexed.order) { + i--; + } + list.add(i, indexed); + } + } + + private void unlink(Indexed indexed) { + for (Key key : indexed.keys) { + List list = byIdentifier.get(key); + list.remove(indexed); + if (list.isEmpty()) { + byIdentifier.remove(key); + } + } + } + + private static final class Indexed { + final Element element; + final String objectClass; + final long order; + Key[] keys; + + Indexed(Element element, String objectClass, long order, Key[] keys) { + this.element = element; + this.objectClass = objectClass; + this.order = order; + this.keys = keys; + } + } + + private static final class Key { + final String objectClass; + final String field; + final String value; + + Key(String objectClass, String field, String value) { + this.objectClass = objectClass; + this.field = field; + this.value = value; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof Key)) { + return false; + } + Key other = (Key) o; + return objectClass.equals(other.objectClass) && field.equals(other.field) && value.equals(other.value); + } + + @Override + public int hashCode() { + return Objects.hash(objectClass, field, value); + } + } +} diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLConnector.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLConnector.java index 6381c755d..4bacc3644 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLConnector.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLConnector.java @@ -27,7 +27,6 @@ package org.forgerock.openicf.connectors.xml; import org.forgerock.openicf.connectors.xml.query.abstracts.Query; -import org.forgerock.openicf.connectors.xml.query.QueryBuilder; import org.forgerock.openicf.connectors.xml.xsdparser.SchemaParser; import java.io.IOException; import java.util.Collection; @@ -215,9 +214,7 @@ public FilterTranslator createFilterTranslator(ObjectClass objClass, Oper @Override public void executeQuery(ObjectClass objClass, Query query, ResultsHandler handler, OperationOptions options) { synchronized (getLock()) { - QueryBuilder queryBuilder = new QueryBuilder(query, objClass); - Collection hits = xmlInstanceHandler.search( - queryBuilder.toString(), objClass); + Collection hits = xmlInstanceHandler.search(query, objClass); int count = 0; for (ConnectorObject hit : hits) { count++; diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslator.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslator.java index deff517de..074666def 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslator.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslator.java @@ -64,11 +64,15 @@ public XMLFilterTranslator(boolean preferUidOverName) { } private String getAttributeName(Attribute attribute) { - String attrName = attribute.getName(); - if (!preferUidOverName && Uid.NAME.equals(attrName)) { - attrName = Name.NAME; - } - return attrName; + return elementName(attribute.getName(), preferUidOverName); + } + + /** + * The element an attribute is stored in: {@code __UID__} is stored as {@code __NAME__} unless + * {@code preferUidOverName}. XMLHandlerImpl looks entries up by the same rule. + */ + static String elementName(String attributeName, boolean preferUidOverName) { + return !preferUidOverName && Uid.NAME.equals(attributeName) ? Name.NAME : attributeName; } @Override @@ -203,7 +207,7 @@ public Query createLessThanOrEqualExpression(LessThanOrEqualFilter filter, boole private Query createComparisonQuery(String name, String operator, String value) { Query query = new QueryImpl(); - query.set(new ComparisonQuery("$x/" + name, operator, escape(value))); + query.set(new ComparisonQuery("$x/" + name, operator, value, escape(value))); return query; } diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandler.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandler.java index 8c1e79f20..5cccc60b4 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandler.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandler.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted 2010 [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC * * $Id$ */ @@ -27,6 +28,7 @@ import java.util.Collection; import java.util.Set; +import org.forgerock.openicf.connectors.xml.query.abstracts.Query; import org.identityconnectors.common.security.GuardedString; import org.identityconnectors.framework.common.objects.Attribute; import org.identityconnectors.framework.common.objects.ConnectorObject; @@ -43,6 +45,8 @@ public interface XMLHandler { Collection search(String query, ObjectClass objectClass); + Collection search(Query query, ObjectClass objectClass); + Uid authenticate(String username, GuardedString password); XMLHandler init(); diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java index 9bf1b9c9c..a17d9a751 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XMLHandlerImpl.java @@ -31,9 +31,11 @@ import org.forgerock.openicf.connectors.xml.util.GuardedStringAccessor; import org.forgerock.openicf.connectors.xml.util.NamespaceLookupUtil; import org.forgerock.openicf.connectors.xml.util.XmlHandlerUtil; -import org.forgerock.openicf.connectors.xml.query.abstracts.Query; +import org.forgerock.openicf.connectors.xml.query.ComparisonQuery; import org.forgerock.openicf.connectors.xml.query.QueryBuilder; import org.forgerock.openicf.connectors.xml.query.XQueryHandler; +import org.forgerock.openicf.connectors.xml.query.abstracts.Query; +import org.forgerock.openicf.connectors.xml.query.abstracts.QueryPart; import com.sun.xml.xsom.XSSchema; import com.sun.xml.xsom.XSSchemaSet; @@ -44,6 +46,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -73,10 +76,8 @@ import org.identityconnectors.framework.common.objects.Schema; import org.identityconnectors.common.Assertions; import org.identityconnectors.framework.common.exceptions.ConnectorException; -import org.identityconnectors.framework.common.objects.AttributeBuilder; import org.identityconnectors.framework.common.objects.AttributeInfoUtil; import org.identityconnectors.framework.common.objects.Uid; -import org.identityconnectors.framework.common.objects.filter.EqualsFilter; import org.w3c.dom.*; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -93,6 +94,7 @@ public class XMLHandlerImpl implements XMLHandler { private Schema connSchema; private XSSchema icfSchema; private XSSchema riSchema; + private final EntryIndex entryIndex; /** Whether the document holds user changes that the file does not have yet. */ private boolean dirty; /** Whether the document was created in memory because the file did not exist, and is not saved yet. */ @@ -115,6 +117,12 @@ public XMLHandlerImpl(XMLConfiguration config, Schema connSchema, XSSchemaSet xs this.riSchema = xsdSchemas.getSchema(1); this.icfSchema = xsdSchemas.getSchema(2); + Set objectClasses = new HashSet(); + for (ObjectClassInfo info : connSchema.getObjectClassInfo()) { + objectClasses.add(info.getType()); + } + entryIndex = new EntryIndex(icfSchema.getTargetNamespace(), riSchema.getTargetNamespace(), objectClasses); + NamespaceLookupUtil.INSTANCE.initialize(icfSchema, riSchema); } @@ -276,6 +284,7 @@ public Uid create(final ObjectClass objClass, final Set attributes) { markDirty(); getDocument().getDocumentElement().appendChild(objElement); + entryIndex.added(objElement); log.info("Exit {0}", method); @@ -292,10 +301,12 @@ public Uid update(ObjectClass objClass, Uid uid, Set replaceAttribute ObjectClassInfo objInfo = connSchema.findObjectClassInfo(objClass.getObjectClassValue()); Map objAttributes = AttributeInfoUtil.toMap(objInfo.getAttributeInfo()); - if (entryExists(objClass, uid, ElementIdentifierFieldType.AUTO)) { - - Element entry = getEntry(objClass, uid, ElementIdentifierFieldType.AUTO); + Element entry = getEntry(objClass, uid, ElementIdentifierFieldType.AUTO); + if (entry == null) { + throw new UnknownUidException("Could not update entry. No entry of type " + objClass.getObjectClassValue() + " with the id " + uid.getUidValue() + " found."); + } + try { for (Attribute attribute : replaceAttributes) { if (!objAttributes.containsKey(attribute.getName())) { @@ -351,8 +362,9 @@ public Uid update(ObjectClass objClass, Uid uid, Set replaceAttribute } } } - } else { - throw new UnknownUidException("Could not update entry. No entry of type " + objClass.getObjectClassValue() + " with the id " + uid.getUidValue() + " found."); + } finally { + // A rejected attribute may leave the entry already changed, so report the change either way. + entryIndex.rekeyed(entry); } log.info("Exit {0}", method); @@ -367,14 +379,14 @@ public void delete(final ObjectClass objClass, final Uid uid) { XmlHandlerUtil.checkObjectType(objClass, riSchema); - if (entryExists(objClass, uid, ElementIdentifierFieldType.AUTO)) { - Element elementToRemove = getEntry(objClass, uid, ElementIdentifierFieldType.AUTO); - getDocument().getDocumentElement().removeChild(elementToRemove); - markDirty(); - log.info("Deleting entry: " + elementToRemove.toString()); - } else { + Element elementToRemove = getEntry(objClass, uid, ElementIdentifierFieldType.AUTO); + if (elementToRemove == null) { throw new UnknownUidException("Deleting entry failed. Could not find an entry of type " + objClass.getObjectClassValue() + " with the uid " + uid.getUidValue()); } + getDocument().getDocumentElement().removeChild(elementToRemove); + markDirty(); + entryIndex.removed(elementToRemove); + log.info("Deleting entry: " + elementToRemove.toString()); log.info("Exit {0}", method); } @@ -414,6 +426,63 @@ public Collection search(String query, ObjectClass objClass) { return results; } + @Override + public Collection search(Query query, ObjectClass objClass) { + List entries = indexedEntries(query, objClass); + if (entries == null) { + return search(new QueryBuilder(query, objClass).toString(), objClass); + } + return toConnectorObjects(entries, objClass); + } + + /** The entries a query selects if the index can tell, in document order; {@code null} otherwise. */ + List indexedEntries(Query query, ObjectClass objClass) { + getDocument(); // throws if there is no document, as the XQuery search did + String objectClass = objClass.getObjectClassValue(); + if (query == null || query.getParts().isEmpty()) { + return entryIndex.all(objectClass); + } + String field = identifierField(query); + if (field == null) { + return null; + } + String value = ((ComparisonQuery) query.getParts().iterator().next()).getValue(); + return entryIndex.find(objectClass, field, value); + } + + /** + * The identifier a query compares, if the query is exactly {@code $x/icf:__UID__ = 'v'} or + * {@code $x/icf:__NAME__ = 'v'} after XMLFilterTranslator's rewrites; otherwise {@code null}. + */ + static String identifierField(Query query) { + if (query.getParts().size() != 1) { + return null; + } + QueryPart part = query.getParts().iterator().next(); + if (!(part instanceof ComparisonQuery)) { + return null; + } + ComparisonQuery comparison = (ComparisonQuery) part; + if (!"=".equals(comparison.getOperator()) || comparison.getValue() == null) { + return null; + } + for (String field : new String[] {Uid.NAME, Name.NAME}) { + if (("$x/" + ICF_NAMESPACE_PREFIX + ":" + field).equals(comparison.getName())) { + return field; + } + } + return null; + } + + private List toConnectorObjects(Collection entries, ObjectClass objClass) { + ConnectorObjectCreator creator = newConnectorObjectCreator(objClass); + List results = new ArrayList(entries.size()); + for (Element entry : entries) { + results.add(creator.createConnectorObject(entry)); + } + return results; + } + @Override public void dispose() { final String method = "serialize"; @@ -435,7 +504,9 @@ public void dispose() { // Callers hold the handler's write lock. Document saved = getDocument(); try { - XmlDocumentWriter.normalizeText(saved); + if (XmlDocumentWriter.normalizeText(saved)) { + entryIndex.rebuild(saved); // a whitespace-only identifier may have just become empty + } checksum = XmlDocumentWriter.write(saved, xmlFile); dirty = false; unsavedNewFile = false; @@ -615,6 +686,7 @@ private void buildDocument() { } else { loadDocument(xmlFile); } + entryIndex.rebuild(document); log.info("Exit {0}", method); } @@ -637,51 +709,17 @@ private Element createDomElement(String elementName, String value) { } private Element getEntry(ObjectClass objClass, Uid uid, ElementIdentifierFieldType identifierField) { - final String method = "getEntry"; - log.info("Entry {0}", method); - - Element result = null; - - // Build search query - XMLFilterTranslator translator = new XMLFilterTranslator(isSupportUid(objClass)); - String idField = getElementIdentifierField(objClass, identifierField); - AttributeBuilder builder = new AttributeBuilder(); - builder.setName(idField); - builder.addValue(uid.getUidValue()); - EqualsFilter equals = new EqualsFilter(builder.build()); - Query query = translator.createEqualsExpression(equals, false); - QueryBuilder queryBuilder = new QueryBuilder(query, objClass); - - // Execute query - XQueryHandler xqHandler = null; - - try { - xqHandler = new XQueryHandler(queryBuilder.toString(), document); - XQResultSequence results = xqHandler.getResultSequence(); - - if (results.next()) { - result = (Element) results.getItem().getNode(); - log.info("Entry found: ", result.toString()); - } - } catch (XQException ex) { - throw ConnectorException.wrap(ex); - } finally { - if (null != xqHandler) { - xqHandler.close(); - } - } - - log.info("Exit {0}", method); + getDocument(); + return entryIndex.first(objClass.getObjectClassValue(), identifierElement(objClass, identifierField), uid.getUidValue()); + } - return result; + /** The identifier element to match, by the rule that XMLFilterTranslator applies to a search. */ + private String identifierElement(ObjectClass objClass, ElementIdentifierFieldType identifierField) { + return XMLFilterTranslator.elementName(getElementIdentifierField(objClass, identifierField), isSupportUid(objClass)); } private boolean entryExists(ObjectClass objClass, Uid uid, ElementIdentifierFieldType identifierField) { - if (getEntry(objClass, uid, identifierField) != null) { - return true; - } - - return false; + return getEntry(objClass, uid, identifierField) != null; } private String getElementIdentifierField(ObjectClass objClass, ElementIdentifierFieldType identifierField) { @@ -731,6 +769,7 @@ private void removeChildrenFromElement(Element element, String childName) { for (Element e : elementsToRemove) { element.removeChild(e); + entryIndex.removed(e); } } diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriter.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriter.java index 61c1e3d9b..b4246652d 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriter.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriter.java @@ -67,8 +67,13 @@ private XmlDocumentWriter() { * {@code //text()[normalize-space(.) = '']}, removed only its first node. Any other run * becomes one Text node with the same value, which is what a reload of the written file gives. * Text inside an entity reference is left alone: the DOM makes it read-only. + * + * @return {@code true} if the tree changed: a whitespace-only run was removed, or a run of + * several nodes or a lone CDATA section was replaced by a Text node; a lone Text node + * that holds more than whitespace is no change */ - static void normalizeText(Node root) { + static boolean normalizeText(Node root) { + boolean changed = false; Node node = root.getFirstChild(); while (node != null) { if (!isText(node)) { @@ -82,6 +87,7 @@ static void normalizeText(Node root) { // A lone Text node, the usual case: nothing to merge. if (isXmlWhitespace(node.getNodeValue())) { parent.removeChild(node); + changed = true; } } else { List run = new ArrayList(); @@ -96,9 +102,11 @@ static void normalizeText(Node root) { for (Node n : run) { parent.removeChild(n); } + changed = true; } node = after != null ? after : following(parent, root); } + return changed; } /** diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/query/ComparisonQuery.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/query/ComparisonQuery.java index e2308e8a1..962b0e74d 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/query/ComparisonQuery.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/query/ComparisonQuery.java @@ -20,6 +20,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted 2010 [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC * * $Id$ */ @@ -30,19 +31,37 @@ public class ComparisonQuery implements QueryPart { - private String name; - private String operator; - private String value; - - - public ComparisonQuery(String name, String operator, String value) { + private final String name; + private final String operator; + private final String value; + private final String literal; + + public ComparisonQuery(String name, String operator, String literal) { + this(name, operator, null, literal); + } + + public ComparisonQuery(String name, String operator, String value, String literal) { this.name = name; this.operator = operator; this.value = value; + this.literal = literal; + } + + public String getName() { + return name; + } + + public String getOperator() { + return operator; + } + + /** The compared value, or {@code null} if only an XQuery literal was given. */ + public String getValue() { + return value; } @Override public String getExpression() { - return String.format("%s %s %s", this.name, this.operator, this.value); + return String.format("%s %s %s", this.name, this.operator, this.literal); } } diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/EntryIndexTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/EntryIndexTests.java new file mode 100644 index 000000000..136b5f0a1 --- /dev/null +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/EntryIndexTests.java @@ -0,0 +1,244 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openicf.connectors.xml; + +import static org.forgerock.openicf.connectors.xml.XmlConnectorTestUtil.ICF_NAMESPACE; +import static org.forgerock.openicf.connectors.xml.XmlConnectorTestUtil.RI_NAMESPACE; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; + +import java.io.StringReader; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; + +import javax.xml.parsers.DocumentBuilderFactory; + +import org.identityconnectors.framework.common.objects.Name; +import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.Uid; +import org.testng.annotations.Test; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.xml.sax.InputSource; + +public class EntryIndexTests { + + private static final String ACCOUNT = ObjectClass.ACCOUNT_NAME; + private static final String GROUP = ObjectClass.GROUP_NAME; + private static final String NAME = Name.NAME; + private static final String UID = Uid.NAME; + + @Test + public void findsEntriesByNameAndUidInDocumentOrder() throws Exception { + EntryIndex index = index(parse(account("u1", "dup") + account("u2", "x") + account("u3", "dup"))); + assertEquals(uids(index.find(ACCOUNT, NAME, "dup")), Arrays.asList("u1", "u3")); + assertEquals(uidOf(index.first(ACCOUNT, NAME, "dup")), "u1"); + assertEquals(uidOf(index.first(ACCOUNT, UID, "u2")), "u2"); + assertNull(index.first(ACCOUNT, NAME, "missing")); + assertTrue(index.find(ACCOUNT, NAME, "missing").isEmpty()); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u1", "u2", "u3")); + } + + @Test + public void findsNestedEntriesAndIgnoresEverythingElse() throws Exception { + EntryIndex index = index(parse("" + account("u1", "a") + "" + + "u2a" + + "u3a" + + "u5a" + + "u4deep")); + assertEquals(uids(index.find(ACCOUNT, NAME, "a")), Arrays.asList("u1")); + assertNull(index.first(ACCOUNT, NAME, "deep")); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u1", "u4")); + } + + @Test + public void classesAreSeparate() throws Exception { + EntryIndex index = index(parse(account("u1", "dup") + + "g1dup")); + assertEquals(uids(index.find(ACCOUNT, NAME, "dup")), Arrays.asList("u1")); + assertEquals(uids(index.find(GROUP, NAME, "dup")), Arrays.asList("g1")); + } + + @Test + public void everyIdentifierValueFindsTheEntryOnce() throws Exception { + EntryIndex index = index(parse("u1" + + "aba")); + assertEquals(uids(index.find(ACCOUNT, NAME, "a")), Arrays.asList("u1")); + assertEquals(uids(index.find(ACCOUNT, NAME, "b")), Arrays.asList("u1")); + } + + @Test + public void valuesAreComparedAsIs() throws Exception { + EntryIndex index = index(parse(account("u1", " padded ") + + "u2ata")); + assertEquals(uids(index.find(ACCOUNT, NAME, " padded ")), Arrays.asList("u1")); + assertTrue(index.find(ACCOUNT, NAME, "padded").isEmpty()); + assertEquals(uids(index.find(ACCOUNT, NAME, "cdata")), Arrays.asList("u2")); + } + + @Test + public void rootMustBeTheContainer() throws Exception { + EntryIndex index = index(parseRaw("" + + account("u1", "a") + "")); + assertTrue(index.all(ACCOUNT).isEmpty()); + } + + @Test + public void addedEntryOfADocumentWithAnotherRootIsNotIndexed() throws Exception { + Document document = parseRaw(""); + EntryIndex index = index(parse(account("u0", "z"))); // a container first: a rebuild must forget it + index.rebuild(document); + Element added = (Element) document.importNode(parse(account("u1", "a")).getDocumentElement().getFirstChild(), true); + document.getDocumentElement().appendChild(added); + index.added(added); + assertTrue(index.all(ACCOUNT).isEmpty()); + assertNull(index.first(ACCOUNT, NAME, "a")); + } + + @Test + public void addedEntryComesLast() throws Exception { + Document document = parse(account("u1", "dup")); + EntryIndex index = index(document); + Element added = (Element) document.importNode(parse(account("u2", "dup")).getDocumentElement().getFirstChild(), true); + document.getDocumentElement().appendChild(added); + index.added(added); + assertEquals(uids(index.find(ACCOUNT, NAME, "dup")), Arrays.asList("u1", "u2")); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u1", "u2")); + } + + @Test + public void removedEntryExposesTheNextDuplicate() throws Exception { + Document document = parse(account("u1", "dup") + account("u2", "dup")); + EntryIndex index = index(document); + Element first = index.first(ACCOUNT, NAME, "dup"); + document.getDocumentElement().removeChild(first); + index.removed(first); + assertEquals(uids(index.find(ACCOUNT, NAME, "dup")), Arrays.asList("u2")); + assertNull(index.first(ACCOUNT, UID, "u1")); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u2")); + } + + @Test + public void removedWrapperTakesItsEntriesAlong() throws Exception { + Document document = parse("" + account("u1", "a") + "" + account("u2", "b")); + EntryIndex index = index(document); + Node wrapper = document.getDocumentElement().getFirstChild(); + document.getDocumentElement().removeChild(wrapper); + index.removed(wrapper); + assertNull(index.first(ACCOUNT, NAME, "a")); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u2")); + } + + @Test + public void rekeyedEntryKeepsItsPlaceInDocumentOrder() throws Exception { + Document document = parse(account("u1", "old") + account("u2", "dup")); + EntryIndex index = index(document); + Element first = index.first(ACCOUNT, UID, "u1"); + first.getElementsByTagNameNS(ICF_NAMESPACE, NAME).item(0).setTextContent("dup"); + index.rekeyed(first); + assertNull(index.first(ACCOUNT, NAME, "old")); + assertEquals(uids(index.find(ACCOUNT, NAME, "dup")), Arrays.asList("u1", "u2")); + } + + @Test + public void rekeyedEntryWithoutANameIsFoundByUidOnly() throws Exception { + Document document = parse(account("u1", "a")); + EntryIndex index = index(document); + Element entry = index.first(ACCOUNT, UID, "u1"); + entry.removeChild(entry.getElementsByTagNameNS(ICF_NAMESPACE, NAME).item(0)); + index.rekeyed(entry); + assertNull(index.first(ACCOUNT, NAME, "a")); + assertEquals(uidOf(index.first(ACCOUNT, UID, "u1")), "u1"); + } + + /** create writes every attribute as a child element, so a schema attribute named like a class gives an entry. */ + @Test + public void addedEntryBringsTheEntriesInsideItAlong() throws Exception { + Document document = parse(account("u1", "a")); + EntryIndex index = index(document); + Element added = (Element) document.importNode(parse("g" + + account("u2", "b") + "").getDocumentElement().getFirstChild(), true); + document.getDocumentElement().appendChild(added); + index.added(added); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u1", "u2")); + assertEquals(uidOf(index.first(ACCOUNT, NAME, "b")), "u2"); + assertSame(index.first(GROUP, NAME, "g"), added); + } + + @Test + public void entryWrittenIntoAnEarlierEntryTakesItsPlaceInDocumentOrder() throws Exception { + Document document = parse("g" + account("u1", "a")); + EntryIndex index = index(document); + Element group = index.first(GROUP, NAME, "g"); + group.appendChild(document.importNode(parse(account("u2", "b")).getDocumentElement().getFirstChild(), true)); + index.rekeyed(group); + assertEquals(uids(index.all(ACCOUNT)), Arrays.asList("u2", "u1")); + assertEquals(uidOf(index.first(ACCOUNT, NAME, "b")), "u2"); + assertSame(index.first(GROUP, NAME, "g"), group); + } + + @Test + public void rekeyedEntryWithoutANewEntryInsideLeavesTheRestAlone() throws Exception { + // Stands in for a count of rebuilds: one would read u2's unreported rename and walk the whole document. + Document document = parse("u1a" + + "" + account("u3", "c") + "" + account("u2", "b")); + EntryIndex index = index(document); + Element second = index.first(ACCOUNT, UID, "u2"); + second.getElementsByTagNameNS(ICF_NAMESPACE, NAME).item(0).setTextContent("renamed"); + index.rekeyed(index.first(ACCOUNT, UID, "u1")); + assertSame(index.first(ACCOUNT, NAME, "b"), second); + assertNull(index.first(ACCOUNT, NAME, "renamed")); + } + + private static EntryIndex index(Document document) { + EntryIndex index = new EntryIndex(ICF_NAMESPACE, RI_NAMESPACE, new HashSet(Arrays.asList(ACCOUNT, GROUP))); + index.rebuild(document); + return index; + } + + private static String account(String uid, String name) { + return "" + uid + "" + name + ""; + } + + private static Document parse(String entries) throws Exception { + return parseRaw("" + entries + ""); + } + + private static Document parseRaw(String xml) throws Exception { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + return factory.newDocumentBuilder().parse(new InputSource(new StringReader(xml))); + } + + private static String uidOf(Element entry) { + return entry.getElementsByTagNameNS(ICF_NAMESPACE, UID).item(0).getTextContent(); + } + + private static List uids(Collection entries) { + List uids = new ArrayList(); + for (Element entry : entries) { + uids.add(uidOf(entry)); + } + return uids; + } +} diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorBundleIT.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorBundleIT.java index 511101e1a..988b5abf7 100644 --- a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorBundleIT.java +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorBundleIT.java @@ -21,8 +21,10 @@ import static org.testng.Assert.assertTrue; import java.io.File; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; import org.identityconnectors.common.security.GuardedString; import org.identityconnectors.framework.api.APIConfiguration; @@ -36,6 +38,7 @@ import org.identityconnectors.framework.common.objects.ConnectorObject; import org.identityconnectors.framework.common.objects.ObjectClass; import org.identityconnectors.framework.common.objects.Uid; +import org.identityconnectors.framework.common.objects.filter.FilterBuilder; import org.testng.annotations.Test; /** @@ -56,12 +59,20 @@ public void bundleSavesLoadsAndFindsEntries() throws Exception { properties.setPropertyValue("xmlFilePath", xmlFile); properties.setPropertyValue("xsdFilePath", XSD_SCHEMA_FILEPATH); properties.setPropertyValue("createFileIfNotExists", true); + // The framework re-applies the filter to what the connector returns; off, a query the connector ignores shows. + api.getResultsHandlerConfiguration().setEnableFilteredResultsHandler(false); ConnectorFacade facade = ConnectorFacadeFactory.getInstance().newInstance(api); Uid alice = facade.create(ObjectClass.ACCOUNT, account("alice"), null); facade.create(ObjectClass.ACCOUNT, account("bob"), null); facade.update(ObjectClass.ACCOUNT, alice, Collections.singleton(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "Changed")), null); + // A filter on a non-identifier attribute is the one call that still runs Saxon XQJ under the bundle's loader. + List changed = new ArrayList(); + facade.search(ObjectClass.ACCOUNT, FilterBuilder.equalTo(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "Changed")), + changed::add, null); + assertEquals(changed.size(), 1); + assertEquals(changed.get(0).getUid().getUidValue(), alice.getUidValue()); Uid bob = facade.authenticate(ObjectClass.ACCOUNT, "bob", new GuardedString("secret-bob".toCharArray()), null); facade.delete(ObjectClass.ACCOUNT, bob, null); diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorScaleTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorScaleTests.java new file mode 100644 index 000000000..d7ab97fd8 --- /dev/null +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorScaleTests.java @@ -0,0 +1,89 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openicf.connectors.xml; + +import static org.forgerock.openicf.connectors.xml.XmlConnectorTestUtil.*; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertTrue; + +import java.io.File; + +import org.identityconnectors.common.security.GuardedString; +import org.identityconnectors.framework.api.ConnectorFacade; +import org.identityconnectors.framework.api.ConnectorFacadeFactory; +import org.identityconnectors.framework.common.objects.Name; +import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.Uid; +import org.identityconnectors.framework.common.objects.filter.EqualsFilter; +import org.identityconnectors.test.common.TestHelpers; +import org.testng.annotations.Test; + +public class XMLConnectorScaleTests { + + private static final int ENTRIES = 20_000; + private static final int LOOKUPS = 20; + /** + * About 0.3 s with the index (measured: 343 ms); the lookups routed around it take tens of + * seconds (measured: 24.8 s and 30.0 s with two routing mutants). + */ + private static final long LIMIT_MILLIS = 5_000L; + + @Test(timeOut = 120_000L) + public void identifierLookupsOnALargeFileStayFast() throws Exception { + File file = getRandomXMLFile(); + String[] names = new String[ENTRIES]; + for (int i = 0; i < ENTRIES; i++) { + names[i] = "user-" + i; + } + writeAccounts(file, names); + // An old mtime keeps the stamp out of the racy window, so the file is parsed once. + setModified(file, System.currentTimeMillis() - 3_600_000L); + + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(file); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + ConnectorFacade facade = ConnectorFacadeFactory.getInstance() + .newInstance(TestHelpers.createTestConfiguration(XMLConnector.class, config)); + try { + facade.test(); // parses the file and builds the index; not timed + + long start = System.nanoTime(); + for (int i = 0; i < LOOKUPS; i++) { + String name = names[ENTRIES - 1 - i * (ENTRIES / LOOKUPS)]; + TestResultsHandler byName = new TestResultsHandler(); + facade.search(ObjectClass.ACCOUNT, new EqualsFilter(new Name(name)), byName, null); + assertEquals(byName.getResultSize(), 1, name); + assertNotNull(facade.getObject(ObjectClass.ACCOUNT, new Uid("uid-" + name), null), name); + assertEquals(facade.authenticate(ObjectClass.ACCOUNT, name, + new GuardedString(("secret-" + name).toCharArray()), null).getUidValue(), "uid-" + name); + } + TestResultsHandler missing = new TestResultsHandler(); + facade.search(ObjectClass.ACCOUNT, new EqualsFilter(new Name("no-such-user")), missing, null); + assertEquals(missing.getResultSize(), 0); + long elapsedMillis = (System.nanoTime() - start) / 1_000_000L; + + assertTrue(elapsedMillis < LIMIT_MILLIS, + (3 * LOOKUPS + 1) + " lookups in " + ENTRIES + " entries took " + elapsedMillis + " ms"); + } finally { + // The handler cache is static: without this, the DOM and the index stay for the rest of the run. + synchronized (XMLConnector.class) { + XMLConnector.XMLHandlerCache.remove(file.getCanonicalPath()); + } + file.delete(); + } + } +} diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorTests.java index 992afa4ea..a0176358a 100644 --- a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorTests.java +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLConnectorTests.java @@ -43,6 +43,7 @@ import java.util.Set; import java.util.concurrent.*; import javax.xml.parsers.DocumentBuilderFactory; +import org.forgerock.openicf.connectors.xml.query.abstracts.Query; import org.forgerock.openicf.connectors.xml.xsdparser.SchemaParser; import org.identityconnectors.common.security.GuardedString; import org.identityconnectors.framework.api.APIConfiguration; @@ -448,6 +449,11 @@ public Collection search(String query, ObjectClass objectClass) throw new UnsupportedOperationException(); } + @Override + public Collection search(Query query, ObjectClass objectClass) { + throw new UnsupportedOperationException(); + } + @Override public Uid authenticate(String username, GuardedString password) { throw new UnsupportedOperationException(); diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslatorTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslatorTests.java index 365d3a788..d1157102f 100644 --- a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslatorTests.java +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLFilterTranslatorTests.java @@ -19,6 +19,7 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted 2010 [name of copyright owner]" + * Portions Copyrighted 2026 3A Systems, LLC * * $Id$ */ @@ -348,4 +349,19 @@ private List getResultsFromQuery(Query query) { return new ArrayList( xmlHandler.search(new QueryBuilder(query, ObjectClass.ACCOUNT).toString(), ObjectClass.ACCOUNT)); } + + @Test + public void comparisonExpressionsAreUnchanged() { + AssertJUnit.assertEquals("$x/icf:__NAME__ = 'O''B'", expression(filterTranslator.createEqualsExpression( + new EqualsFilter(AttributeBuilder.build(ATTR_NAME, "O'B")), false))); + AssertJUnit.assertEquals("$x/icf:__NAME__ != 'x'", expression(filterTranslator.createEqualsExpression( + new EqualsFilter(AttributeBuilder.build(ATTR_NAME, "x")), true))); + AssertJUnit.assertEquals("$x/ri:lastname >= 'M'", expression(filterTranslator.createGreaterThanOrEqualExpression( + new GreaterThanOrEqualFilter(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "M")), false))); + } + + private static String expression(Query query) { + AssertJUnit.assertEquals(1, query.getParts().size()); + return query.getParts().iterator().next().getExpression(); + } } diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerIndexTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerIndexTests.java new file mode 100644 index 000000000..0a3a07103 --- /dev/null +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerIndexTests.java @@ -0,0 +1,363 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ +package org.forgerock.openicf.connectors.xml; + +import static org.forgerock.openicf.connectors.xml.XmlConnectorTestUtil.*; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertNotNull; +import static org.testng.Assert.assertNull; +import static org.testng.Assert.assertSame; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.expectThrows; + +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import org.forgerock.openicf.connectors.xml.query.ComparisonQuery; +import org.forgerock.openicf.connectors.xml.query.QueryBuilder; +import org.forgerock.openicf.connectors.xml.query.QueryImpl; +import org.forgerock.openicf.connectors.xml.query.XQueryHandler; +import org.forgerock.openicf.connectors.xml.query.abstracts.Query; +import org.forgerock.openicf.connectors.xml.xsdparser.SchemaParser; +import org.identityconnectors.common.security.GuardedString; +import org.identityconnectors.framework.common.exceptions.AlreadyExistsException; +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.identityconnectors.framework.common.exceptions.UnknownUidException; +import org.identityconnectors.framework.common.objects.Attribute; +import org.identityconnectors.framework.common.objects.AttributeBuilder; +import org.identityconnectors.framework.common.objects.ConnectorObject; +import org.identityconnectors.framework.common.objects.Name; +import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.Uid; +import org.identityconnectors.framework.common.objects.filter.EqualsFilter; +import org.identityconnectors.framework.common.objects.filter.GreaterThanOrEqualFilter; +import org.identityconnectors.framework.common.objects.filter.StartsWithFilter; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; +import org.w3c.dom.Element; +import org.w3c.dom.Node; + +import javax.xml.xquery.XQResultSequence; + +public class XMLHandlerIndexTests { + + private XMLHandlerImpl handler; + + @BeforeMethod + public void setUp() throws Exception { + File file = getRandomXMLFile(); + Files.copy(getTestFile("index-fixture.xml").toPath(), file.toPath()); + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(file); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + SchemaParser parser = new SchemaParser(XMLConnector.class, config.getXsdFilePath()); + handler = new XMLHandlerImpl(config, parser.parseSchema(), parser.getXsdSchema()); + handler.init(); + } + + @Test + public void deleteRemovesTheFirstDuplicateAndExposesTheNext() { + handler.delete(ObjectClass.ACCOUNT, new Uid("u1")); + assertEquals(byXQuery(ObjectClass.ACCOUNT, Name.NAME, "dup"), Collections.singletonList("u2|dup")); + expectThrows(AlreadyExistsException.class, () -> handler.create(ObjectClass.ACCOUNT, account("dup"))); + + handler.delete(ObjectClass.ACCOUNT, new Uid("u2")); + handler.create(ObjectClass.ACCOUNT, account("dup")); + } + + @Test + public void renamedEntryIsFoundByItsNewNameOnly() { + handler.update(ObjectClass.ACCOUNT, new Uid("u1"), + Collections.singleton(AttributeBuilder.build(Name.NAME, "renamed"))); + expectThrows(AlreadyExistsException.class, () -> handler.create(ObjectClass.ACCOUNT, account("renamed"))); + + handler.update(ObjectClass.ACCOUNT, new Uid("u1"), + Collections.singleton(AttributeBuilder.build(Name.NAME, "dup"))); + handler.create(ObjectClass.ACCOUNT, account("renamed")); + } + + @Test + public void failedRenameLeavesNoStaleName() { + expectThrows(IllegalArgumentException.class, + () -> handler.update(ObjectClass.ACCOUNT, new Uid("u1"), renameThenFail("a"))); + // The rename was applied before the failure (#158): u1 is no longer "dup" in the document. + assertEquals(byXQuery(ObjectClass.ACCOUNT, Name.NAME, "a"), Collections.singletonList("u1|a")); + assertEquals(describe(handler.search(equalsQuery(Name.NAME, "a"), ObjectClass.ACCOUNT)), + Collections.singletonList("u1|a")); // the index has the new name too + handler.delete(ObjectClass.ACCOUNT, new Uid("u2")); + handler.create(ObjectClass.ACCOUNT, account("dup")); // a stale "dup" for u1 in the index would refuse this + } + + /** + * Renames to {@code newName}, then fails on an unknown attribute. update() applies the attributes + * in order, so the rename is in the document when it throws. A two-valued __NAME__ cannot be used: + * AttributeBuilder.build(Name.NAME, "a", "b") throws before update() runs. + */ + static Set renameThenFail(String newName) { + Set attributes = new LinkedHashSet(); + attributes.add(AttributeBuilder.build(Name.NAME, newName)); + attributes.add(AttributeBuilder.build("no-such-attribute", "x")); + return attributes; + } + + @Test + public void authenticateFindsTheEntryByName() { + handler.create(ObjectClass.ACCOUNT, account("eve")); + Uid uid = handler.authenticate("eve", new GuardedString("secret-eve".toCharArray())); + assertEquals(uid.getUidValue() + "|eve", byXQuery(ObjectClass.ACCOUNT, Name.NAME, "eve").get(0)); + } + + @Test + public void identifierWithAnAmpersandIsFound() { + // The XQuery literal 'a&b' is not valid: create's duplicate check failed on it before the index. + Uid uid = handler.create(ObjectClass.ACCOUNT, account("a&b")); + Query byName = new XMLFilterTranslator(true).createEqualsExpression( + new EqualsFilter(AttributeBuilder.build(Name.NAME, "a&b")), false); + assertEquals(describe(handler.search(byName, ObjectClass.ACCOUNT)), + Collections.singletonList(uid.getUidValue() + "|a&b")); + assertEquals(handler.authenticate("a&b", new GuardedString("secret-a&b".toCharArray())), uid); + } + + @Test + public void identifierLookupsUseTheHandlersOwnNamespaces() throws Exception { + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(getRandomXMLFile()); + Files.copy(getTestFile("index-fixture.xml").toPath(), config.getXmlFilePath().toPath()); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + SchemaParser parser = new SchemaParser(XMLConnector.class, XSD_SCHEMA_FILEPATH); + XMLHandlerImpl own = new XMLHandlerImpl(config, parser.parseSchema(), parser.getXsdSchema()); + + // Next to the original, so that its import of resource-schema-1.xsd resolves. + File otherXsd = getTestFile("other-ri-namespace.xsd"); + String xsd = new String(Files.readAllBytes(XSD_SCHEMA_FILEPATH.toPath()), StandardCharsets.UTF_8); + Files.write(otherXsd.toPath(), xsd.replace("instances/ef2bc95b-76e0-48e2-86d6-4d4f44d4e4a4", "instances/other") + .getBytes(StandardCharsets.UTF_8)); + try { + XMLConfiguration otherConfig = new XMLConfiguration(); + otherConfig.setXmlFilePath(getRandomXMLFile()); + otherConfig.setXsdFilePath(otherXsd); + SchemaParser otherParser = new SchemaParser(XMLConnector.class, otherXsd); + // Sets the JVM-wide namespaces to its own: it is now the last configured handler. + new XMLHandlerImpl(otherConfig, otherParser.parseSchema(), otherParser.getXsdSchema()); + own.init(); + assertEquals(describe(own.search(equalsQuery(Name.NAME, "dup"), ObjectClass.ACCOUNT)), + Arrays.asList("u1|dup", "u2|dup")); + } finally { + Files.delete(otherXsd.toPath()); + // Puts the JVM-wide namespaces back for the tests that run after this one. + new XMLHandlerImpl(config, parser.parseSchema(), parser.getXsdSchema()); + } + } + + /** {@code $x/icf:FIELD = 'VALUE'} exactly as XMLFilterTranslator builds it, without the framework's value checks. */ + static Query equalsQuery(String field, String value) { + Query query = new QueryImpl(); + query.set(new ComparisonQuery("$x/icf:" + field, "=", value, "'" + value.replaceAll("'", "''") + "'")); + return query; + } + + List byXQuery(ObjectClass objectClass, String field, String value) { + return describe(handler.search(new QueryBuilder(equalsQuery(field, value), objectClass).toString(), objectClass)); + } + + static List describe(Collection objects) { + List result = new ArrayList(); + for (ConnectorObject object : objects) { + result.add(object.getUid().getUidValue() + "|" + (object.getName() == null ? null : object.getName().getNameValue())); + } + return result; + } + + private static final String[][] PROBES = { + {"__ACCOUNT__", "__NAME__", "dup"}, {"__ACCOUNT__", "__NAME__", "no-uid"}, + {"__ACCOUNT__", "__NAME__", "two-a"}, {"__ACCOUNT__", "__NAME__", "two-b"}, + {"__ACCOUNT__", "__NAME__", " padded "}, {"__ACCOUNT__", "__NAME__", "padded"}, + {"__ACCOUNT__", "__NAME__", "cdata"}, {"__ACCOUNT__", "__NAME__", "foreign"}, + {"__ACCOUNT__", "__NAME__", "nested"}, {"__ACCOUNT__", "__NAME__", "late"}, + {"__ACCOUNT__", "__NAME__", " "}, {"__ACCOUNT__", "__NAME__", ""}, + {"__ACCOUNT__", "__NAME__", "renamed"}, {"__ACCOUNT__", "__NAME__", "missing"}, + {"__ACCOUNT__", "__NAME__", "inner"}, {"__ACCOUNT__", "__NAME__", "holder"}, + {"__ACCOUNT__", "__NAME__", "foreign-name"}, + {"__ACCOUNT__", "__UID__", "u1"}, {"__ACCOUNT__", "__UID__", "u7"}, + {"__ACCOUNT__", "__UID__", "u8"}, {"__ACCOUNT__", "__UID__", "u12"}, + {"__ACCOUNT__", "__UID__", ""}, {"__ACCOUNT__", "__UID__", "missing"}, + {"__GROUP__", "__NAME__", "dup"}, + }; + + private void assertIndexAgreesWithXQuery(String step) throws Exception { + for (String[] probe : PROBES) { + ObjectClass objectClass = new ObjectClass(probe[0]); + Query query = equalsQuery(probe[1], probe[2]); + assertSameEntries(handler.indexedEntries(query, objectClass), xqueryEntries(query, objectClass), + step + " " + Arrays.toString(probe)); + } + for (ObjectClass objectClass : Arrays.asList(ObjectClass.ACCOUNT, ObjectClass.GROUP)) { + assertSameEntries(handler.indexedEntries(null, objectClass), xqueryEntries(null, objectClass), + step + " all " + objectClass); + assertSameEntries(handler.indexedEntries(new QueryImpl(), objectClass), xqueryEntries(new QueryImpl(), objectClass), + step + " empty query " + objectClass); + } + } + + private List xqueryEntries(Query query, ObjectClass objectClass) throws Exception { + XQueryHandler xquery = new XQueryHandler(new QueryBuilder(query, objectClass).toString(), handler.getDocument()); + try { + List entries = new ArrayList(); + XQResultSequence sequence = xquery.getResultSequence(); + while (sequence.next()) { + entries.add((Element) sequence.getItem().getNode()); + } + return entries; + } finally { + xquery.close(); + } + } + + private static void assertSameEntries(List actual, List expected, String message) { + assertNotNull(actual, message + ": the index did not answer"); + assertEquals(actual.size(), expected.size(), message); + for (int i = 0; i < expected.size(); i++) { + assertSame(actual.get(i), expected.get(i), message + " #" + i); + } + } + + @Test + public void indexAgreesWithXQueryAfterChanges() throws Exception { + assertIndexAgreesWithXQuery("loaded"); + handler.create(ObjectClass.ACCOUNT, account("dup-new")); + assertIndexAgreesWithXQuery("created"); + handler.update(ObjectClass.ACCOUNT, new Uid("u2"), Collections.singleton(AttributeBuilder.build(Name.NAME, "renamed"))); + handler.update(ObjectClass.ACCOUNT, new Uid("u2"), Collections.singleton(AttributeBuilder.build(Name.NAME, "dup"))); + assertIndexAgreesWithXQuery("renamed back to a duplicate"); + handler.delete(ObjectClass.ACCOUNT, new Uid("u1")); + assertIndexAgreesWithXQuery("first duplicate deleted"); + handler.update(ObjectClass.ACCOUNT, new Uid("u11"), Collections.singleton(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "L11"))); + assertIndexAgreesWithXQuery("value holding an entry replaced"); + expectThrows(IllegalArgumentException.class, + () -> handler.update(ObjectClass.ACCOUNT, new Uid("u4"), renameThenFail("x"))); + assertIndexAgreesWithXQuery("failed rename"); + handler.dispose(); + assertIndexAgreesWithXQuery("saved"); + } + + @Test + public void saveRebuildsTheIndexWhenItEmptiesAnIdentifier() throws Exception { + // Stands in for a schema with an optional __NAME__, where update can store a whitespace-only name + // that the save empties; the handler would not accept the value, so the DOM is edited directly. + Element u1 = handler.indexedEntries(equalsQuery("__UID__", "u1"), ObjectClass.ACCOUNT).get(0); + for (Node child = u1.getFirstChild(); child != null; child = child.getNextSibling()) { + if (child.getNodeType() == Node.ELEMENT_NODE && Name.NAME.equals(child.getLocalName())) { + child.setTextContent(" "); + } + } + handler.create(ObjectClass.ACCOUNT, account("x")); + handler.dispose(); + assertIndexAgreesWithXQuery("saved after a name became blank"); + } + + @Test + public void queryOnAnotherAttributeFallsBackToXQuery() { + Query query = new XMLFilterTranslator(true).createEqualsExpression( + new EqualsFilter(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "L2")), false); + assertNull(handler.indexedEntries(query, ObjectClass.ACCOUNT)); + assertEquals(describe(handler.search(query, ObjectClass.ACCOUNT)), Arrays.asList("u2|dup")); + } + + @Test + public void searchWithoutADocumentFailsAsBefore() throws Exception { + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(getRandomXMLFile()); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + SchemaParser parser = new SchemaParser(XMLConnector.class, config.getXsdFilePath()); + XMLHandlerImpl uninitialized = new XMLHandlerImpl(config, parser.parseSchema(), parser.getXsdSchema()); + ConnectorException byIdentifier = expectThrows(ConnectorException.class, + () -> uninitialized.search(equalsQuery("__NAME__", "dup"), ObjectClass.ACCOUNT)); + assertTrue(byIdentifier.getMessage().startsWith("Data file does not exists: "), byIdentifier.getMessage()); + ConnectorException all = expectThrows(ConnectorException.class, + () -> uninitialized.search(new QueryImpl(), ObjectClass.ACCOUNT)); + assertTrue(all.getMessage().startsWith("Data file does not exists: "), all.getMessage()); + } + + @Test + public void onlySingleEqualityOnAnIdentifierGoesToTheIndex() { + XMLFilterTranslator translator = new XMLFilterTranslator(true); + Query byName = translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build(Name.NAME, "x")), false); + Query byUid = translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build(Uid.NAME, "x")), false); + assertEquals(XMLHandlerImpl.identifierField(byName), Name.NAME); + assertEquals(XMLHandlerImpl.identifierField(byUid), Uid.NAME); + assertEquals(XMLHandlerImpl.identifierField(new XMLFilterTranslator(false).createEqualsExpression( + new EqualsFilter(AttributeBuilder.build(Uid.NAME, "x")), false)), Name.NAME); + + assertNull(XMLHandlerImpl.identifierField( + translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build(Name.NAME, "x")), true))); + assertNull(XMLHandlerImpl.identifierField( + translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build("__name__", "x")), false))); + assertNull(XMLHandlerImpl.identifierField( + translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "x")), false))); + assertNull(XMLHandlerImpl.identifierField(translator.createAndExpression(byName, + translator.createEqualsExpression(new EqualsFilter(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "y")), false)))); + assertNull(XMLHandlerImpl.identifierField(translator.createStartsWithExpression( + new StartsWithFilter(AttributeBuilder.build(Name.NAME, "x")), false))); + assertNull(XMLHandlerImpl.identifierField(translator.createGreaterThanOrEqualExpression( + new GreaterThanOrEqualFilter(AttributeBuilder.build(Name.NAME, "x")), false))); + } + + @Test + public void lookupsAndSavesNeverUseNodeLists() throws Exception { + File file = getRandomXMLFile(); + writeAccounts(file, "alice", "bob", "carol"); + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(file); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + SchemaParser parser = new SchemaParser(XMLConnector.class, config.getXsdFilePath()); + XMLHandlerImpl fresh = new XMLHandlerImpl(config, parser.parseSchema(), parser.getXsdSchema()); + fresh.init(); + + fresh.search(equalsQuery(Name.NAME, "bob"), ObjectClass.ACCOUNT); + fresh.search(equalsQuery(Uid.NAME, "uid-carol"), ObjectClass.ACCOUNT); + fresh.search((Query) null, ObjectClass.ACCOUNT); + fresh.authenticate("alice", new GuardedString("secret-alice".toCharArray())); + fresh.create(ObjectClass.ACCOUNT, account("dave")); + fresh.update(ObjectClass.ACCOUNT, new Uid("uid-bob"), + Collections.singleton(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "Changed"))); + fresh.delete(ObjectClass.ACCOUNT, new Uid("uid-carol")); + fresh.dispose(); + + org.testng.Assert.assertFalse(XercesNodeLists.used(fresh.getDocument())); + } + + @Test + public void replacedValueTakesTheEntriesInsideItAlong() { + handler.update(ObjectClass.ACCOUNT, new Uid("u11"), + Collections.singleton(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "L11"))); + handler.create(ObjectClass.ACCOUNT, account("inner")); // a stale "inner" for u12 in the index would refuse this + } + + @Test + public void classWithoutUidIsFoundByNameWhenAskedByUid() { + // __GROUP__ has no __UID__ element: its Uid is the name. + handler.delete(ObjectClass.GROUP, new Uid("dup")); + expectThrows(UnknownUidException.class, () -> handler.delete(ObjectClass.GROUP, new Uid("dup"))); + } +} diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerReloadTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerReloadTests.java index ad216d4c0..f59633d9d 100644 --- a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerReloadTests.java +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerReloadTests.java @@ -156,6 +156,9 @@ public void externalCopyWithAnOlderMtimeIsLoaded() throws Exception { writeAccounts(file, "carol"); setModified(file, now() - 2 * HOUR); assertEquals(names(), List.of("carol")); + // names() reads through XQuery; authenticate goes through the index + assertEquals(call(h -> h.authenticate("carol", new GuardedString("secret-carol".toCharArray()))), + new Uid("uid-carol")); } @Test diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriterTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriterTests.java index dc43436bf..04da40018 100644 --- a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriterTests.java +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XmlDocumentWriterTests.java @@ -80,7 +80,7 @@ public void followingWithARootOutsideTheAncestorsEndsAtTheTop() throws Exception public void normalizedDocumentIsLeftAlone() throws Exception { Document document = parse("x"); Node text = only(document, "a").getFirstChild(); - XmlDocumentWriter.normalizeText(document); + assertFalse(XmlDocumentWriter.normalizeText(document)); assertSingleText(only(document, "a"), "x"); assertSame(only(document, "a").getFirstChild(), text); } @@ -88,7 +88,7 @@ public void normalizedDocumentIsLeftAlone() throws Exception { @Test public void whitespaceOnlyTextIsRemoved() throws Exception { Document document = parse("\n \t \n \n x\n"); - XmlDocumentWriter.normalizeText(document); + assertTrue(XmlDocumentWriter.normalizeText(document)); assertNull(only(document, "a").getFirstChild()); for (Node n = document.getDocumentElement().getFirstChild(); n != null; n = n.getNextSibling()) { assertEquals(n.getNodeType(), Node.ELEMENT_NODE); @@ -122,14 +122,14 @@ public void whitespaceOnlyRunOfSeveralNodesIsRemovedWhole() throws Exception { @Test public void adjacentTextAndCdataBecomeOneTextNode() throws Exception { Document document = parse(" "); - XmlDocumentWriter.normalizeText(document); + assertTrue(XmlDocumentWriter.normalizeText(document)); assertSingleText(only(document, "a"), " x"); - XmlDocumentWriter.normalizeText(document); + assertTrue(XmlDocumentWriter.normalizeText(document)); assertSingleText(only(document, "b"), "z"); } diff --git a/OpenICF-xml-connector/src/test/resources/test/xml_store/index-fixture.xml b/OpenICF-xml-connector/src/test/resources/test/xml_store/index-fixture.xml new file mode 100644 index 000000000..6890fed58 --- /dev/null +++ b/OpenICF-xml-connector/src/test/resources/test/xml_store/index-fixture.xml @@ -0,0 +1,15 @@ + + + u1dupL1 + u2dupL2 + no-uidL3 + u4two-atwo-bL4 + u5 padded L5 + u6ataL6 + u7foreign + u8nestedL8 + u9L9lateforeign-name + u10 L10 + u11holderu12inner + dupDS +