From 8253557d03502a70e9863350615d4f7aea463fc2 Mon Sep 17 00:00:00 2001 From: Robert Munteanu Date: Fri, 28 Aug 2015 14:44:03 +0300 Subject: [PATCH] OAK-3313 - Many tests leak DocumentNodeStore instances --- .../plugins/blob/ClusterRepositoryInfoTest.java | 16 +- .../jackrabbit/oak/plugins/document/BlobTest.java | 11 +- .../oak/plugins/document/CheckpointsTest.java | 6 +- .../oak/plugins/document/CollisionTest.java | 13 +- .../oak/plugins/document/CommitQueueTest.java | 9 +- .../document/DocumentMkBuilderProvider.java | 65 +++ .../plugins/document/DocumentNodeStoreTest.java | 488 +++++++++------------ .../plugins/document/LastRevRecoveryAgentTest.java | 3 +- .../oak/plugins/document/LastRevRecoveryTest.java | 8 +- .../document/LastRevSingleNodeRecoveryTest.java | 14 +- .../oak/plugins/document/NodeStoreDiffTest.java | 10 +- .../plugins/document/SharedBlobStoreGCTest.java | 6 + .../oak/plugins/document/SimpleTest.java | 113 +++-- .../oak/plugins/document/UnmergedBranchTest.java | 8 +- .../oak/plugins/document/ValueMapTest.java | 5 +- 15 files changed, 396 insertions(+), 379 deletions(-) create mode 100644 oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMkBuilderProvider.java diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ClusterRepositoryInfoTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ClusterRepositoryInfoTest.java index eb27670..c64d6ca 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ClusterRepositoryInfoTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/ClusterRepositoryInfoTest.java @@ -20,7 +20,7 @@ import static org.hamcrest.CoreMatchers.instanceOf; import junit.framework.Assert; import org.apache.commons.io.FileUtils; -import org.apache.jackrabbit.oak.plugins.document.DocumentMK; +import org.apache.jackrabbit.oak.plugins.document.DocumentMkBuilderProvider; import org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore; import org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreUtils; import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; @@ -29,6 +29,7 @@ import org.apache.jackrabbit.oak.spi.blob.BlobStore; import org.junit.After; import org.junit.Assume; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; import java.io.File; @@ -40,6 +41,9 @@ import java.io.IOException; public class ClusterRepositoryInfoTest { static BlobStore blobStore; + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + @BeforeClass public static void setup() { try { @@ -52,14 +56,14 @@ public class ClusterRepositoryInfoTest { @Test public void differentCluster() throws Exception { - DocumentNodeStore ds1 = new DocumentMK.Builder() + DocumentNodeStore ds1 = builderProvider.newBuilder() .setAsyncDelay(0) .setDocumentStore(new MemoryDocumentStore()) .setBlobStore(blobStore) .getNodeStore(); String repoId1 = ClusterRepositoryInfo.createId(ds1); - DocumentNodeStore ds2 = new DocumentMK.Builder() + DocumentNodeStore ds2 = builderProvider.newBuilder() .setAsyncDelay(0) .setDocumentStore(new MemoryDocumentStore()) .setBlobStore(blobStore) @@ -72,7 +76,7 @@ public class ClusterRepositoryInfoTest { @Test public void sameCluster() throws Exception { MemoryDocumentStore store = new MemoryDocumentStore(); - DocumentNodeStore ds1 = new DocumentMK.Builder() + DocumentNodeStore ds1 = builderProvider.newBuilder() .setAsyncDelay(0) .setDocumentStore(store) .setClusterId(1) @@ -81,7 +85,7 @@ public class ClusterRepositoryInfoTest { String repoId1 = ClusterRepositoryInfo.createId(ds1); ds1.runBackgroundOperations(); - DocumentNodeStore ds2 = new DocumentMK.Builder() + DocumentNodeStore ds2 = builderProvider.newBuilder() .setAsyncDelay(0) .setDocumentStore(store) .setClusterId(2) @@ -96,7 +100,7 @@ public class ClusterRepositoryInfoTest { @Test public void checkGetIdWhenNotRegistered() { MemoryDocumentStore store = new MemoryDocumentStore(); - DocumentNodeStore ds1 = new DocumentMK.Builder() + DocumentNodeStore ds1 = builderProvider.newBuilder() .setAsyncDelay(0) .setDocumentStore(store) .setClusterId(1) diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BlobTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BlobTest.java index c292399..459f5b9 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BlobTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BlobTest.java @@ -29,6 +29,7 @@ import org.apache.jackrabbit.oak.json.BlobSerializer; import org.apache.jackrabbit.oak.plugins.blob.BlobStoreBlob; import org.apache.jackrabbit.oak.plugins.memory.ArrayBasedBlob; import org.apache.jackrabbit.oak.spi.blob.MemoryBlobStore; +import org.junit.Rule; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +41,9 @@ import com.mongodb.DB; */ public class BlobTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private static final Logger LOG = LoggerFactory.getLogger(RandomizedClusterTest.class); // private static final boolean MONGO_DB = true; @@ -59,10 +63,10 @@ public class BlobTest { MongoUtils.dropCollections(MongoUtils.getConnection().getDB()); } } - + @Test public void addBlobs() throws Exception { - DocumentMK mk = new DocumentMK.Builder(). + DocumentMK mk = builderProvider.newBuilder(). setMongoDB(openMongoConnection()).open(); long blobSize = TOTAL_SIZE / DOCUMENT_COUNT; ArrayList blobIds = new ArrayList(); @@ -76,13 +80,12 @@ public class BlobTest { for (String id : blobIds) { assertEquals(blobSize, mk.getLength(id)); } - mk.dispose(); } @Test public void testBlobSerialization() throws Exception{ TestBlobStore blobStore = new TestBlobStore(); - DocumentMK mk = new DocumentMK.Builder().setBlobStore(blobStore).open(); + DocumentMK mk = builderProvider.newBuilder().setBlobStore(blobStore).open(); BlobSerializer blobSerializer = mk.getNodeStore().getBlobSerializer(); Blob blob = new BlobStoreBlob(blobStore, "foo"); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CheckpointsTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CheckpointsTest.java index 214209f..f6524fd 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CheckpointsTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CheckpointsTest.java @@ -28,6 +28,7 @@ import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.stats.Clock; import org.junit.Before; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; import com.google.common.collect.ImmutableMap; @@ -40,6 +41,9 @@ import static org.junit.Assert.assertTrue; public class CheckpointsTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private Clock clock; private DocumentNodeStore store; @@ -47,7 +51,7 @@ public class CheckpointsTest { @Before public void setUp() throws InterruptedException { clock = new Clock.Virtual(); - store = new DocumentMK.Builder().clock(clock).getNodeStore(); + store = builderProvider.newBuilder().clock(clock).getNodeStore(); } @Test diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CollisionTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CollisionTest.java index 9c681d6..b9604a6 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CollisionTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CollisionTest.java @@ -18,7 +18,7 @@ package org.apache.jackrabbit.oak.plugins.document; import java.util.concurrent.atomic.AtomicInteger; -import org.apache.jackrabbit.oak.plugins.document.util.Utils; +import org.junit.Rule; import org.junit.Test; import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES; @@ -28,16 +28,19 @@ import static org.junit.Assert.assertEquals; public class CollisionTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private static final AtomicInteger COUNTER = new AtomicInteger(); // OAK-2342 @Test public void purge() throws Exception { - DocumentMK mk1 = new DocumentMK.Builder().setClusterId(1).open(); + DocumentMK mk1 = builderProvider.newBuilder().setClusterId(1).open(); DocumentNodeStore ns1 = mk1.getNodeStore(); DocumentStore store = ns1.getDocumentStore(); - DocumentMK mk2 = new DocumentMK.Builder().setClusterId(2) + DocumentMK mk2 = builderProvider.newBuilder().setClusterId(2) .setDocumentStore(store).open(); DocumentNodeStore ns2 = mk2.getNodeStore(); @@ -49,7 +52,7 @@ public class CollisionTest { // restart node store ns1.dispose(); - mk1 = new DocumentMK.Builder().setClusterId(1) + mk1 = builderProvider.newBuilder().setClusterId(1) .setDocumentStore(store).open(); ns1 = mk1.getNodeStore(); @@ -59,7 +62,7 @@ public class CollisionTest { // restart other node store ns2.dispose(); - mk2 = new DocumentMK.Builder().setClusterId(2) + mk2 = builderProvider.newBuilder().setClusterId(2) .setDocumentStore(store).open(); ns2 = mk2.getNodeStore(); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitQueueTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitQueueTest.java index 9c5eb05..1781a3d 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitQueueTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitQueueTest.java @@ -31,6 +31,7 @@ import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.commit.Observer; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.junit.Rule; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -43,6 +44,9 @@ import static org.junit.Assert.assertFalse; */ public class CommitQueueTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private static final Logger LOG = LoggerFactory.getLogger(CommitQueueTest.class); private static final int NUM_WRITERS = 10; @@ -53,7 +57,7 @@ public class CommitQueueTest { @Test public void concurrentCommits() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); AtomicBoolean running = new AtomicBoolean(true); Closeable observer = store.addObserver(new Observer() { @@ -178,7 +182,7 @@ public class CommitQueueTest { // OAK-2868 @Test public void branchCommitMustNotBlockTrunkCommit() throws Exception { - final DocumentNodeStore ds = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore ds = builderProvider.newBuilder().getNodeStore(); // simulate start of a branch commit Commit c = ds.newCommit(ds.getHeadRevision().asBranchRevision(), null); @@ -201,7 +205,6 @@ public class CommitQueueTest { assertFalse("Commit did not succeed within 3 seconds", t.isAlive()); ds.canceled(c); - ds.dispose(); assertNoExceptions(); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMkBuilderProvider.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMkBuilderProvider.java new file mode 100644 index 0000000..dbf29f3 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentMkBuilderProvider.java @@ -0,0 +1,65 @@ +package org.apache.jackrabbit.oak.plugins.document; + +import java.util.List; + +import org.junit.rules.ExternalResource; + +import com.google.common.collect.Lists; + +/** + * The DocumentMkBuilderProvider is a JUnit @Rule which automatically disposes created DocumentNodeStore instances + * + *

Usage samples are below

+ * + *

Before:

+ * + * 
+ *  @Test public void someTest() {
+ *      DocumentNodeStore = new DocumentMK.Builder().getNodeStore();
+ *  }
+ * + *

After:

+ * + * 
+ *  @Rule
+    public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider();
+ *  
+ *  @Test public void someTest() {
+ *      DocumentNodeStore = builderProvider.newBuilder().getNodeStore();
+ *  }
+ * + */ +public class DocumentMkBuilderProvider extends ExternalResource { + + private List builders = Lists.newArrayList(); + + @Override + protected void after() { + for ( DocumentMkBuilderProvider.DisposingDocumentMkBuilder builder : builders ) { + builder.dispose(); + } + } + + public DocumentMK.Builder newBuilder() { + DocumentMkBuilderProvider.DisposingDocumentMkBuilder builder = new DisposingDocumentMkBuilder(); + builders.add(builder); + return builder; + } + + private static class DisposingDocumentMkBuilder extends DocumentMK.Builder { + + private boolean initialised = false; + + @Override + public DocumentNodeStore getNodeStore() { + initialised = true; + return super.getNodeStore(); + } + + public void dispose() { + if ( initialised ) { + getNodeStore().dispose(); + } + } + } +} \ No newline at end of file diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java index 71e929f..2f03e16 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java @@ -50,6 +50,7 @@ import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -89,10 +90,14 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.apache.jackrabbit.oak.stats.Clock; import org.junit.After; +import org.junit.Rule; import org.junit.Test; public class DocumentNodeStoreTest { - + + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + @After public void tearDown() { Revision.resetClockToDefault(); @@ -112,9 +117,9 @@ public class DocumentNodeStoreTest { return null; } }; - final DocumentNodeStore store1 = new DocumentMK.Builder().setAsyncDelay(0) + final DocumentNodeStore store1 = builderProvider.newBuilder().setAsyncDelay(0) .setDocumentStore(testStore).setClusterId(1).getNodeStore(); - DocumentNodeStore store2 = new DocumentMK.Builder().setAsyncDelay(0) + DocumentNodeStore store2 = builderProvider.newBuilder().setAsyncDelay(0) .setDocumentStore(docStore).setClusterId(2).getNodeStore(); NodeBuilder builder = store2.getRoot().builder(); @@ -156,14 +161,11 @@ public class DocumentNodeStoreTest { root = store1.getRoot(); // now node2 is visible assertTrue(root.hasChildNode("node2")); - - store1.dispose(); - store2.dispose(); } @Test public void childNodeCache() throws Exception { - DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); NodeBuilder builder = store.getRoot().builder(); int max = (int) (100 * 1.5); SortedSet children = new TreeSet(); @@ -180,7 +182,6 @@ public class DocumentNodeStoreTest { store.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); int numEntries = Iterables.size(store.getRoot().getChildNodeEntries()); assertEquals(max - 1, numEntries); - store.dispose(); } @Test @@ -197,7 +198,7 @@ public class DocumentNodeStoreTest { return super.query(collection, fromKey, toKey, limit); } }; - DocumentNodeStore store = new DocumentMK.Builder() + DocumentNodeStore store = builderProvider.newBuilder() .setDocumentStore(docStore).getNodeStore(); NodeBuilder root = store.getRoot().builder(); for (int i = 0; i < 10; i++) { @@ -217,8 +218,6 @@ public class DocumentNodeStoreTest { e.getNodeState(); } assertEquals(0, counter.get()); - - store.dispose(); } @Test @@ -240,7 +239,7 @@ public class DocumentNodeStoreTest { } }; final List exceptions = new ArrayList(); - final DocumentMK mk = new DocumentMK.Builder() + final DocumentMK mk = builderProvider.newBuilder() .setDocumentStore(docStore).setAsyncDelay(0).open(); final DocumentNodeStore store = mk.getNodeStore(); @@ -307,20 +306,18 @@ public class DocumentNodeStoreTest { doc = docStore.find(NODES, id); assertNull("document with id " + id + " must not have _deletedOnce despite rollback", doc.get(NodeDocument.DELETED_ONCE)); - - mk.dispose(); } // OAK-1662 @Test public void getNewestRevision() throws Exception { DocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns1 = new DocumentMK.Builder() + DocumentNodeStore ns1 = builderProvider.newBuilder() .setDocumentStore(docStore).setAsyncDelay(0) .setClusterId(1).getNodeStore(); ns1.getRoot(); ns1.runBackgroundOperations(); - DocumentNodeStore ns2 = new DocumentMK.Builder() + DocumentNodeStore ns2 = builderProvider.newBuilder() .setDocumentStore(docStore).setAsyncDelay(0) .setClusterId(2).getNodeStore(); ns2.getRoot(); @@ -335,16 +332,13 @@ public class DocumentNodeStoreTest { NodeBuilder b2 = ns2.getRoot().builder(); b2.setProperty("q", "value"); ns2.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY); - - ns1.dispose(); - ns2.dispose(); } @Test public void commitHookChangesOnBranch() throws Exception { final int NUM_NODES = DocumentRootBuilder.UPDATE_LIMIT / 2; final int NUM_PROPS = 10; - DocumentNodeStore ns = new DocumentMK.Builder().getNodeStore(); + DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); for (int i = 0; i < NUM_NODES; i++) { NodeBuilder c = builder.child("n" + i); @@ -394,8 +388,6 @@ public class DocumentNodeStoreTest { assertEquals("test", p.getValue(Type.STRING)); } } - - ns.dispose(); } // OAK-1814 @@ -405,14 +397,14 @@ public class DocumentNodeStoreTest { clock.waitUntil(System.currentTimeMillis()); Revision.setClock(clock); MemoryDocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore nodeStore1 = new DocumentMK.Builder() + DocumentNodeStore nodeStore1 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(1) .setAsyncDelay(0).clock(clock).getNodeStore(); nodeStore1.runBackgroundOperations(); - DocumentNodeStore nodeStore2 = new DocumentMK.Builder() + DocumentNodeStore nodeStore2 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(2) .setAsyncDelay(0).clock(clock).getNodeStore(); - DocumentNodeStore nodeStore3 = new DocumentMK.Builder() + DocumentNodeStore nodeStore3 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(3) .setAsyncDelay(0).clock(clock).getNodeStore(); @@ -438,10 +430,6 @@ public class DocumentNodeStoreTest { NodeState state = doc.getNodeAtRevision(nodeStore3, nodeStore3.getHeadRevision(), null); assertNotNull(state); - - nodeStore1.dispose(); - nodeStore2.dispose(); - nodeStore3.dispose(); } @Test @@ -450,7 +438,7 @@ public class DocumentNodeStoreTest { clock.waitUntil(System.currentTimeMillis()); Revision.setClock(clock); MemoryDocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns1 = new DocumentMK.Builder() + DocumentNodeStore ns1 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(1) .setAsyncDelay(0).clock(clock).getNodeStore(); NodeBuilder builder1 = ns1.getRoot().builder(); @@ -458,7 +446,7 @@ public class DocumentNodeStoreTest { ns1.merge(builder1, EmptyHook.INSTANCE, CommitInfo.EMPTY); ns1.runBackgroundOperations(); - DocumentNodeStore ns2 = new DocumentMK.Builder() + DocumentNodeStore ns2 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(2) .setAsyncDelay(0).clock(clock).getNodeStore(); @@ -488,9 +476,6 @@ public class DocumentNodeStoreTest { doc = docStore.find(NODES, Utils.getIdFromPath("/node")); Long mod2 = (Long) doc.get(MODIFIED_IN_SECS); assertTrue("" + mod2 + " < " + mod1, mod2 >= mod1); - - ns1.dispose(); - ns2.dispose(); } // OAK-1861 @@ -510,7 +495,7 @@ public class DocumentNodeStoreTest { return super.query(collection, fromKey, toKey, limit); } }; - DocumentNodeStore ns = new DocumentMK.Builder() + DocumentNodeStore ns = builderProvider.newBuilder() .setDocumentStore(docStore) .setAsyncDelay(0).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); @@ -538,14 +523,14 @@ public class DocumentNodeStoreTest { @Test public void readFromPreviousDoc() throws CommitFailedException { DocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns = new DocumentMK.Builder() + DocumentNodeStore ns = builderProvider.newBuilder() .setDocumentStore(docStore).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); builder.child("test").setProperty("prop", "initial"); ns.merge(builder, EmptyHook.INSTANCE, CommitInfo.EMPTY); ns.dispose(); - ns = new DocumentMK.Builder().setClusterId(2).setAsyncDelay(0) + ns = builderProvider.newBuilder().setClusterId(2).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); builder = ns.getRoot().builder(); builder.child("test").setProperty("prop", "value"); @@ -584,10 +569,10 @@ public class DocumentNodeStoreTest { Revision.setClock(clock); DocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns1 = new DocumentMK.Builder().setAsyncDelay(0) + DocumentNodeStore ns1 = builderProvider.newBuilder().setAsyncDelay(0) .clock(clock).setDocumentStore(docStore).setClusterId(1) .getNodeStore(); - DocumentNodeStore ns2 = new DocumentMK.Builder().setAsyncDelay(0) + DocumentNodeStore ns2 = builderProvider.newBuilder().setAsyncDelay(0) .clock(clock).setDocumentStore(docStore).setClusterId(2) .getNodeStore(); @@ -640,17 +625,14 @@ public class DocumentNodeStoreTest { assertTrue(diff.modified.contains("/test")); assertEquals(1, diff.added.size()); assertTrue(diff.added.contains("/test/foo")); - - ns1.dispose(); - ns2.dispose(); } @Test public void updateClusterState() { DocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns1 = new DocumentMK.Builder().setAsyncDelay(0) + DocumentNodeStore ns1 = builderProvider.newBuilder().setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); - DocumentNodeStore ns2 = new DocumentMK.Builder().setAsyncDelay(0) + DocumentNodeStore ns2 = builderProvider.newBuilder().setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); ns1.updateClusterState(); @@ -671,14 +653,12 @@ public class DocumentNodeStoreTest { assertEquals(1, (int) inactive.keySet().iterator().next()); assertEquals(1, active.size()); assertEquals(2, (int) active.keySet().iterator().next()); - - ns2.dispose(); } // OAK-2288 @Test public void mergedBranchVisibility() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder() + final DocumentNodeStore store = builderProvider.newBuilder() .setAsyncDelay(0).getNodeStore(); DocumentStore docStore = store.getDocumentStore(); @@ -706,8 +686,6 @@ public class DocumentNodeStoreTest { // must be visible at the revision of the merged branch assertTrue(store.getRoot().getChildNode("test").getChildNode("node").exists()); - - store.dispose(); } // OAK-2308 @@ -718,7 +696,7 @@ public class DocumentNodeStoreTest { MemoryDocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore store1 = new DocumentMK.Builder() + DocumentNodeStore store1 = builderProvider.newBuilder() .setDocumentStore(docStore) .setAsyncDelay(0).clock(clock).getNodeStore(); @@ -746,14 +724,11 @@ public class DocumentNodeStoreTest { agent.recover(store1.getClusterId()); // start a second store - DocumentNodeStore store2 = new DocumentMK.Builder() + DocumentNodeStore store2 = builderProvider.newBuilder() .setDocumentStore(docStore) .setAsyncDelay(0).clock(clock).getNodeStore(); // must see /test/node assertTrue(store2.getRoot().getChildNode("test").getChildNode("node").exists()); - - store2.dispose(); - store1.dispose(); } // OAK-2336 @@ -768,7 +743,7 @@ public class DocumentNodeStoreTest { return super.find(collection, key); } }; - DocumentNodeStore ns = new DocumentMK.Builder() + DocumentNodeStore ns = builderProvider.newBuilder() .setDocumentStore(store).setAsyncDelay(0).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); String testId = Utils.getIdFromPath("/test"); @@ -804,8 +779,6 @@ public class DocumentNodeStoreTest { fail("must not access previous document: " + id); } } - - ns.dispose(); } // OAK-2345 @@ -815,7 +788,7 @@ public class DocumentNodeStoreTest { clock.waitUntil(System.currentTimeMillis()); Revision.setClock(clock); MemoryDocumentStore docStore = new MemoryDocumentStore(); - DocumentNodeStore ns1 = new DocumentMK.Builder() + DocumentNodeStore ns1 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(1) .setAsyncDelay(0).clock(clock).getNodeStore(); NodeBuilder builder = ns1.getRoot().builder(); @@ -825,7 +798,7 @@ public class DocumentNodeStoreTest { ns1.dispose(); // start other cluster node - DocumentNodeStore ns2 = new DocumentMK.Builder() + DocumentNodeStore ns2 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(2) .setAsyncDelay(0).clock(clock).getNodeStore(); assertNotNull(ns2.getRevisionComparator().getRevisionSeen(r)); @@ -836,12 +809,11 @@ public class DocumentNodeStoreTest { + REMEMBER_REVISION_ORDER_MILLIS + 1000); // start cluster 2 again - ns2 = new DocumentMK.Builder() + ns2 = builderProvider.newBuilder() .setDocumentStore(docStore).setClusterId(2) .setAsyncDelay(0).clock(clock).getNodeStore(); // now r is considered old and revisionSeen is null assertNull(ns2.getRevisionComparator().getRevisionSeen(r)); - ns2.dispose(); } // OAK-1782 @@ -862,7 +834,7 @@ public class DocumentNodeStoreTest { indexedProperty, startValue, limit); } }; - final DocumentNodeStore ns = new DocumentMK.Builder() + final DocumentNodeStore ns = builderProvider.newBuilder() .setDocumentStore(store).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); // make sure we have enough children to trigger diffManyChildren @@ -907,8 +879,6 @@ public class DocumentNodeStoreTest { // 1) query the first 50 children to find out there are many // 2) query for the changed children between the two revisions assertTrue(numQueries.get() <= 2); - - store.dispose(); } // OAK-2359 @@ -923,7 +893,7 @@ public class DocumentNodeStoreTest { return super.find(collection, key); } }; - DocumentNodeStore store = new DocumentMK.Builder() + DocumentNodeStore store = builderProvider.newBuilder() .setClusterId(1).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); @@ -956,8 +926,6 @@ public class DocumentNodeStoreTest { reads.clear(); doc.getValueMap("foo").get(removedAt); assertNoPreviousDocs(reads); - - store.dispose(); } // OAK-2464 @@ -972,7 +940,7 @@ public class DocumentNodeStoreTest { return super.find(collection, key); } }; - DocumentNodeStore store = new DocumentMK.Builder() + DocumentNodeStore store = builderProvider.newBuilder() .setClusterId(1).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); @@ -1010,8 +978,6 @@ public class DocumentNodeStoreTest { assertEquals("Should not go to DocStore::find when doc child cache is filled by reading", 0, reads.size()); assertFalse("Non existing children should be reported as such", nonExistingChild.exists()); - - store.dispose(); } @Test @@ -1025,7 +991,7 @@ public class DocumentNodeStoreTest { return super.find(collection, key); } }; - DocumentNodeStore store = new DocumentMK.Builder() + DocumentNodeStore store = builderProvider.newBuilder() .setUseSimpleRevision(true) .setClusterId(1).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); @@ -1051,7 +1017,6 @@ public class DocumentNodeStoreTest { assertTrue("DocStore should be queried when no doc child cache entry has all children", reads.size() > 0); assertFalse("Non existing children should be reported as such", nonExistingChild.exists()); - store.dispose(); } @Test @@ -1088,7 +1053,7 @@ public class DocumentNodeStoreTest { } }; - DocumentNodeStore store = new DocumentMK.Builder() + DocumentNodeStore store = builderProvider.newBuilder() .setUseSimpleRevision(true) .setClusterId(1).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); @@ -1122,257 +1087,218 @@ public class DocumentNodeStoreTest { assertTrue("Fully cached entry in doc child cache should be able to find existing children" + " even if doc store sort order is incompatible to that of Java", reads.size() > 0); assertTrue("Existing children should be reported as such", existingChild.exists()); - - store.dispose(); } @Test public void mergeInternalDocAcrossCluster() throws Exception { MemoryDocumentStore docStore = new MemoryDocumentStore(); - final DocumentNodeStore store1 = new DocumentMK.Builder() + final DocumentNodeStore store1 = builderProvider.newBuilder() .setDocumentStore(docStore).setAsyncDelay(0) .setClusterId(1) .getNodeStore(); store1.setEnableConcurrentAddRemove(true); - final DocumentNodeStore store2 = new DocumentMK.Builder() + final DocumentNodeStore store2 = builderProvider.newBuilder() .setDocumentStore(docStore).setAsyncDelay(0) .setClusterId(2) .getNodeStore(); store2.setEnableConcurrentAddRemove(true); - try { - NodeState root; - NodeBuilder builder; + NodeState root; + NodeBuilder builder; - //Prepare repo - root = store1.getRoot(); - builder = root.builder(); - builder.child(":hidden").child("deleteDeleted"); - builder.child(":hidden").child("deleteChanged"); - builder.child(":hidden").child("changeDeleted"); - merge(store1, builder); - store1.runBackgroundOperations(); - store2.runBackgroundOperations(); - - //Changes in store1 - root = store1.getRoot(); - builder = root.builder(); - builder.child("visible"); - builder.child(":hidden").child("b"); - builder.child(":hidden").child("deleteDeleted").remove(); - builder.child(":hidden").child("changeDeleted").remove(); - builder.child(":hidden").child("deleteChanged").setProperty("foo", "bar"); - builder.child(":dynHidden").child("c"); - builder.child(":dynHidden").child("childWithProp").setProperty("foo", "bar"); - merge(store1, builder); + //Prepare repo + root = store1.getRoot(); + builder = root.builder(); + builder.child(":hidden").child("deleteDeleted"); + builder.child(":hidden").child("deleteChanged"); + builder.child(":hidden").child("changeDeleted"); + merge(store1, builder); + store1.runBackgroundOperations(); + store2.runBackgroundOperations(); - //Changes in store2 + //Changes in store1 + root = store1.getRoot(); + builder = root.builder(); + builder.child("visible"); + builder.child(":hidden").child("b"); + builder.child(":hidden").child("deleteDeleted").remove(); + builder.child(":hidden").child("changeDeleted").remove(); + builder.child(":hidden").child("deleteChanged").setProperty("foo", "bar"); + builder.child(":dynHidden").child("c"); + builder.child(":dynHidden").child("childWithProp").setProperty("foo", "bar"); + merge(store1, builder); - //root would hold reference to store2 root state after initial repo initialization - root = store2.getRoot(); + //Changes in store2 - //The hidden node itself should be creatable across cluster concurrently + //root would hold reference to store2 root state after initial repo initialization + root = store2.getRoot(); + + //The hidden node itself should be creatable across cluster concurrently + builder = root.builder(); + builder.child(":dynHidden"); + merge(store2, builder); + + //Children of hidden node should be creatable across cluster concurrently + builder = root.builder(); + builder.child(":hidden").child("b"); + builder.child(":dynHidden").child("c"); + merge(store2, builder); + + //Deleted deleted conflict of internal node should work across cluster concurrently + builder = root.builder(); + builder.child(":hidden").child("deleteDeleted").remove(); + merge(store2, builder); + + //Avoid repeated merge tries ... fail early + store2.setMaxBackOffMillis(0); + + boolean commitFailed = false; + try { builder = root.builder(); - builder.child(":dynHidden"); + builder.child("visible"); merge(store2, builder); + } catch (CommitFailedException cfe) { + commitFailed = true; + } + assertTrue("Concurrent creation of visible node across cluster must fail", commitFailed); - //Children of hidden node should be creatable across cluster concurrently + commitFailed = false; + try { builder = root.builder(); - builder.child(":hidden").child("b"); - builder.child(":dynHidden").child("c"); + builder.child(":dynHidden").child("childWithProp").setProperty("foo", "bar"); merge(store2, builder); + } catch (CommitFailedException cfe) { + commitFailed = true; + } + assertTrue("Concurrent creation of hidden node with properties across cluster must fail", commitFailed); - //Deleted deleted conflict of internal node should work across cluster concurrently + commitFailed = false; + try { builder = root.builder(); - builder.child(":hidden").child("deleteDeleted").remove(); + builder.child(":hidden").child("deleteChanged").remove(); merge(store2, builder); + } catch (CommitFailedException cfe) { + commitFailed = true; + } + assertTrue("Delete changed merge across cluster must fail even under hidden tree", commitFailed); - //Avoid repeated merge tries ... fail early - store2.setMaxBackOffMillis(0); - - boolean commitFailed = false; - try { - builder = root.builder(); - builder.child("visible"); - merge(store2, builder); - } catch (CommitFailedException cfe) { - commitFailed = true; - } - assertTrue("Concurrent creation of visible node across cluster must fail", commitFailed); - - commitFailed = false; - try { - builder = root.builder(); - builder.child(":dynHidden").child("childWithProp").setProperty("foo", "bar"); - merge(store2, builder); - } catch (CommitFailedException cfe) { - commitFailed = true; - } - assertTrue("Concurrent creation of hidden node with properties across cluster must fail", commitFailed); - - commitFailed = false; - try { - builder = root.builder(); - builder.child(":hidden").child("deleteChanged").remove(); - merge(store2, builder); - } catch (CommitFailedException cfe) { - commitFailed = true; - } - assertTrue("Delete changed merge across cluster must fail even under hidden tree", commitFailed); - - commitFailed = false; - try { - builder = root.builder(); - builder.child(":hidden").child("changeDeleted").setProperty("foo", "bar"); - merge(store2, builder); - } catch (CommitFailedException cfe) { - commitFailed = true; - } - assertTrue("Change deleted merge across cluster must fail even under hidden tree", commitFailed); - } finally { - store2.dispose(); - store1.dispose(); + commitFailed = false; + try { + builder = root.builder(); + builder.child(":hidden").child("changeDeleted").setProperty("foo", "bar"); + merge(store2, builder); + } catch (CommitFailedException cfe) { + commitFailed = true; } + assertTrue("Change deleted merge across cluster must fail even under hidden tree", commitFailed); } @Test public void mergeDeleteDeleteEmptyInternalDoc() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - NodeBuilder builder = store.getRoot().builder(); - builder.child(":a"); - builder.child(":b"); - merge(store, builder); - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1"}, new String[]{":a"}, - new String[]{":2"}, new String[]{":b"}, - new String[]{":3"}, new String[]{":a", ":b"}, - true, "Delete-delete merge conflicts for internal docs should be resolved"); - } finally { - store.dispose(); - } + NodeBuilder builder = store.getRoot().builder(); + builder.child(":a"); + builder.child(":b"); + merge(store, builder); + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1"}, new String[]{":a"}, + new String[]{":2"}, new String[]{":b"}, + new String[]{":3"}, new String[]{":a", ":b"}, + true, "Delete-delete merge conflicts for internal docs should be resolved"); } @Test public void mergeDeleteDeleteNonEmptyInternalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - NodeBuilder builder = store.getRoot().builder(); - builder.child(":a").setProperty("foo", "bar"); - builder.child(":b"); - merge(store, builder); - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1"}, new String[]{":a"}, - new String[]{":2"}, new String[]{":b"}, - new String[]{":3"}, new String[]{":a", ":b"}, - false, "Delete-delete merge conflicts for non-empty internal docs should fail"); - } finally { - store.dispose(); - } + NodeBuilder builder = store.getRoot().builder(); + builder.child(":a").setProperty("foo", "bar"); + builder.child(":b"); + merge(store, builder); + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1"}, new String[]{":a"}, + new String[]{":2"}, new String[]{":b"}, + new String[]{":3"}, new String[]{":a", ":b"}, + false, "Delete-delete merge conflicts for non-empty internal docs should fail"); } @Test public void mergeDeleteDeleteNormalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - NodeBuilder builder = store.getRoot().builder(); - builder.child("a"); - builder.child("b"); - merge(store, builder); - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1"}, new String[]{"a"}, - new String[]{":2"}, new String[]{"b"}, - new String[]{":3"}, new String[]{"a", "b"}, - false, "Delete-delete merge conflicts for normal docs should fail"); - } finally { - store.dispose(); - } + NodeBuilder builder = store.getRoot().builder(); + builder.child("a"); + builder.child("b"); + merge(store, builder); + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1"}, new String[]{"a"}, + new String[]{":2"}, new String[]{"b"}, + new String[]{":3"}, new String[]{"a", "b"}, + false, "Delete-delete merge conflicts for normal docs should fail"); } @Test public void mergeAddAddEmptyInternalDoc() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1", ":a"}, new String[]{}, - new String[]{":2", ":b"}, new String[]{}, - new String[]{":3", ":a", ":b"}, new String[]{}, - true, "Add-add merge conflicts for internal docs should be resolvable"); - } finally { - store.dispose(); - } + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1", ":a"}, new String[]{}, + new String[]{":2", ":b"}, new String[]{}, + new String[]{":3", ":a", ":b"}, new String[]{}, + true, "Add-add merge conflicts for internal docs should be resolvable"); } @Test public void mergeAddAddNonEmptyInternalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1", ":a"}, new String[]{}, true, - new String[]{":2", ":b"}, new String[]{}, true, - new String[]{":3", ":a", ":b"}, new String[]{}, false, - false, "Add-add merge conflicts for non empty internal docs should fail"); - } finally { - store.dispose(); - } + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1", ":a"}, new String[]{}, true, + new String[]{":2", ":b"}, new String[]{}, true, + new String[]{":3", ":a", ":b"}, new String[]{}, false, + false, "Add-add merge conflicts for non empty internal docs should fail"); } @Test public void mergeAddAddNormalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1", "a"}, new String[]{}, - new String[]{":2", "b"}, new String[]{}, - new String[]{":3", "a", "b"}, new String[]{}, - false, "Add-add merge conflicts for normal docs should fail"); - } finally { - store.dispose(); - } + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1", "a"}, new String[]{}, + new String[]{":2", "b"}, new String[]{}, + new String[]{":3", "a", "b"}, new String[]{}, + false, "Add-add merge conflicts for normal docs should fail"); } @Test public void mergeDeleteChangedInternalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - NodeBuilder builder = store.getRoot().builder(); - builder.child(":a"); - builder.child(":b"); - merge(store, builder); - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1", ":a"}, new String[]{}, true, - new String[]{":2", ":b"}, new String[]{}, true, - new String[]{":3"}, new String[]{":a", ":b"}, false, - false, "Delete changed merge conflicts for internal docs should fail"); - } finally { - store.dispose(); - } + NodeBuilder builder = store.getRoot().builder(); + builder.child(":a"); + builder.child(":b"); + merge(store, builder); + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1", ":a"}, new String[]{}, true, + new String[]{":2", ":b"}, new String[]{}, true, + new String[]{":3"}, new String[]{":a", ":b"}, false, + false, "Delete changed merge conflicts for internal docs should fail"); } @Test public void mergeChangeDeletedInternalDocShouldFail() throws Exception { - final DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore store = builderProvider.newBuilder().getNodeStore(); store.setEnableConcurrentAddRemove(true); - try { - NodeBuilder builder = store.getRoot().builder(); - builder.child(":a"); - builder.child(":b"); - merge(store, builder); - SingleInstanceConflictUtility.generateConflict(store, - new String[]{":1"}, new String[]{":a"}, false, - new String[]{":2"}, new String[]{":b"}, false, - new String[]{":3", ":a", ":b"}, new String[]{}, true, - false, "Change deleted merge conflicts for internal docs should fail"); - } finally { - store.dispose(); - } + NodeBuilder builder = store.getRoot().builder(); + builder.child(":a"); + builder.child(":b"); + merge(store, builder); + SingleInstanceConflictUtility.generateConflict(store, + new String[]{":1"}, new String[]{":a"}, false, + new String[]{":2"}, new String[]{":b"}, false, + new String[]{":3", ":a", ":b"}, new String[]{}, true, + false, "Change deleted merge conflicts for internal docs should fail"); } @Test @@ -1567,7 +1493,7 @@ public class DocumentNodeStoreTest { final int NUM_NODES = DocumentRootBuilder.UPDATE_LIMIT / 2; final int NUM_PROPS = 10; final int REBASE_COUNT = 5; - final DocumentNodeStore ns = new DocumentMK.Builder().getNodeStore(); + final DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); for (int i = 0; i < NUM_NODES / 2; i++) { @@ -1599,30 +1525,36 @@ public class DocumentNodeStoreTest { System.out.println("Starting the final merge "+ new Date()); merge(ns, builder); - - ns.dispose(); } // OAK-2642 @Test public void dispose() throws CommitFailedException, InterruptedException { final BlockingQueue updates = new ArrayBlockingQueue(1); + // when disposing of the DocumentNodeStore instances the updates queue + // becomes full due to the pending operations being flushed. + // This flag ensures that after the main test is completed all + // updates are processed without being blocked + final AtomicBoolean throttleUpdates = new AtomicBoolean(true); MemoryDocumentStore docStore = new MemoryDocumentStore() { @Override public void update(Collection collection, List keys, UpdateOp updateOp) { - for (String k : keys) { - try { - updates.put(k); - } catch (InterruptedException e) { - throw new RuntimeException(e); + + if ( throttleUpdates.get() ) { + for (String k : keys) { + try { + updates.put(k); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } } } super.update(collection, keys, updateOp); } }; - final DocumentNodeStore store = new DocumentMK.Builder() + final DocumentNodeStore store = builderProvider.newBuilder() .setClusterId(1).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); updates.clear(); @@ -1670,7 +1602,7 @@ public class DocumentNodeStoreTest { } // start new store with clusterId 2 - DocumentNodeStore store2 = new DocumentMK.Builder() + DocumentNodeStore store2 = builderProvider.newBuilder() .setClusterId(2).setAsyncDelay(0) .setDocumentStore(docStore).getNodeStore(); @@ -1689,12 +1621,13 @@ public class DocumentNodeStoreTest { node.child("child-2"); merge(store2, builder); } + throttleUpdates.set(false); } // OAK-2695 @Test public void dispatch() throws Exception { - DocumentNodeStore ns = new DocumentMK.Builder().getNodeStore(); + DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore(); Revision from = ns.getHeadRevision(); NodeBuilder builder = ns.getRoot().builder(); @@ -1713,13 +1646,11 @@ public class DocumentNodeStoreTest { return true; } }); - - ns.dispose(); } @Test public void rootRevision() throws Exception { - DocumentNodeStore ns = new DocumentMK.Builder().getNodeStore(); + DocumentNodeStore ns = builderProvider.newBuilder().getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); builder.child("foo").child("child"); @@ -1735,8 +1666,6 @@ public class DocumentNodeStoreTest { assertTrue(child instanceof DocumentNodeState); DocumentNodeState state = (DocumentNodeState) child; assertEquals(head, state.getRootRevision()); - - ns.dispose(); } @Test @@ -1753,7 +1682,7 @@ public class DocumentNodeStoreTest { return super.query(collection, fromKey, toKey, limit); } }; - DocumentNodeStore ns = new DocumentMK.Builder() + DocumentNodeStore ns = builderProvider.newBuilder() .setDocumentStore(store).setAsyncDelay(0).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); @@ -1789,9 +1718,6 @@ public class DocumentNodeStoreTest { assertEquals(1, added.size()); assertEquals("node", added.get(0)); assertEquals("must not run queries", 0, numQueries.get()); - - ns.dispose(); - } // OAK-1970 @@ -1816,7 +1742,7 @@ public class DocumentNodeStoreTest { return super.query(collection, fromKey, toKey, indexedProperty, startValue, limit); } }; - DocumentNodeStore ns = new DocumentMK.Builder().clock(clock) + DocumentNodeStore ns = builderProvider.newBuilder().clock(clock) .setDocumentStore(ds).setAsyncDelay(0).getNodeStore(); NodeBuilder builder = ns.getRoot().builder(); @@ -1855,8 +1781,6 @@ public class DocumentNodeStoreTest { // startValue must be based on the revision of the before state // and not when '/test' was last modified assertEquals(beforeModified, (long) startValues.get(0)); - - ns.dispose(); } // OAK-2620 @@ -1882,7 +1806,7 @@ public class DocumentNodeStoreTest { return super.findAndUpdate(collection, update); } }; - DocumentNodeStore ds = new DocumentMK.Builder() + DocumentNodeStore ds = builderProvider.newBuilder() .setDocumentStore(store) .setAsyncDelay(0).getNodeStore(); ds.setMaxBackOffMillis(0); // do not retry merges @@ -1921,8 +1845,6 @@ public class DocumentNodeStoreTest { // expected } - ds.dispose(); - for (String s : failure) { fail(s); } @@ -1944,7 +1866,7 @@ public class DocumentNodeStoreTest { return super.findAndUpdate(collection, update); } }; - DocumentNodeStore ds = new DocumentMK.Builder() + DocumentNodeStore ds = builderProvider.newBuilder() .setDocumentStore(store) .setAsyncDelay(0).getNodeStore(); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java index 69737de..0cdf113 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgentTest.java @@ -107,7 +107,8 @@ public class LastRevRecoveryAgentTest { @After public void tearDown(){ - sharedStore.dispose(); + ds1.dispose(); + // ds2.dispose(); - fails for Mongo and RDB as the first dispose call also disposes the sharedStore ClusterNodeInfo.resetClockToDefault(); Revision.resetClockToDefault(); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java index 5ceefdc..4183e4b 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryTest.java @@ -34,6 +34,7 @@ import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.apache.jackrabbit.oak.stats.Clock; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import static com.google.common.collect.Lists.newArrayList; @@ -43,6 +44,9 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class LastRevRecoveryTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private Clock clock; private DocumentNodeStore ds1; private DocumentNodeStore ds2; @@ -58,7 +62,7 @@ public class LastRevRecoveryTest { // disable lease check because we fiddle with the virtual clock final boolean leaseCheck = false; sharedStore = new MemoryDocumentStore(); - ds1 = new DocumentMK.Builder() + ds1 = builderProvider.newBuilder() .clock(clock) .setLeaseCheck(leaseCheck) .setAsyncDelay(0) @@ -66,7 +70,7 @@ public class LastRevRecoveryTest { .getNodeStore(); c1Id = ds1.getClusterId(); - ds2 = new DocumentMK.Builder() + ds2 = builderProvider.newBuilder() .clock(clock) .setLeaseCheck(leaseCheck) .setAsyncDelay(0) diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevSingleNodeRecoveryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevSingleNodeRecoveryTest.java index 595fe59..84d5723 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevSingleNodeRecoveryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/LastRevSingleNodeRecoveryTest.java @@ -46,6 +46,8 @@ public class LastRevSingleNodeRecoveryTest { private DocumentMK mk; + private DocumentMK mk2; + public LastRevSingleNodeRecoveryTest(DocumentStoreFixture fixture) { this.fixture = fixture; } @@ -112,13 +114,14 @@ public class LastRevSingleNodeRecoveryTest { // so that the current time is more than the current lease end clock.waitUntil(clock.getTime() + mk.getClusterInfo().getLeaseTime() + 1000); // Recreate mk instance, to simulate fail condition and recovery on start - mk = openMK(0, mk.getNodeStore().getDocumentStore()); + // Make sure to use a different variable for cleanup ; mk should not be disposed here + mk2 = openMK(0, mk.getNodeStore().getDocumentStore()); - int pendingCount = mk.getPendingWriteCount(); + int pendingCount = mk2.getPendingWriteCount(); // Immediately check again, now should not have done any changes. - LastRevRecoveryAgent recoveryAgent = mk.getNodeStore().getLastRevRecoveryAgent(); + LastRevRecoveryAgent recoveryAgent = mk2.getNodeStore().getLastRevRecoveryAgent(); /** Now there should have been pendingCount updates **/ - assertEquals(pendingCount, recoveryAgent.recover(mk.getClusterInfo().getId())); + assertEquals(pendingCount, recoveryAgent.recover(mk2.getClusterInfo().getId())); } @Test @@ -223,6 +226,9 @@ public class LastRevSingleNodeRecoveryTest { Revision.resetClockToDefault(); ClusterNodeInfo.resetClockToDefault(); mk.dispose(); + if ( mk2 != null ) { + mk2.dispose(); + } fixture.dispose(); } } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java index 45295b7..e560cf7 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeStoreDiffTest.java @@ -39,19 +39,23 @@ import org.apache.jackrabbit.oak.spi.commit.EditorHook; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import static org.junit.Assert.assertFalse; public class NodeStoreDiffTest { - private NodeStore ns; + + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + + private DocumentNodeStore ns; private final TestDocumentStore tds = new TestDocumentStore(); @Before public void setUp() throws IOException { - ns = new DocumentMK.Builder() + ns = builderProvider.newBuilder() .setDocumentStore(tds) .setUseSimpleRevision(true) //To simplify debugging .setAsyncDelay(0) diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SharedBlobStoreGCTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SharedBlobStoreGCTest.java index 7939c27..8b3c044 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SharedBlobStoreGCTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SharedBlobStoreGCTest.java @@ -173,6 +173,8 @@ public class SharedBlobStoreGCTest { DataStoreUtils.cleanup(cluster1.getDataStore(), cluster1.getDate()); FileUtils.cleanDirectory((new File(DataStoreUtils.getHomeDir())).getParentFile()); DataStoreUtils.time = -1; + cluster1.getDocumentNodeStore().dispose(); + cluster2.getDocumentNodeStore().dispose(); } class Cluster { @@ -273,6 +275,10 @@ public class SharedBlobStoreGCTest { public Date getDate() { return startDate; } + + public DocumentNodeStore getDocumentNodeStore() { + return ds; + } } } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java index bbfa22e..9d67981 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/SimpleTest.java @@ -30,6 +30,7 @@ import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.commons.json.JsopBuilder; import org.apache.jackrabbit.oak.plugins.document.DocumentNodeState.Children; import org.apache.jackrabbit.oak.plugins.document.util.Utils; +import org.junit.Rule; import org.junit.Test; import com.google.common.collect.Lists; @@ -40,16 +41,13 @@ import com.mongodb.DB; */ public class SimpleTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + private static final boolean MONGO_DB = false; // private static final boolean MONGO_DB = true; @Test - public void test() { - DocumentMK mk = new DocumentMK.Builder().open(); - mk.dispose(); - } - - @Test public void pathToId() { assertEquals("0:/", Utils.getIdFromPath("/")); assertEquals("/", Utils.getPathFromId("0:/")); @@ -71,7 +69,7 @@ public class SimpleTest { @Test public void addNodeGetNode() { - DocumentMK mk = new DocumentMK.Builder().open(); + DocumentMK mk = builderProvider.newBuilder().open(); DocumentStore s = mk.getDocumentStore(); DocumentNodeStore ns = mk.getNodeStore(); Revision rev = Revision.fromString(mk.getHeadRevision()); @@ -86,7 +84,6 @@ public class SimpleTest { PropertyState p = n2.getProperty("name"); assertNotNull(p); assertEquals("Hello", p.getValue(Type.STRING)); - mk.dispose(); } @Test @@ -119,8 +116,6 @@ public class SimpleTest { assertEquals("{\":id\":\"/test/a@r4-0-1\",\"x\":1,\":childNodeCount\":0}", r4); r4 = mk.getNodes("/test/b", rev4, 0, 0, Integer.MAX_VALUE, ":id"); assertEquals("{\":id\":\"/test/b@r3-0-1\",\":childNodeCount\":0}", r4); - - mk.dispose(); } @Test @@ -136,7 +131,6 @@ public class SimpleTest { // the previous commit should be rolled back now, // so this should work mk.commit("/", "+\"b\": {}", null, null); - mk.dispose(); } @Test @@ -168,7 +162,6 @@ public class SimpleTest { assertEquals("+\"/t2\":{}\n+\"/t3\":{}", diff13); String diff34 = mk.diff(rev3, rev4, "/", 0).trim(); assertEquals("^\"/t3\":{}", diff34); - mk.dispose(); } @Test @@ -199,7 +192,6 @@ public class SimpleTest { assertEquals("{\":childNodeCount\":0}", test); String test2 = mk.getNodes("/test2", rev, 0, 0, Integer.MAX_VALUE, null); assertEquals("{\":childNodeCount\":0}", test2); - mk.dispose(); } @Test @@ -211,7 +203,6 @@ public class SimpleTest { assertEquals("{\"x\":\"1\",\"child\":{},\":childNodeCount\":1}", test); test = mk.getNodes("/test", rev, 0, 0, Integer.MAX_VALUE, null); assertNull(test); - mk.dispose(); } @Test @@ -223,7 +214,6 @@ public class SimpleTest { assertEquals("{\"x\":\"1\",\"child\":{},\":childNodeCount\":1}", test); test = mk.getNodes("/test", rev, 0, 0, Integer.MAX_VALUE, null); assertEquals("{\"x\":\"1\",\"child\":{},\":childNodeCount\":1}", test); - mk.dispose(); } @Test @@ -266,7 +256,6 @@ public class SimpleTest { assertEquals("{\"test\":1,\"test\":{},\":childNodeCount\":1}", test); // System.out.println(test); - mk.dispose(); } @Test @@ -394,59 +383,55 @@ public class SimpleTest { @Test public void commitRoot() { DocumentMK mk = createMK(); - try { - DocumentStore store = mk.getDocumentStore(); - Revision head = Revision.fromString(mk.getHeadRevision()); - head = Revision.fromString(mk.commit("", "+\"/test\":{\"foo\":{}}", head.toString(), null)); - - // root node must not have the revision - NodeDocument rootDoc = store.find(Collection.NODES, "0:/"); - - //As we update the childStatus flag the commit root would shift - //one layer above - // assertNotNull(rootDoc); - // assertFalse(rootDoc.containsRevision(head)); - - // test node must have head in revisions - NodeDocument node = store.find(Collection.NODES, "1:/test"); - //assertNotNull(node); - //assertTrue(node.containsRevision(head)); - - // foo must not have head in revisions and must refer to test - // as commit root (depth = 1) - NodeDocument foo = store.find(Collection.NODES, "2:/test/foo"); - assertNotNull(foo); - assertFalse(foo.containsRevision(head)); - assertEquals("/", foo.getCommitRootPath(head)); - - head = Revision.fromString(mk.commit("", "+\"/bar\":{}+\"/test/foo/bar\":{}", head.toString(), null)); - - // root node is root of commit - rootDoc = store.find(Collection.NODES, "0:/"); - assertNotNull(rootDoc); - assertTrue(rootDoc.containsRevision(head)); - - // /bar refers to root nodes a commit root - NodeDocument bar = store.find(Collection.NODES, "1:/bar"); - assertNotNull(bar); - assertEquals("/", bar.getCommitRootPath(head)); - - // /test/foo/bar refers to root nodes a commit root - bar = store.find(Collection.NODES, "3:/test/foo/bar"); - assertNotNull(bar); - assertEquals("/", bar.getCommitRootPath(head)); - - } finally { - mk.dispose(); - } + DocumentStore store = mk.getDocumentStore(); + Revision head = Revision.fromString(mk.getHeadRevision()); + head = Revision.fromString(mk.commit("", "+\"/test\":{\"foo\":{}}", head.toString(), null)); + + // root node must not have the revision + NodeDocument rootDoc = store.find(Collection.NODES, "0:/"); + + //As we update the childStatus flag the commit root would shift + //one layer above + // assertNotNull(rootDoc); + // assertFalse(rootDoc.containsRevision(head)); + + // test node must have head in revisions + NodeDocument node = store.find(Collection.NODES, "1:/test"); + //assertNotNull(node); + //assertTrue(node.containsRevision(head)); + + // foo must not have head in revisions and must refer to test + // as commit root (depth = 1) + NodeDocument foo = store.find(Collection.NODES, "2:/test/foo"); + assertNotNull(foo); + assertFalse(foo.containsRevision(head)); + assertEquals("/", foo.getCommitRootPath(head)); + + head = Revision.fromString(mk.commit("", "+\"/bar\":{}+\"/test/foo/bar\":{}", head.toString(), null)); + + // root node is root of commit + rootDoc = store.find(Collection.NODES, "0:/"); + assertNotNull(rootDoc); + assertTrue(rootDoc.containsRevision(head)); + + // /bar refers to root nodes a commit root + NodeDocument bar = store.find(Collection.NODES, "1:/bar"); + assertNotNull(bar); + assertEquals("/", bar.getCommitRootPath(head)); + + // /test/foo/bar refers to root nodes a commit root + bar = store.find(Collection.NODES, "3:/test/foo/bar"); + assertNotNull(bar); + assertEquals("/", bar.getCommitRootPath(head)); + } - private static DocumentMK createMK() { + private DocumentMK createMK() { return createMK(false); } - private static DocumentMK createMK(boolean useSimpleRevision) { - DocumentMK.Builder builder = new DocumentMK.Builder(); + private DocumentMK createMK(boolean useSimpleRevision) { + DocumentMK.Builder builder = builderProvider.newBuilder(); if (MONGO_DB) { DB db = MongoUtils.getConnection().getDB(); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java index aad3cc9..485259f 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/UnmergedBranchTest.java @@ -25,6 +25,7 @@ import java.util.TreeMap; import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore; import org.apache.jackrabbit.oak.plugins.document.util.Utils; +import org.junit.Rule; import org.junit.Test; import static org.junit.Assert.assertFalse; @@ -32,6 +33,9 @@ import static org.junit.Assert.assertTrue; public class UnmergedBranchTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); + @Test public void purgeUnmergedBranch() throws Exception { DocumentStore testStore = new MemoryDocumentStore(); @@ -89,8 +93,8 @@ public class UnmergedBranchTest { return mk.getNodeStore().getDocumentStore().find(Collection.NODES, Utils.getIdFromPath("/")); } - private static DocumentMK create(DocumentStore ds, int clusterId){ - return new DocumentMK.Builder().setAsyncDelay(0) + private DocumentMK create(DocumentStore ds, int clusterId){ + return builderProvider.newBuilder().setAsyncDelay(0) .setDocumentStore(ds).setClusterId(clusterId).open(); } } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java index d0815ed..f5ed9dc 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ValueMapTest.java @@ -27,6 +27,7 @@ import org.apache.jackrabbit.oak.plugins.document.util.Utils; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.EmptyHook; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.junit.Rule; import org.junit.Test; import com.google.common.collect.Iterators; @@ -42,6 +43,8 @@ import static org.junit.Assert.assertTrue; * Test cases for OAK-1233. */ public class ValueMapTest { + @Rule + public DocumentMkBuilderProvider builderProvider = new DocumentMkBuilderProvider(); @Test public void previousDocs1() { @@ -146,7 +149,7 @@ public class ValueMapTest { // OAK-2433 @Test public void mergeSorted() throws Exception { - DocumentNodeStore store = new DocumentMK.Builder().setAsyncDelay(0).getNodeStore(); + DocumentNodeStore store = builderProvider.newBuilder().setAsyncDelay(0).getNodeStore(); DocumentStore docStore = store.getDocumentStore(); String id = Utils.getIdFromPath("/"); -- 2.5.0