From 1c087d5b3f5ac78087cd8f7963725aa009e73ae1 Mon Sep 17 00:00:00 2001 From: Juri Leino Date: Thu, 25 Apr 2024 15:37:17 +0200 Subject: [PATCH 1/8] [test] add test that triggers defragmentation after update - add constructor to ExistXmldbEmbeddedServer that allows passing in configuration properties - create new test that updates a document with fragmentation limit set to -1 --- .../exist/test/ExistXmldbEmbeddedServer.java | 12 +++ .../update/UpdateInsertTriggersDefrag.java | 78 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java diff --git a/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java b/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java index c39ea1f8452..cd54c9e7b4d 100644 --- a/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java +++ b/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.Map; +import java.util.Properties; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -98,6 +99,17 @@ public ExistXmldbEmbeddedServer(final boolean asGuest, final boolean disableAuto this.asGuest = asGuest; } + /** + * @param asGuest Use the guest account, default is the admin account + * @param disableAutoDeploy Whether auto-deployment of XARs should be disabled + * @param useTemporaryStorage Whether the data and journal folder should use temporary storage + * @param settings set properties + */ + public ExistXmldbEmbeddedServer(final boolean asGuest, final boolean disableAutoDeploy, final boolean useTemporaryStorage, final Properties settings) { + this.existEmbeddedServer = new ExistEmbeddedServer(null, null, settings, disableAutoDeploy, useTemporaryStorage); + this.asGuest = asGuest; + } + @Override protected void before() throws Throwable { startDb(); diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java new file mode 100644 index 00000000000..bfb68395976 --- /dev/null +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java @@ -0,0 +1,78 @@ +/* + * eXist-db Open Source Native XML Database + * Copyright (C) 2001 The eXist-db Authors + * + * info@exist-db.org + * http://www.exist-db.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ +package org.exist.xquery.update; + +import org.exist.test.ExistXmldbEmbeddedServer; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.xmldb.api.base.Collection; +import org.xmldb.api.base.ResourceSet; +import org.xmldb.api.modules.CollectionManagementService; +import org.xmldb.api.modules.XMLResource; +import org.xmldb.api.modules.XQueryService; + +import static org.exist.util.PropertiesBuilder.propertiesBuilder; +import static org.exist.storage.DBBroker.PROPERTY_XUPDATE_FRAGMENTATION_FACTOR; +import static org.exist.test.TestConstants.TEST_COLLECTION_URI; +import static org.exist.test.TestConstants.TEST_XML_URI; +import static org.junit.Assert.assertEquals; + +public class UpdateInsertTriggersDefrag { + @ClassRule + public static final ExistXmldbEmbeddedServer exist = new ExistXmldbEmbeddedServer(false, true, true, + propertiesBuilder().put(PROPERTY_XUPDATE_FRAGMENTATION_FACTOR, -1).build()); + private final String path = TEST_COLLECTION_URI + "/" + TEST_XML_URI.toString(); + private Collection testCollection; + private CollectionManagementService collectionService; + + @Before + public void setUp() throws Exception { + collectionService = (CollectionManagementService) exist.getRoot().getService("CollectionManagementService","1.0"); + + testCollection = collectionService.createCollection(TEST_COLLECTION_URI.lastSegment().toString()); + final XMLResource doc = (XMLResource) testCollection.createResource(TEST_XML_URI.toString(), XMLResource.RESOURCE_TYPE); + + doc.setContent("initial"); + testCollection.storeResource(doc); + } + + @After + public void tearDown() throws Exception { + collectionService.removeCollection(testCollection.getName()); + testCollection.close(); + } + + @Test + public void triggerDefragAfterUpdate() throws Exception { + final XQueryService queryService = (XQueryService) testCollection.getService("XPathQueryService", "1.0"); + + final String update = "update insert new node into doc('" + path + "')//list"; + final ResourceSet updateResult = queryService.queryResource(path, update); + assertEquals("Update expression returns an empty sequence", 0, updateResult.getSize()); + + final ResourceSet itemResult = queryService.queryResource(path, "//item"); + assertEquals("Both items are returned", 2, itemResult.getSize()); + } + +} From 9ca293bba9f0e6646e1905f2743fb79bb62d464f Mon Sep 17 00:00:00 2001 From: Juri Leino Date: Tue, 9 Apr 2024 15:13:01 +0200 Subject: [PATCH 2/8] [bugfix] NPE in NativeBroker.defragXMLResource A regression introduced in c553667 causes a NullPointerException to be thrown whenever an XML-resource needs to be defragmented. This will be triggered on frequent writes to any XML-resource and will effectively remove the file from the database and from any indexes. --- exist-core/src/main/java/org/exist/storage/NativeBroker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exist-core/src/main/java/org/exist/storage/NativeBroker.java b/exist-core/src/main/java/org/exist/storage/NativeBroker.java index 88bd6ce72cd..485d263f41d 100644 --- a/exist-core/src/main/java/org/exist/storage/NativeBroker.java +++ b/exist-core/src/main/java/org/exist/storage/NativeBroker.java @@ -3033,7 +3033,7 @@ public Object start() { } }.run(); // create a copy of the old doc to copy the nodes into it - final DocumentImpl tempDoc = new DocumentImpl(null, null, doc.getCollection(), doc.getDocId(), doc.getFileURI()); + final DocumentImpl tempDoc = new DocumentImpl(null, doc.getDocId(), doc); tempDoc.copyOf(this, doc, doc); final StreamListener listener = getIndexController().getStreamListener(doc, ReindexMode.STORE); // copy the nodes From 1e83f0bd05f964ebc5d426265d5db1ef7e8482ea Mon Sep 17 00:00:00 2001 From: Juri Leino Date: Mon, 14 Oct 2024 17:06:45 +0200 Subject: [PATCH 3/8] [bugfix] PermissionDeniedExcpetion in defragXMLResource --- exist-core/src/main/java/org/exist/storage/NativeBroker.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exist-core/src/main/java/org/exist/storage/NativeBroker.java b/exist-core/src/main/java/org/exist/storage/NativeBroker.java index 485d263f41d..028fb149ddb 100644 --- a/exist-core/src/main/java/org/exist/storage/NativeBroker.java +++ b/exist-core/src/main/java/org/exist/storage/NativeBroker.java @@ -3033,8 +3033,7 @@ public Object start() { } }.run(); // create a copy of the old doc to copy the nodes into it - final DocumentImpl tempDoc = new DocumentImpl(null, doc.getDocId(), doc); - tempDoc.copyOf(this, doc, doc); + final DocumentImpl tempDoc = new DocumentImpl(null, pool, doc.getCollection(), doc.getDocId(), doc.getFileURI()); final StreamListener listener = getIndexController().getStreamListener(doc, ReindexMode.STORE); // copy the nodes final NodeList nodes = doc.getChildNodes(); @@ -3067,7 +3066,7 @@ public Object start() { if (LOG.isDebugEnabled()) { LOG.debug("Defragmentation took {} ms.", (System.currentTimeMillis() - start)); } - } catch(final PermissionDeniedException | IOException e) { + } catch(IOException e) { LOG.error(e); } } From f88c402a8942a471387405a3e5ae241b28847f00 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 20 Oct 2024 12:55:52 +0200 Subject: [PATCH 4/8] [bugfix] Correct class name --- ...tTriggersDefrag.java => UpdateInsertTriggersDefragTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename exist-core/src/test/java/org/exist/xquery/update/{UpdateInsertTriggersDefrag.java => UpdateInsertTriggersDefragTest.java} (98%) diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java similarity index 98% rename from exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java rename to exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java index bfb68395976..f0f3cfb9821 100644 --- a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefrag.java +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java @@ -38,7 +38,7 @@ import static org.exist.test.TestConstants.TEST_XML_URI; import static org.junit.Assert.assertEquals; -public class UpdateInsertTriggersDefrag { +public class UpdateInsertTriggersDefragTest { @ClassRule public static final ExistXmldbEmbeddedServer exist = new ExistXmldbEmbeddedServer(false, true, true, propertiesBuilder().put(PROPERTY_XUPDATE_FRAGMENTATION_FACTOR, -1).build()); From a8fe2784d44b8de6ccfcc810dafb885c91e539b1 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 20 Oct 2024 13:00:35 +0200 Subject: [PATCH 5/8] [bugfix] Fix class members visibility --- .../org/exist/xquery/update/UpdateInsertTriggersDefragTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java index f0f3cfb9821..1b220f3fa28 100644 --- a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java @@ -39,9 +39,11 @@ import static org.junit.Assert.assertEquals; public class UpdateInsertTriggersDefragTest { + @ClassRule public static final ExistXmldbEmbeddedServer exist = new ExistXmldbEmbeddedServer(false, true, true, propertiesBuilder().put(PROPERTY_XUPDATE_FRAGMENTATION_FACTOR, -1).build()); + private final String path = TEST_COLLECTION_URI + "/" + TEST_XML_URI.toString(); private Collection testCollection; private CollectionManagementService collectionService; From bf4675fc3d72b2498c687dd5e3abf623179d9889 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 20 Oct 2024 13:01:45 +0200 Subject: [PATCH 6/8] [bugfix] Make sure not to leak XML Resource --- .../xquery/update/UpdateInsertTriggersDefragTest.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java index 1b220f3fa28..f8cf8b419c2 100644 --- a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java @@ -53,10 +53,11 @@ public void setUp() throws Exception { collectionService = (CollectionManagementService) exist.getRoot().getService("CollectionManagementService","1.0"); testCollection = collectionService.createCollection(TEST_COLLECTION_URI.lastSegment().toString()); - final XMLResource doc = (XMLResource) testCollection.createResource(TEST_XML_URI.toString(), XMLResource.RESOURCE_TYPE); + try (final XMLResource doc = (XMLResource) testCollection.createResource(TEST_XML_URI.toString(), XMLResource.RESOURCE_TYPE)) { - doc.setContent("initial"); - testCollection.storeResource(doc); + doc.setContent("initial"); + testCollection.storeResource(doc); + }; } @After From 9ef85e66d6a1d7a6b7039fdd60abf60f13b2ba5e Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 20 Oct 2024 13:12:00 +0200 Subject: [PATCH 7/8] [bugfix] Remove Collection is not needed as the test uses Temporary Storage --- .../org/exist/xquery/update/UpdateInsertTriggersDefragTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java index f8cf8b419c2..d9e4fa14578 100644 --- a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java @@ -62,7 +62,6 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { - collectionService.removeCollection(testCollection.getName()); testCollection.close(); } From 69555880f11464e724a744a7446cc6d00d4cb1ac Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Sun, 20 Oct 2024 13:32:59 +0200 Subject: [PATCH 8/8] [refactor] Use Embedded API directly. We should not use XML:DB API in tests unless we are intending to explicitly test the XML:DB API implementation --- .../exist/test/ExistXmldbEmbeddedServer.java | 11 --- .../UpdateInsertTriggersDefragTest.java | 86 +++++++++++-------- 2 files changed, 52 insertions(+), 45 deletions(-) diff --git a/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java b/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java index cd54c9e7b4d..8e1b46546a6 100644 --- a/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java +++ b/exist-core/src/main/java/org/exist/test/ExistXmldbEmbeddedServer.java @@ -99,17 +99,6 @@ public ExistXmldbEmbeddedServer(final boolean asGuest, final boolean disableAuto this.asGuest = asGuest; } - /** - * @param asGuest Use the guest account, default is the admin account - * @param disableAutoDeploy Whether auto-deployment of XARs should be disabled - * @param useTemporaryStorage Whether the data and journal folder should use temporary storage - * @param settings set properties - */ - public ExistXmldbEmbeddedServer(final boolean asGuest, final boolean disableAutoDeploy, final boolean useTemporaryStorage, final Properties settings) { - this.existEmbeddedServer = new ExistEmbeddedServer(null, null, settings, disableAutoDeploy, useTemporaryStorage); - this.asGuest = asGuest; - } - @Override protected void before() throws Throwable { startDb(); diff --git a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java index d9e4fa14578..291b493d63d 100644 --- a/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java +++ b/exist-core/src/test/java/org/exist/xquery/update/UpdateInsertTriggersDefragTest.java @@ -21,60 +21,78 @@ */ package org.exist.xquery.update; -import org.exist.test.ExistXmldbEmbeddedServer; -import org.junit.After; +import com.evolvedbinary.j8fu.function.Consumer2E; +import org.exist.EXistException; +import org.exist.collections.Collection; +import org.exist.security.PermissionDeniedException; +import org.exist.source.StringSource; +import org.exist.storage.BrokerPool; +import org.exist.storage.DBBroker; +import org.exist.storage.txn.Txn; +import org.exist.test.ExistEmbeddedServer; +import org.exist.test.TestConstants; +import org.exist.util.MimeType; +import org.exist.util.StringInputSource; +import org.exist.xquery.XPathException; +import org.exist.xquery.value.Sequence; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; -import org.xmldb.api.base.Collection; -import org.xmldb.api.base.ResourceSet; -import org.xmldb.api.modules.CollectionManagementService; -import org.xmldb.api.modules.XMLResource; -import org.xmldb.api.modules.XQueryService; +import java.io.IOException; +import java.util.Optional; + +import static org.exist.test.Util.executeQuery; +import static org.exist.test.Util.withCompiledQuery; import static org.exist.util.PropertiesBuilder.propertiesBuilder; -import static org.exist.storage.DBBroker.PROPERTY_XUPDATE_FRAGMENTATION_FACTOR; -import static org.exist.test.TestConstants.TEST_COLLECTION_URI; -import static org.exist.test.TestConstants.TEST_XML_URI; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class UpdateInsertTriggersDefragTest { @ClassRule - public static final ExistXmldbEmbeddedServer exist = new ExistXmldbEmbeddedServer(false, true, true, - propertiesBuilder().put(PROPERTY_XUPDATE_FRAGMENTATION_FACTOR, -1).build()); - - private final String path = TEST_COLLECTION_URI + "/" + TEST_XML_URI.toString(); - private Collection testCollection; - private CollectionManagementService collectionService; + public static final ExistEmbeddedServer existEmbeddedServer = new ExistEmbeddedServer(propertiesBuilder().put(DBBroker.PROPERTY_XUPDATE_FRAGMENTATION_FACTOR, -1).build(), true, true); @Before public void setUp() throws Exception { - collectionService = (CollectionManagementService) exist.getRoot().getService("CollectionManagementService","1.0"); + final BrokerPool brokerPool = existEmbeddedServer.getBrokerPool(); + try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject())); + final Txn transaction = brokerPool.getTransactionManager().beginTransaction()) { - testCollection = collectionService.createCollection(TEST_COLLECTION_URI.lastSegment().toString()); - try (final XMLResource doc = (XMLResource) testCollection.createResource(TEST_XML_URI.toString(), XMLResource.RESOURCE_TYPE)) { - - doc.setContent("initial"); - testCollection.storeResource(doc); - }; - } + // store the test document in the test collection + try (final Collection testCollection = broker.getOrCreateCollection(transaction, TestConstants.TEST_COLLECTION_URI)) { + broker.storeDocument(transaction, TestConstants.TEST_XML_URI, new StringInputSource("initial"), MimeType.XML_TYPE, testCollection); + } - @After - public void tearDown() throws Exception { - testCollection.close(); + transaction.commit(); + } } @Test public void triggerDefragAfterUpdate() throws Exception { - final XQueryService queryService = (XQueryService) testCollection.getService("XPathQueryService", "1.0"); + final String updateQuery = "update insert new node into doc('" + TestConstants.TEST_COLLECTION_URI + "/" + TestConstants.TEST_XML_URI + "')//list"; + assertQuery(updateQuery, updateResults -> + assertTrue("Update expression returns an empty sequence", updateResults.isEmpty()) + ); - final String update = "update insert new node into doc('" + path + "')//list"; - final ResourceSet updateResult = queryService.queryResource(path, update); - assertEquals("Update expression returns an empty sequence", 0, updateResult.getSize()); - - final ResourceSet itemResult = queryService.queryResource(path, "//item"); - assertEquals("Both items are returned", 2, itemResult.getSize()); + final String searchQuery = "doc('" + TestConstants.TEST_COLLECTION_URI + "/" + TestConstants.TEST_XML_URI + "')//item"; + assertQuery(searchQuery, searchResults -> + assertEquals("Both items are returned", 2, searchResults.getItemCount()) + ); } + private void assertQuery(final String query, final Consumer2E assertions) throws EXistException, XPathException, PermissionDeniedException, IOException { + final BrokerPool brokerPool = existEmbeddedServer.getBrokerPool(); + try (final DBBroker broker = brokerPool.get(Optional.of(brokerPool.getSecurityManager().getSystemSubject())); + final Txn transaction = brokerPool.getTransactionManager().beginTransaction()) { + + withCompiledQuery(broker, new StringSource(query), compiledQuery -> { + final Sequence results = executeQuery(broker, compiledQuery); + assertions.accept(results); + return null; + }); + + transaction.commit(); + } + } }