From 60131204d1c71c9bda6bd5ba4b55f2d12fa4444e Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Sat, 28 Jul 2018 15:50:33 +0100 Subject: [PATCH 1/5] Deprecate low level non-application operations --- .../org/apache/jena/tdb/StoreConnection.java | 10 +- .../org/apache/jena/tdb/sys/TDBMaker.java | 40 +++++-- .../org/apache/jena/tdb/TestTDBFactory.java | 109 +++++++++--------- .../org/apache/jena/tdb/store/TestLoader.java | 4 +- .../tdb/transaction/TestTransRestart.java | 5 +- 5 files changed, 95 insertions(+), 73 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java b/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java index 978ee785e1f..4a6125d9885 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java @@ -228,10 +228,18 @@ public static synchronized StoreConnection make(Location location, StoreParams p StoreConnection sConn = cache.get(location) ; if (sConn != null) return sConn ; - DatasetGraphTDB dsg = DatasetBuilderStd.create(location, params) ; + DatasetGraphTDB dsg = build(location, params) ; sConn = _makeAndCache(dsg) ; return sConn ; } + + /** + * Build storage {@link DatasetGraphTDB}. + * This operation is the primitive that creates the storage-level DatasetGraphTDB. + */ + private static DatasetGraphTDB build(Location location, StoreParams params) { + return DatasetBuilderStd.create(location, params) ; + } /** Make a StoreConnection based on any StoreParams at the location or the system defaults. */ public static StoreConnection make(Location location) { diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/sys/TDBMaker.java b/jena-tdb/src/main/java/org/apache/jena/tdb/sys/TDBMaker.java index 2d4acde19a0..67fa12d929a 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/sys/TDBMaker.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/sys/TDBMaker.java @@ -18,9 +18,11 @@ package org.apache.jena.tdb.sys; +import org.apache.jena.sparql.core.DatasetGraph; import org.apache.jena.tdb.StoreConnection ; +import org.apache.jena.tdb.TDBFactory; import org.apache.jena.tdb.base.file.Location ; -import org.apache.jena.tdb.setup.DatasetBuilderStd ; +import org.apache.jena.tdb.setup.DatasetBuilderStd; import org.apache.jena.tdb.setup.StoreParams ; import org.apache.jena.tdb.store.DatasetGraphTDB ; import org.apache.jena.tdb.transaction.DatasetGraphTransaction ; @@ -29,7 +31,7 @@ public class TDBMaker { - // ---- Transactional + // ---- Transactional dataset maker. // This hides details from the top level TDB Factory (e.g. for testing) /** Create a DatasetGraph that supports transactions */ @@ -50,24 +52,42 @@ public static DatasetGraphTransaction createDatasetGraphTransaction() { return createDatasetGraphTransaction(Location.mem()) ; } - private static DatasetGraphTransaction _create(Location location) { - // No need to cache StoreConnection does all that. - return new DatasetGraphTransaction(location) ; - } - + /** + * Release a {@code Location}. + * Do not use a {@code Dataset} at this location without + * remaking it via {@link TDBFactory}. + */ public static void releaseLocation(Location location) { StoreConnection.release(location) ; } + /** + * Reset the making and caching of datasets. + * Applications should not use this operation. + * All dataset must be rebuild with {@link TDBFactory} + * after calling this operation. + * Use {@link TDBFactory#release(DatasetGraph)} with care. + * @deprecated + */ + @Deprecated public static void reset() { StoreConnection.reset() ; } + // Make a DatasetGraphTransaction + private static DatasetGraphTransaction _create(Location location) { + // No need to cache StoreConnection does all that. + return new DatasetGraphTransaction(location) ; + } + // ---- Raw non-transactional - /** Create a non-transactional TDB dataset graph. - * Bypasses StoreConnection caching. - * Use at your own risk. + /** + * Create a non-transactional TDB dataset graph. + * Use at your own risk. + * This function does not attempt to share databases at the same location. + * @deprecated Use {@link TDBFactory} or {@link StoreConnection}. */ + @Deprecated public static DatasetGraphTDB createDatasetGraphTDB(Location loc, StoreParams params) { return DatasetBuilderStd.create(loc, params) ; } } diff --git a/jena-tdb/src/test/java/org/apache/jena/tdb/TestTDBFactory.java b/jena-tdb/src/test/java/org/apache/jena/tdb/TestTDBFactory.java index 35853b7a4c3..22c1a5e7c3f 100644 --- a/jena-tdb/src/test/java/org/apache/jena/tdb/TestTDBFactory.java +++ b/jena-tdb/src/test/java/org/apache/jena/tdb/TestTDBFactory.java @@ -38,74 +38,69 @@ public class TestTDBFactory extends BaseTest static Quad quad1 = SSE.parseQuad("(_

1)") ; static Quad quad2 = SSE.parseQuad("(_

1)") ; - @Before public void before() - { - StoreConnection.reset() ; - FileOps.clearDirectory(DIR) ; + @Before + public void before() { + StoreConnection.reset(); + FileOps.clearDirectory(DIR); } - - @After public void after() - { - FileOps.clearDirectory(DIR) ; + + @After + public void after() { + FileOps.clearDirectory(DIR); } - - @Test public void testTDBFactory1() - { - TDBMaker.reset() ; - DatasetGraph dg1 = TDBFactory.createDatasetGraph(Location.mem("FOO")) ; - DatasetGraph dg2 = TDBFactory.createDatasetGraph(Location.mem("FOO")) ; - dg1.add(quad1) ; - assertTrue(dg2.contains(quad1)) ; + + @Test + public void testTDBFactory1() { + DatasetGraph dg1 = TDBFactory.createDatasetGraph(Location.mem("FOO")); + DatasetGraph dg2 = TDBFactory.createDatasetGraph(Location.mem("FOO")); + dg1.add(quad1); + assertTrue(dg2.contains(quad1)); } - - @Test public void testTDBFactory2() - { - TDBMaker.reset() ; + + @Test + public void testTDBFactory2() { // The unnamed location is unique each time. - DatasetGraph dg1 = TDBFactory.createDatasetGraph(Location.mem()) ; - DatasetGraph dg2 = TDBFactory.createDatasetGraph(Location.mem()) ; - dg1.add(quad1) ; - assertFalse(dg2.contains(quad1)) ; + DatasetGraph dg1 = TDBFactory.createDatasetGraph(Location.mem()); + DatasetGraph dg2 = TDBFactory.createDatasetGraph(Location.mem()); + dg1.add(quad1); + assertFalse(dg2.contains(quad1)); } - @Test public void testTDBMakerTxn1() - { - TDBMaker.reset() ; - DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.create(DIR)) ; - DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.create(DIR)) ; - - DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph() ; - DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph() ; - - assertSame(dgBase1, dgBase2) ; + @Test + public void testTDBMakerTxn1() { + DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.create(DIR)); + DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.create(DIR)); + + DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph(); + DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph(); + + assertSame(dgBase1, dgBase2); } - - @Test public void testTDBMakerTxn2() - { + + @Test + public void testTDBMakerTxn2() { // Named memory locations - TDBMaker.reset() ; - DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.mem("FOO")) ; - DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.mem("FOO")) ; - - DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph() ; - DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph() ; - - assertSame(dgBase1, dgBase2) ; + DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.mem("FOO")); + DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.mem("FOO")); + + DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph(); + DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph(); + + assertSame(dgBase1, dgBase2); } - - @Test public void testTDBMakerTxn3() - { + + @Test + public void testTDBMakerTxn3() { // Un-named memory locations - TDBMaker.reset() ; - DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.mem()) ; - DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.mem()) ; - - DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph() ; - DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph() ; - - assertNotSame(dgBase1, dgBase2) ; + DatasetGraph dg1 = TDBMaker.createDatasetGraphTransaction(Location.mem()); + DatasetGraph dg2 = TDBMaker.createDatasetGraphTransaction(Location.mem()); + + DatasetGraph dgBase1 = ((DatasetGraphTransaction)dg1).getBaseDatasetGraph(); + DatasetGraph dgBase2 = ((DatasetGraphTransaction)dg2).getBaseDatasetGraph(); + + assertNotSame(dgBase1, dgBase2); } - + @Test public void testTDBFresh01() { boolean b = TDBFactory.inUseLocation(DIR) ; assertFalse("Expect false before any creation attempted", b) ; diff --git a/jena-tdb/src/test/java/org/apache/jena/tdb/store/TestLoader.java b/jena-tdb/src/test/java/org/apache/jena/tdb/store/TestLoader.java index 0ebf6ef1c73..71a11ff4a08 100644 --- a/jena-tdb/src/test/java/org/apache/jena/tdb/store/TestLoader.java +++ b/jena-tdb/src/test/java/org/apache/jena/tdb/store/TestLoader.java @@ -34,7 +34,7 @@ import org.apache.jena.tdb.TDB ; import org.apache.jena.tdb.TDBLoader ; import org.apache.jena.tdb.base.file.Location ; -import org.apache.jena.tdb.sys.TDBMaker ; +import org.apache.jena.tdb.setup.DatasetBuilderStd; import org.junit.AfterClass ; import org.junit.BeforeClass ; import org.junit.Test ; @@ -60,7 +60,7 @@ static public void afterClass() { } static DatasetGraphTDB fresh() { - return TDBMaker.createDatasetGraphTDB(Location.mem(), null) ; + return DatasetBuilderStd.create(Location.mem()) ; } @Test diff --git a/jena-tdb/src/test/java/org/apache/jena/tdb/transaction/TestTransRestart.java b/jena-tdb/src/test/java/org/apache/jena/tdb/transaction/TestTransRestart.java index e05cbd3a0fa..ce1559a2714 100644 --- a/jena-tdb/src/test/java/org/apache/jena/tdb/transaction/TestTransRestart.java +++ b/jena-tdb/src/test/java/org/apache/jena/tdb/transaction/TestTransRestart.java @@ -33,10 +33,10 @@ import org.apache.jena.tdb.base.file.FileFactory ; import org.apache.jena.tdb.base.file.Location ; import org.apache.jena.tdb.base.objectfile.ObjectFile ; +import org.apache.jena.tdb.setup.DatasetBuilderStd; import org.apache.jena.tdb.store.DatasetGraphTDB ; import org.apache.jena.tdb.sys.Names ; import org.apache.jena.tdb.sys.SystemTDB ; -import org.apache.jena.tdb.sys.TDBMaker ; import org.junit.After ; import org.junit.Before ; import org.junit.Test ; @@ -70,14 +70,13 @@ public class TestTransRestart extends BaseTest { cleanup() ; } - private static DatasetGraphTDB createPlain(Location location) { return TDBMaker.createDatasetGraphTDB(location, null) ; } + private static DatasetGraphTDB createPlain(Location location) { return DatasetBuilderStd.create(location) ; } private void setupPlain() { // Make without transactions. DatasetGraphTDB dsg = createPlain(location) ; dsg.add(quad1) ; dsg.close() ; - StoreConnection.release(location) ; return ; } From f8cbb72bad4b900cc2765941cda69038f45f523d Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Sat, 28 Jul 2018 16:45:08 +0100 Subject: [PATCH 2/5] Correct javadoc on StoreConnection.getBaseDataset(). --- .../src/main/java/org/apache/jena/tdb/StoreConnection.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java b/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java index 4a6125d9885..3a7cdecae88 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/StoreConnection.java @@ -129,10 +129,9 @@ public DatasetGraphTxn begin(TxnType mode, String label) { } /** - * Testing operation - do not use the base dataset without knowing how the - * transaction system uses it. The base dataset may not reflect the true state - * if pending commits are queued. - * @see #flush + * Internal operation - to get a dataset for application use, call a + * {@link TDBFactory} function. Do not use the base dataset without knowing how the + * transaction system uses it. */ public DatasetGraphTDB getBaseDataset() { checkValid(); From 95b0393b5121a243d69382a9a854916419dd252e Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Sat, 28 Jul 2018 17:29:59 +0100 Subject: [PATCH 3/5] Fixup javadoc --- .../main/java/org/apache/jena/tdb/setup/Build.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/setup/Build.java b/jena-tdb/src/main/java/org/apache/jena/tdb/setup/Build.java index b9b852345bd..a6ec2c865c5 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/setup/Build.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/setup/Build.java @@ -46,9 +46,6 @@ public static TupleIndex openTupleIndex(Location location, String indexName, Str public static TupleIndex openTupleIndex(Location location, String indexName, String primary, String indexOrder, int readCacheSize, int writeCacheSize, int dftKeyLength, int dftValueLength) { - // XXX replace with: - // return DatasetBuilderStd.stdBuilder().makeTupleIndex(location, indexName, primary, indexOrder) ; - // All this to BuilderDB. StoreParamsBuilder spb = StoreParams.builder() ; spb.blockReadCacheSize(readCacheSize) ; spb.blockWriteCacheSize(writeCacheSize) ; @@ -68,7 +65,6 @@ public static NodeTable makeNodeTable(Location location, StoreParams params) { return dbBuild.makeNodeTable(location, params) ; } - //XXX Reorg all calls to NodeTableBuilder to this argument order. public static NodeTable makeNodeTable(Location location, String indexNode2Id, int node2NodeIdCacheSize, String indexId2Node, int nodeId2NodeCacheSize, @@ -80,8 +76,7 @@ public static NodeTable makeNodeTable(Location location, return makeNodeTable(location, spb.build()) ; } - /** Choose the StoreParams. This is the policy applied when creating or reattaching to a database. - * (extracted and put here to keep the size of DatasetBuildStd + /** Choose the StoreParams. This is the policy applied when creating or re-attaching to a database. *

* If the location has parameters in a tdb.cfg file, use them, as modified by any * application-supplied internal parameters. @@ -107,13 +102,13 @@ public static NodeTable makeNodeTable(Location location, * providing some parameters in the {@link TDBFactory} call. *

* This includes changing filenames, indexing choices and block size. - * Otherwise, the database may be permanetly and irrecovably corrupted. + * Otherwise, the database may be permanently and irrevocably corrupted. * You have been warned. * * @param location The place where the database is or will be. * @param isNew Whether the database is being created or whether there is an existing database. * @param pApp Application-provide store parameters. - * @param pLoc Store parameters foud at the location. + * @param pLoc Store parameters found at the location. * @param pDft System default store parameters. * @return StoreParams * From e338300c958ffaadc84b30abb3a262d5b42635e6 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Sat, 28 Jul 2018 17:30:19 +0100 Subject: [PATCH 4/5] Add StoreConnection constructor to non-API class --- .../jena/tdb/transaction/DatasetGraphTransaction.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java b/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java index c02379eeb5d..859ca1196d2 100644 --- a/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java +++ b/jena-tdb/src/main/java/org/apache/jena/tdb/transaction/DatasetGraphTransaction.java @@ -69,7 +69,11 @@ public class DatasetGraphTransaction extends DatasetGraphTrackActive implements private boolean isClosed = false; public DatasetGraphTransaction(Location location) { - sConn = StoreConnection.make(location) ; + this(StoreConnection.make(location)) ; + } + + public DatasetGraphTransaction(StoreConnection sConn) { + this.sConn = sConn; } public Location getLocation() { @@ -135,7 +139,7 @@ public DatasetGraphTDB get() { if ( sConn.haveUsedInTransaction() ) throw new TDBTransactionException("Not in a transaction") ; - // Never used in a transaction - return underlying database for old + // Never been used in a transaction - return underlying database for old // style (non-transactional) usage. return sConn.getBaseDataset() ; } From 6f2516763f3380c4e4f9d86833f9d9610d7594a6 Mon Sep 17 00:00:00 2001 From: Andy Seaborne Date: Sat, 28 Jul 2018 22:29:18 +0100 Subject: [PATCH 5/5] Make test more tolerant of slow VMs. --- .../sparql/transaction/AbstractTestTransactionLifecycle.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jena-arq/src/test/java/org/apache/jena/sparql/transaction/AbstractTestTransactionLifecycle.java b/jena-arq/src/test/java/org/apache/jena/sparql/transaction/AbstractTestTransactionLifecycle.java index f593b7cf359..399d26fa139 100644 --- a/jena-arq/src/test/java/org/apache/jena/sparql/transaction/AbstractTestTransactionLifecycle.java +++ b/jena-arq/src/test/java/org/apache/jena/sparql/transaction/AbstractTestTransactionLifecycle.java @@ -586,7 +586,7 @@ public Boolean call() { Future f1 = executor.submit(callable); Future f2 = executor.submit(callable); // Wait longer than the cumulative threads sleep - assertTrue(f1.get(4, TimeUnit.SECONDS)); + assertTrue(f1.get(10, TimeUnit.SECONDS)); assertTrue(f2.get(1, TimeUnit.SECONDS)); } finally { executor.shutdownNow();