ATLAS-398 Delete trait that exists but not linked to entity results in "400 Bad…

release-log.txt

@@ -7,6 +7,7 @@ ATLAS-409 Atlas will not import avro tables with schema read from a file (dosset
 ATLAS-379 Create sqoop and falcon metadata addons (venkatnrangan,bvellanki,sowmyaramesh via shwethags)
 
 ALL CHANGES:
+ATLAS-398 Delete trait that exists but not linked to entity results in "400 Bad request". It should result "404 not found" (ndjouhr via shwethags)
 ATLAS-372 Expose entity deletion through REST API (dkantor via shwethags)
 ATLAS-452 Exceptions while running HiveHookIT#testAlterTableRename (shwethags)
 ATLAS-388 UI : On creating Tag, the page to be reset for creating new Tag (Anilg via shwethags)

repository/src/main/java/org/apache/atlas/repository/MetadataRepository.java

@@ -23,6 +23,7 @@ import org.apache.atlas.typesystem.ITypedReferenceableInstance;
 import org.apache.atlas.typesystem.ITypedStruct;
 import org.apache.atlas.typesystem.exception.EntityExistsException;
 import org.apache.atlas.typesystem.exception.EntityNotFoundException;
+import org.apache.atlas.typesystem.exception.TraitNotFoundException;
 import org.apache.atlas.typesystem.types.AttributeInfo;
 import org.apache.atlas.typesystem.types.IDataType;
 import org.apache.atlas.typesystem.types.TypeUtils;
@@ -140,7 +141,7 @@ public interface MetadataRepository {
      * @param traitNameToBeDeleted name of the trait
      * @throws RepositoryException
      */
-    void deleteTrait(String guid, String traitNameToBeDeleted) throws EntityNotFoundException, RepositoryException;
+    void deleteTrait(String guid, String traitNameToBeDeleted) throws TraitNotFoundException, EntityNotFoundException, RepositoryException;
 
     /**
      * Adds/Updates the property to the entity that corresponds to the GUID

repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepository.java

@@ -34,6 +34,7 @@ import org.apache.atlas.typesystem.ITypedReferenceableInstance;
 import org.apache.atlas.typesystem.ITypedStruct;
 import org.apache.atlas.typesystem.exception.EntityExistsException;
 import org.apache.atlas.typesystem.exception.EntityNotFoundException;
+import org.apache.atlas.typesystem.exception.TraitNotFoundException;
 import org.apache.atlas.typesystem.types.AttributeInfo;
 import org.apache.atlas.typesystem.types.ClassType;
 import org.apache.atlas.typesystem.types.IDataType;
@@ -239,16 +240,19 @@ public class GraphBackedMetadataRepository implements MetadataRepository {
      */
     @Override
     @GraphTransaction
-    public void deleteTrait(String guid, String traitNameToBeDeleted) throws EntityNotFoundException, RepositoryException {
+    public void deleteTrait(String guid, String traitNameToBeDeleted) throws TraitNotFoundException, EntityNotFoundException, RepositoryException {
         LOG.info("Deleting trait={} from entity={}", traitNameToBeDeleted, guid);
-        try {
-            Vertex instanceVertex = graphHelper.getVertexForGUID(guid);
+        
+        Vertex instanceVertex = graphHelper.getVertexForGUID(guid);
 
-            List<String> traitNames = GraphHelper.getTraitNames(instanceVertex);
-            if (!traitNames.contains(traitNameToBeDeleted)) {
-                throw new EntityNotFoundException(
+        List<String> traitNames = GraphHelper.getTraitNames(instanceVertex);
+        if (!traitNames.contains(traitNameToBeDeleted)) {
+                throw new TraitNotFoundException(
                         "Could not find trait=" + traitNameToBeDeleted + " in the repository for entity: " + guid);
-            }
+        }
+
+        try {
+            
 
             final String entityTypeName = GraphHelper.getTypeName(instanceVertex);
             String relationshipLabel = GraphHelper.getTraitLabel(entityTypeName, traitNameToBeDeleted);

repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java

@@ -37,6 +37,7 @@ import org.apache.atlas.typesystem.ITypedStruct;
 import org.apache.atlas.typesystem.Referenceable;
 import org.apache.atlas.typesystem.Struct;
 import org.apache.atlas.typesystem.exception.EntityNotFoundException;
+import org.apache.atlas.typesystem.exception.TraitNotFoundException;
 import org.apache.atlas.typesystem.persistence.Id;
 import org.apache.atlas.typesystem.types.ClassType;
 import org.apache.atlas.typesystem.types.DataTypes;
@@ -337,13 +338,13 @@ public class GraphBackedMetadataRepositoryTest {
         Assert.assertTrue(modificationTimestampPostUpdate > modificationTimestampPreUpdate);
     }
 
-    @Test(expectedExceptions = RepositoryException.class)
+    @Test(expectedExceptions = EntityNotFoundException.class)
     public void testDeleteTraitForNonExistentEntity() throws Exception {
         repositoryService.deleteTrait(UUID.randomUUID().toString(), TestUtils.PII);
         Assert.fail();
     }
 
-    @Test(expectedExceptions = RepositoryException.class)
+    @Test(expectedExceptions = TraitNotFoundException.class)
     public void testDeleteTraitForNonExistentTrait() throws Exception {
         final String aGUID = getGUID();
         repositoryService.deleteTrait(aGUID, "PCI");

server-api/src/main/java/org/apache/atlas/typesystem/exception/TraitNotFoundException.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.atlas.typesystem.exception;
+import org.apache.atlas.AtlasException;
+
+/**
+ * A simple wrapper for  404.
+ * Thrown when a requested trait can not be found.
+ */
+public class TraitNotFoundException extends AtlasException{
+    public TraitNotFoundException() {
+    }
+
+    public TraitNotFoundException(String message) {
+        super(message);
+    }
+
+    public TraitNotFoundException(String message, Throwable cause) {
+        super(message, cause);
+    }
+
+    public TraitNotFoundException(Throwable cause) {
+        super(cause);
+    }
+
+    public TraitNotFoundException(String message, Throwable cause, boolean enableSuppression,
+            boolean writableStackTrace) {
+        super(message, cause, enableSuppression, writableStackTrace);
+    }
+}

webapp/src/main/java/org/apache/atlas/web/resources/EntityResource.java

@@ -26,6 +26,7 @@ import org.apache.atlas.typesystem.Referenceable;
 import org.apache.atlas.typesystem.exception.EntityExistsException;
 import org.apache.atlas.typesystem.exception.EntityNotFoundException;
 import org.apache.atlas.typesystem.exception.TypeNotFoundException;
+import org.apache.atlas.typesystem.exception.TraitNotFoundException;
 import org.apache.atlas.typesystem.json.InstanceSerialization;
 import org.apache.atlas.typesystem.types.ValueConversionException;
 import org.apache.atlas.utils.ParamChecker;
@@ -566,6 +567,9 @@ public class EntityResource {
         } catch (EntityNotFoundException | TypeNotFoundException e) {
             LOG.error("An entity with GUID={} does not exist", guid, e);
             throw new WebApplicationException(Servlets.getErrorResponse(e, Response.Status.NOT_FOUND));
+        } catch (TraitNotFoundException e) {
+            LOG.error("The trait name={} for entity={} does not exist.", traitName, guid, e);
+            throw new WebApplicationException(Servlets.getErrorResponse(e, Response.Status.NOT_FOUND));
         } catch (AtlasException | IllegalArgumentException e) {
             LOG.error("Unable to delete trait name={} for entity={}", traitName, guid, e);
             throw new WebApplicationException(Servlets.getErrorResponse(e, Response.Status.BAD_REQUEST));

webapp/src/test/java/org/apache/atlas/web/resources/EntityJerseyResourceIT.java

@@ -69,6 +69,7 @@ import java.util.UUID;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.fail;
 
+
 /**
  * Integration tests for Entity Jersey Resource.
  */
@@ -594,7 +595,32 @@ public class EntityJerseyResourceIT extends BaseResourceIT {
                 "trait=" + traitName + " should be defined in type system before it can be deleted");
         Assert.assertNotNull(response.get(AtlasClient.STACKTRACE));
     }
+@Test(dependsOnMethods = "testSubmitEntity()")
+    public void testDeleteExistentTraitNonExistentForEntity() throws Exception {
+    
+        final String guid = tableId._getId();
+        final String traitName = "PII_Trait" + randomString();
+        HierarchicalTypeDefinition<TraitType> piiTrait = TypesUtil
+                .createTraitTypeDef(traitName, ImmutableList.<String>of(),
+                        TypesUtil.createRequiredAttrDef("type", DataTypes.STRING_TYPE));
+        String traitDefinitionAsJSON = TypesSerialization$.MODULE$.toJson(piiTrait, true);
+        createType(traitDefinitionAsJSON);
+        
+        ClientResponse clientResponse = service.path(ENTITIES).path(guid).path(TRAITS).path(traitName)
+                .accept(Servlets.JSON_MEDIA_TYPE).type(Servlets.JSON_MEDIA_TYPE)
+                .method(HttpMethod.DELETE, ClientResponse.class);
+        
+        Assert.assertEquals(clientResponse.getStatus(), Response.Status.NOT_FOUND.getStatusCode());
+
+        String responseAsString = clientResponse.getEntity(String.class);
+        Assert.assertNotNull(responseAsString);
 
+        JSONObject response = new JSONObject(responseAsString);
+        Assert.assertNotNull(response.get(AtlasClient.ERROR));
+        Assert.assertNotNull(response.get(AtlasClient.STACKTRACE));
+     
+        
+    }
     private String random() {
         return RandomStringUtils.random(10);
     }