BUG-38723 Return 404 instead of 400 or 500 when entity is not found

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

@@ -30,6 +30,7 @@ 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.EntityNotFoundException;
 import org.apache.hadoop.metadata.repository.MetadataRepository;
 import org.apache.hadoop.metadata.repository.graph.GraphProvider;
 import org.slf4j.Logger;
@@ -75,10 +76,10 @@ public class HiveLineageService implements LineageService {
 
             HIVE_TABLE_SCHEMA_QUERY = conf.getString(
                     "metadata.lineage.hive.table.schema.query",
-                    "hive_table where name=\"?\", columns");
+                    "hive_table where name=\"%s\", columns");
             HIVE_TABLE_EXISTS_QUERY = conf.getString(
                     "metadata.lineage.hive.table.exists.query",
-                    "from hive_table where name=\"?\"");
+                    "from hive_table where name=\"%s\"");
         } catch (MetadataException e) {
             throw new RuntimeException(e);
         }
@@ -106,7 +107,7 @@ public class HiveLineageService implements LineageService {
      */
     @Override
     @GraphTransaction
-    public String getOutputs(String tableName) throws DiscoveryException {
+    public String getOutputs(String tableName) throws MetadataException {
         LOG.info("Fetching lineage outputs for tableName={}", tableName);
         ParamChecker.notEmpty(tableName, "table name cannot be null");
         validateTableExists(tableName);
@@ -134,7 +135,7 @@ public class HiveLineageService implements LineageService {
      */
     @Override
     @GraphTransaction
-    public String getOutputsGraph(String tableName) throws DiscoveryException {
+    public String getOutputsGraph(String tableName) throws MetadataException {
         LOG.info("Fetching lineage outputs graph for tableName={}", tableName);
         ParamChecker.notEmpty(tableName, "table name cannot be null");
         validateTableExists(tableName);
@@ -155,7 +156,7 @@ public class HiveLineageService implements LineageService {
      */
     @Override
     @GraphTransaction
-    public String getInputs(String tableName) throws DiscoveryException {
+    public String getInputs(String tableName) throws MetadataException {
         LOG.info("Fetching lineage inputs for tableName={}", tableName);
         ParamChecker.notEmpty(tableName, "table name cannot be null");
         validateTableExists(tableName);
@@ -183,7 +184,7 @@ public class HiveLineageService implements LineageService {
      */
     @Override
     @GraphTransaction
-    public String getInputsGraph(String tableName) throws DiscoveryException {
+    public String getInputsGraph(String tableName) throws MetadataException {
         LOG.info("Fetching lineage inputs graph for tableName={}", tableName);
         ParamChecker.notEmpty(tableName, "table name cannot be null");
         validateTableExists(tableName);
@@ -204,12 +205,12 @@ public class HiveLineageService implements LineageService {
      */
     @Override
     @GraphTransaction
-    public String getSchema(String tableName) throws DiscoveryException {
+    public String getSchema(String tableName) throws MetadataException {
         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);
+        final String schemaQuery = String.format(HIVE_TABLE_SCHEMA_QUERY, tableName);
         return discoveryService.searchByDSL(schemaQuery);
     }
 
@@ -218,11 +219,11 @@ public class HiveLineageService implements LineageService {
      *
      * @param tableName table name
      */
-    private void validateTableExists(String tableName) throws DiscoveryException {
-        String tableExistsQuery = HIVE_TABLE_EXISTS_QUERY.replace("?", tableName);
+    private void validateTableExists(String tableName) throws MetadataException {
+        final String tableExistsQuery = String.format(HIVE_TABLE_EXISTS_QUERY, tableName);
         GremlinQueryResult queryResult = discoveryService.evaluate(tableExistsQuery);
         if (!(queryResult.rows().length() > 0)) {
-            throw new IllegalArgumentException(tableName + " does not exist");
+            throw new EntityNotFoundException(tableName + " does not exist");
         }
     }
 }

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

@@ -18,6 +18,8 @@
 
 package org.apache.hadoop.metadata.discovery;
 
+import org.apache.hadoop.metadata.MetadataException;
+
 /**
  * Lineage service interface.
  */
@@ -29,7 +31,7 @@ public interface LineageService {
      * @param tableName tableName
      * @return Outputs as JSON
      */
-    String getOutputs(String tableName) throws DiscoveryException;
+    String getOutputs(String tableName) throws MetadataException;
 
     /**
      * Return the lineage outputs graph for the given tableName.
@@ -37,7 +39,7 @@ public interface LineageService {
      * @param tableName tableName
      * @return Outputs Graph as JSON
      */
-    String getOutputsGraph(String tableName) throws DiscoveryException;
+    String getOutputsGraph(String tableName) throws MetadataException;
 
     /**
      * Return the lineage inputs for the given tableName.
@@ -45,7 +47,7 @@ public interface LineageService {
      * @param tableName tableName
      * @return Inputs as JSON
      */
-    String getInputs(String tableName) throws DiscoveryException;
+    String getInputs(String tableName) throws MetadataException;
 
     /**
      * Return the lineage inputs graph for the given tableName.
@@ -53,7 +55,7 @@ public interface LineageService {
      * @param tableName tableName
      * @return Inputs Graph as JSON
      */
-    String getInputsGraph(String tableName) throws DiscoveryException;
+    String getInputsGraph(String tableName) throws MetadataException;
 
     /**
      * Return the schema for the given tableName.
@@ -61,5 +63,5 @@ public interface LineageService {
      * @param tableName tableName
      * @return Schema as JSON
      */
-    String getSchema(String tableName) throws DiscoveryException;
+    String getSchema(String tableName) throws MetadataException;
 }

repository/src/main/java/org/apache/hadoop/metadata/repository/EntityNotFoundException.java

+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.metadata.repository;
+
+import org.apache.hadoop.metadata.MetadataException;
+
+/**
+ * A simple wrapper for 404.
+ */
+public class EntityNotFoundException extends MetadataException {
+    public EntityNotFoundException() {
+    }
+
+    public EntityNotFoundException(String message) {
+        super(message);
+    }
+
+    public EntityNotFoundException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public EntityNotFoundException(Throwable cause) {
+        super(cause);
+    }
+
+    public EntityNotFoundException(String message, Throwable cause, boolean enableSuppression,
+                                   boolean writableStackTrace) {
+        super(message, cause, enableSuppression, writableStackTrace);
+    }
+}

repository/src/main/java/org/apache/hadoop/metadata/repository/MetadataRepository.java

@@ -130,7 +130,7 @@ public interface MetadataRepository {
      * @return a list of trait names for the given entity guid
      * @throws RepositoryException
      */
-    List<String> getTraitNames(String guid) throws RepositoryException;
+    List<String> getTraitNames(String guid) throws MetadataException;
 
     /**
      * Adds a new trait to an existing entity represented by a guid.

repository/src/main/java/org/apache/hadoop/metadata/repository/graph/GraphBackedMetadataRepository.java

@@ -30,6 +30,7 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.metadata.GraphTransaction;
 import org.apache.hadoop.metadata.MetadataException;
 import org.apache.hadoop.metadata.repository.Constants;
+import org.apache.hadoop.metadata.repository.EntityNotFoundException;
 import org.apache.hadoop.metadata.repository.MetadataRepository;
 import org.apache.hadoop.metadata.repository.RepositoryException;
 import org.apache.hadoop.metadata.typesystem.IReferenceableInstance;
@@ -89,7 +90,8 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
     private final TitanGraph titanGraph;
 
     @Inject
-    public GraphBackedMetadataRepository(GraphProvider<TitanGraph> graphProvider) throws MetadataException {
+    public GraphBackedMetadataRepository(
+            GraphProvider<TitanGraph> graphProvider) throws MetadataException {
         this.typeSystem = TypeSystem.getInstance();
 
         this.titanGraph = graphProvider.get();
@@ -124,7 +126,8 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
     }
 
     @Override
-    public String getFieldNameInVertex(IDataType<?> dataType, AttributeInfo aInfo) throws MetadataException {
+    public String getFieldNameInVertex(IDataType<?> dataType,
+                                       AttributeInfo aInfo) throws MetadataException {
         return getQualifiedName(dataType, aInfo.name);
     }
 
@@ -137,7 +140,8 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
         return EDGE_LABEL_PREFIX + typeName + "." + attrName;
     }
 
-    public String getEdgeLabel(ITypedInstance typedInstance, AttributeInfo aInfo) throws MetadataException {
+    public String getEdgeLabel(ITypedInstance typedInstance,
+                               AttributeInfo aInfo) throws MetadataException {
         IDataType dataType = typeSystem.getDataType(IDataType.class, typedInstance.getTypeName());
         return getEdgeLabel(dataType, aInfo);
     }
@@ -169,11 +173,11 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
         }
     }
 
-    private Vertex getVertexForGUID(String guid) throws RepositoryException {
+    private Vertex getVertexForGUID(String guid) throws EntityNotFoundException {
         Vertex instanceVertex = GraphHelper.findVertexByGUID(titanGraph, guid);
         if (instanceVertex == null) {
             LOG.debug("Could not find a vertex for guid={}", guid);
-            throw new RepositoryException(
+            throw new EntityNotFoundException(
                     "Could not find an entity in the repository for guid: " + guid);
         }
 
@@ -208,7 +212,7 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
      */
     @Override
     @GraphTransaction
-    public List<String> getTraitNames(String guid) throws RepositoryException {
+    public List<String> getTraitNames(String guid) throws MetadataException {
         LOG.info("Retrieving trait names for entity={}", guid);
         Vertex instanceVertex = getVertexForGUID(guid);
         return getTraitNames(instanceVertex);
@@ -252,7 +256,7 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
             ((TitanVertex) instanceVertex)
                     .addProperty(Constants.TRAIT_NAMES_PROPERTY_KEY, traitName);
 
-        } catch (MetadataException e) {
+        } catch (Exception e) {
             throw new RepositoryException(e);
         }
     }
@@ -315,21 +319,20 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
 
     @Override
     @GraphTransaction
-    public void updateEntity(String guid, String property, String value) throws RepositoryException {
+    public void updateEntity(String guid,
+                             String property, String value) throws RepositoryException {
         LOG.info("Adding property {} for entity guid {}", property, guid);
 
         try {
-            Vertex instanceVertex = GraphHelper.findVertexByGUID(titanGraph, guid);
-            if (instanceVertex == null) {
-                throw new RepositoryException("Could not find a vertex for guid " + guid);
-            }
+            Vertex instanceVertex = getVertexForGUID(guid);
 
             LOG.debug("Found a vertex {} for guid {}", instanceVertex, guid);
             String typeName = instanceVertex.getProperty(Constants.ENTITY_TYPE_PROPERTY_KEY);
             ClassType type = typeSystem.getDataType(ClassType.class, typeName);
             AttributeInfo attributeInfo = type.fieldMapping.fields.get(property);
             if (attributeInfo == null) {
-                throw new MetadataException("Invalid property " + property + " for entity " + typeName);
+                throw new MetadataException("Invalid property "
+                        + property + " for entity " + typeName);
             }
 
             DataTypes.TypeCategory attrTypeCategory = attributeInfo.dataType().getTypeCategory();
@@ -343,8 +346,9 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
                 throw new RepositoryException("Update of " + attrTypeCategory + " is not supported");
             }
 
-            instanceToGraphMapper.mapAttributesToVertex(getIdFromVertex(typeName, instanceVertex), instance,
-                    instanceVertex, new HashMap<Id, Vertex>(), attributeInfo, attributeInfo.dataType());
+            instanceToGraphMapper.mapAttributesToVertex(getIdFromVertex(typeName, instanceVertex),
+                    instance, instanceVertex, new HashMap<Id, Vertex>(),
+                    attributeInfo, attributeInfo.dataType());
         } catch (Exception e) {
             throw new RepositoryException(e);
         }

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

@@ -22,6 +22,7 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import org.apache.hadoop.metadata.RepositoryMetadataModule;
 import org.apache.hadoop.metadata.discovery.graph.GraphBackedDiscoveryService;
+import org.apache.hadoop.metadata.repository.EntityNotFoundException;
 import org.apache.hadoop.metadata.services.DefaultMetadataService;
 import org.apache.hadoop.metadata.typesystem.Referenceable;
 import org.apache.hadoop.metadata.typesystem.TypesDef;
@@ -174,7 +175,7 @@ public class HiveLineageServiceTest {
         Assert.fail();
     }
 
-    @Test (expectedExceptions = IllegalArgumentException.class)
+    @Test (expectedExceptions = EntityNotFoundException.class)
     public void testGetInputsBadTableName() throws Exception {
         hiveLineageService.getInputs("blah");
         Assert.fail();
@@ -223,7 +224,7 @@ public class HiveLineageServiceTest {
         Assert.fail();
     }
 
-    @Test (expectedExceptions = IllegalArgumentException.class)
+    @Test (expectedExceptions = EntityNotFoundException.class)
     public void testGetOutputsBadTableName() throws Exception {
         hiveLineageService.getOutputs("blah");
         Assert.fail();
@@ -285,7 +286,7 @@ public class HiveLineageServiceTest {
         Assert.fail();
     }
 
-    @Test (expectedExceptions = IllegalArgumentException.class)
+    @Test (expectedExceptions = EntityNotFoundException.class)
     public void testGetSchemaBadTableName() throws Exception {
         hiveLineageService.getSchema("blah");
         Assert.fail();

repository/src/test/java/org/apache/hadoop/metadata/repository/graph/GraphBackedMetadataRepositoryTest.java

@@ -29,6 +29,7 @@ import org.apache.hadoop.metadata.TestUtils;
 import org.apache.hadoop.metadata.discovery.graph.GraphBackedDiscoveryService;
 import org.apache.hadoop.metadata.repository.BaseTest;
 import org.apache.hadoop.metadata.repository.Constants;
+import org.apache.hadoop.metadata.repository.EntityNotFoundException;
 import org.apache.hadoop.metadata.repository.RepositoryException;
 import org.apache.hadoop.metadata.typesystem.IStruct;
 import org.apache.hadoop.metadata.typesystem.ITypedReferenceableInstance;
@@ -227,7 +228,7 @@ public class GraphBackedMetadataRepositoryTest {
         Assert.assertEquals(traitNames.size(), 0);
     }
 
-    @Test (expectedExceptions = RepositoryException.class)
+    @Test (expectedExceptions = EntityNotFoundException.class)
     public void testGetTraitNamesForBadEntity() throws Exception {
         repositoryService.getTraitNames(UUID.randomUUID().toString());
         Assert.fail();

webapp/src/main/java/org/apache/hadoop/metadata/web/resources/EntityResource.java

@@ -142,7 +142,8 @@ public class EntityResource {
                 response.put(MetadataServiceClient.DEFINITION, entityDefinition);
                 status = Response.Status.OK;
             } else {
-                response.put(MetadataServiceClient.ERROR, Servlets.escapeJsonString(String.format("An entity with GUID={%s} does not exist", guid)));
+                response.put(MetadataServiceClient.ERROR, Servlets.escapeJsonString(
+                        String.format("An entity with GUID={%s} does not exist", guid)));
             }
 
             return Response.status(status).entity(response).build();
@@ -209,12 +210,15 @@ public class EntityResource {
                            @QueryParam("property") String property,
                            @QueryParam("value") String value) {
         try {
+            Preconditions.checkNotNull(property, "Entity property cannot be null");
+            Preconditions.checkNotNull(value, "Entity value cannot be null");
+
             metadataService.updateEntity(guid, property, value);
 
             JSONObject response = new JSONObject();
             response.put(MetadataServiceClient.REQUEST_ID, Thread.currentThread().getName());
             return Response.ok(response).build();
-        } catch (MetadataException e) {
+        } catch (MetadataException | IllegalArgumentException e) {
             LOG.error("Unable to add property {} to entity id {}", property, guid, e);
             throw new WebApplicationException(
                     Servlets.getErrorResponse(e, Response.Status.BAD_REQUEST));

webapp/src/main/java/org/apache/hadoop/metadata/web/resources/HiveLineageResource.java

@@ -22,6 +22,7 @@ import org.apache.hadoop.metadata.MetadataServiceClient;
 import org.apache.hadoop.metadata.ParamChecker;
 import org.apache.hadoop.metadata.discovery.DiscoveryException;
 import org.apache.hadoop.metadata.discovery.LineageService;
+import org.apache.hadoop.metadata.repository.EntityNotFoundException;
 import org.apache.hadoop.metadata.web.util.Servlets;
 import org.codehaus.jettison.json.JSONObject;
 import org.slf4j.Logger;
@@ -80,6 +81,10 @@ public class HiveLineageResource {
             response.put(MetadataServiceClient.RESULTS, new JSONObject(jsonResult));
 
             return Response.ok(response).build();
+        } catch (EntityNotFoundException e) {
+            LOG.error("table entity not found for {}", tableName, e);
+            throw new WebApplicationException(
+                    Servlets.getErrorResponse(e, Response.Status.NOT_FOUND));
         } catch (DiscoveryException | IllegalArgumentException e) {
             LOG.error("Unable to get lineage inputs graph for table {}", tableName, e);
             throw new WebApplicationException(
@@ -114,6 +119,10 @@ public class HiveLineageResource {
             response.put(MetadataServiceClient.RESULTS, new JSONObject(jsonResult));
 
             return Response.ok(response).build();
+        } catch (EntityNotFoundException e) {
+            LOG.error("table entity not found for {}", tableName, e);
+            throw new WebApplicationException(
+                    Servlets.getErrorResponse(e, Response.Status.NOT_FOUND));
         } catch (DiscoveryException | IllegalArgumentException e) {
             LOG.error("Unable to get lineage outputs graph for table {}", tableName, e);
             throw new WebApplicationException(
@@ -148,6 +157,10 @@ public class HiveLineageResource {
             response.put(MetadataServiceClient.RESULTS, new JSONObject(jsonResult));
 
             return Response.ok(response).build();
+        } catch (EntityNotFoundException e) {
+            LOG.error("table entity not found for {}", tableName, e);
+            throw new WebApplicationException(
+                    Servlets.getErrorResponse(e, Response.Status.NOT_FOUND));
         } catch (DiscoveryException | IllegalArgumentException e) {
             LOG.error("Unable to get schema for table {}", tableName, e);
             throw new WebApplicationException(

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

@@ -152,6 +152,22 @@ public class EntityJerseyResourceIT extends BaseResourceIT {
         tableInstance.set("level", 4);
     }
 
+    @Test(dependsOnMethods = "testSubmitEntity", expectedExceptions = IllegalArgumentException.class)
+    public void testAddNullProperty() throws Exception {
+        final String guid = tableId._getId();
+        //add property
+        addProperty(guid, null, "foo bar");
+        Assert.fail();
+    }
+
+    @Test(dependsOnMethods = "testSubmitEntity", expectedExceptions = IllegalArgumentException.class)
+    public void testAddNullPropertyValue() throws Exception {
+        final String guid = tableId._getId();
+        //add property
+        addProperty(guid, "description", null);
+        Assert.fail();
+    }
+
     @Test(dependsOnMethods = "testSubmitEntity")
     public void testAddReferenceProperty() throws Exception {
         //Create new db instance
@@ -362,6 +378,25 @@ public class EntityJerseyResourceIT extends BaseResourceIT {
         Assert.assertNotNull(response.get(MetadataServiceClient.GUID));
     }
 
+    @Test(dependsOnMethods = "testAddTrait")
+    public void testAddExistingTrait() throws Exception {
+        final String traitName = "PII_Trait";
+
+        Struct traitInstance = new Struct(traitName);
+        String traitInstanceAsJSON = InstanceSerialization.toJson(traitInstance, true);
+        LOG.debug("traitInstanceAsJSON = " + traitInstanceAsJSON);
+
+        final String guid = tableId._getId();
+        ClientResponse clientResponse = service
+                .path("api/metadata/entities")
+                .path(guid)
+                .path(TRAITS)
+                .accept(MediaType.APPLICATION_JSON)
+                .type(MediaType.APPLICATION_JSON)
+                .method(HttpMethod.POST, ClientResponse.class, traitInstanceAsJSON);
+        Assert.assertEquals(clientResponse.getStatus(), Response.Status.BAD_REQUEST.getStatusCode());
+    }
+
     @Test(dependsOnMethods = "testGetTraitNames")
     public void testAddTraitWithAttribute() throws Exception {
             final String traitName = "P_I_I";

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

@@ -193,7 +193,7 @@ public class HiveLineageJerseyResourceIT extends BaseResourceIT {
                 .type(MediaType.APPLICATION_JSON)
                 .method(HttpMethod.GET, ClientResponse.class);
         Assert.assertEquals(clientResponse.getStatus(),
-                Response.Status.BAD_REQUEST.getStatusCode());
+                Response.Status.NOT_FOUND.getStatusCode());
     }
 
     private void setUpTypes() throws Exception {