ATLAS-1744 Error when searching type with attribute name order , limit , offset (sumasai)

intg/src/main/java/org/apache/atlas/AtlasErrorCode.java

@@ -52,6 +52,7 @@ public enum AtlasErrorCode {
     PATCH_FOR_UNKNOWN_TYPE(400, "ATLAS-400-00-017", "{0} - patch references unknown type {1}"),
     PATCH_INVALID_DATA(400, "ATLAS-400-00-018", "{0} - patch data is invalid for type {1}"),
     TYPE_NAME_INVALID_FORMAT(400, "ATLAS-400-00-019", "{0}: invalid name for {1}.  Names must consist of a letter followed by a sequence of letter, number, or '_' characters"),
+    ATTRIBUTE_NAME_INVALID(400, "ATLAS-400-00-020", "{0}: invalid name. Attribute name must not contain query keywords"),
     INVALID_PARAMETERS(400, "ATLAS-400-00-01A", "invalid parameters: {0}"),
     CLASSIFICATION_ALREADY_ASSOCIATED(400, "ATLAS-400-00-01B", "instance {0} already is associated with classification {1}"),
     CONSTRAINT_INVERSE_REF_ATTRIBUTE_INVALID_TYPE(400, "ATLAS-400-00-01C", "{0}.{1}: invalid {2} constraint. Attribute {3} is not an entity type"),

intg/src/main/java/org/apache/atlas/type/AtlasTypeUtil.java

@@ -99,7 +99,6 @@ public class AtlasTypeUtil {
             && StringUtils.endsWith(typeName, ATLAS_TYPE_MAP_SUFFIX);
     }
 
-
     public static boolean isValidTypeName(String typeName) {
         Matcher m = NAME_PATTERN.matcher(typeName);
 
@@ -120,15 +119,6 @@ public class AtlasTypeUtil {
         return InvalidTraitTypeNameErrorMessage;
     }
 
-    public static void validateType(AtlasBaseTypeDef typeDef) throws AtlasBaseException {
-        boolean isValidName = (typeDef instanceof AtlasClassificationDef) ? isValidTraitTypeName(typeDef.getName())
-            : isValidTypeName(typeDef.getName());
-
-        if (!isValidName) {
-            throw new AtlasBaseException(AtlasErrorCode.TYPE_NAME_INVALID_FORMAT, typeDef.getName(), typeDef.getCategory().name());
-        }
-    }
-
     public static String getStringValue(Map map, Object key) {
         Object ret = map != null ? map.get(key) : null;
 

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

@@ -41,7 +41,9 @@ import org.apache.atlas.repository.audit.EntityAuditRepository;
 import org.apache.atlas.repository.graph.DeleteHandler;
 import org.apache.atlas.repository.graph.GraphBackedMetadataRepository;
 import org.apache.atlas.repository.graph.GraphBackedSearchIndexer;
+import org.apache.atlas.repository.store.graph.AtlasEntityDefStore;
 import org.apache.atlas.repository.store.graph.AtlasEntityStore;
+import org.apache.atlas.repository.store.graph.v1.AtlasEntityDefStoreV1;
 import org.apache.atlas.repository.store.graph.v1.AtlasEntityStoreV1;
 import org.apache.atlas.repository.store.graph.v1.AtlasTypeDefGraphStoreV1;
 import org.apache.atlas.repository.store.graph.v1.DeleteHandlerV1;
@@ -76,6 +78,9 @@ public class RepositoryMetadataModule extends com.google.inject.AbstractModule {
         // bind the ITypeStore interface to an implementation
         bind(ITypeStore.class).to(GraphBackedTypeStore.class).asEagerSingleton();
         bind(AtlasTypeDefStore.class).to(AtlasTypeDefGraphStoreV1.class).asEagerSingleton();
+
+        //For testing
+        bind(AtlasEntityDefStore.class).to(AtlasEntityDefStoreV1.class).asEagerSingleton();
         bind(AtlasTypeRegistry.class).asEagerSingleton();
 
         //GraphBackedSearchIndexer must be an eager singleton to force the search index creation to happen before

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasAbstractDefStoreV1.java

@@ -17,8 +17,19 @@
  */
 package org.apache.atlas.repository.store.graph.v1;
 
+import org.apache.atlas.ApplicationProperties;
+import org.apache.atlas.AtlasErrorCode;
+import org.apache.atlas.AtlasException;
+import org.apache.atlas.exception.AtlasBaseException;
+import org.apache.atlas.model.typedef.AtlasBaseTypeDef;
+import org.apache.atlas.model.typedef.AtlasStructDef;
+import org.apache.atlas.query.QueryParser;
 import org.apache.atlas.type.AtlasTypeRegistry;
 
+import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 /**
  * Abstract typedef-store for v1 format.
  */
@@ -26,8 +37,40 @@ public abstract class AtlasAbstractDefStoreV1 {
     protected final AtlasTypeDefGraphStoreV1 typeDefStore;
     protected final AtlasTypeRegistry        typeRegistry;
 
+    private static final String  NAME_REGEX         = "[a-zA-Z][a-zA-Z0-9_ ]*";
+    private static final Pattern NAME_PATTERN       = Pattern.compile(NAME_REGEX);
+
+    private static final String ALLOW_RESERVED_KEYWORDS = "atlas.types.allowReservedKeywords";
+
     public AtlasAbstractDefStoreV1(AtlasTypeDefGraphStoreV1 typeDefStore, AtlasTypeRegistry typeRegistry) {
         this.typeDefStore = typeDefStore;
         this.typeRegistry = typeRegistry;
     }
+
+    public void validateType(AtlasBaseTypeDef typeDef) throws AtlasBaseException {
+        if (!isValidName(typeDef.getName())) {
+            throw new AtlasBaseException(AtlasErrorCode.TYPE_NAME_INVALID_FORMAT, typeDef.getName(), typeDef.getCategory().name());
+        }
+
+        try {
+            final boolean allowReservedKeywords = ApplicationProperties.get().getBoolean(ALLOW_RESERVED_KEYWORDS, false);
+
+            if (!allowReservedKeywords && typeDef instanceof AtlasStructDef) {
+                final List<AtlasStructDef.AtlasAttributeDef> attributeDefs = ((AtlasStructDef) typeDef).getAttributeDefs();
+                for (AtlasStructDef.AtlasAttributeDef attrDef : attributeDefs) {
+                    if (QueryParser.isKeyword(attrDef.getName())) {
+                        throw new AtlasBaseException(AtlasErrorCode.ATTRIBUTE_NAME_INVALID, attrDef.getName(), typeDef.getCategory().name());
+                    }
+                }
+            }
+        } catch (AtlasException e) {
+            throw new AtlasBaseException(AtlasErrorCode.INTERNAL_ERROR, "Could not load configuration");
+        }
+    }
+
+    public boolean isValidName(String typeName) {
+        Matcher m = NAME_PATTERN.matcher(typeName);
+
+        return m.matches();
+    }
 }

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasClassificationDefStoreV1.java

@@ -27,7 +27,6 @@ import org.apache.atlas.repository.store.graph.AtlasClassificationDefStore;
 import org.apache.atlas.type.AtlasClassificationType;
 import org.apache.atlas.type.AtlasType;
 import org.apache.atlas.type.AtlasTypeRegistry;
-import org.apache.atlas.type.AtlasTypeUtil;
 import org.apache.atlas.typesystem.types.DataTypes.TypeCategory;
 import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
@@ -36,6 +35,8 @@ import org.slf4j.LoggerFactory;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * ClassificationDef store in v1 format.
@@ -43,6 +44,10 @@ import java.util.List;
 public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 implements AtlasClassificationDefStore {
     private static final Logger LOG = LoggerFactory.getLogger(AtlasClassificationDefStoreV1.class);
 
+    private static final String  TRAIT_NAME_REGEX   = "[a-zA-Z][a-zA-Z0-9_ .]*";
+
+    private static final Pattern TRAIT_NAME_PATTERN = Pattern.compile(TRAIT_NAME_REGEX);
+
     public AtlasClassificationDefStoreV1(AtlasTypeDefGraphStoreV1 typeDefStore, AtlasTypeRegistry typeRegistry) {
         super(typeDefStore, typeRegistry);
     }
@@ -53,7 +58,7 @@ public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 imple
             LOG.debug("==> AtlasClassificationDefStoreV1.preCreate({})", classificationDef);
         }
 
-        AtlasTypeUtil.validateType(classificationDef);
+        validateType(classificationDef);
 
         AtlasType type = typeRegistry.getType(classificationDef.getName());
 
@@ -173,7 +178,7 @@ public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 imple
             LOG.debug("==> AtlasClassificationDefStoreV1.update({})", classifiDef);
         }
 
-        AtlasTypeUtil.validateType(classifiDef);
+        validateType(classifiDef);
 
         AtlasClassificationDef ret = StringUtils.isNotBlank(classifiDef.getGuid())
                   ? updateByGuid(classifiDef.getGuid(), classifiDef) : updateByName(classifiDef.getName(), classifiDef);
@@ -192,7 +197,7 @@ public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 imple
             LOG.debug("==> AtlasClassificationDefStoreV1.updateByName({}, {})", name, classificationDef);
         }
 
-        AtlasTypeUtil.validateType(classificationDef);
+        validateType(classificationDef);
 
         AtlasType type = typeRegistry.getType(classificationDef.getName());
 
@@ -224,7 +229,7 @@ public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 imple
             LOG.debug("==> AtlasClassificationDefStoreV1.updateByGuid({})", guid);
         }
 
-        AtlasTypeUtil.validateType(classificationDef);
+        validateType(classificationDef);
 
         AtlasType type = typeRegistry.getTypeByGuid(guid);
 
@@ -375,4 +380,11 @@ public class AtlasClassificationDefStoreV1 extends AtlasAbstractDefStoreV1 imple
 
         return ret;
     }
+
+    @Override
+    public boolean isValidName(String typeName) {
+        Matcher m = TRAIT_NAME_PATTERN.matcher(typeName);
+
+        return m.matches();
+    }
 }

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityDefStoreV1.java

@@ -17,6 +17,7 @@
  */
 package org.apache.atlas.repository.store.graph.v1;
 
+import com.google.inject.Inject;
 import org.apache.atlas.AtlasErrorCode;
 import org.apache.atlas.exception.AtlasBaseException;
 import org.apache.atlas.model.typedef.AtlasEntityDef;
@@ -26,7 +27,6 @@ import org.apache.atlas.repository.store.graph.AtlasEntityDefStore;
 import org.apache.atlas.type.AtlasEntityType;
 import org.apache.atlas.type.AtlasType;
 import org.apache.atlas.type.AtlasTypeRegistry;
-import org.apache.atlas.type.AtlasTypeUtil;
 import org.apache.atlas.typesystem.types.DataTypes.TypeCategory;
 import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
@@ -42,6 +42,7 @@ import java.util.List;
 public class AtlasEntityDefStoreV1 extends AtlasAbstractDefStoreV1 implements AtlasEntityDefStore {
     private static final Logger LOG = LoggerFactory.getLogger(AtlasEntityDefStoreV1.class);
 
+    @Inject
     public AtlasEntityDefStoreV1(AtlasTypeDefGraphStoreV1 typeDefStore, AtlasTypeRegistry typeRegistry) {
         super(typeDefStore, typeRegistry);
     }
@@ -52,7 +53,7 @@ public class AtlasEntityDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasEntityDefStoreV1.preCreate({})", entityDef);
         }
 
-        AtlasTypeUtil.validateType(entityDef);
+        validateType(entityDef);
 
         AtlasType type = typeRegistry.getType(entityDef.getName());
 
@@ -172,7 +173,7 @@ public class AtlasEntityDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasEntityDefStoreV1.update({})", entityDef);
         }
 
-        AtlasTypeUtil.validateType(entityDef);
+        validateType(entityDef);
 
         AtlasEntityDef ret = StringUtils.isNotBlank(entityDef.getGuid()) ? updateByGuid(entityDef.getGuid(), entityDef)
                                                                          : updateByName(entityDef.getName(), entityDef);
@@ -190,7 +191,7 @@ public class AtlasEntityDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasEntityDefStoreV1.updateByName({}, {})", name, entityDef);
         }
 
-        AtlasTypeUtil.validateType(entityDef);
+        validateType(entityDef);
 
         AtlasType type = typeRegistry.getType(entityDef.getName());
 
@@ -222,7 +223,7 @@ public class AtlasEntityDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasEntityDefStoreV1.updateByGuid({})", guid);
         }
 
-        AtlasTypeUtil.validateType(entityDef);
+        validateType(entityDef);
 
         AtlasType type = typeRegistry.getTypeByGuid(guid);
 

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEnumDefStoreV1.java

@@ -52,7 +52,7 @@ public class AtlasEnumDefStoreV1 extends AtlasAbstractDefStoreV1 implements Atla
             LOG.debug("==> AtlasEnumDefStoreV1.create({})", enumDef);
         }
 
-        AtlasTypeUtil.validateType(enumDef);
+        validateType(enumDef);
 
         AtlasVertex vertex = typeDefStore.findTypeVertexByName(enumDef.getName());
 
@@ -143,7 +143,7 @@ public class AtlasEnumDefStoreV1 extends AtlasAbstractDefStoreV1 implements Atla
             LOG.debug("==> AtlasEnumDefStoreV1.update({})", enumDef);
         }
 
-        AtlasTypeUtil.validateType(enumDef);
+        validateType(enumDef);
 
         AtlasEnumDef ret = StringUtils.isNotBlank(enumDef.getGuid()) ? updateByGuid(enumDef.getGuid(), enumDef)
                                                                      : updateByName(enumDef.getName(), enumDef);
@@ -161,7 +161,7 @@ public class AtlasEnumDefStoreV1 extends AtlasAbstractDefStoreV1 implements Atla
             LOG.debug("==> AtlasEnumDefStoreV1.updateByName({}, {})", name, enumDef);
         }
 
-        AtlasTypeUtil.validateType(enumDef);
+        validateType(enumDef);
 
         AtlasVertex vertex = typeDefStore.findTypeVertexByNameAndCategory(name, TypeCategory.ENUM);
 
@@ -188,7 +188,7 @@ public class AtlasEnumDefStoreV1 extends AtlasAbstractDefStoreV1 implements Atla
             LOG.debug("==> AtlasEnumDefStoreV1.updateByGuid({})", guid);
         }
 
-        AtlasTypeUtil.validateType(enumDef);
+        validateType(enumDef);
 
         AtlasVertex vertex = typeDefStore.findTypeVertexByGuidAndCategory(guid, TypeCategory.ENUM);
 

repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasStructDefStoreV1.java

@@ -64,7 +64,7 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasStructDefStoreV1.preCreate({})", structDef);
         }
 
-        AtlasTypeUtil.validateType(structDef);
+        validateType(structDef);
 
         AtlasType type = typeRegistry.getType(structDef.getName());
 
@@ -183,7 +183,7 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasStructDefStoreV1.update({})", structDef);
         }
 
-        AtlasTypeUtil.validateType(structDef);
+        validateType(structDef);
 
         AtlasStructDef ret = StringUtils.isNotBlank(structDef.getGuid()) ? updateByGuid(structDef.getGuid(), structDef)
                                                                          : updateByName(structDef.getName(), structDef);
@@ -201,7 +201,7 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasStructDefStoreV1.updateByName({}, {})", name, structDef);
         }
 
-        AtlasTypeUtil.validateType(structDef);
+        validateType(structDef);
 
         AtlasType type = typeRegistry.getType(structDef.getName());
 
@@ -233,7 +233,7 @@ public class AtlasStructDefStoreV1 extends AtlasAbstractDefStoreV1 implements At
             LOG.debug("==> AtlasStructDefStoreV1.updateByGuid({})", guid);
         }
 
-        AtlasTypeUtil.validateType(structDef);
+        validateType(structDef);
 
         AtlasType type = typeRegistry.getTypeByGuid(guid);
 

repository/src/main/scala/org/apache/atlas/query/QueryParser.scala

@@ -421,6 +421,10 @@ object QueryParser extends StandardTokenParsers with QueryKeywords with Expressi
       case g ~ ce => ce
     }
 
+    def isKeyword(s: String) = queryreservedWords.contains(s)
+
+    def isDelimiter(s: String) = querydelims.contains(s)
+
 }
 
 class QueryLexer(val keywords: Seq[String], val delims: Seq[String]) extends StdLexical with ImplicitConversions {

repository/src/test/java/org/apache/atlas/discovery/GraphBackedDiscoveryServiceTest.java

@@ -1006,6 +1006,18 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
         System.out.println("results = " + results);
     }
 
+    @Test
+    public void testSearchForTypeWithReservedKeywordAttributes() throws Exception {
+        createTypesWithReservedKeywordAttributes();
+
+        String dslQuery = "from OrderType where `order` = 1";
+        String jsonResults = searchByDSL(dslQuery);
+        assertNotNull(jsonResults);
+
+        JSONObject results = new JSONObject(jsonResults);
+        System.out.println("results = " + results);
+    }
+
     /*
      * Type Hierarchy is:
      *   A(a)
@@ -1028,6 +1040,15 @@ public class GraphBackedDiscoveryServiceTest extends BaseRepositoryTest {
         TypeSystem.getInstance().defineClassTypes(A, B, C, D);
     }
 
+    private void createTypesWithReservedKeywordAttributes() throws Exception {
+        HierarchicalTypeDefinition orderType = createClassTypeDef("OrderType", null, createRequiredAttrDef("order", DataTypes.INT_TYPE));
+
+        HierarchicalTypeDefinition limitType =
+            createClassTypeDef("LimitType", null, createOptionalAttrDef("limit", DataTypes.BOOLEAN_TYPE));
+
+        TypeSystem.getInstance().defineClassTypes(orderType, limitType);
+    }
+
     private void createInstances() throws Exception {
         Referenceable instance = new Referenceable("D");
         instance.set("d", 1);

repository/src/test/java/org/apache/atlas/repository/store/graph/AtlasEntityDefStoreV1Test.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.repository.store.graph;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.inject.Inject;
+import org.apache.atlas.AtlasErrorCode;
+import org.apache.atlas.RepositoryMetadataModule;
+import org.apache.atlas.exception.AtlasBaseException;
+import org.apache.atlas.model.typedef.AtlasEntityDef;
+import org.apache.atlas.repository.graph.AtlasGraphProvider;
+import org.apache.atlas.type.AtlasTypeUtil;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Guice;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.fail;
+
+/**
+ * Tests for AtlasEntityStoreV1
+ */
+@Guice(modules = RepositoryMetadataModule.class)
+public class AtlasEntityDefStoreV1Test {
+
+    @Inject
+    private
+    AtlasEntityDefStore entityDefStore;
+
+    @DataProvider
+    public Object[][] invalidAttributeNameWithReservedKeywords(){
+        AtlasEntityDef invalidAttrNameType =
+            AtlasTypeUtil.createClassTypeDef("Invalid_Attribute_Type", "description", ImmutableSet.<String>of(),
+                AtlasTypeUtil.createRequiredAttrDef("order", "string"),
+                AtlasTypeUtil.createRequiredAttrDef("limit", "string"));
+
+        return new Object[][] {{
+            invalidAttrNameType
+        }};
+    }
+
+    @Test(dataProvider = "invalidAttributeNameWithReservedKeywords")
+    public void testCreateTypeWithReservedKeywords(AtlasEntityDef atlasEntityDef) {
+        try {
+            entityDefStore.create(atlasEntityDef, null);
+        } catch (AtlasBaseException e) {
+            Assert.assertEquals(e.getAtlasErrorCode(), AtlasErrorCode.ATTRIBUTE_NAME_INVALID);
+        }
+    }
+
+    @AfterClass
+    public void clear(){
+        AtlasGraphProvider.cleanup();
+    }
+}