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 bf6befb0..281a29e8 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 @@ -50,7 +50,11 @@ public class ConcurrentXMLHandler implements XMLHandler { private volatile int invokers = 0; public ConcurrentXMLHandler(XMLConfiguration config, Schema connSchema, XSSchemaSet xsdSchemas) { - proxy = new XMLHandlerImpl(config, connSchema, xsdSchemas); + this(new XMLHandlerImpl(config, connSchema, xsdSchemas)); + } + + ConcurrentXMLHandler(XMLHandler proxy) { + this.proxy = proxy; } @Override diff --git a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConnectorObjectCreator.java b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConnectorObjectCreator.java index a2066552..4072c3ea 100644 --- a/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConnectorObjectCreator.java +++ b/OpenICF-xml-connector/src/main/java/org/forgerock/openicf/connectors/xml/ConnectorObjectCreator.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$ */ @@ -40,7 +41,6 @@ import org.identityconnectors.framework.common.objects.ObjectClass; import org.identityconnectors.framework.common.objects.Uid; import org.w3c.dom.Node; -import org.w3c.dom.NodeList; class ConnectorObjectCreator { @@ -49,7 +49,6 @@ class ConnectorObjectCreator { private Map attributeClassMap; private Map attributeInfoMap; private ObjectClass objectClass; - private NodeList nodeList; private ConnectorObjectBuilder conObjBuilder; protected ConnectorObjectCreator(HashMap attrClasses, HashMap attrInfos, ObjectClass objClass) { @@ -58,18 +57,17 @@ protected ConnectorObjectCreator(HashMap attrClasses, HashMap + * A file rewritten in place with the same size within one tick of the modification time clock + * keeps all of these. That can only go unnoticed while the stamp is racy: when it was taken, the + * modification time was less than {@link #RACY_WINDOW_MILLIS} before the clock, or after it. + * Callers compare the content ({@link #checksum}) behind a racy stamp. + */ +final class FileStamp { + + /** The coarsest common tick: 2 s on FAT; 1 s on HFS+ and ext3. */ + static final long RACY_WINDOW_MILLIS = 2000L; + + /** No file: no modification time, which every existing file has. */ + static final FileStamp MISSING = new FileStamp(null, -1L, null, false); + + /** The file could not be read: matches nothing, not even itself. */ + private static final FileStamp UNKNOWN = new FileStamp(null, -1L, null, true); + + private final FileTime lastModified; + private final long size; + private final Object fileKey; + private final boolean racy; + + private FileStamp(FileTime lastModified, long size, Object fileKey, boolean racy) { + this.lastModified = lastModified; + this.size = size; + this.fileKey = fileKey; + this.racy = racy; + } + + static FileStamp read(File file) { + long now = System.currentTimeMillis(); + try { + BasicFileAttributes attributes = Files.readAttributes(file.toPath(), BasicFileAttributes.class); + FileTime modified = attributes.lastModifiedTime(); + return new FileStamp(modified, attributes.size(), attributes.fileKey(), + now - modified.toMillis() < RACY_WINDOW_MILLIS); + } catch (NoSuchFileException e) { + return MISSING; + } catch (IOException e) { + return UNKNOWN; + } + } + + /** The CRC-32 of the file's content. */ + static long checksum(File file) throws IOException { + CRC32 crc = new CRC32(); + try (InputStream in = new CheckedInputStream(Files.newInputStream(file.toPath()), crc)) { + byte[] buffer = new byte[64 * 1024]; + while (in.read(buffer) != -1) { + // the stream updates the checksum + } + } + return crc.getValue(); + } + + boolean isRacy() { + return racy; + } + + /** Whether the file could be read, or was known to be missing, when the stamp was taken. */ + boolean isKnown() { + return this != UNKNOWN; + } + + boolean sameState(FileStamp other) { + if (this == UNKNOWN || other == UNKNOWN) { + return false; + } + return Objects.equals(lastModified, other.lastModified) + && size == other.size + && Objects.equals(fileKey, other.fileKey); + } +} 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 1fbe90da..6381c755 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 @@ -55,7 +55,8 @@ public class XMLConnector implements Connector, AuthenticateOp, CreateOp, DeleteOp, SearchOp, SchemaOp, TestOp, UpdateOp { private static final Log log = Log.getLog(XMLConnector.class); - private static /*volatile*/ final Map XMLHandlerCache = new HashMap(1); + /** Guarded by {@code XMLConnector.class}. */ + static final Map XMLHandlerCache = new HashMap(1); private XMLConfiguration config; private XMLHandler xmlInstanceHandler = null; @@ -77,23 +78,23 @@ public Configuration getConfiguration() { @Override public void init(Configuration configuration) { this.config = Assertions.nullChecked((XMLConfiguration) configuration, "config"); - synchronized (XMLConnector.class) { - try { - String canonicalPath = config.getXmlFilePath().getCanonicalPath(); - ConcurrentXMLHandler handler = XMLHandlerCache.get(canonicalPath); - + ConcurrentXMLHandler handler; + try { + String canonicalPath = config.getXmlFilePath().getCanonicalPath(); + synchronized (XMLConnector.class) { + handler = XMLHandlerCache.get(canonicalPath); if (null == handler) { SchemaParser schemaParser = new SchemaParser(XMLConnector.class, config.getXsdFilePath()); handler = new ConcurrentXMLHandler(config, schema(), schemaParser.getXsdSchema()); XMLHandlerCache.put(canonicalPath, handler); } - xmlInstanceHandler = handler.init(); - } - catch (IOException ex) { - log.error(ex, "Failed to get the CanonicalPath of {0}", config.getXmlFilePath()); - throw new ConnectorIOException(ex); } + } catch (IOException ex) { + log.error(ex, "Failed to get the CanonicalPath of {0}", config.getXmlFilePath()); + throw new ConnectorIOException(ex); } + // Outside the class monitor: loading one file, or waiting while another call saves it, must not hold up connectors on other files. + xmlInstanceHandler = handler.init(); log.info("XMLConnector initialized"); } 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 dfd5e608..9bf1b9c9 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 @@ -39,6 +39,8 @@ import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -46,6 +48,8 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.zip.CRC32; +import java.util.zip.CheckedInputStream; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -74,6 +78,7 @@ 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; public class XMLHandlerImpl implements XMLHandler { @@ -82,13 +87,20 @@ public class XMLHandlerImpl implements XMLHandler { * Setup logging for the {@link XMLHandlerImpl}. */ private static final Log log = Log.getLog(XMLHandlerImpl.class); + private static final String DEFER_NODE_EXPANSION = "http://apache.org/xml/features/dom/defer-node-expansion"; private XMLConfiguration config; private volatile Document document; private Schema connSchema; private XSSchema icfSchema; private XSSchema riSchema; - private long lastModified = 0l; - private volatile long version = 0l; + /** 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. */ + private boolean unsavedNewFile; + /** The file as last seen: at the last load, save attempt, new document, or content-confirmed check. */ + private FileStamp stamp = FileStamp.MISSING; + /** The CRC-32 of the bytes last parsed or written. */ + private long checksum; public static final String XSI_NAMESPACE = "http://www.w3.org/2001/XMLSchema-instance"; public static final String ICF_NAMESPACE_PREFIX = "icf"; public static final String RI_NAMESPACE_PREFIX = "ri"; @@ -108,10 +120,61 @@ public XMLHandlerImpl(XMLConfiguration config, Schema connSchema, XSSchemaSet xs @Override public XMLHandler init() { - buildDocument(); + if (document != null && fileAppearedOverNewDocument()) { + dropNewDocument(); + } + if (document == null || (!dirty && !unsavedNewFile && fileHasChanged())) { + buildDocument(); + } return this; } + /** + * Whether a file appeared where a new document is not saved yet. Such a file wins over the + * document: the connector never loaded it, so a save would replace a whole store. This is the + * only reload of a new document: a partial write of its failed save is not such a file, + * because the save takes the stamp again. If that stamp could not be read, the file is still + * taken for the partial write: only a save leaves a new document with such a stamp. + */ + private boolean fileAppearedOverNewDocument() { + File xmlFile = config.getXmlFilePath(); + return unsavedNewFile && stamp.isKnown() && xmlFile.isFile() && !stamp.sameState(FileStamp.read(xmlFile)); + } + + /** Drops the new document with its changes: the next init() loads the file, or starts a new document if the file is gone. */ + private void dropNewDocument() { + if (dirty) { + log.error("{0} appeared before the new document was saved; keeping the file and dropping the changes made to the new document", config.getXmlFilePath()); + dirty = false; + } + document = null; + } + + /** A racy stamp cannot rule out a same-size write in the same tick, so the content decides. */ + private boolean fileHasChanged() { + File xmlFile = config.getXmlFilePath(); + FileStamp current = FileStamp.read(xmlFile); + if (!stamp.sameState(current)) { + return true; + } + if (!stamp.isRacy()) { + return false; + } + try { + if (FileStamp.checksum(xmlFile) != checksum) { + return true; + } + } catch (IOException e) { + return true; + } + stamp = current; // the content is what we hold; the stamp stops being racy once it is old enough + return false; + } + + private void markDirty() { + dirty = true; + } + @Override public Uid create(final ObjectClass objClass, final Set attributes) { final String method = "create"; @@ -211,6 +274,7 @@ public Uid create(final ObjectClass objClass, final Set attributes) { log.info("Creating new entry: {0}", attributes.toString()); } + markDirty(); getDocument().getDocumentElement().appendChild(objElement); log.info("Exit {0}", method); @@ -266,6 +330,7 @@ public Uid update(ObjectClass objClass, Uid uid, Set replaceAttribute } // Remove existing nodes from entry + markDirty(); removeChildrenFromElement(entry, prefixAttributeName(attributeName)); // Add updated nodes to entry @@ -305,6 +370,7 @@ public void delete(final ObjectClass objClass, final Uid uid) { 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 { throw new UnknownUidException("Deleting entry failed. Could not find an entry of type " + objClass.getObjectClassValue() + " with the uid " + uid.getUidValue()); @@ -322,18 +388,7 @@ public Collection search(String query, ObjectClass objClass) { if (query != null && !query.isEmpty() && objClass != null) { - ObjectClassInfo objInfo = connSchema.findObjectClassInfo(objClass.getObjectClassValue()); - Set objAttributes = objInfo.getAttributeInfo(); - - // Map with the attribute-names and what class they are - HashMap attributeClassMap = new HashMap(); - for (AttributeInfo info : objAttributes) { - attributeClassMap.put(info.getName(), info.getType().getSimpleName()); - } - - // Map with the AttributeInfo for each attribute - HashMap attributeInfoMap = - new HashMap(AttributeInfoUtil.toMap(objInfo.getAttributeInfo())); + ConnectorObjectCreator conObjCreator = newConnectorObjectCreator(objClass); XQueryHandler xqHandler = null; try { @@ -341,17 +396,9 @@ public Collection search(String query, ObjectClass objClass) { XQResultSequence queryResult = xqHandler.getResultSequence(); - ConnectorObjectCreator conObjCreator = - new ConnectorObjectCreator(attributeClassMap, attributeInfoMap, objClass); - while (queryResult.next()) { - Node resultNode = queryResult.getItem().getNode(); - - NodeList nodes = resultNode.getChildNodes(); - - ConnectorObject conObj = conObjCreator.createConnectorObject(nodes); - results.add(conObj); + results.add(conObjCreator.createConnectorObject(queryResult.getItem().getNode())); } } catch (XQException ex) { log.error("Error while searching: {0}", ex); @@ -367,31 +414,37 @@ public Collection search(String query, ObjectClass objClass) { return results; } - private boolean isExternallyModified() { - boolean modified = false; - if (config.getXmlFilePath().exists()) { - modified = lastModified != config.getXmlFilePath().lastModified(); - } - return modified; - } - @Override public void dispose() { final String method = "serialize"; log.info("Entry {0}", method); - if (version != lastModified && isExternallyModified()) { - log.error("UPDATE COLLUSION: File has been modified after read into memory and the data in memory has not been synced before."); + if (!dirty && !unsavedNewFile) { + log.info("Exit {0}: nothing to save", method); + return; + } + File xmlFile = config.getXmlFilePath(); + if (fileAppearedOverNewDocument()) { + dropNewDocument(); + log.info("Exit {0}: {1} appeared before the new document was saved; not overwriting it", method, xmlFile); + return; + } + // A path that holds no file has nothing to overwrite: the save creates the file. + if (xmlFile.isFile() && !stamp.sameState(FileStamp.read(xmlFile))) { + log.error("UPDATE COLLISION: {0} has changed since it was loaded or saved; overwriting it with the data in memory.", xmlFile); } - // Callers hold the handler's write lock. Document saved = getDocument(); try { XmlDocumentWriter.normalizeText(saved); - XmlDocumentWriter.write(saved, config.getXmlFilePath()); + checksum = XmlDocumentWriter.write(saved, xmlFile); + dirty = false; + unsavedNewFile = false; log.info("Saving changes to xml file"); } catch (TransformerException | SAXException | IOException ex) { log.error("Failed saving changes to xml file: {0}", ex); throw ConnectorException.wrap(ex); + } finally { + stamp = FileStamp.read(xmlFile); } log.info("Exit {0}", method); @@ -486,22 +539,56 @@ private void createDocument() { + icfSchema.getTargetNamespace() + " " + config.getXsdIcfFilePath()); } + // Not read again: a file that appeared since the check found none is not the connector's to overwrite. + stamp = FileStamp.MISSING; + // Not a user change: if the first save fails, a file that appears afterwards is loaded instead. + unsavedNewFile = true; log.info("Exit {0}", method); } + ConnectorObjectCreator newConnectorObjectCreator(ObjectClass objClass) { + ObjectClassInfo objInfo = connSchema.findObjectClassInfo(objClass.getObjectClassValue()); + HashMap attributeClassMap = new HashMap(); + for (AttributeInfo info : objInfo.getAttributeInfo()) { + attributeClassMap.put(info.getName(), info.getType().getSimpleName()); + } + HashMap attributeInfoMap = + new HashMap(AttributeInfoUtil.toMap(objInfo.getAttributeInfo())); + return new ConnectorObjectCreator(attributeClassMap, attributeInfoMap, objClass); + } + private void loadDocument(File xmlFile) { final String method = "loadDocument"; log.info("Entry {0}", method); DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance(); docBuilderFactory.setNamespaceAware(true); - DocumentBuilder docBuilder; + try { + // Build every node now: a deferred node is built on its first read, and readers run in parallel. + docBuilderFactory.setAttribute(DEFER_NODE_EXPANSION, Boolean.FALSE); + } catch (IllegalArgumentException ex) { + log.warn("The XML parser {0} does not support {1}", docBuilderFactory.getClass().getName(), DEFER_NODE_EXPANSION); + } try { - docBuilder = docBuilderFactory.newDocumentBuilder(); - document = docBuilder.parse(xmlFile); - lastModified = xmlFile.lastModified(); - version = lastModified; + DocumentBuilder docBuilder = docBuilderFactory.newDocumentBuilder(); + // Taken before reading: a write during the read shows as a change at the next init(). + FileStamp loadedStamp = FileStamp.read(xmlFile); + CRC32 crc = new CRC32(); + Document loaded; + // The checksum is of the bytes parsed: the parser reads on to the end, past what follows the root. + try (InputStream in = new CheckedInputStream(Files.newInputStream(xmlFile.toPath()), crc)) { + InputSource source = new InputSource(in); + source.setSystemId(xmlFile.toURI().toASCIIString()); + loaded = docBuilder.parse(source); + } + // Same text shape as a saved document: CDATA and split runs become one text node, blank text goes. + XmlDocumentWriter.normalizeText(loaded); + // Nothing changes unless the parse succeeded: a malformed file keeps failing until it is fixed. + document = loaded; + stamp = loadedStamp; + checksum = crc.getValue(); + unsavedNewFile = false; log.info("Loading XML document from: {0}", xmlFile.getPath()); } catch (ParserConfigurationException ex) { throw ConnectorException.wrap(ex); diff --git a/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/FileStampTests.java b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/FileStampTests.java new file mode 100644 index 00000000..85e3e951 --- /dev/null +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/FileStampTests.java @@ -0,0 +1,156 @@ +/* + * 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.getRandomXMLFile; +import static org.forgerock.openicf.connectors.xml.XmlConnectorTestUtil.setModified; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.StandardCopyOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.zip.CRC32; + +import org.testng.SkipException; +import org.testng.annotations.Test; + +public class FileStampTests { + + private static final long HOUR = 3_600_000L; + + @Test + public void unchangedFileMatches() throws IOException { + File file = write(getRandomXMLFile(), "abc", now() - HOUR); + assertTrue(FileStamp.read(file).sameState(FileStamp.read(file))); + } + + @Test + public void olderMtimeIsAChange() throws IOException { + File file = write(getRandomXMLFile(), "abc", now() - HOUR); + FileStamp before = FileStamp.read(file); + setModified(file, now() - 2 * HOUR); + assertFalse(before.sameState(FileStamp.read(file))); + } + + @Test + public void sizeChangeWithTheSameMtimeIsAChange() throws IOException { + long past = now() - HOUR; + File file = write(getRandomXMLFile(), "abc", past); + FileStamp before = FileStamp.read(file); + write(file, "abcd", past); + assertFalse(before.sameState(FileStamp.read(file))); + } + + @Test + public void replacedFileIsAChange() throws IOException { + long past = now() - HOUR; + File file = write(getRandomXMLFile(), "abc", past); + if (Files.readAttributes(file.toPath(), BasicFileAttributes.class).fileKey() == null) { + throw new SkipException("The file system reports no file key"); + } + FileStamp before = FileStamp.read(file); + File copy = write(getRandomXMLFile(), "abc", past); + Files.move(copy.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING); + setModified(file, past); + assertFalse(before.sameState(FileStamp.read(file))); + } + + @Test + public void unreadableStampMatchesNothing() throws IOException { + File loop = getRandomXMLFile(); + try { + Files.createSymbolicLink(loop.toPath(), loop.toPath().getFileName()); // a link to itself + } catch (IOException | UnsupportedOperationException e) { + throw new SkipException("Cannot create a symbolic link here: " + e); + } + try { + Files.readAttributes(loop.toPath(), BasicFileAttributes.class); + throw new SkipException("The file system resolves a link to itself"); + } catch (NoSuchFileException e) { + throw new SkipException("The file system reports a link to itself as missing"); + } catch (IOException expected) { + // ELOOP: the path is neither readable nor known to be missing + } + assertFalse(FileStamp.read(loop).sameState(FileStamp.read(loop))); + assertFalse(FileStamp.read(loop).sameState(FileStamp.MISSING)); + assertFalse(FileStamp.MISSING.sameState(FileStamp.read(loop))); + assertFalse(FileStamp.read(loop).isKnown()); + } + + @Test + public void readableAndMissingFilesAreKnown() throws IOException { + assertTrue(FileStamp.read(write(getRandomXMLFile(), "abc", now() - HOUR)).isKnown()); + assertTrue(FileStamp.read(getRandomXMLFile()).isKnown()); + } + + @Test + public void missingFileMatchesMissing() { + assertTrue(FileStamp.read(getRandomXMLFile()).sameState(FileStamp.MISSING)); + } + + @Test + public void stampIsRacyOnlyNearTheClock() throws IOException { + File file = write(getRandomXMLFile(), "abc", now() + HOUR); + assertTrue(FileStamp.read(file).isRacy()); + setModified(file, now() - HOUR); + assertFalse(FileStamp.read(file).isRacy()); + } + + @Test + public void stampIsRacyJustAfterAWrite() throws IOException { + // An outside write of the same size in the same tick right after a load or save looks like no change. + File file = write(getRandomXMLFile(), "abc", now() - 500); + assertTrue(FileStamp.read(file).isRacy()); + setModified(file, now() - 3_000); + assertFalse(FileStamp.read(file).isRacy()); + } + + @Test + public void checksumFollowsTheContent() throws IOException { + File file = write(getRandomXMLFile(), "abc", now() - HOUR); + CRC32 crc = new CRC32(); + crc.update("abc".getBytes(StandardCharsets.UTF_8)); + assertEquals(FileStamp.checksum(file), crc.getValue()); + write(file, "abd", now() - HOUR); + assertNotEquals(FileStamp.checksum(file), crc.getValue()); + } + + @Test + public void deletedFileIsAChange() throws IOException { + File file = write(getRandomXMLFile(), "abc", now() - HOUR); + FileStamp before = FileStamp.read(file); + Files.delete(file.toPath()); + assertFalse(before.sameState(FileStamp.read(file))); + } + + private static File write(File file, String content, long modified) throws IOException { + Files.write(file.toPath(), content.getBytes(StandardCharsets.UTF_8)); + setModified(file, modified); + return file; + } + + private static long now() { + return System.currentTimeMillis(); + } +} 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 5e7e5be1..511101e1 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,6 +21,7 @@ import static org.testng.Assert.assertTrue; import java.io.File; +import java.util.Arrays; import java.util.Collections; import org.identityconnectors.common.security.GuardedString; @@ -50,7 +51,7 @@ public void bundleSavesLoadsAndFindsEntries() throws Exception { ConnectorInfoManager manager = ConnectorInfoManagerFactory.getInstance().getLocalManager(bundle.toURI().toURL()); assertEquals(manager.getConnectorInfos().size(), 1); APIConfiguration api = manager.getConnectorInfos().get(0).createDefaultAPIConfiguration(); - File xmlFile = getRandomXMLFile(); + File xmlFile = getRandomXMLFile(); // no file yet: the first call creates a new document under the bundle's loader ConfigurationProperties properties = api.getConfigurationProperties(); properties.setPropertyValue("xmlFilePath", xmlFile); properties.setPropertyValue("xsdFilePath", XSD_SCHEMA_FILEPATH); @@ -64,8 +65,14 @@ public void bundleSavesLoadsAndFindsEntries() throws Exception { Uid bob = facade.authenticate(ObjectClass.ACCOUNT, "bob", new GuardedString("secret-bob".toCharArray()), null); facade.delete(ObjectClass.ACCOUNT, bob, null); - assertEquals(namesInFile(xmlFile), Collections.singletonList("alice")); + assertEquals(namesInFile(xmlFile), Arrays.asList("alice")); ConnectorObject read = facade.getObject(ObjectClass.ACCOUNT, alice, null); assertEquals(AttributeUtil.getStringValue(read.getAttributeByName(ATTR_ACCOUNT_LAST_NAME)), "Changed"); + + // An outside edit, so that a call parses a file under the bundle's loader. + writeAccounts(xmlFile, "carol"); + setModified(xmlFile, System.currentTimeMillis() - 3_600_000L); + ConnectorObject carol = facade.getObject(ObjectClass.ACCOUNT, new Uid("uid-carol"), null); + assertEquals(carol.getName().getNameValue(), "carol"); } } 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 a134130b..992afa4e 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 @@ -36,8 +36,12 @@ import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.attribute.FileTime; +import java.util.Arrays; +import java.util.Collection; import java.util.HashSet; import java.util.Set; +import java.util.concurrent.*; import javax.xml.parsers.DocumentBuilderFactory; import org.forgerock.openicf.connectors.xml.xsdparser.SchemaParser; import org.identityconnectors.common.security.GuardedString; @@ -285,15 +289,14 @@ public void valuesSurviveASaveAndAReload() throws Exception { @Test public void disposeNeverUsesNodeLists() throws Exception { - String icf = "http://openidm.forgerock.com/xml/ns/public/resource/openicf/resource-schema-1.xsd"; - String ri = "http://openidm.forgerock.com/xml/ns/public/resource/instances/ef2bc95b-76e0-48e2-86d6-4d4f44d4e4a4"; - Files.write(xmlFile.toPath(), ("\n" - + " \n a\n \n" - + " \n b\n \n" - + "\n").getBytes(StandardCharsets.UTF_8)); XMLHandlerImpl handler = newHandler(); - handler.init(); + handler.init(); // no file yet: a new document, which dispose() saves + Document document = handler.getDocument(); + // Two children: Xerces allocates no node list cache for a parent with fewer. + document.getDocumentElement().appendChild(document.createElementNS(RI_NAMESPACE, "ri:" + ACCOUNT_TYPE)); + document.getDocumentElement().appendChild(document.createElementNS(RI_NAMESPACE, "ri:" + ACCOUNT_TYPE)); handler.dispose(); + AssertJUnit.assertTrue(xmlFile.exists()); AssertJUnit.assertFalse(XercesNodeLists.used(handler.getDocument())); } @@ -333,10 +336,11 @@ public void failedSaveIsLoggedAndThrown() throws Exception { } @Test - public void saveWithoutADocumentThrowsConnectorException() { + public void saveWithoutADocumentSavesNothing() throws Exception { XMLHandlerImpl handler = newHandler(); // init() never ran, as when it fails on the first load - ConnectorException e = expectThrows(ConnectorException.class, handler::dispose); - AssertJUnit.assertTrue(e.getMessage(), e.getMessage().startsWith("Data file does not exists: ")); + String out = captureStdOut(handler::dispose); + AssertJUnit.assertTrue(out, out.contains("Exit serialize: nothing to save")); + AssertJUnit.assertFalse(xmlFile.exists()); } @Test @@ -347,6 +351,114 @@ public void successfulSaveLogsItsExit() throws Exception { AssertJUnit.assertTrue(out, out.contains("Exit serialize" + System.lineSeparator())); } + @Test + public void readOnlyOperationsDoNotRewriteTheFile() throws Exception { + Uid uid = facade.create(ObjectClass.ACCOUNT, getRequiredAccountAttributes(), null); + FileTime past = FileTime.fromMillis(System.currentTimeMillis() - 3_600_000L); + Files.setLastModifiedTime(xmlFile.toPath(), past); + byte[] before = Files.readAllBytes(xmlFile.toPath()); + + facade.search(ObjectClass.ACCOUNT, null, new TestResultsHandler(), null); + facade.getObject(ObjectClass.ACCOUNT, uid, null); + facade.authenticate(ObjectClass.ACCOUNT, ATTR_ACCOUNT_VALUE_NAME, + new GuardedString(ATTR_ACCOUNT_VALUE_PASSWORD.toCharArray()), null); + facade.test(); + facade.schema(); + + AssertJUnit.assertEquals(past, Files.getLastModifiedTime(xmlFile.toPath())); + AssertJUnit.assertTrue(Arrays.equals(before, Files.readAllBytes(xmlFile.toPath()))); + } + + @Test(timeOut = 60_000L) + public void initOnOneFileDoesNotWaitForAnotherFile() throws Exception { + File slowFile = getRandomXMLFile(); + String key = slowFile.getCanonicalPath(); + BlockingHandler blocking = new BlockingHandler(); + synchronized (XMLConnector.class) { + XMLConnector.XMLHandlerCache.put(key, new ConcurrentXMLHandler(blocking)); + } + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future slow = executor.submit(() -> new XMLConnector().init(configFor(slowFile))); + AssertJUnit.assertTrue(blocking.entered.await(10, TimeUnit.SECONDS)); + + Future other = executor.submit(() -> { + XMLConnector connector = new XMLConnector(); + connector.init(configFor(getRandomXMLFile())); + connector.dispose(); + }); + other.get(10, TimeUnit.SECONDS); + + blocking.release.countDown(); + slow.get(10, TimeUnit.SECONDS); + } finally { + blocking.release.countDown(); + executor.shutdownNow(); + synchronized (XMLConnector.class) { + XMLConnector.XMLHandlerCache.remove(key); + } + } + } + + private static XMLConfiguration configFor(File xmlFile) { + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(xmlFile); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + config.setCreateFileIfNotExists(true); + return config; + } + + /** A handler whose init() blocks until released. */ + private static final class BlockingHandler implements XMLHandler { + final CountDownLatch entered = new CountDownLatch(1); + final CountDownLatch release = new CountDownLatch(1); + + @Override + public XMLHandler init() { + entered.countDown(); + try { + release.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + return this; + } + + @Override + public void dispose() { + } + + @Override + public Uid create(ObjectClass objClass, Set attributes) { + throw new UnsupportedOperationException(); + } + + @Override + public Uid update(ObjectClass objClass, Uid uid, Set replaceAttributes) { + throw new UnsupportedOperationException(); + } + + @Override + public void delete(ObjectClass objClass, Uid uid) { + throw new UnsupportedOperationException(); + } + + @Override + public Collection search(String query, ObjectClass objectClass) { + throw new UnsupportedOperationException(); + } + + @Override + public Uid authenticate(String username, GuardedString password) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean isSupportUid(ObjectClass objectClass) { + throw new UnsupportedOperationException(); + } + } + private XMLHandlerImpl newHandler() { XMLConfiguration config = new XMLConfiguration(); config.setXmlFilePath(xmlFile); 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 new file mode 100644 index 00000000..ad216d4c --- /dev/null +++ b/OpenICF-xml-connector/src/test/java/org/forgerock/openicf/connectors/xml/XMLHandlerReloadTests.java @@ -0,0 +1,843 @@ +/* + * 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.assertFalse; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.expectThrows; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.PrintStream; +import java.io.UncheckedIOException; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.InvalidPathException; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.function.Function; + +import javax.xml.parsers.DocumentBuilder; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +import org.forgerock.openicf.connectors.xml.query.QueryBuilder; +import org.forgerock.openicf.connectors.xml.xsdparser.SchemaParser; +import org.identityconnectors.common.security.GuardedString; +import org.identityconnectors.framework.common.exceptions.ConnectorException; +import org.identityconnectors.framework.common.objects.Attribute; +import org.identityconnectors.framework.common.objects.AttributeBuilder; +import org.identityconnectors.framework.common.objects.AttributeUtil; +import org.identityconnectors.framework.common.objects.ConnectorObject; +import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.Uid; +import org.testng.SkipException; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; +import org.w3c.dom.DOMException; + +public class XMLHandlerReloadTests { + + private static final long HOUR = 3_600_000L; + + private File file; + private ConcurrentXMLHandler handler; + + @BeforeMethod + public void setUp() { + file = getRandomXMLFile(); + handler = new ConcurrentXMLHandler(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + } + + @Test + public void readOnlyCallsDoNotWriteTheFile() throws Exception { + writeAccounts(file, "alice"); + FileTime past = FileTime.fromMillis(now() - HOUR); + Files.setLastModifiedTime(file.toPath(), past); + byte[] before = Files.readAllBytes(file.toPath()); + + names(); + call(h -> h.authenticate("alice", new GuardedString("secret-alice".toCharArray()))); + + assertEquals(Files.getLastModifiedTime(file.toPath()), past); + assertEquals(Files.readAllBytes(file.toPath()), before); + } + + @Test + public void newFileIsCreatedByTheFirstCall() { + // Guards existing behaviour: with createFileIfNotExists even test() creates the file. + assertFalse(file.exists()); + call(h -> null); + assertTrue(file.exists()); + } + + @Test + public void changeIsInTheFileWhenTheLastUserLeaves() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + call(h -> h.create(ObjectClass.ACCOUNT, account("bob"))); + assertEquals(namesInFile(file), List.of("alice", "bob")); + } + + @Test + public void failedUpdateLeavesTheFileInAgreementWithMemory() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + Set twoLastNames = Collections.singleton(AttributeBuilder.build(ATTR_ACCOUNT_LAST_NAME, "A", "B")); + List inMemory; + handler.init(); // holds the document: no reload and no save until the last dispose() + try { + expectThrows(IllegalArgumentException.class, + () -> call(h -> h.update(ObjectClass.ACCOUNT, new Uid("uid-alice"), twoLastNames))); + inMemory = lastNames(handler.search(allAccounts(), ObjectClass.ACCOUNT)); + } finally { + handler.dispose(); + } + assertEquals(lastNamesIn(file), inMemory); + } + + @Test + public void deleteIsInTheFileWhenTheLastUserLeaves() throws Exception { + writeAccounts(file, "alice", "bobby"); + setModified(file, now() - HOUR); + call(h -> { + h.delete(ObjectClass.ACCOUNT, new Uid("uid-bobby")); + return null; + }); + assertEquals(namesInFile(file), List.of("alice")); + } + + @Test + public void unchangedFileIsServedFromMemory() throws Exception { + long past = now() - HOUR; + writeAccounts(file, "alice"); + setModified(file, past); + assertEquals(names(), List.of("alice")); + + // Same size, same mtime, same file, stamp not racy: indistinguishable from no change. + writeAccounts(file, "bobby"); + setModified(file, past); + assertEquals(names(), List.of("alice")); + } + + @Test + public void externalCopyWithAnOlderMtimeIsLoaded() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + assertEquals(names(), List.of("alice")); + + writeAccounts(file, "carol"); + setModified(file, now() - 2 * HOUR); + assertEquals(names(), List.of("carol")); + } + + @Test + public void malformedEditFailsEveryCallUntilFixed() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - 2 * HOUR); + assertEquals(names(), List.of("alice")); + + Files.write(file.toPath(), " expectThrows(ConnectorException.class, () -> handler.init())); + captureStdErr(() -> expectThrows(ConnectorException.class, () -> handler.init())); + + writeAccounts(file, "bob"); + setModified(file, now() - HOUR / 2); + assertEquals(names(), List.of("bob")); + } + + @Test + public void sameSizeEditInsideTheRacyWindowIsLoaded() throws Exception { + long future = now() + HOUR; // a stamp at or after the clock is racy + writeAccounts(file, "alice"); + setModified(file, future); + assertEquals(names(), List.of("alice")); + + writeAccounts(file, "bobby"); + setModified(file, future); + assertEquals(names(), List.of("bobby")); + } + + @Test + public void racyLoadIsNotParsedAgain() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() + HOUR); // racy for as long as the test runs + String first = captureStdOut(() -> names()); + assertTrue(first.contains("Loading XML document from: " + file.getPath()), first); + String second = captureStdOut(() -> names()); + assertFalse(second.contains("Loading XML document from"), second); + } + + @Test + public void racyLoadWithMarkupAfterTheRootIsNotParsedAgain() throws Exception { + writeAccounts(file, "alice"); + // Far past any parser buffer: the checksum taken while parsing covers it only if the parser reads to the end. + String after = "\n\n"; + Files.write(file.toPath(), after.getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND); + setModified(file, now() + HOUR); // racy for as long as the test runs + String first = captureStdOut(() -> names()); + assertTrue(first.contains("Loading XML document from: " + file.getPath()), first); + String second = captureStdOut(() -> names()); + assertFalse(second.contains("Loading XML document from"), second); + } + + @Test + public void unreadableFileBehindARacyStampFails() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() + HOUR); // racy for as long as the test runs + assertEquals(names(), List.of("alice")); + makeUnreadable(file); + try { + expectThrows(ConnectorException.class, () -> handler.init()); + } finally { + file.setReadable(true, false); + } + } + + @Test + public void stampConfirmedByContentStopsBeingRacy() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - FileStamp.RACY_WINDOW_MILLIS + 1_000L); // racy for one more second + assertEquals(names(), List.of("alice")); + if (now() - Files.getLastModifiedTime(file.toPath()).toMillis() >= FileStamp.RACY_WINDOW_MILLIS) { + // A slow load, or a file system that truncates mtimes to seconds: the stamp taken was not racy. + throw new SkipException("The load of " + file + " fell outside the racy window"); + } + Thread.sleep(1_500L); // the same mtime is now outside the window + names(); // racy stamp, same content: the stamp is taken again + makeUnreadable(file); + try { + assertEquals(names(), List.of("alice")); // nothing reads the file any more + } finally { + file.setReadable(true, false); + } + } + + @Test + public void relativeDtdIsResolvedAgainstTheFile() throws Exception { + File dtd = new File(file.getParentFile(), file.getName() + ".dtd"); + Files.write(dtd.toPath(), "".getBytes(StandardCharsets.UTF_8)); + writeAccounts(file, "alice"); + String xml = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8) + .replace("\nalice<", "&who;<"); + Files.write(file.toPath(), xml.getBytes(StandardCharsets.UTF_8)); + assertEquals(names(), List.of("alice")); + } + + @Test + public void cdataValueOfALoadedFileIsRead() throws Exception { + writeAccounts(file, "alice"); + String xml = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8) + .replace("Last-alice", ""); + Files.write(file.toPath(), xml.getBytes(StandardCharsets.UTF_8)); + setModified(file, now() - HOUR); + assertEquals(call(h -> lastNames(h.search(allAccounts(), ObjectClass.ACCOUNT))), List.of("Last-alice")); + assertEquals(call(h -> lastNames(h.search(allAccounts(), ObjectClass.ACCOUNT))), List.of("Last-alice")); + } + + @Test + public void whitespaceOnlyValueOfALoadedFileIsAbsent() throws Exception { + writeAccounts(file, "alice"); + String xml = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8) + .replace("Last-alice", " "); + Files.write(file.toPath(), xml.getBytes(StandardCharsets.UTF_8)); + setModified(file, now() - HOUR); + // As after a save and a reload: the blank text is gone, and so is the attribute. + assertEquals(call(h -> lastNames(h.search(allAccounts(), ObjectClass.ACCOUNT))), Collections.singletonList(null)); + } + + @Test + public void relativeDtdIsResolvedUnderANonAsciiDirectory() throws Exception { + // Unique per run: a directory left by a killed run would otherwise turn this test into a skip. + File dir = new File(file.getParentFile(), "\u0434\u0438\u0440-\u00e9-" + file.getName()); + try { + // A path the platform encoding cannot hold (Linux under LANG=C) throws here, before anything is created. + Files.createDirectory(dir.toPath()); + } catch (InvalidPathException | IOException e) { + throw new SkipException("Cannot create a directory with a non-ASCII name: " + dir, e); + } + try { + File store = new File(dir, "store.xml"); + File dtd = new File(dir, "store.dtd"); + Files.write(dtd.toPath(), "".getBytes(StandardCharsets.UTF_8)); + writeAccounts(store, "alice"); + String xml = new String(Files.readAllBytes(store.toPath()), StandardCharsets.UTF_8) + .replace("\nalice<", "&who;<"); + Files.write(store.toPath(), xml.getBytes(StandardCharsets.UTF_8)); + XMLHandler other = new ConcurrentXMLHandler(config(store), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + other.init(); + try { + List found = new ArrayList(); + for (ConnectorObject object : other.search(allAccounts(), ObjectClass.ACCOUNT)) { + found.add(object.getName().getNameValue()); + } + assertEquals(found, List.of("alice")); + } finally { + other.dispose(); + } + } finally { + File[] children = dir.listFiles(); + if (children != null) { + for (File child : children) { + child.delete(); + } + } + dir.delete(); + } + } + + @Test + public void outsideEditDuringAChangeIsReportedAndOverwritten() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - 2 * HOUR); + handler.init(); + handler.create(ObjectClass.ACCOUNT, account("bob")); + writeAccounts(file, "carol"); + setModified(file, now() - HOUR); + String errors = captureStdErr(handler::dispose); + assertTrue(errors.contains("UPDATE COLLISION: " + file + " has changed since it was loaded or saved"), errors); + assertEquals(namesInFile(file), List.of("alice", "bob")); + } + + @Test + public void ownChangeIsSavedWithoutACollision() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + String errors = captureStdErr(() -> call(h -> h.create(ObjectClass.ACCOUNT, account("bob")))); + assertFalse(errors.contains("UPDATE COLLISION"), errors); + } + + @Test + public void deletedFileIsRecreatedWithoutACollision() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + assertEquals(names(), List.of("alice")); + Files.delete(file.toPath()); + String errors = captureStdErr(() -> assertEquals(names(), List.of())); + assertFalse(errors.contains("UPDATE COLLISION"), errors); + } + + @Test + public void retryAfterAFailedSaveDoesNotReportItsOwnPartialWrite() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + byte[] before = Files.readAllBytes(file.toPath()); + XMLHandlerImpl impl = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); + Set bob = account("bob"); + bob.add(AttributeBuilder.build(ATTR_ACCOUNT_FIRST_NAME, "x\uD800y")); // an unpaired surrogate: no encoder can write it + impl.create(ObjectClass.ACCOUNT, bob); + captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); + assertFalse(Arrays.equals(Files.readAllBytes(file.toPath()), before)); // the failed save truncated the file + String errors = captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); + assertFalse(errors.contains("UPDATE COLLISION"), errors); + } + + @Test + public void ownSaveIsNotParsedAgain() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + call(h -> h.create(ObjectClass.ACCOUNT, account("bob"))); // the stamp after this save is racy + String log = captureStdOut(() -> assertEquals(names(), List.of("alice", "bob"))); + assertFalse(log.contains("Loading XML document from"), log); + } + + @Test + public void failedSaveKeepsTheChangeInMemoryAndRetriesIt() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - 2 * HOUR); + handler.init(); + handler.create(ObjectClass.ACCOUNT, account("bob")); + Files.delete(file.toPath()); + assertTrue(file.mkdir()); // a directory cannot be written, whoever runs the test + try { + captureStdErr(() -> expectThrows(ConnectorException.class, handler::dispose)); + } finally { + assertTrue(file.delete()); + } + + // An outside file appears: it must not replace the unsaved change, which overwrites it and says so. + writeAccounts(file, "carol"); + setModified(file, now() - HOUR); + String errors = captureStdErr(() -> assertEquals(names(), List.of("alice", "bob"))); + assertTrue(errors.contains("UPDATE COLLISION"), errors); + assertEquals(namesInFile(file), List.of("alice", "bob")); + } + + @Test + public void unsavedNewDocumentGivesWayToAFileThatAppears() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: an empty document in memory + assertTrue(file.mkdir()); + try { + captureStdErr(() -> expectThrows(ConnectorException.class, handler::dispose)); + } finally { + assertTrue(file.delete()); + } + + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + byte[] appeared = Files.readAllBytes(file.toPath()); + assertEquals(names(), List.of("alice")); + assertEquals(Files.readAllBytes(file.toPath()), appeared); // loaded, not written back + } + + @Test + public void changedNewDocumentGivesWayToAFileThatAppears() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: a new document in memory + handler.create(ObjectClass.ACCOUNT, account("bob")); + assertTrue(file.mkdir()); + try { + captureStdErr(() -> expectThrows(ConnectorException.class, handler::dispose)); + } finally { + assertTrue(file.delete()); + } + + // A store appears (a restore, a late mount): the connector never loaded it, so a save would replace all of it. + writeAccounts(file, "alice", "carol"); + setModified(file, now() - HOUR); + byte[] appeared = Files.readAllBytes(file.toPath()); + String errors = captureStdErr(() -> assertEquals(names(), List.of("alice", "carol"))); + assertEquals(Files.readAllBytes(file.toPath()), appeared); + assertTrue(errors.contains(file + " appeared before the new document was saved"), errors); + } + + @Test + public void fileThatAppearsDuringAChangeToANewDocumentIsKept() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: a new document in memory + handler.create(ObjectClass.ACCOUNT, account("bob")); + writeAccounts(file, "alice", "carol"); + setModified(file, now() - HOUR); + byte[] appeared = Files.readAllBytes(file.toPath()); + String errors = captureStdErr(handler::dispose); + assertEquals(Files.readAllBytes(file.toPath()), appeared); + assertTrue(errors.contains(file + " appeared before the new document was saved"), errors); + assertEquals(names(), List.of("alice", "carol")); + } + + @Test + public void fileThatAppearsDuringTheFirstCallIsNotOverwritten() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: an empty new document + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + byte[] appeared = Files.readAllBytes(file.toPath()); + captureStdErr(handler::dispose); + assertEquals(Files.readAllBytes(file.toPath()), appeared); + assertEquals(names(), List.of("alice")); + } + + @Test + public void loadedStoreMovedBackBeforeTheNewDocumentIsSavedIsKept() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + assertEquals(names(), List.of("alice")); + File moved = new File(file.getPath() + ".moved"); + assertTrue(file.renameTo(moved)); + handler.init(); // no file: a new document replaces the loaded one + assertTrue(moved.renameTo(file)); // the loaded file is back, with its file key, size and mtime + handler.dispose(); + assertEquals(namesInFile(file), List.of("alice")); + } + + @Test + public void fileThatAppearsAsTheNewDocumentIsCreatedIsNotOverwritten() throws Exception { + XMLHandlerImpl impl = new XMLHandlerImpl(config(new AppearingFile(file)), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); // the check finds no file, and one appears right after it + byte[] appeared = Files.readAllBytes(file.toPath()); + impl.dispose(); + assertEquals(Files.readAllBytes(file.toPath()), appeared); + assertEquals(names(), List.of("alice")); + } + + @Test + public void changeToANewDocumentIsSavedOnceThePathIsFree() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: a new document in memory + handler.create(ObjectClass.ACCOUNT, account("bob")); + assertTrue(file.mkdir()); + try { + captureStdErr(() -> expectThrows(ConnectorException.class, handler::dispose)); + } finally { + assertTrue(file.delete()); + } + + // The stamp changed, but no file appeared: the change is saved, and no file is overwritten. + String errors = captureStdErr(() -> assertEquals(names(), List.of("bob"))); + assertFalse(errors.contains("UPDATE COLLISION"), errors); + assertEquals(namesInFile(file), List.of("bob")); + } + + @Test + public void changesDroppedForAFileThatAppearsStayDroppedWhenItGoes() throws Exception { + assertFalse(file.exists()); + handler.init(); // no file: a new document in memory + handler.create(ObjectClass.ACCOUNT, account("bob")); + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + captureStdErr(handler::dispose); // keeps the file and drops bob + Files.delete(file.toPath()); + assertEquals(names(), List.of()); + assertEquals(namesInFile(file), List.of()); + } + + @Test + public void ownPartialWriteOfANewDocumentIsSavedAgain() throws Exception { + XMLHandlerImpl impl = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); // no file: a new document in memory + Set bob = account("bob"); + bob.add(AttributeBuilder.build(ATTR_ACCOUNT_FIRST_NAME, "x\uD800y")); // an unpaired surrogate: no encoder can write it + impl.create(ObjectClass.ACCOUNT, bob); + captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); + assertTrue(file.exists()); // the failed save left a partial file + captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); // not a file that appeared: saved again + } + + @Test + public void ownPartialWriteOfAnUnchangedNewDocumentIsNotLoaded() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - HOUR); + XMLHandlerImpl impl = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); // the checksum is now the loaded file's, which no partial write matches + Files.delete(file.toPath()); + impl.init(); // no file: a new document in memory, with no change to save but the file itself + org.w3c.dom.Element root = impl.getDocument().getDocumentElement(); + // An unpaired surrogate: no encoder can write it + org.w3c.dom.Node unwritable = root.appendChild(impl.getDocument().createTextNode("x\uD800y")); + captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); + assertTrue(file.exists()); // the failed save left a partial file + root.removeChild(unwritable); + + impl.init(); // not a file that appeared: not parsed + impl.dispose(); + assertEquals(namesInFile(file), List.of()); + } + + @Test + public void ownPartialWriteBehindAnUnreadableStampIsSavedAgain() throws Exception { + StatFailingFile store = new StatFailingFile(file); + XMLHandlerImpl impl = new XMLHandlerImpl(config(store), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); // no file: a new document in memory + impl.create(ObjectClass.ACCOUNT, account("bob")); + org.w3c.dom.Element root = impl.getDocument().getDocumentElement(); + // An unpaired surrogate: no encoder can write it + org.w3c.dom.Node unwritable = root.appendChild(impl.getDocument().createTextNode("x\uD800y")); + store.statFails = true; // the stamp the failed save takes cannot be read + try { + captureStdErr(() -> expectThrows(ConnectorException.class, impl::dispose)); + } finally { + store.statFails = false; + } + assertTrue(file.exists()); // the failed save left a partial file + root.removeChild(unwritable); + + String errors = captureStdErr(() -> { + impl.init(); // not a file that appeared: neither dropped nor parsed + impl.dispose(); + }); + assertFalse(errors.contains("appeared before the new document was saved"), errors); + assertEquals(namesInFile(file), List.of("bob")); + } + + @Test + public void savedNewFileIsNotWrittenAgain() throws Exception { + call(h -> null); // creates the file + String log = captureStdOut(() -> call(h -> null)); + assertTrue(log.contains("Exit serialize: nothing to save"), log); + } + + @Test + public void failedDeleteOfANestedEntryLeavesTheFileAlone() throws Exception { + writeAccounts(file, "alice"); + // An XSD-invalid hand edit: getEntry's descendant query finds the entry, removeChild on the container rejects it. + String xml = new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8) + .replace(" \n", " \n \n") + .replace(" \n", " \n \n"); + Files.write(file.toPath(), xml.getBytes(StandardCharsets.UTF_8)); + FileTime past = FileTime.fromMillis(now() - HOUR); + Files.setLastModifiedTime(file.toPath(), past); + byte[] before = Files.readAllBytes(file.toPath()); + + expectThrows(DOMException.class, () -> call(h -> { + h.delete(ObjectClass.ACCOUNT, new Uid("uid-alice")); + return null; + })); + + assertEquals(Files.getLastModifiedTime(file.toPath()), past); + assertEquals(Files.readAllBytes(file.toPath()), before); + } + + @Test + public void externalEditWithANewerMtimeIsLoaded() throws Exception { + writeAccounts(file, "alice"); + setModified(file, now() - 2 * HOUR); + assertEquals(names(), List.of("alice")); + + writeAccounts(file, "bob"); + setModified(file, now() - HOUR); + assertEquals(names(), List.of("bob")); + } + + /** A store path where a file appears as soon as a check finds none there. */ + private static final class AppearingFile extends File { + + AppearingFile(File file) { + super(file.getPath()); + } + + @Override + public boolean exists() { + if (super.exists()) { + return true; + } + try { + writeAccounts(this, "alice"); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return false; + } + } + + /** + * A store path whose stamp cannot be read while {@code statFails} is set: the stamp is then read through a link to itself. + * Only callers of {@code toPath()} are diverted, such as {@code FileStamp}. {@code isFile()} and + * {@code XmlDocumentWriter.write}, which opens a {@code FileOutputStream} on the path string, still reach the store. + * A writer that opened {@code toPath()} would fail on the link before writing a byte, and the test would find no + * partial file. + */ + private static final class StatFailingFile extends File { + + boolean statFails; + private final File loop; + + StatFailingFile(File file) { + super(file.getPath()); + loop = new File(file.getPath() + ".loop"); + try { + Files.createSymbolicLink(loop.toPath(), loop.toPath().getFileName()); + } catch (IOException | UnsupportedOperationException e) { + throw new SkipException("Cannot create a symbolic link here: " + e); + } + try { + Files.readAttributes(loop.toPath(), BasicFileAttributes.class); + throw new SkipException("The file system resolves a link to itself"); + } catch (NoSuchFileException e) { + throw new SkipException("The file system reports a link to itself as missing"); + } catch (IOException expected) { + // ELOOP: the path is neither readable nor known to be missing + } + } + + @Override + public Path toPath() { + return statFails ? loop.toPath() : super.toPath(); + } + } + + /** Makes {@code file} unreadable, or skips the test where its owner can read it anyway (root, Windows). */ + private static void makeUnreadable(File file) { + if (!file.setReadable(false, false) || Files.isReadable(file.toPath())) { + file.setReadable(true, false); + throw new SkipException("Cannot make " + file + " unreadable here"); + } + } + + @Test + public void loadedDocumentIsFullyExpanded() throws Exception { + writeAccounts(file, "alice"); + XMLHandlerImpl impl = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); + assertFalse(impl.getDocument() instanceof org.apache.xerces.dom.DeferredDocumentImpl, + impl.getDocument().getClass().getName()); + } + + @Test + public void connectorObjectIsReadWithoutNodeLists() throws Exception { + writeAccounts(file, "alice"); + XMLHandlerImpl impl = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()); + impl.init(); + org.w3c.dom.Element entry = (org.w3c.dom.Element) impl.getDocument() + .getElementsByTagNameNS(RI_NAMESPACE, ObjectClass.ACCOUNT_NAME).item(0); + assertFalse(XercesNodeLists.used(impl.getDocument())); + + ConnectorObject object = impl.newConnectorObjectCreator(ObjectClass.ACCOUNT).createConnectorObject(entry); + + assertEquals(object.getUid().getUidValue(), "uid-alice"); + assertEquals(object.getName().getNameValue(), "alice"); + assertEquals(AttributeUtil.getStringValue(object.getAttributeByName(ATTR_ACCOUNT_LAST_NAME)), "Last-alice"); + assertFalse(XercesNodeLists.used(impl.getDocument())); + } + + @Test + public void parserWithoutTheDeferAttributeStillLoads() throws Exception { + writeAccounts(file, "alice"); + withDocumentBuilderFactory(NoDeferAttributeFactory.class, () -> assertEquals(names(), List.of("alice"))); + } + + @Test + public void parserWithoutTheDeferAttributeIsReported() throws Exception { + writeAccounts(file, "alice"); + String log = captureStdOut(() -> withDocumentBuilderFactory(NoDeferAttributeFactory.class, () -> names())); + assertTrue(log.contains("The XML parser " + NoDeferAttributeFactory.class.getName() + + " does not support http://apache.org/xml/features/dom/defer-node-expansion"), log); + } + + /** Runs {@code action} with {@code factory} as the JAXP DocumentBuilderFactory, the way a JVM-wide setting selects it. */ + private static void withDocumentBuilderFactory(Class factory, Runnable action) { + String property = "javax.xml.parsers.DocumentBuilderFactory"; + String previous = System.getProperty(property); + System.setProperty(property, factory.getName()); + try { + action.run(); + } finally { + if (previous == null) { + System.clearProperty(property); + } else { + System.setProperty(property, previous); + } + } + } + + /** A JAXP parser that supports none of the Xerces attributes: setAttribute throws, as the JAXP contract allows. */ + public static final class NoDeferAttributeFactory extends DocumentBuilderFactory { + + private final DocumentBuilderFactory xerces = new org.apache.xerces.jaxp.DocumentBuilderFactoryImpl(); + + @Override + public DocumentBuilder newDocumentBuilder() throws ParserConfigurationException { + xerces.setNamespaceAware(isNamespaceAware()); + return xerces.newDocumentBuilder(); + } + + @Override + public void setAttribute(String name, Object value) { + throw new IllegalArgumentException("Not supported: " + name); + } + + @Override + public Object getAttribute(String name) { + throw new IllegalArgumentException("Not supported: " + name); + } + + @Override + public void setFeature(String name, boolean value) throws ParserConfigurationException { + throw new ParserConfigurationException("Not supported: " + name); + } + + @Override + public boolean getFeature(String name) throws ParserConfigurationException { + throw new ParserConfigurationException("Not supported: " + name); + } + } + + /** One framework call: init, the operation, dispose. */ + private T call(Function operation) { + handler.init(); + try { + return operation.apply(handler); + } finally { + handler.dispose(); + } + } + + private List names() { + return call(h -> { + List names = new ArrayList(); + for (ConnectorObject object : h.search(allAccounts(), ObjectClass.ACCOUNT)) { + names.add(object.getName().getNameValue()); + } + return names; + }); + } + + private static String allAccounts() { + return new QueryBuilder(null, ObjectClass.ACCOUNT).toString(); + } + + private static List lastNames(Collection objects) { + List values = new ArrayList(); + for (ConnectorObject object : objects) { + Attribute lastName = object.getAttributeByName(ATTR_ACCOUNT_LAST_NAME); + values.add(lastName == null ? null : AttributeUtil.getStringValue(lastName)); + } + return values; + } + + private static List lastNamesIn(File file) { + XMLHandler fresh = new XMLHandlerImpl(config(file), schemaParser().parseSchema(), schemaParser().getXsdSchema()).init(); + return lastNames(fresh.search(allAccounts(), ObjectClass.ACCOUNT)); + } + + private static XMLConfiguration config(File file) { + XMLConfiguration config = new XMLConfiguration(); + config.setXmlFilePath(file); + config.setXsdFilePath(XSD_SCHEMA_FILEPATH); + config.setCreateFileIfNotExists(true); + return config; + } + + private static SchemaParser schemaParser() { + return new SchemaParser(XMLConnector.class, XSD_SCHEMA_FILEPATH); + } + + private static String captureStdOut(Runnable action) throws UnsupportedEncodingException { + PrintStream original = System.out; + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + System.setOut(new PrintStream(buffer, true, StandardCharsets.UTF_8.name())); + try { + action.run(); + } finally { + System.setOut(original); + } + return buffer.toString(StandardCharsets.UTF_8.name()); + } + + private static String captureStdErr(Runnable action) throws UnsupportedEncodingException { + PrintStream original = System.err; + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + System.setErr(new PrintStream(buffer, true, StandardCharsets.UTF_8.name())); + try { + action.run(); + } finally { + System.setErr(original); + } + return buffer.toString(StandardCharsets.UTF_8.name()); + } + + private static long now() { + return System.currentTimeMillis(); + } +}