BUG-38698 hive lineage api's return 200 for table that does not exist

repository/src/main/java/org/apache/hadoop/metadata/discovery/HiveLineageService.java

@@ -22,10 +22,12 @@ import com.thinkaurelius.titan.core.TitanGraph;
 import org.apache.commons.configuration.PropertiesConfiguration;
 import org.apache.hadoop.metadata.GraphTransaction;
 import org.apache.hadoop.metadata.MetadataException;
+import org.apache.hadoop.metadata.ParamChecker;
 import org.apache.hadoop.metadata.PropertiesUtil;
 import org.apache.hadoop.metadata.discovery.graph.DefaultGraphPersistenceStrategy;
 import org.apache.hadoop.metadata.discovery.graph.GraphBackedDiscoveryService;
 import org.apache.hadoop.metadata.query.Expressions;
+import org.apache.hadoop.metadata.query.GremlinQueryResult;
 import org.apache.hadoop.metadata.query.HiveLineageQuery;
 import org.apache.hadoop.metadata.query.HiveWhereUsedQuery;
 import org.apache.hadoop.metadata.repository.MetadataRepository;
@@ -56,6 +58,7 @@ public class HiveLineageService implements LineageService {
     private static final String HIVE_PROCESS_OUTPUT_ATTRIBUTE_NAME;
 
     private static final String HIVE_TABLE_SCHEMA_QUERY;
+    private static final String HIVE_TABLE_EXISTS_QUERY;
 
     static {
         // todo - externalize this using type system - dog food
@@ -63,7 +66,6 @@ public class HiveLineageService implements LineageService {
             PropertiesConfiguration conf = PropertiesUtil.getApplicationProperties();
             HIVE_TABLE_TYPE_NAME =
                 conf.getString("metadata.lineage.hive.table.type.name",  "DataSet");
-                conf.getString("metadata.lineage.hive.table.type.name",  "hive_table");
             HIVE_PROCESS_TYPE_NAME =
                 conf.getString("metadata.lineage.hive.process.type.name", "Process");
             HIVE_PROCESS_INPUT_ATTRIBUTE_NAME =
@@ -74,6 +76,9 @@ public class HiveLineageService implements LineageService {
             HIVE_TABLE_SCHEMA_QUERY = conf.getString(
                     "metadata.lineage.hive.table.schema.query",
                     "hive_table where name=\"?\", columns");
+            HIVE_TABLE_EXISTS_QUERY = conf.getString(
+                    "metadata.lineage.hive.table.exists.query",
+                    "from hive_table where name=\"?\"");
         } catch (MetadataException e) {
             throw new RuntimeException(e);
         }
@@ -103,6 +108,8 @@ public class HiveLineageService implements LineageService {
     @GraphTransaction
     public String getOutputs(String tableName) throws DiscoveryException {
         LOG.info("Fetching lineage outputs for tableName={}", tableName);
+        ParamChecker.notEmpty(tableName, "table name cannot be null");
+        validateTableExists(tableName);
 
         HiveWhereUsedQuery outputsQuery = new HiveWhereUsedQuery(
                 HIVE_TABLE_TYPE_NAME, tableName, HIVE_PROCESS_TYPE_NAME,
@@ -129,6 +136,8 @@ public class HiveLineageService implements LineageService {
     @GraphTransaction
     public String getOutputsGraph(String tableName) throws DiscoveryException {
         LOG.info("Fetching lineage outputs graph for tableName={}", tableName);
+        ParamChecker.notEmpty(tableName, "table name cannot be null");
+        validateTableExists(tableName);
 
         HiveWhereUsedQuery outputsQuery = new HiveWhereUsedQuery(
                 HIVE_TABLE_TYPE_NAME, tableName, HIVE_PROCESS_TYPE_NAME,
@@ -148,6 +157,8 @@ public class HiveLineageService implements LineageService {
     @GraphTransaction
     public String getInputs(String tableName) throws DiscoveryException {
         LOG.info("Fetching lineage inputs for tableName={}", tableName);
+        ParamChecker.notEmpty(tableName, "table name cannot be null");
+        validateTableExists(tableName);
 
         HiveLineageQuery inputsQuery = new HiveLineageQuery(
                 HIVE_TABLE_TYPE_NAME, tableName, HIVE_PROCESS_TYPE_NAME,
@@ -174,6 +185,8 @@ public class HiveLineageService implements LineageService {
     @GraphTransaction
     public String getInputsGraph(String tableName) throws DiscoveryException {
         LOG.info("Fetching lineage inputs graph for tableName={}", tableName);
+        ParamChecker.notEmpty(tableName, "table name cannot be null");
+        validateTableExists(tableName);
 
         HiveLineageQuery inputsQuery = new HiveLineageQuery(
                 HIVE_TABLE_TYPE_NAME, tableName, HIVE_PROCESS_TYPE_NAME,
@@ -192,8 +205,24 @@ public class HiveLineageService implements LineageService {
     @Override
     @GraphTransaction
     public String getSchema(String tableName) throws DiscoveryException {
-        // todo - validate if indeed this is a table type and exists
+        LOG.info("Fetching schema for tableName={}", tableName);
+        ParamChecker.notEmpty(tableName, "table name cannot be null");
+        validateTableExists(tableName);
+
         String schemaQuery = HIVE_TABLE_SCHEMA_QUERY.replace("?", tableName);
         return discoveryService.searchByDSL(schemaQuery);
     }
+
+    /**
+     * Validate if indeed this is a table type and exists.
+     *
+     * @param tableName table name
+     */
+    private void validateTableExists(String tableName) throws DiscoveryException {
+        String tableExistsQuery = HIVE_TABLE_EXISTS_QUERY.replace("?", tableName);
+        GremlinQueryResult queryResult = discoveryService.evaluate(tableExistsQuery);
+        if (!(queryResult.rows().length() > 0)) {
+            throw new IllegalArgumentException(tableName + " does not exist");
+        }
+    }
 }

repository/src/main/java/org/apache/hadoop/metadata/discovery/graph/GraphBackedDiscoveryService.java

@@ -23,8 +23,6 @@ import com.thinkaurelius.titan.core.TitanIndexQuery;
 import com.thinkaurelius.titan.core.TitanProperty;
 import com.thinkaurelius.titan.core.TitanVertex;
 import com.tinkerpop.blueprints.Vertex;
-import com.tinkerpop.gremlin.groovy.Gremlin;
-import com.tinkerpop.gremlin.java.GremlinPipeline;
 import org.apache.hadoop.metadata.GraphTransaction;
 import org.apache.hadoop.metadata.MetadataServiceClient;
 import org.apache.hadoop.metadata.discovery.DiscoveryException;
@@ -123,13 +121,18 @@ public class GraphBackedDiscoveryService implements DiscoveryService {
     @GraphTransaction
     public String searchByDSL(String dslQuery) throws DiscoveryException {
         LOG.info("Executing dsl query={}", dslQuery);
+        GremlinQueryResult queryResult = evaluate(dslQuery);
+        return queryResult.toJson();
+    }
+
+    public GremlinQueryResult evaluate(String dslQuery) throws DiscoveryException {
+        LOG.info("Executing dsl query={}", dslQuery);
         try {
             QueryParser queryParser = new QueryParser();
             Either<Parsers.NoSuccess, Expressions.Expression> either = queryParser.apply(dslQuery);
             if (either.isRight()) {
                 Expressions.Expression expression = either.right().get();
-                GremlinQueryResult queryResult = evaluate(expression);
-                return queryResult.toJson();
+                return evaluate(expression);
             }
         } catch (Exception e) { // unable to catch ExpressionException
             throw new DiscoveryException("Invalid expression : " + dslQuery, e);

repository/src/test/java/org/apache/hadoop/metadata/discovery/HiveLineageServiceTest.java

@@ -162,6 +162,24 @@ public class HiveLineageServiceTest {
         Assert.assertTrue(paths.length() > 0);
     }
 
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetInputsTableNameNull() throws Exception {
+        hiveLineageService.getInputs(null);
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetInputsTableNameEmpty() throws Exception {
+        hiveLineageService.getInputs("");
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetInputsBadTableName() throws Exception {
+        hiveLineageService.getInputs("blah");
+        Assert.fail();
+    }
+
     @Test
     public void testGetInputsGraph() throws Exception {
         JSONObject results = new JSONObject(
@@ -193,6 +211,24 @@ public class HiveLineageServiceTest {
         Assert.assertTrue(paths.length() > 0);
     }
 
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetOututsTableNameNull() throws Exception {
+        hiveLineageService.getOutputs(null);
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetOutputsTableNameEmpty() throws Exception {
+        hiveLineageService.getOutputs("");
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetOutputsBadTableName() throws Exception {
+        hiveLineageService.getOutputs("blah");
+        Assert.fail();
+    }
+
     @Test
     public void testGetOutputsGraph() throws Exception {
         JSONObject results = new JSONObject(hiveLineageService.getOutputsGraph("sales_fact"));
@@ -237,6 +273,24 @@ public class HiveLineageServiceTest {
         }
     }
 
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetSchemaTableNameNull() throws Exception {
+        hiveLineageService.getSchema(null);
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetSchemaTableNameEmpty() throws Exception {
+        hiveLineageService.getSchema("");
+        Assert.fail();
+    }
+
+    @Test (expectedExceptions = IllegalArgumentException.class)
+    public void testGetSchemaBadTableName() throws Exception {
+        hiveLineageService.getSchema("blah");
+        Assert.fail();
+    }
+
     private void setUpTypes() throws Exception {
         TypesDef typesDef = createTypeDefinitions();
         String typesAsJSON = TypesSerialization.toJson(typesDef);

webapp/src/test/java/org/apache/hadoop/metadata/web/resources/HiveLineageJerseyResourceIT.java

@@ -166,6 +166,36 @@ public class HiveLineageJerseyResourceIT extends BaseResourceIT {
         }
     }
 
+    @Test
+    public void testSchemaForEmptyTable() throws Exception {
+        WebResource resource = service
+                .path(BASE_URI)
+                .path("")
+                .path("schema");
+
+        ClientResponse clientResponse = resource
+                .accept(MediaType.APPLICATION_JSON)
+                .type(MediaType.APPLICATION_JSON)
+                .method(HttpMethod.GET, ClientResponse.class);
+        Assert.assertEquals(clientResponse.getStatus(),
+                Response.Status.NOT_FOUND.getStatusCode());
+    }
+
+    @Test
+    public void testSchemaForInvalidTable() throws Exception {
+        WebResource resource = service
+                .path(BASE_URI)
+                .path("blah")
+                .path("schema");
+
+        ClientResponse clientResponse = resource
+                .accept(MediaType.APPLICATION_JSON)
+                .type(MediaType.APPLICATION_JSON)
+                .method(HttpMethod.GET, ClientResponse.class);
+        Assert.assertEquals(clientResponse.getStatus(),
+                Response.Status.BAD_REQUEST.getStatusCode());
+    }
+
     private void setUpTypes() throws Exception {
         TypesDef typesDef = createTypeDefinitions();
         createType(typesDef);