HBase Improvement - Fixed a bug for inserting NULL values and refactored some tests
Project: http://git-wip-us.apache.org/repos/asf/metamodel/repo Commit: http://git-wip-us.apache.org/repos/asf/metamodel/commit/738a97d0 Tree: http://git-wip-us.apache.org/repos/asf/metamodel/tree/738a97d0 Diff: http://git-wip-us.apache.org/repos/asf/metamodel/diff/738a97d0 Branch: refs/heads/master Commit: 738a97d042ffffcfb00db71b5199a0bc0663432a Parents: 1dc52f6 Author: Gerard Dellemann <[email protected]> Authored: Tue Jun 19 11:27:22 2018 +0200 Committer: Gerard Dellemann <[email protected]> Committed: Tue Jun 19 11:27:22 2018 +0200 ---------------------------------------------------------------------- .../org/apache/metamodel/hbase/HBaseClient.java | 18 ++- .../hbase/HBaseRowDeletionBuilder.java | 6 +- .../apache/metamodel/hbase/DeleteRowTest.java | 7 +- .../apache/metamodel/hbase/DropTableTest.java | 14 +-- .../apache/metamodel/hbase/HBaseClientTest.java | 102 +++++++++++++-- .../hbase/HBaseUpdateCallbackTest.java | 10 ++ .../apache/metamodel/hbase/InsertRowTest.java | 123 ++++++------------- 7 files changed, 158 insertions(+), 122 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/main/java/org/apache/metamodel/hbase/HBaseClient.java ---------------------------------------------------------------------- diff --git a/hbase/src/main/java/org/apache/metamodel/hbase/HBaseClient.java b/hbase/src/main/java/org/apache/metamodel/hbase/HBaseClient.java index 0594a5b..e73a643 100644 --- a/hbase/src/main/java/org/apache/metamodel/hbase/HBaseClient.java +++ b/hbase/src/main/java/org/apache/metamodel/hbase/HBaseClient.java @@ -76,8 +76,8 @@ final class HBaseClient { if (i != indexOfIdColumn) { // NullChecker is already forced within the HBaseColumn class final byte[] columnFamily = Bytes.toBytes(columns[i].getColumnFamily()); - // An HBaseColumn doesn't need a qualifier, this only works when the qualifier is empty (not null). - // Otherwise NullPointer exceptions will happen + // An HBaseColumn doesn't need a qualifier, this only works when the qualifier is empty (not + // null). Otherwise NullPointer exceptions will happen byte[] qualifier = null; if (columns[i].getQualifier() != null) { qualifier = Bytes.toBytes(columns[i].getQualifier()); @@ -85,7 +85,15 @@ final class HBaseClient { qualifier = Bytes.toBytes(new String("")); } final byte[] value = getValueAsByteArray(values[i]); - put.addColumn(columnFamily, qualifier, value); + // A NULL value, doesn't get inserted in HBase + // TODO: Do we delete the cell (and therefore the qualifier) if the table get's updated? + if (value == null) { + logger.warn("The value of column '{}:{}' is null. This insertion is skipped", columns[i] + .getColumnFamily() + .toString(), columns[i].getQualifier().toString()); + } else { + put.addColumn(columnFamily, qualifier, value); + } } } // Add the put to the table @@ -187,7 +195,9 @@ final class HBaseClient { */ private byte[] getValueAsByteArray(final Object value) { byte[] valueAsByteArray; - if (value instanceof byte[]) { + if (value == null) { + valueAsByteArray = null; + } else if (value instanceof byte[]) { valueAsByteArray = (byte[]) value; } else { valueAsByteArray = Bytes.toBytes(value.toString()); http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/main/java/org/apache/metamodel/hbase/HBaseRowDeletionBuilder.java ---------------------------------------------------------------------- diff --git a/hbase/src/main/java/org/apache/metamodel/hbase/HBaseRowDeletionBuilder.java b/hbase/src/main/java/org/apache/metamodel/hbase/HBaseRowDeletionBuilder.java index 078fe5b..e8afda8 100644 --- a/hbase/src/main/java/org/apache/metamodel/hbase/HBaseRowDeletionBuilder.java +++ b/hbase/src/main/java/org/apache/metamodel/hbase/HBaseRowDeletionBuilder.java @@ -40,14 +40,14 @@ public class HBaseRowDeletionBuilder extends AbstractRowDeletionBuilder { /** * Creates a {@link HBaseRowDeletionBuilder} - * @param hBaseWriter + * @param dataContext * @param table - * @throws IllegalArgumentException when the hBaseWriter is null + * @throws IllegalArgumentException when the dataContext is null */ public HBaseRowDeletionBuilder(final HBaseDataContext dataContext, final Table table) { super(table); if (dataContext == null) { - throw new IllegalArgumentException("hBaseClient cannot be null"); + throw new IllegalArgumentException("DataContext cannot be null"); } this._dataContext = dataContext; } http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/test/java/org/apache/metamodel/hbase/DeleteRowTest.java ---------------------------------------------------------------------- diff --git a/hbase/src/test/java/org/apache/metamodel/hbase/DeleteRowTest.java b/hbase/src/test/java/org/apache/metamodel/hbase/DeleteRowTest.java index 4e588ca..65a8431 100644 --- a/hbase/src/test/java/org/apache/metamodel/hbase/DeleteRowTest.java +++ b/hbase/src/test/java/org/apache/metamodel/hbase/DeleteRowTest.java @@ -55,14 +55,14 @@ public class DeleteRowTest extends HBaseUpdateCallbackTest { } /** - * Creating a HBaseRowDeletionBuilder with the hBaseClient null, should throw an exception + * Creating a HBaseRowDeletionBuilder with the DataContext null, should throw an exception * * @throws IOException */ @Test - public void testHBaseClientNullAtBuilder() throws IOException { + public void testDataContextNullAtBuilder() throws IOException { exception.expect(IllegalArgumentException.class); - exception.expectMessage("hBaseClient cannot be null"); + exception.expectMessage("DataContext cannot be null"); final HBaseTable existingTable = createAndAddTableToDatastore(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); new HBaseRowDeletionBuilder(null, existingTable); @@ -83,7 +83,6 @@ public class DeleteRowTest extends HBaseUpdateCallbackTest { getUpdateCallback().deleteFrom(existingTable).execute(); } - /** * Goodflow. Deleting a row, that doesn't exist, should not throw an exception * http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/test/java/org/apache/metamodel/hbase/DropTableTest.java ---------------------------------------------------------------------- diff --git a/hbase/src/test/java/org/apache/metamodel/hbase/DropTableTest.java b/hbase/src/test/java/org/apache/metamodel/hbase/DropTableTest.java index e3493e9..c94febc 100644 --- a/hbase/src/test/java/org/apache/metamodel/hbase/DropTableTest.java +++ b/hbase/src/test/java/org/apache/metamodel/hbase/DropTableTest.java @@ -18,7 +18,8 @@ */ package org.apache.metamodel.hbase; -import static org.junit.Assert.*; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import java.io.IOException; @@ -54,17 +55,6 @@ public class DropTableTest extends HBaseUpdateCallbackTest { } /** - * Creating a HBaseClient with the tableName null, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithTableNameNull() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage("Can't drop a table without having the tableName"); - - new HBaseClient(getDataContext().getConnection()).dropTable(null); - } - - /** * Goodflow. Dropping a table successfully. * * @throws IOException http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/test/java/org/apache/metamodel/hbase/HBaseClientTest.java ---------------------------------------------------------------------- diff --git a/hbase/src/test/java/org/apache/metamodel/hbase/HBaseClientTest.java b/hbase/src/test/java/org/apache/metamodel/hbase/HBaseClientTest.java index 31d6a0d..f46fd9e 100644 --- a/hbase/src/test/java/org/apache/metamodel/hbase/HBaseClientTest.java +++ b/hbase/src/test/java/org/apache/metamodel/hbase/HBaseClientTest.java @@ -19,6 +19,7 @@ package org.apache.metamodel.hbase; import java.util.LinkedHashSet; +import java.util.Map; import java.util.Set; import org.junit.Before; @@ -26,7 +27,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -public class HBaseClientTest extends HBaseTestCase { +public class HBaseClientTest extends HBaseUpdateCallbackTest { @Rule public ExpectedException exception = ExpectedException.none(); @@ -36,11 +37,75 @@ public class HBaseClientTest extends HBaseTestCase { super.setUp(); } + /* Inserting a row */ + + /** + * Inserting a row with the columns null, should throw a exception + */ + @Test + public void testInsertRowWithColumnsNull() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage( + "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); + + final Object[] values = new String[] { "Values" }; + new HBaseClient(getDataContext().getConnection()).insertRow("tableName", null, values, 0); + } + + /** + * Inserting a row with with the values null, should throw a exception + */ + @Test + public void testInsertRowWithValuesNull() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage( + "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); + + final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); + final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); + final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); + new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, null, 0); + } + /** - * Creating a HBaseClient with the tableName null, should throw a exception + * Inserting a row with with the indexOfIdColumn out of bounce, should throw a exception */ @Test - public void testCreatingTheHBaseClientWithTableNameNull() { + public void testInsertRowWithIndexOfIdColumnOutOfBounce() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage( + "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); + + final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); + final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); + final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); + final Object[] values = new String[] { "Values" }; + new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, values, 10); + } + + /** + * Inserting a row with with the rowKey null, should throw a exception + */ + @Test + public void testInsertRowWithRowKeyNull() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage( + "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); + + final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); + final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); + final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); + final Object[] values = new String[] { null }; + new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, values, 0); + } + + /* Creating a table */ + + /** + * Creating a table with the tableName null, should throw a exception + */ + @Test + public void testCreateTableWithTableNameNull() { exception.expect(IllegalArgumentException.class); exception.expectMessage("Can't create a table without having the tableName or columnFamilies"); @@ -50,10 +115,10 @@ public class HBaseClientTest extends HBaseTestCase { } /** - * Creating a HBaseClient with the tableName null, should throw a exception + * Creating a table with the columnFamilies null, should throw a exception */ @Test - public void testCreatingTheHBaseClientWithColumnFamiliesNull() { + public void testCreateTableWithColumnFamiliesNull() { exception.expect(IllegalArgumentException.class); exception.expectMessage("Can't create a table without having the tableName or columnFamilies"); @@ -61,10 +126,10 @@ public class HBaseClientTest extends HBaseTestCase { } /** - * Creating a HBaseClient with the tableName null, should throw a exception + * Creating a table with the columnFamilies empty, should throw a exception */ @Test - public void testCreatingTheHBaseClientWithColumnFamiliesEmpty() { + public void testCreateTableWithColumnFamiliesEmpty() { exception.expect(IllegalArgumentException.class); exception.expectMessage("Can't create a table without having the tableName or columnFamilies"); @@ -72,11 +137,13 @@ public class HBaseClientTest extends HBaseTestCase { new HBaseClient(getDataContext().getConnection()).createTable("1", columnFamilies); } + /* Deleting a row */ + /** - * Creating a HBaseClient with the tableName null, should throw a exception + * Deleting a row with the tableName null, should throw a exception */ @Test - public void testDeleteRowWithoutTableName() { + public void testDeleteRowWithTableNameNull() { exception.expect(IllegalArgumentException.class); exception.expectMessage("Can't delete a row without having tableName or rowKey"); @@ -84,13 +151,26 @@ public class HBaseClientTest extends HBaseTestCase { } /** - * Creating a HBaseClient with the rowKey null, should throw a exception + * Deleting a row with the rowKey null, should throw a exception */ @Test - public void testCreatingTheHBaseClientWithRowKeyNull() { + public void testDeleteRowWithRowKeyNull() { exception.expect(IllegalArgumentException.class); exception.expectMessage("Can't delete a row without having tableName or rowKey"); new HBaseClient(getDataContext().getConnection()).deleteRow("tableName", null); } + + /* Dropping/deleting a table */ + + /** + * Dropping a table with the tableName null, should throw a exception + */ + @Test + public void testDropTableWithTableNameNull() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("Can't drop a table without having the tableName"); + + new HBaseClient(getDataContext().getConnection()).dropTable(null); + } } http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/test/java/org/apache/metamodel/hbase/HBaseUpdateCallbackTest.java ---------------------------------------------------------------------- diff --git a/hbase/src/test/java/org/apache/metamodel/hbase/HBaseUpdateCallbackTest.java b/hbase/src/test/java/org/apache/metamodel/hbase/HBaseUpdateCallbackTest.java index 7d03641..403f95d 100644 --- a/hbase/src/test/java/org/apache/metamodel/hbase/HBaseUpdateCallbackTest.java +++ b/hbase/src/test/java/org/apache/metamodel/hbase/HBaseUpdateCallbackTest.java @@ -241,6 +241,16 @@ public abstract class HBaseUpdateCallbackTest extends HBaseTestCase { } } + /** + * Converts a list of {@link HBaseColumn}'s to an array of {@link HBaseColumn}'s + * + * @param columns + * @return Array of {@link HBaseColumn} + */ + protected static HBaseColumn[] convertToHBaseColumnsArray(List<HBaseColumn> columns) { + return columns.toArray(new HBaseColumn[columns.size()]); + } + protected HBaseUpdateCallback getUpdateCallback() { return updateCallback; } http://git-wip-us.apache.org/repos/asf/metamodel/blob/738a97d0/hbase/src/test/java/org/apache/metamodel/hbase/InsertRowTest.java ---------------------------------------------------------------------- diff --git a/hbase/src/test/java/org/apache/metamodel/hbase/InsertRowTest.java b/hbase/src/test/java/org/apache/metamodel/hbase/InsertRowTest.java index 1c21cba..b380d33 100644 --- a/hbase/src/test/java/org/apache/metamodel/hbase/InsertRowTest.java +++ b/hbase/src/test/java/org/apache/metamodel/hbase/InsertRowTest.java @@ -25,6 +25,10 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Get; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.metamodel.MetaModelException; import org.apache.metamodel.insert.RowInsertionBuilder; import org.apache.metamodel.schema.MutableTable; @@ -96,82 +100,6 @@ public class InsertRowTest extends HBaseUpdateCallbackTest { } /** - * Creating a HBaseClient with the tableName null, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithTableNameNull() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); - - final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); - final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); - final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); - final Object[] values = new String[] { "Values" }; - new HBaseClient(getDataContext().getConnection()).insertRow(null, columns, values, 0); - } - - /** - * Creating a HBaseClient with the columns null, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithColumnsNull() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); - - final Object[] values = new String[] { "Values" }; - new HBaseClient(getDataContext().getConnection()).insertRow("tableName", null, values, 0); - } - - /** - * Creating a HBaseClient with the values null, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithValuesNull() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); - - final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); - final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); - final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); - new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, null, 0); - } - - /** - * Creating a HBaseClient with the indexOfIdColumn out of bounce, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithIndexOfIdColumnOutOfBounce() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); - - final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); - final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); - final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); - final Object[] values = new String[] { "Values" }; - new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, values, 10); - } - - /** - * Creating a HBaseClient with the rowKey null, should throw a exception - */ - @Test - public void testCreatingTheHBaseClientWithRowKeyNull() { - exception.expect(IllegalArgumentException.class); - exception.expectMessage( - "Can't insert a row without having (correct) tableName, columns, values or indexOfIdColumn"); - - final HBaseTable table = createHBaseTable(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); - final Map<HBaseColumn, Object> row = createRow(table, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR, false); - final HBaseColumn[] columns = convertToHBaseColumnsArray(getHBaseColumnsFromRow(row)); - final Object[] values = new String[] { null }; - new HBaseClient(getDataContext().getConnection()).insertRow(table.getName(), columns, values, 0); - } - - /** * Inserting a row without setting enough values directly on the HBaseClient, should throw exception. * NOTE: This exception is already prevented when using the {@link HBaseRowInsertionBuilder} * @throws IOException @@ -198,6 +126,34 @@ public class InsertRowTest extends HBaseUpdateCallbackTest { } /** + * Inserting a row with with a value null, should get skipped + * @throws IOException + */ + @Test + public void testInsertRowWithValueNull() throws IOException { + final HBaseTable table = createAndAddTableToDatastore(TABLE_NAME, HBaseDataContext.FIELD_ID, CF_FOO, CF_BAR); + + RowInsertionBuilder insertBuilder = getUpdateCallback() + .insertInto(table) + .value(new HBaseColumn(HBaseDataContext.FIELD_ID, null, table), RK_1) + .value(new HBaseColumn(CF_FOO, Q_BAH, table), V_WORLD) + .value(new HBaseColumn(CF_FOO, Q_HELLO, table), null) + .value(new HBaseColumn(CF_BAR, Q_HEY, table), V_YO); + insertBuilder.execute(); + + try (org.apache.hadoop.hbase.client.Table hBaseTable = getDataContext().getConnection().getTable(TableName + .valueOf(TABLE_NAME))) { + final Get get = new Get(Bytes.toBytes(RK_1)); + final Result result = hBaseTable.get(get); + + assertFalse(result.isEmpty()); + assertEquals(V_WORLD, new String(result.getValue(Bytes.toBytes(CF_FOO), Bytes.toBytes(Q_BAH)))); + assertNull(result.getValue(Bytes.toBytes(CF_FOO), Bytes.toBytes(Q_HELLO))); + assertEquals(V_YO, new String(result.getValue(Bytes.toBytes(CF_BAR), Bytes.toBytes(Q_HEY)))); + } + } + + /** * Goodflow. Using an existing table and columns, should work * * @throws IOException @@ -251,21 +207,12 @@ public class InsertRowTest extends HBaseUpdateCallbackTest { RowInsertionBuilder insertBuilder = getUpdateCallback() .insertInto(table) + .value(new HBaseColumn(HBaseDataContext.FIELD_ID, null, table), RK_1) .value(new HBaseColumn(CF_FOO, Q_BAH, table), V_WORLD) .value(new HBaseColumn(CF_FOO, Q_HELLO, table), V_THERE) .value(new HBaseColumn(CF_BAR, Q_HEY, table), V_YO); - assertEquals("INSERT INTO HBase.table_for_junit(foo:bah,foo:hello,bar:hey) " - + "VALUES (\"world\",\"there\",\"yo\")", insertBuilder.toSql()); - } - - /** - * Converts a list of {@link HBaseColumn}'s to an array of {@link HBaseColumn}'s - * - * @param columns - * @return Array of {@link HBaseColumn} - */ - private static HBaseColumn[] convertToHBaseColumnsArray(List<HBaseColumn> columns) { - return columns.toArray(new HBaseColumn[columns.size()]); + assertEquals("INSERT INTO HBase.table_for_junit(_id,foo:bah,foo:hello,bar:hey) " + + "VALUES (\"junit1\",\"world\",\"there\",\"yo\")", insertBuilder.toSql()); } }
