From 7a8182a2a3dc026767e44fca68aeb64c6b0b64d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20S=C3=B8rensen?= Date: Mon, 26 Dec 2016 21:28:40 +0100 Subject: [PATCH] METAMODEL-1133: Made PojoDataContext thread-safe with CopyOnWrite list --- CHANGES.md | 10 ++- .../org/apache/metamodel/util/FileHelper.java | 1 - pojo/pom.xml | 45 +++++----- .../pojo/ArrayTableDataProvider.java | 9 +- .../metamodel/pojo/MapTableDataProvider.java | 10 ++- .../pojo/ObjectTableDataProvider.java | 10 ++- .../metamodel/pojo/PojoDataContext.java | 4 +- .../apache/metamodel/pojo/PojoDataSet.java | 5 +- .../metamodel/pojo/TableDataProvider.java | 2 + .../metamodel/pojo/PojoDataContextTest.java | 83 +++++++++++++++++-- 10 files changed, 131 insertions(+), 48 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 427ac2148..66db4ddd2 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,11 +1,15 @@ -### Apache MetaModel 4.5.5 +### Apache MetaModel 4.5.6 + * [METAMODEL-1133] - Made PojoDataContext thread-safe. * [METAMODEL-1132] - Support native paging on SQL Server and Oracle database. + +### Apache MetaModel 4.5.5 + * [METAMODEL-1128] - Fixed bug pertaining to ElasticSearch REST data set scrolling. * [METAMODEL-1118] - Fixed bug pertaining to cloning of FilterItem.LogicalOperator in compiled queries. * [METAMODEL-1111] - Added WHERE rewrite for Oracle when empty strings are considered as NULL. - * [METAMODEL-1122] - Optimized the way the Cassandra module executes primary key lookup queries. - * [METAMODEL-1109] - Fixed diacritics/encoding issue with Fixed Width reader. + * [METAMODEL-1122] - Optimized the way the Cassandra module executes primary key lookup queries. + * [METAMODEL-1109] - Fixed diacritics/encoding issue with Fixed Width reader. * [METAMODEL-1115] - Added support for passing your own PartnerConnection object to the Salesforce.com connector. * [METAMODEL-1113] - Fixed support for ColumnNamingStrategy in CSV connector. * [METAMODEL-1114] - Added support for ColumnNamingStrategy in EBCDIC connector. diff --git a/core/src/main/java/org/apache/metamodel/util/FileHelper.java b/core/src/main/java/org/apache/metamodel/util/FileHelper.java index 7465fd00d..c955ec68a 100644 --- a/core/src/main/java/org/apache/metamodel/util/FileHelper.java +++ b/core/src/main/java/org/apache/metamodel/util/FileHelper.java @@ -147,7 +147,6 @@ public static Reader getReader(InputStream inputStream, String encoding) throws int unread; // auto-detect byte-order-mark - @SuppressWarnings("resource") final PushbackInputStream pushbackInputStream = new PushbackInputStream(inputStream, bom.length); final int n = pushbackInputStream.read(bom, 0, bom.length); diff --git a/pojo/pom.xml b/pojo/pom.xml index cdedeb18b..67f5b5079 100644 --- a/pojo/pom.xml +++ b/pojo/pom.xml @@ -1,23 +1,16 @@ - - + + MetaModel org.apache.metamodel @@ -32,11 +25,15 @@ under the License. MetaModel-core ${project.version} - - org.slf4j - slf4j-nop - test - + + com.google.guava + guava + + + org.slf4j + slf4j-nop + test + junit junit diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/ArrayTableDataProvider.java b/pojo/src/main/java/org/apache/metamodel/pojo/ArrayTableDataProvider.java index ac95fca26..926c8079a 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/ArrayTableDataProvider.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/ArrayTableDataProvider.java @@ -22,6 +22,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; import org.apache.metamodel.util.SimpleTableDef; @@ -33,11 +34,11 @@ public class ArrayTableDataProvider implements TableDataProvider { private static final long serialVersionUID = 1L; private final SimpleTableDef _tableDef; - private final Collection _arrays; + private final CopyOnWriteArrayList _arrays; public ArrayTableDataProvider(SimpleTableDef tableDef, Collection arrays) { _tableDef = tableDef; - _arrays = arrays; + _arrays = new CopyOnWriteArrayList<>(arrays); } @Override @@ -71,4 +72,8 @@ public void insert(Map recordData) { _arrays.add(record); } + @Override + public void remove(Object[] next) { + _arrays.remove(next); + } } diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/MapTableDataProvider.java b/pojo/src/main/java/org/apache/metamodel/pojo/MapTableDataProvider.java index e8993f6a4..bd56f52fa 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/MapTableDataProvider.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/MapTableDataProvider.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; import org.apache.metamodel.util.SimpleTableDef; @@ -34,11 +35,11 @@ public class MapTableDataProvider implements TableDataProvider> _maps; + private final CopyOnWriteArrayList> _maps; public MapTableDataProvider(SimpleTableDef tableDef, Collection> maps) { _tableDef = tableDef; - _maps = maps; + _maps = new CopyOnWriteArrayList<>(maps); } @Override @@ -65,4 +66,9 @@ public Object getValue(String column, Map record) { public void insert(Map recordData) { _maps.add(recordData); } + + @Override + public void remove(Map next) { + _maps.remove(next); + } } diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/ObjectTableDataProvider.java b/pojo/src/main/java/org/apache/metamodel/pojo/ObjectTableDataProvider.java index 54c2a772c..f6bf117dc 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/ObjectTableDataProvider.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/ObjectTableDataProvider.java @@ -25,6 +25,7 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; +import java.util.concurrent.CopyOnWriteArrayList; import java.util.Set; import java.util.TreeMap; @@ -43,7 +44,7 @@ public final class ObjectTableDataProvider implements TableDataProvider { private static final long serialVersionUID = 1L; private final String _tableName; - private final Collection _collection; + private final CopyOnWriteArrayList _collection; private final Class _class; private final SimpleTableDef _tableDef; private final Map> _fieldTypes; @@ -58,7 +59,7 @@ public ObjectTableDataProvider(String tableName, Class cls) { public ObjectTableDataProvider(String tableName, Class cls, Collection collection) { _tableName = tableName; - _collection = collection; + _collection = new CopyOnWriteArrayList<>(collection); _class = cls; _fieldTypes = new HashMap>(); _tableDef = createTableDef(); @@ -73,6 +74,11 @@ public String getName() { public Iterator iterator() { return _collection.iterator(); } + + @Override + public void remove(E next) { + _collection.remove(next); + } @Override public SimpleTableDef getTableDef() { diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataContext.java b/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataContext.java index 2de3d590f..61ee38160 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataContext.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataContext.java @@ -142,9 +142,7 @@ protected String getMainSchemaName() throws MetaModelException { @Override public void executeUpdate(UpdateScript update) { PojoUpdateCallback updateCallback = new PojoUpdateCallback(this); - synchronized (this) { - update.run(updateCallback); - } + update.run(updateCallback); } protected void addTableDataProvider(TableDataProvider tableDataProvider) { diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataSet.java b/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataSet.java index 1c86ef1a1..b9bf3dfbc 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataSet.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/PojoDataSet.java @@ -41,7 +41,6 @@ final class PojoDataSet extends AbstractDataSet { public PojoDataSet(TableDataProvider pojoTable, SelectItem[] selectItems) { super(selectItems); _pojoTable = pojoTable; - _iterator = _pojoTable.iterator(); } @@ -55,7 +54,7 @@ public boolean next() { return false; } } - + @Override public Row getRow() { final int size = getHeader().size(); @@ -74,6 +73,6 @@ public Row getRow() { * Used by DELETE statements to delete a record. */ protected void remove() { - _iterator.remove(); + _pojoTable.remove(_next); } } diff --git a/pojo/src/main/java/org/apache/metamodel/pojo/TableDataProvider.java b/pojo/src/main/java/org/apache/metamodel/pojo/TableDataProvider.java index 7786517a0..5faa4a249 100644 --- a/pojo/src/main/java/org/apache/metamodel/pojo/TableDataProvider.java +++ b/pojo/src/main/java/org/apache/metamodel/pojo/TableDataProvider.java @@ -35,4 +35,6 @@ public interface TableDataProvider extends HasName, Iterable, Serializable public Object getValue(String column, E record); public void insert(Map recordData); + + public void remove(E next); } diff --git a/pojo/src/test/java/org/apache/metamodel/pojo/PojoDataContextTest.java b/pojo/src/test/java/org/apache/metamodel/pojo/PojoDataContextTest.java index d5e093cc9..b0f3c68bf 100644 --- a/pojo/src/test/java/org/apache/metamodel/pojo/PojoDataContextTest.java +++ b/pojo/src/test/java/org/apache/metamodel/pojo/PojoDataContextTest.java @@ -18,27 +18,37 @@ */ package org.apache.metamodel.pojo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; - -import junit.framework.TestCase; +import java.util.Random; +import java.util.concurrent.atomic.AtomicReference; import org.apache.metamodel.DataContext; import org.apache.metamodel.UpdateCallback; import org.apache.metamodel.UpdateScript; +import org.apache.metamodel.create.CreateTable; import org.apache.metamodel.data.DataSet; +import org.apache.metamodel.insert.InsertInto; import org.apache.metamodel.query.Query; import org.apache.metamodel.schema.ColumnType; import org.apache.metamodel.schema.Schema; import org.apache.metamodel.schema.Table; import org.apache.metamodel.util.SimpleTableDef; +import org.junit.Assert; +import org.junit.Test; -public class PojoDataContextTest extends TestCase { +public class PojoDataContextTest { + @Test public void testExampleScenario() throws Exception { Collection persons = new ArrayList(); persons.add(new Person("Bono", 42)); @@ -64,13 +74,14 @@ public void testExampleScenario() throws Exception { DataSet dataSet = dc.query().from("persons").innerJoin("titles").on("name", "name").selectAll().execute(); - assertEquals("[persons.age, persons.name, titles.name, titles.title]", - Arrays.toString(dataSet.getSelectItems())); + assertEquals("[persons.age, persons.name, titles.name, titles.title]", Arrays.toString(dataSet + .getSelectItems())); assertTrue(dataSet.next()); assertEquals("Row[values=[42, Elvis Presley, Elvis Presley, The King]]", dataSet.getRow().toString()); assertFalse(dataSet.next()); } + @Test public void testScenarioWithMap() throws Exception { final SimpleTableDef tableDef = new SimpleTableDef("bar", new String[] { "col1", "col2", "col3" }, new ColumnType[] { ColumnType.VARCHAR, ColumnType.INTEGER, ColumnType.BOOLEAN }); @@ -84,6 +95,7 @@ public void testScenarioWithMap() throws Exception { runScenario(tableDataProvider); } + @Test public void testScenarioWithArrays() throws Exception { final SimpleTableDef tableDef = new SimpleTableDef("bar", new String[] { "col1", "col2", "col3" }, new ColumnType[] { ColumnType.VARCHAR, ColumnType.INTEGER, ColumnType.BOOLEAN }); @@ -97,6 +109,7 @@ public void testScenarioWithArrays() throws Exception { runScenario(tableDataProvider); } + @Test public void testScenarioWithObjects() throws Exception { final Collection collection = new ArrayList(); collection.add(new FoobarBean("2", 1000, true)); @@ -109,6 +122,60 @@ public void testScenarioWithObjects() throws Exception { runScenario(tableDataProvider); } + @Test + public void testThreadSafety() throws Exception { + final PojoDataContext dataContext = new PojoDataContext(); + dataContext.executeUpdate(new CreateTable(dataContext.getDefaultSchema(), "tbl").withColumn("foo").ofType( + ColumnType.STRING)); + + final Table table = dataContext.getTableByQualifiedLabel("tbl"); + + final AtomicReference exception = new AtomicReference(); + + final Runnable runnable = new Runnable() { + @Override + public void run() { + try { + final Random random = new Random(); + for (int i = 0; i < 50; i++) { + dataContext.executeUpdate(new InsertInto(table).value("foo", "test" + random.nextInt(1000))); + try (final DataSet dataSet = dataContext.query().from(table).selectAll().execute()) { + while (dataSet.next()) { + final Object value = dataSet.getRow().getValue(0); + Assert.assertNotNull(value); + } + } + } + } catch (Exception e) { + exception.set(e); + } + } + }; + + // start four threads, doing the same write+read stuff + final Thread t1 = new Thread(runnable); + final Thread t2 = new Thread(runnable); + final Thread t3 = new Thread(runnable); + final Thread t4 = new Thread(runnable); + t1.start(); + t2.start(); + t3.start(); + t4.start(); + + t1.join(); + t2.join(); + t3.join(); + t4.join(); + + assertNull(exception.get()); + + final DataSet dataSet = dataContext.query().from(table).selectCount().execute(); + assertTrue(dataSet.next()); + final Object count = dataSet.getRow().getValue(0); + assertEquals("200", count.toString()); + dataSet.close(); + } + private void runScenario(TableDataProvider tableDataProvider) { final PojoDataContext dc = new PojoDataContext("foo", tableDataProvider); @@ -171,12 +238,12 @@ public void run(UpdateCallback callback) { Query qUnordered = dc.query().from("bar").select("col1").toQuery().selectDistinct(); ds = dc.executeQuery(qUnordered); assertTrue(ds.next()); - //Check both possibilities for the order, because not for certain - if(ds.getRow().toString().equals("Row[values=[2]]")){ + // Check both possibilities for the order, because not for certain + if (ds.getRow().toString().equals("Row[values=[2]]")) { assertEquals("Row[values=[2]]", ds.getRow().toString()); assertTrue(ds.next()); assertEquals("Row[values=[3]]", ds.getRow().toString()); - }else{ + } else { assertEquals("Row[values=[3]]", ds.getRow().toString()); assertTrue(ds.next()); assertEquals("Row[values=[2]]", ds.getRow().toString());