From 8cb7fc48abe4c1fdb80ed39cc0eeac7f8937f729 Mon Sep 17 00:00:00 2001 From: Abraham Elmahrek Date: Fri, 19 Dec 2014 13:59:42 -0800 Subject: [PATCH] SQOOP-1930: Sqoop2: Enforce a non empty schema name and column names (Veena Basavaraj via Abraham Elmahrek) --- .../apache/sqoop/schema/ByteArraySchema.java | 12 ++++--- .../org/apache/sqoop/schema/NullSchema.java | 3 +- .../java/org/apache/sqoop/schema/Schema.java | 8 ++--- .../org/apache/sqoop/schema/SchemaError.java | 4 ++- .../org/apache/sqoop/schema/type/Column.java | 6 +++- .../org/apache/sqoop/json/TestSchemaBean.java | 3 -- .../json/util/TestSchemaSerialization.java | 31 +++++++++++++++++++ 7 files changed, 51 insertions(+), 16 deletions(-) diff --git a/common/src/main/java/org/apache/sqoop/schema/ByteArraySchema.java b/common/src/main/java/org/apache/sqoop/schema/ByteArraySchema.java index 4e2ab960..804ebf0c 100644 --- a/common/src/main/java/org/apache/sqoop/schema/ByteArraySchema.java +++ b/common/src/main/java/org/apache/sqoop/schema/ByteArraySchema.java @@ -20,14 +20,16 @@ import org.apache.sqoop.schema.type.Binary; /*** - * Schema holding a single field of Binary data - * Used to support connectors to schemaless / unstructured systems - * Such as HDFS or Kafka + * Schema holding a single field of Binary data Used to support connectors to + * schemaless / unstructured systems Such as HDFS or Kafka */ public class ByteArraySchema extends Schema { + private static final String BYTE_ARRAY_SCHEMA_NAME = "ByteArraySchema"; + private static final String BYTE_ARRAY_COLUMN_NAME = "ByteArraySchema_Bytes"; + public static final ByteArraySchema instance = (ByteArraySchema) new ByteArraySchema() - .addColumn(new Binary("ByteArraySchema_Bytes")); + .addColumn(new Binary(BYTE_ARRAY_COLUMN_NAME)); public static ByteArraySchema getInstance() { return instance; @@ -35,6 +37,6 @@ public static ByteArraySchema getInstance() { // To avoid instantiation private ByteArraySchema() { - super("ByteArraySchema"); + super(BYTE_ARRAY_SCHEMA_NAME); } } diff --git a/common/src/main/java/org/apache/sqoop/schema/NullSchema.java b/common/src/main/java/org/apache/sqoop/schema/NullSchema.java index c3393a6f..4a9ae116 100644 --- a/common/src/main/java/org/apache/sqoop/schema/NullSchema.java +++ b/common/src/main/java/org/apache/sqoop/schema/NullSchema.java @@ -19,6 +19,7 @@ public class NullSchema extends Schema { + private static final String NULL_SCHEMA_NAME = "NullSchema"; public static final NullSchema instance = new NullSchema(); public static NullSchema getInstance() { @@ -27,6 +28,6 @@ public static NullSchema getInstance() { private NullSchema() { // empty string is the name - super(""); + super(NULL_SCHEMA_NAME); } } diff --git a/common/src/main/java/org/apache/sqoop/schema/Schema.java b/common/src/main/java/org/apache/sqoop/schema/Schema.java index 9eef72b2..5951e36a 100644 --- a/common/src/main/java/org/apache/sqoop/schema/Schema.java +++ b/common/src/main/java/org/apache/sqoop/schema/Schema.java @@ -65,7 +65,9 @@ private Schema() { public Schema(String name) { this(); - assert name != null; + if (StringUtils.isEmpty(name)) { + throw new SqoopException(SchemaError.SCHEMA_0006, "Schema: " + name); + } this.name = name; } @@ -80,10 +82,6 @@ public Schema(String name) { * @return a reference to this object */ public Schema addColumn(Column column) { - if(column.getName() == null) { - throw new SqoopException(SchemaError.SCHEMA_0001, "Column: " + column); - } - if(columNames.contains(column.getName())) { throw new SqoopException(SchemaError.SCHEMA_0002, "Column: " + column); } diff --git a/common/src/main/java/org/apache/sqoop/schema/SchemaError.java b/common/src/main/java/org/apache/sqoop/schema/SchemaError.java index d430a643..33e46721 100644 --- a/common/src/main/java/org/apache/sqoop/schema/SchemaError.java +++ b/common/src/main/java/org/apache/sqoop/schema/SchemaError.java @@ -34,7 +34,9 @@ public enum SchemaError implements ErrorCode { SCHEMA_0004("Non-null target column has no matching source column"), - SCHEMA_0005("No matching method available for source and target schemas") + SCHEMA_0005("No matching method available for source and target schemas"), + + SCHEMA_0006("Schema without name"), ; diff --git a/common/src/main/java/org/apache/sqoop/schema/type/Column.java b/common/src/main/java/org/apache/sqoop/schema/type/Column.java index 2d7eac21..6f1f8d8b 100644 --- a/common/src/main/java/org/apache/sqoop/schema/type/Column.java +++ b/common/src/main/java/org/apache/sqoop/schema/type/Column.java @@ -18,6 +18,8 @@ package org.apache.sqoop.schema.type; import org.apache.commons.lang.StringUtils; +import org.apache.sqoop.common.SqoopException; +import org.apache.sqoop.schema.SchemaError; /** * Base class for all the supported types in the Sqoop {@link #Schema} @@ -42,7 +44,9 @@ public Column(String name) { } public Column(String name, Boolean nullable) { - assert !StringUtils.isEmpty(name); + if (StringUtils.isEmpty(name)) { + throw new SqoopException(SchemaError.SCHEMA_0001, "Column name: " + name); + } this.name = name; setNullable(nullable); } diff --git a/common/src/test/java/org/apache/sqoop/json/TestSchemaBean.java b/common/src/test/java/org/apache/sqoop/json/TestSchemaBean.java index e8bc0854..960a13e9 100644 --- a/common/src/test/java/org/apache/sqoop/json/TestSchemaBean.java +++ b/common/src/test/java/org/apache/sqoop/json/TestSchemaBean.java @@ -37,13 +37,10 @@ public class TestSchemaBean extends TestSchemaSerialization { protected Schema transfer(Schema schema) { SchemaBean extractBean = new SchemaBean(schema); JSONObject extractJson = extractBean.extract(true); - String transferredString = extractJson.toJSONString(); - JSONObject restoreJson = JSONUtils.parse(transferredString); SchemaBean restoreBean = new SchemaBean(); restoreBean.restore(restoreJson); - return restoreBean.getSchema(); } } diff --git a/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java b/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java index f0475568..9031c997 100644 --- a/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java +++ b/common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java @@ -23,7 +23,9 @@ import java.util.Collections; import java.util.HashSet; +import org.apache.sqoop.common.SqoopException; import org.apache.sqoop.json.JSONUtils; +import org.apache.sqoop.schema.ByteArraySchema; import org.apache.sqoop.schema.NullSchema; import org.apache.sqoop.schema.Schema; import org.apache.sqoop.schema.type.Array; @@ -61,6 +63,35 @@ public void testNullSchemaObject() { transferAndAssert(NullSchema.getInstance()); } + @SuppressWarnings("unused") + @Test(expected = SqoopException.class) + public void testEmptySchemaName() { + Schema schema = new Schema(""); + } + + @SuppressWarnings("unused") + @Test(expected = SqoopException.class) + public void testNullSchemaName() { + Schema schema = new Schema(null); + } + + @SuppressWarnings("unused") + @Test(expected = SqoopException.class) + public void testSchemaWithNullColumnName() { + Schema schema = new Schema("test").addColumn(new Text(null)); + } + + @SuppressWarnings("unused") + @Test(expected = SqoopException.class) + public void testSchemaWithEmptyColumnName() { + Schema schema = new Schema("test").addColumn(new Text("")); + } + + @Test + public void testByteArraySchemaObject() { + transferAndAssert(ByteArraySchema.getInstance()); + } + @Test public void testArray() { // create an array type containing decimals