ATLAS-2419: DSL Semantic Validation. Fix for limit & range. Signed-off-by: apoorvnaik <[email protected]>
Project: http://git-wip-us.apache.org/repos/asf/atlas/repo Commit: http://git-wip-us.apache.org/repos/asf/atlas/commit/c35f82ca Tree: http://git-wip-us.apache.org/repos/asf/atlas/tree/c35f82ca Diff: http://git-wip-us.apache.org/repos/asf/atlas/diff/c35f82ca Branch: refs/heads/master Commit: c35f82ca1a3af5bbb1565afe6b9ff7358ed5c243 Parents: 1fee4a5 Author: Ashutosh Mestry <[email protected]> Authored: Thu Jan 25 19:09:20 2018 -0800 Committer: apoorvnaik <[email protected]> Committed: Thu Jan 25 19:26:09 2018 -0800 ---------------------------------------------------------------------- .../java/org/apache/atlas/AtlasErrorCode.java | 16 + .../java/org/apache/atlas/query/AtlasDSL.java | 26 +- .../java/org/apache/atlas/query/DSLVisitor.java | 119 +++---- .../org/apache/atlas/query/GremlinClause.java | 4 +- .../apache/atlas/query/GremlinClauseList.java | 101 ++++++ .../atlas/query/GremlinQueryComposer.java | 347 ++++++++++--------- .../apache/atlas/query/IdentifierHelper.java | 98 +++--- .../java/org/apache/atlas/query/Lookup.java | 5 +- .../apache/atlas/query/RegistryBasedLookup.java | 50 ++- .../atlas/query/SelectClauseComposer.java | 52 ++- .../org/apache/atlas/query/DSLQueriesTest.java | 125 ++++--- .../atlas/query/GremlinQueryComposerTest.java | 196 ++++++----- 12 files changed, 686 insertions(+), 453 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java ---------------------------------------------------------------------- diff --git a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java index 3289b48..7d88547 100644 --- a/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java +++ b/intg/src/main/java/org/apache/atlas/AtlasErrorCode.java @@ -103,7 +103,23 @@ public enum AtlasErrorCode { SAVED_SEARCH_CHANGE_USER(400, "ATLAS-400-00-056", "saved-search {0} can not be moved from user {1} to {2}"), INVALID_QUERY_PARAM_LENGTH(400, "ATLAS-400-00-057" , "Length of query param {0} exceeds the limit"), INVALID_QUERY_LENGTH(400, "ATLAS-400-00-058" , "Invalid query length, update {0} to change the limit" ), + // DSL related error codes INVALID_DSL_QUERY(400, "ATLAS-400-00-059" , "Invalid DSL query: {0} | Reason: {1}. Please refer to Atlas DSL grammar for more information" ), + INVALID_DSL_GROUPBY(400, "ATLAS-400-00-05A", "DSL Semantic Error - GroupBy attribute {0} is non-primitive"), + INVALID_DSL_UNKNOWN_TYPE(400, "ATLAS-400-00-05B", "DSL Semantic Error - {0} type not found"), + INVALID_DSL_UNKNOWN_CLASSIFICATION(400, "ATLAS-400-00-05C", "DSL Semantic Error - {0} classification not found"), + INVALID_DSL_UNKNOWN_ATTR_TYPE(400, "ATLAS-400-00-05D", "DSL Semantic Error - {0} attribute not found for type {1}"), + INVALID_DSL_ORDERBY(400, "ATLAS-400-00-05E", "DSL Semantic Error - OrderBy attribute {0} is non-primitive"), + INVALID_DSL_FROM(400, "ATLAS-400-00-05F", "DSL Semantic Error - From source {0} is not a valid Entity/Classification type"), + INVALID_DSL_SELECT_REFERRED_ATTR(400, "ATLAS-400-00-060", "DSL Semantic Error - Select clause has multiple referred attributes {0}"), + INVALID_DSL_SELECT_INVALID_AGG(400, "ATLAS-400-00-061", "DSL Semantic Error - Select clause has aggregation on referred attributes {0}"), + INVALID_DSL_SELECT_ATTR_MIXING(400, "ATLAS-400-00-062", "DSL Semantic Error - Select clause has simple and referred attributes"), + INVALID_DSL_HAS_ATTRIBUTE(400, "ATLAS-400-00-063", "DSL Semantic Error - No attribute {0} exists for type {1}"), + INVALID_DSL_QUALIFIED_NAME(400, "ATLAS-400-00-064", "DSL Semantic Error - Qualified name for {0} failed!"), + INVALID_DSL_QUALIFIED_NAME2(400, "ATLAS-400-00-065", "DSL Semantic Error - Qualified name for {0} failed for type {1}. Cause: {2}"), + INVALID_DSL_DUPLICATE_ALIAS(400, "ATLAS-400-00-066", "DSL Semantic Error - Duplicate alias found: '{0}' for type '{1}' already present."), + INVALID_DSL_INVALID_DATE(400, "ATLAS-400-00-067", "DSL Semantic Error - Date format: {0}."), + INVALID_DSL_HAS_PROPERTY(400, "ATLAS-400-00-068", "DSL Semantic Error - Property needs to be a primitive type: {0}"), // All Not found enums go here TYPE_NAME_NOT_FOUND(404, "ATLAS-404-00-001", "Given typename {0} was invalid"), http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java b/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java index 61a34b1..b8a744b 100644 --- a/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java +++ b/repository/src/main/java/org/apache/atlas/query/AtlasDSL.java @@ -104,6 +104,8 @@ public class AtlasDSL { } public static class Translator { + private static final Logger LOG = LoggerFactory.getLogger(Translator.class); + private final AtlasDSLParser.QueryContext queryContext; private final AtlasTypeRegistry typeRegistry; private final int offset; @@ -123,33 +125,21 @@ public class AtlasDSL { GremlinQueryComposer gremlinQueryComposer = new GremlinQueryComposer(typeRegistry, queryMetadata, limit, offset); DSLVisitor dslVisitor = new DSLVisitor(gremlinQueryComposer); - try { - queryContext.accept(dslVisitor); + queryContext.accept(dslVisitor); - processErrorList(gremlinQueryComposer, null); + processErrorList(gremlinQueryComposer); - return new GremlinQuery(gremlinQueryComposer.get(), queryMetadata.hasSelect()); - } catch (Exception e) { - processErrorList(gremlinQueryComposer, e); - } + String gremlinQuery = gremlinQueryComposer.get(); - return null; + return new GremlinQuery(gremlinQuery, queryMetadata.hasSelect()); } - private void processErrorList(GremlinQueryComposer gremlinQueryComposer, Exception e) throws AtlasBaseException { + private void processErrorList(GremlinQueryComposer gremlinQueryComposer) throws AtlasBaseException { final String errorMessage; if (CollectionUtils.isNotEmpty(gremlinQueryComposer.getErrorList())) { errorMessage = StringUtils.join(gremlinQueryComposer.getErrorList(), ", "); - } else { - errorMessage = e != null ? (e.getMessage() != null ? e.getMessage() : e.toString()) : null; - } - - if (errorMessage != null) { - if (e != null) { - throw new AtlasBaseException(AtlasErrorCode.INVALID_DSL_QUERY, e, this.query, errorMessage); - } - + LOG.warn("DSL Errors: {}", errorMessage); throw new AtlasBaseException(AtlasErrorCode.INVALID_DSL_QUERY, this.query, errorMessage); } } http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/DSLVisitor.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/DSLVisitor.java b/repository/src/main/java/org/apache/atlas/query/DSLVisitor.java index 9a213a3..2f9787e 100644 --- a/repository/src/main/java/org/apache/atlas/query/DSLVisitor.java +++ b/repository/src/main/java/org/apache/atlas/query/DSLVisitor.java @@ -26,15 +26,12 @@ import org.slf4j.LoggerFactory; import java.util.*; -import static org.apache.atlas.query.antlr4.AtlasDSLParser.RULE_whereClause; - public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { private static final Logger LOG = LoggerFactory.getLogger(DSLVisitor.class); private static final String AND = "AND"; private static final String OR = "OR"; - private Set<Integer> visitedRuleIndexes = new HashSet<>(); private final GremlinQueryComposer gremlinQueryComposer; public DSLVisitor(GremlinQueryComposer gremlinQueryComposer) { @@ -42,44 +39,6 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { } @Override - public Void visitSpaceDelimitedQueries(SpaceDelimitedQueriesContext ctx) { - addVisitedRule(ctx.getRuleIndex()); - return super.visitSpaceDelimitedQueries(ctx); - } - - @Override - public Void visitCommaDelimitedQueries(CommaDelimitedQueriesContext ctx) { - addVisitedRule(ctx.getRuleIndex()); - return super.visitCommaDelimitedQueries(ctx); - } - - @Override - public Void visitIsClause(IsClauseContext ctx) { - if (LOG.isDebugEnabled()) { - LOG.debug("=> DSLVisitor.visitIsClause({})", ctx); - } - - if(!hasVisitedRule(RULE_whereClause)) { - gremlinQueryComposer.addFromIsA(ctx.arithE().getText(), ctx.identifier().getText()); - } - - return super.visitIsClause(ctx); - } - - @Override - public Void visitHasClause(HasClauseContext ctx) { - if (LOG.isDebugEnabled()) { - LOG.debug("=> DSLVisitor.visitHasClause({})", ctx); - } - - if(!hasVisitedRule(RULE_whereClause)) { - gremlinQueryComposer.addFromProperty(ctx.arithE().getText(), ctx.identifier().getText()); - } - - return super.visitHasClause(ctx); - } - - @Override public Void visitLimitOffset(LimitOffsetContext ctx) { if (LOG.isDebugEnabled()) { LOG.debug("=> DSLVisitor.visitLimitOffset({})", ctx); @@ -159,7 +118,6 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { LOG.debug("=> DSLVisitor.visitWhereClause({})", ctx); } - addVisitedRule(ctx.getRuleIndex()); ExprContext expr = ctx.expr(); processExpr(expr, gremlinQueryComposer); return super.visitWhereClause(ctx); @@ -171,7 +129,7 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { LOG.debug("=> DSLVisitor.visitFromExpression({})", ctx); } - FromSrcContext fromSrc = ctx.fromSrc(); + FromSrcContext fromSrc = ctx.fromSrc(); AliasExprContext aliasExpr = fromSrc.aliasExpr(); if (aliasExpr != null) { @@ -188,11 +146,13 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { @Override public Void visitSingleQrySrc(SingleQrySrcContext ctx) { - if (!hasVisitedRule(RULE_whereClause)) { - if (ctx.fromExpression() == null) { - if (ctx.expr() != null && gremlinQueryComposer.hasFromClause()) { - processExpr(ctx.expr(), gremlinQueryComposer); - } + if (ctx.fromExpression() == null) { + if (ctx.expr() != null && !gremlinQueryComposer.hasFromClause()) { + inferFromClause(ctx); + } + + if (ctx.expr() != null && gremlinQueryComposer.hasFromClause()) { + processExpr(ctx.expr(), gremlinQueryComposer); } } @@ -210,6 +170,43 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { return super.visitGroupByExpression(ctx); } + private Void visitIsClause(GremlinQueryComposer gqc, IsClauseContext ctx) { + if (LOG.isDebugEnabled()) { + LOG.debug("=> DSLVisitor.visitIsClause({})", ctx); + } + + gqc.addIsA(ctx.arithE().getText(), ctx.identifier().getText()); + return super.visitIsClause(ctx); + } + + private void visitHasClause(GremlinQueryComposer gqc, HasClauseContext ctx) { + if (LOG.isDebugEnabled()) { + LOG.debug("=> DSLVisitor.visitHasClause({})", ctx); + } + + gqc.addFromProperty(ctx.arithE().getText(), ctx.identifier().getText()); + super.visitHasClause(ctx); + } + + private void inferFromClause(SingleQrySrcContext ctx) { + if (ctx.fromExpression() != null) { + return; + } + + if (ctx.expr() != null && gremlinQueryComposer.hasFromClause()) { + return; + } + + if (ctx.expr().compE() != null && ctx.expr().compE().isClause() != null && ctx.expr().compE().isClause().arithE() != null) { + gremlinQueryComposer.addFrom(ctx.expr().compE().isClause().arithE().getText()); + return; + } + + if (ctx.expr().compE() != null && ctx.expr().compE().hasClause() != null && ctx.expr().compE().hasClause().arithE() != null) { + gremlinQueryComposer.addFrom(ctx.expr().compE().hasClause().arithE().getText()); + } + } + private void processExpr(final ExprContext expr, GremlinQueryComposer gremlinQueryComposer) { if (CollectionUtils.isNotEmpty(expr.exprRight())) { processExprRight(expr, gremlinQueryComposer); @@ -281,11 +278,11 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { } if (comparisonClause != null) { - String lhs = comparisonClause.arithE(0).getText(); - String op, rhs; + String lhs = comparisonClause.arithE(0).getText(); + String op, rhs; AtomEContext atomECtx = comparisonClause.arithE(1).multiE().atomE(); if (atomECtx.literal() == null || - (atomECtx.literal() != null && atomECtx.literal().valueArray() == null)) { + (atomECtx.literal() != null && atomECtx.literal().valueArray() == null)) { op = comparisonClause.operator().getText().toUpperCase(); rhs = comparisonClause.arithE(1).getText(); } else { @@ -300,31 +297,23 @@ public class DSLVisitor extends AtlasDSLParserBaseVisitor<Void> { } if (compE != null && compE.isClause() != null) { - gremlinQueryComposer.addFromIsA(compE.isClause().arithE().getText(), compE.isClause().identifier().getText()); + visitIsClause(gremlinQueryComposer, compE.isClause()); } if (compE != null && compE.hasClause() != null) { - gremlinQueryComposer.addFromProperty(compE.hasClause().arithE().getText(), compE.hasClause().identifier().getText()); + visitHasClause(gremlinQueryComposer, compE.hasClause()); } } private String getInClause(AtomEContext atomEContext) { - StringBuilder sb = new StringBuilder(); + StringBuilder sb = new StringBuilder(); ValueArrayContext valueArrayContext = atomEContext.literal().valueArray(); - int startIdx = 1; - int endIdx = valueArrayContext.children.size() - 1; + int startIdx = 1; + int endIdx = valueArrayContext.children.size() - 1; for (int i = startIdx; i < endIdx; i++) { sb.append(valueArrayContext.getChild(i)); } return sb.toString(); } - - private void addVisitedRule(int ruleIndex) { - visitedRuleIndexes.add(ruleIndex); - } - - private boolean hasVisitedRule(int ruleIndex) { - return visitedRuleIndexes.contains(ruleIndex); - } -} +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/GremlinClause.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/GremlinClause.java b/repository/src/main/java/org/apache/atlas/query/GremlinClause.java index 5a4ab4c..a02514d 100644 --- a/repository/src/main/java/org/apache/atlas/query/GremlinClause.java +++ b/repository/src/main/java/org/apache/atlas/query/GremlinClause.java @@ -36,11 +36,11 @@ enum GremlinClause { AND("and(%s)"), NESTED_START("__"), NESTED_HAS_OPERATOR("has('%s', %s(%s))"), - LIMIT("limit(%s)"), + LIMIT("limit(local, %s).limit(%s)"), ORDER_BY("order().by('%s')"), ORDER_BY_DESC("order().by('%s', decr)"), OUT("out('%s')"), - RANGE("range(%s, %s + %s)"), + RANGE("range(local, %s, %s + %s).range(%s, %s + %s)"), SELECT("select('%s')"), TO_LIST("toList()"), TEXT_CONTAINS("has('%s', org.janusgraph.core.attribute.Text.textRegex(%s))"), http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java b/repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java new file mode 100644 index 0000000..7117977 --- /dev/null +++ b/repository/src/main/java/org/apache/atlas/query/GremlinClauseList.java @@ -0,0 +1,101 @@ +/** + * 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.query; + +import org.apache.atlas.type.AtlasEntityType; + +import java.util.LinkedList; +import java.util.List; + +class GremlinClauseList { + private final List<GremlinQueryComposer.GremlinClauseValue> list; + + GremlinClauseList() { + this.list = new LinkedList<>(); + } + + public void add(GremlinQueryComposer.GremlinClauseValue g) { + list.add(g); + } + + public void add(int idx, GremlinQueryComposer.GremlinClauseValue g) { + list.add(idx, g); + } + + public void add(GremlinQueryComposer.GremlinClauseValue g, AtlasEntityType t) { + add(g); + } + + public void add(int idx, GremlinQueryComposer.GremlinClauseValue g, AtlasEntityType t) { + add(idx, g); + } + + public void add(GremlinClause clause, String... args) { + list.add(new GremlinQueryComposer.GremlinClauseValue(clause, clause.get(args))); + } + + public void add(int i, GremlinClause clause, String... args) { + list.add(i, new GremlinQueryComposer.GremlinClauseValue(clause, clause.get(args))); + } + + public GremlinQueryComposer.GremlinClauseValue getAt(int i) { + return list.get(i); + } + + public String getValue(int i) { + return list.get(i).getValue(); + } + + public GremlinQueryComposer.GremlinClauseValue get(int i) { + return list.get(i); + } + + public int size() { + return list.size(); + } + + public int contains(GremlinClause clause) { + for (int i = 0; i < list.size(); i++) { + if (list.get(i).getClause() == clause) + return i; + } + + return -1; + } + + public boolean isEmpty() { + return list.size() == 0 || containsGVLimit(); + } + + private boolean containsGVLimit() { + return list.size() == 3 && + list.get(0).getClause() == GremlinClause.G && + list.get(1).getClause() == GremlinClause.V && + list.get(2).getClause() == GremlinClause.LIMIT; + } + + public void clear() { + list.clear(); + } + + public GremlinQueryComposer.GremlinClauseValue remove(int index) { + GremlinQueryComposer.GremlinClauseValue gcv = get(index); + list.remove(index); + return gcv; + } +} http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java b/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java index 95ba461..0686334 100644 --- a/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java +++ b/repository/src/main/java/org/apache/atlas/query/GremlinQueryComposer.java @@ -18,7 +18,9 @@ package org.apache.atlas.query; import com.google.common.annotations.VisibleForTesting; +import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.exception.AtlasBaseException; +import org.apache.atlas.model.TypeCategory; import org.apache.atlas.model.discovery.SearchParameters; import org.apache.atlas.model.typedef.AtlasStructDef; import org.apache.atlas.type.AtlasEntityType; @@ -88,24 +90,24 @@ public class GremlinQueryComposer { LOG.debug("addFrom(typeName={})", typeName); } - IdentifierHelper.Advice ta = getAdvice(typeName); + IdentifierHelper.IdentifierMetadata ta = getIdMetadata(typeName); if(context.shouldRegister(ta.get())) { context.registerActive(ta.get()); - IdentifierHelper.Advice ia = getAdvice(ta.get()); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(ta.get()); if (ia.isTrait()) { - add(GremlinClause.TRAIT, ia.get()); + add(GremlinClause.TRAIT, ia); } else { if (ia.hasSubtypes()) { add(GremlinClause.HAS_TYPE_WITHIN, ia.getSubTypes()); } else { - add(GremlinClause.HAS_TYPE, ia.get()); + add(GremlinClause.HAS_TYPE, ia); } } } else { - IdentifierHelper.Advice ia = getAdvice(ta.get()); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(ta.get()); introduceType(ia); } } @@ -119,16 +121,16 @@ public class GremlinQueryComposer { addFrom(typeName); } - add(GremlinClause.HAS_PROPERTY, - IdentifierHelper.getQualifiedName(lookup, context, attribute)); + add(GremlinClause.HAS_PROPERTY, getIdMetadata(attribute)); } - public void addFromIsA(String typeName, String traitName) { + public void addIsA(String typeName, String traitName) { if (!isNestedQuery) { addFrom(typeName); } - add(GremlinClause.TRAIT, traitName); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(traitName); + add(GremlinClause.TRAIT, ia); } public void addWhere(String lhs, String operator, String rhs) { @@ -136,64 +138,51 @@ public class GremlinQueryComposer { LOG.debug("addWhere(lhs={}, operator={}, rhs={})", lhs, operator, rhs); } - String currentType = context.getActiveTypeName(); - SearchParameters.Operator op = SearchParameters.Operator.fromString(operator); - IdentifierHelper.Advice org = null; - IdentifierHelper.Advice lhsI = getAdvice(lhs); + String currentType = context.getActiveTypeName(); + IdentifierHelper.IdentifierMetadata org = null; + IdentifierHelper.IdentifierMetadata lhsI = getIdMetadata(lhs); if (!lhsI.isPrimitive()) { introduceType(lhsI); org = lhsI; - lhsI = getAdvice(lhs); + lhsI = getIdMetadata(lhs); } - if (StringUtils.isEmpty(lhsI.getQualifiedName())) { - if (LOG.isDebugEnabled()) { - LOG.debug("unknown identifier '" + lhsI.getRaw() + "'"); - } - - context.getErrorList().add("unknown identifier '" + lhsI.getRaw() + "'"); - } else { - if (lhsI.isDate()) { - rhs = parseDate(rhs); - } - - rhs = addQuotesIfNecessary(rhs); - if (op == SearchParameters.Operator.LIKE) { - add(GremlinClause.TEXT_CONTAINS, lhsI.getQualifiedName(), getFixedRegEx(rhs)); - } else if (op == SearchParameters.Operator.IN) { - add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), "within", rhs); - } else { - add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), op.getSymbols()[1], rhs); - } + if (!context.validator.isValidQualifiedName(lhsI.getQualifiedName(), lhsI.getRaw())) { + return; + } - if (org != null && org.getIntroduceType()) { - add(GremlinClause.DEDUP); - add(GremlinClause.IN, org.getEdgeLabel()); - context.registerActive(currentType); - } + if (lhsI.isDate()) { + rhs = parseDate(rhs); } - } - private String getQualifiedName(IdentifierHelper.Advice ia) { - String s = ia.getQualifiedName(); - if(StringUtils.isEmpty(s)) { - s = ia.getRaw(); - getErrorList().add(String.format("Error: %s is invalid", ia.getRaw())); + SearchParameters.Operator op = SearchParameters.Operator.fromString(operator); + rhs = addQuotesIfNecessary(rhs); + if (op == SearchParameters.Operator.LIKE) { + add(GremlinClause.TEXT_CONTAINS, lhsI.getQualifiedName(), IdentifierHelper.getFixedRegEx(rhs)); + } else if (op == SearchParameters.Operator.IN) { + add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), "within", rhs); + } else { + add(GremlinClause.HAS_OPERATOR, lhsI.getQualifiedName(), op.getSymbols()[1], rhs); } - return s; + if (org != null && org.isReferredType()) { + add(GremlinClause.DEDUP); + add(GremlinClause.IN, org.getEdgeLabel()); + context.registerActive(currentType); + } } - private String getFixedRegEx(String rhs) { - return rhs.replace("*", ".*").replace('?', '.'); + private String getQualifiedName(IdentifierHelper.IdentifierMetadata ia) { + return context.validator.isValidQualifiedName(ia.getQualifiedName(), ia.getRaw()) ? + ia.getQualifiedName() : ia.getRaw(); } public void addAndClauses(List<String> clauses) { - queryClauses.add(GremlinClause.AND, String.join(",", clauses)); + add(GremlinClause.AND, String.join(",", clauses)); } public void addOrClauses(List<String> clauses) { - queryClauses.add(GremlinClause.OR, String.join(",", clauses)); + add(GremlinClause.OR, String.join(",", clauses)); } public void addSelect(SelectClauseComposer selectClauseComposer) { @@ -214,7 +203,12 @@ public class GremlinQueryComposer { } for (int i = 0; i < scc.getItems().length; i++) { - IdentifierHelper.Advice ia = getAdvice(scc.getItem(i)); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(scc.getItem(i)); + + if(scc.isAggregatorWithArgument(i) && !ia.isPrimitive()) { + context.check(false, AtlasErrorCode.INVALID_DSL_SELECT_INVALID_AGG, ia.getQualifiedName()); + return; + } if (!scc.getItem(i).equals(scc.getLabel(i))) { context.addAlias(scc.getLabel(i), getQualifiedName(ia)); @@ -225,25 +219,29 @@ public class GremlinQueryComposer { } scc.isSelectNoop = hasNoopCondition(ia); - if(scc.isSelectNoop) { + if (scc.isSelectNoop) { return; } if (introduceType(ia)) { + scc.incrementTypesIntroduced(); scc.isSelectNoop = !ia.hasParts(); - if(ia.hasParts()) { - scc.assign(i, getQualifiedName(getAdvice(ia.get())), GremlinClause.INLINE_GET_PROPERTY); + if (ia.hasParts()) { + scc.assign(i, getQualifiedName(getIdMetadata(ia.get())), GremlinClause.INLINE_GET_PROPERTY); } } else { scc.assign(i, getQualifiedName(ia), GremlinClause.INLINE_GET_PROPERTY); + scc.incrementPrimitiveType(); } } + + context.validator.check(!scc.hasMultipleReferredTypes(), + AtlasErrorCode.INVALID_DSL_SELECT_REFERRED_ATTR, Integer.toString(scc.getIntroducedTypesCount())); + context.validator.check(!scc.hasMixedAttributes(), AtlasErrorCode.INVALID_DSL_SELECT_ATTR_MIXING); } - private boolean hasNoopCondition(IdentifierHelper.Advice ia) { - return ia.isPrimitive() == false && - ia.isAttribute() == false && - context.hasAlias(ia.getRaw()); + private boolean hasNoopCondition(IdentifierHelper.IdentifierMetadata ia) { + return !ia.isPrimitive() && !ia.isAttribute() && context.hasAlias(ia.getRaw()); } public GremlinQueryComposer createNestedProcessor() { @@ -284,7 +282,7 @@ public class GremlinQueryComposer { } if (offset.equalsIgnoreCase("0")) { - add(GremlinClause.LIMIT, limit); + add(GremlinClause.LIMIT, limit, limit); } else { addRangeClause(offset, limit); } @@ -342,7 +340,7 @@ public class GremlinQueryComposer { LOG.debug("addOrderBy(name={}, isDesc={})", name, isDesc); } - IdentifierHelper.Advice ia = getAdvice(name); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(name); if (queryMetadata.hasSelect() && queryMetadata.hasGroupBy()) { addSelectTransformation(this.context.selectClauseComposer, getQualifiedName(ia), isDesc); } else if (queryMetadata.hasGroupBy()) { @@ -370,7 +368,7 @@ public class GremlinQueryComposer { GremlinClause.SELECT_FN; } if (StringUtils.isEmpty(orderByQualifiedAttrName)) { - queryClauses.add(0, fn, + add(0, fn, selectClauseComposer.getLabelHeader(), selectClauseComposer.hasAssignmentExpr() ? selectClauseComposer.getAssignmentExprString(): EMPTY_STRING, selectClauseComposer.getItemsString(), EMPTY_STRING); @@ -381,14 +379,15 @@ public class GremlinQueryComposer { sortClause = isDesc ? GremlinClause.INLINE_SORT_DESC : GremlinClause.INLINE_SORT_ASC; } String idxStr = String.valueOf(itemIdx); - queryClauses.add(0, fn, + add(0, fn, selectClauseComposer.getLabelHeader(), selectClauseComposer.hasAssignmentExpr() ? selectClauseComposer.getAssignmentExprString(): EMPTY_STRING, selectClauseComposer.getItemsString(), sortClause.get(idxStr, idxStr) ); } - queryClauses.add(GremlinClause.INLINE_TRANSFORM_CALL); + + add(GremlinClause.INLINE_TRANSFORM_CALL); } private String addQuotesIfNecessary(String rhs) { @@ -410,7 +409,7 @@ public class GremlinQueryComposer { try { return DSL_DATE_FORMAT.get().parse(s).getTime(); } catch (ParseException ex) { - context.errorList.add(ex.getMessage()); + context.validator.check(ex, AtlasErrorCode.INVALID_DSL_INVALID_DATE); } return -1; @@ -453,36 +452,28 @@ public class GremlinQueryComposer { } } - private boolean introduceType(IdentifierHelper.Advice ia) { - if (ia.getIntroduceType()) { + private boolean introduceType(IdentifierHelper.IdentifierMetadata ia) { + if (ia.isReferredType()) { add(GremlinClause.OUT, ia.getEdgeLabel()); context.registerActive(ia); } - return ia.getIntroduceType(); + return ia.isReferredType(); } - private IdentifierHelper.Advice getAdvice(String actualTypeName) { + private IdentifierHelper.IdentifierMetadata getIdMetadata(String actualTypeName) { return IdentifierHelper.create(context, lookup, actualTypeName); } - private void add(GremlinClause clause, String... args) { - queryClauses.add(new GremlinClauseValue(clause, clause.get(args))); - } - - private void add(int idx, GremlinClause clause, String... args) { - queryClauses.add(idx, new GremlinClauseValue(clause, clause.get(args))); - } - private void addRangeClause(String startIndex, String endIndex) { if (LOG.isDebugEnabled()) { LOG.debug("addRangeClause(startIndex={}, endIndex={})", startIndex, endIndex); } if (queryMetadata.hasSelect()) { - add(queryClauses.size() - 1, GremlinClause.RANGE, startIndex, startIndex, endIndex); + add(queryClauses.size() - 1, GremlinClause.RANGE, startIndex, startIndex, endIndex, startIndex, startIndex, endIndex); } else { - add(GremlinClause.RANGE, startIndex, startIndex, endIndex); + add(GremlinClause.RANGE, startIndex, startIndex, endIndex, startIndex, startIndex, endIndex); } } @@ -491,8 +482,8 @@ public class GremlinQueryComposer { LOG.debug("addOrderByClause(name={})", name, descr); } - IdentifierHelper.Advice ia = getAdvice(name); - add((!descr) ? GremlinClause.ORDER_BY : GremlinClause.ORDER_BY_DESC, getQualifiedName(ia)); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(name); + add((!descr) ? GremlinClause.ORDER_BY : GremlinClause.ORDER_BY_DESC, ia); } private void addGroupByClause(String name) { @@ -500,8 +491,8 @@ public class GremlinQueryComposer { LOG.debug("addGroupByClause(name={})", name); } - IdentifierHelper.Advice ia = getAdvice(name); - add(GremlinClause.GROUP_BY, getQualifiedName(ia)); + IdentifierHelper.IdentifierMetadata ia = getIdMetadata(name); + add(GremlinClause.GROUP_BY, ia); } public boolean hasFromClause() { @@ -509,7 +500,23 @@ public class GremlinQueryComposer { queryClauses.contains(GremlinClause.HAS_TYPE_WITHIN) != -1; } - private static class GremlinClauseValue { + private void add(GremlinClause clause, IdentifierHelper.IdentifierMetadata ia) { + if(context != null && !context.validator.isValid(context, clause, ia)) { + return; + } + + add(clause, (ia.getQualifiedName() == null ? ia.get() : ia.getQualifiedName())); + } + + private void add(GremlinClause clause, String... args) { + queryClauses.add(new GremlinClauseValue(clause, clause.get(args))); + } + + private void add(int idx, GremlinClause clause, String... args) { + queryClauses.add(idx, new GremlinClauseValue(clause, clause.get(args))); + } + + static class GremlinClauseValue { private final GremlinClause clause; private final String value; @@ -527,124 +534,42 @@ public class GremlinQueryComposer { } } - private static class GremlinClauseList { - private final List<GremlinClauseValue> list; - - private GremlinClauseList() { - this.list = new LinkedList<>(); - } - - public void add(GremlinClauseValue g) { - list.add(g); - } - - public void add(int idx, GremlinClauseValue g) { - list.add(idx, g); - } - - public void add(GremlinClauseValue g, AtlasEntityType t) { - add(g); - } - - public void add(int idx, GremlinClauseValue g, AtlasEntityType t) { - add(idx, g); - } - - public void add(GremlinClause clause, String... args) { - list.add(new GremlinClauseValue(clause, clause.get(args))); - } - - public void add(int i, GremlinClause clause, String... args) { - list.add(i, new GremlinClauseValue(clause, clause.get(args))); - } - - public GremlinClauseValue getAt(int i) { - return list.get(i); - } - - public String getValue(int i) { - return list.get(i).value; - } - - public GremlinClauseValue get(int i) { - return list.get(i); - } - - public int size() { - return list.size(); - } - - public int contains(GremlinClause clause) { - for (int i = 0; i < list.size(); i++) { - if (list.get(i).getClause() == clause) - return i; - } - - return -1; - } - - public boolean isEmpty() { - return list.size() == 0 || containsGVLimit(); - } - - private boolean containsGVLimit() { - return list.size() == 3 && - list.get(0).clause == GremlinClause.G && - list.get(1).clause == GremlinClause.V && - list.get(2).clause == GremlinClause.LIMIT; - } - - public void clear() { - list.clear(); - } - - public GremlinClauseValue remove(int index) { - GremlinClauseValue gcv = get(index); - list.remove(index); - return gcv; - } - } - @VisibleForTesting static class Context { private static final AtlasStructType UNKNOWN_TYPE = new AtlasStructType(new AtlasStructDef()); private final Lookup lookup; - private final List<String> errorList = new ArrayList<>(); private final Map<String, String> aliasMap = new HashMap<>(); private AtlasType activeType; private SelectClauseComposer selectClauseComposer; + private ClauseValidator validator; public Context(Lookup lookup) { this.lookup = lookup; + validator = new ClauseValidator(lookup); } public void registerActive(String typeName) { if(shouldRegister(typeName)) { try { activeType = lookup.getType(typeName); - aliasMap.put(typeName, typeName); } catch (AtlasBaseException e) { - errorList.add(e.getMessage()); + validator.check(e, AtlasErrorCode.INVALID_DSL_UNKNOWN_TYPE, typeName); activeType = UNKNOWN_TYPE; } } } - public void registerActive(IdentifierHelper.Advice advice) { - if (StringUtils.isNotEmpty(advice.getTypeName())) { - registerActive(advice.getTypeName()); + public void registerActive(IdentifierHelper.IdentifierMetadata identifierMetadata) { + if (validator.check(StringUtils.isNotEmpty(identifierMetadata.getTypeName()), + AtlasErrorCode.INVALID_DSL_UNKNOWN_TYPE, identifierMetadata.getRaw())) { + registerActive(identifierMetadata.getTypeName()); } else { - errorList.add("unknown identifier '" + advice.getRaw() + "'"); activeType = UNKNOWN_TYPE; } } - public AtlasType getActiveType() { - return activeType; - } - public AtlasEntityType getActiveEntityType() { return (activeType instanceof AtlasEntityType) ? (AtlasEntityType) activeType : @@ -655,6 +580,10 @@ public class GremlinQueryComposer { return activeType.getTypeName(); } + public AtlasType getActiveType() { + return activeType; + } + public boolean shouldRegister(String typeName) { return activeType == null || (activeType != null && !StringUtils.equals(getActiveTypeName(), typeName)) && @@ -683,7 +612,7 @@ public class GremlinQueryComposer { public void addAlias(String alias, String typeName) { if(aliasMap.containsKey(alias)) { - errorList.add(String.format("Duplicate alias found: %s for type %s already present.", alias, getActiveEntityType())); + check(false, AtlasErrorCode.INVALID_DSL_DUPLICATE_ALIAS, alias, getActiveTypeName()); return; } @@ -691,7 +620,81 @@ public class GremlinQueryComposer { } public List<String> getErrorList() { + return validator.getErrorList(); + } + + public boolean error(AtlasBaseException e, AtlasErrorCode ec, String t, String name) { + return validator.check(e, ec, t, name); + } + + public boolean check(boolean condition, AtlasErrorCode vm, String... args) { + return validator.check(condition, vm, args); + } + } + + private static class ClauseValidator { + private final Lookup lookup; + List<String> errorList = new ArrayList<>(); + + public ClauseValidator(Lookup lookup) { + this.lookup = lookup; + } + + public boolean isValid(Context ctx, GremlinClause clause, IdentifierHelper.IdentifierMetadata ia) { + switch (clause) { + case TRAIT: + return check(ia.isTrait(), AtlasErrorCode.INVALID_DSL_UNKNOWN_CLASSIFICATION, ia.getRaw()); + + case HAS_TYPE: + TypeCategory typeCategory = ctx.getActiveType().getTypeCategory(); + return check(StringUtils.isNotEmpty(ia.getTypeName()) && + typeCategory == TypeCategory.CLASSIFICATION || typeCategory == TypeCategory.ENTITY, + AtlasErrorCode.INVALID_DSL_UNKNOWN_TYPE, ia.getRaw()); + + case HAS_PROPERTY: + return check(ia.isPrimitive(), AtlasErrorCode.INVALID_DSL_HAS_PROPERTY, ia.getRaw()); + + case ORDER_BY: + return check(ia.isPrimitive(), AtlasErrorCode.INVALID_DSL_ORDERBY, ia.getRaw()); + + case GROUP_BY: + return check(ia.isPrimitive(), AtlasErrorCode.INVALID_DSL_SELECT_INVALID_AGG, ia.getRaw()); + + default: + return (getErrorList().size() == 0); + } + } + + public boolean check(Exception ex, AtlasErrorCode vm, String... args) { + String[] extraArgs = getExtraSlotArgs(args, ex.getMessage()); + return check(false, vm, extraArgs); + } + + private String[] getExtraSlotArgs(String[] args, String s) { + String[] argsPlus1 = new String[args.length + 1]; + System.arraycopy(args, 0, argsPlus1, 0, args.length); + argsPlus1[args.length] = s; + return argsPlus1; + } + + public boolean check(boolean condition, AtlasErrorCode vm, String... args) { + if(!condition) { + addError(vm, args); + } + + return condition; + } + + public void addError(AtlasErrorCode ec, String... messages) { + errorList.add(ec.getFormattedErrorMessage(messages)); + } + + public List<String> getErrorList() { return errorList; } + + public boolean isValidQualifiedName(String qualifiedName, String raw) { + return check(StringUtils.isNotEmpty(qualifiedName), AtlasErrorCode.INVALID_DSL_QUALIFIED_NAME, raw); + } } } http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java b/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java index ab91800..87a8dc7 100644 --- a/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java +++ b/repository/src/main/java/org/apache/atlas/query/IdentifierHelper.java @@ -6,9 +6,9 @@ * 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 - * + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> * 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. @@ -18,6 +18,7 @@ package org.apache.atlas.query; +import org.apache.atlas.AtlasErrorCode; import org.apache.atlas.exception.AtlasBaseException; import org.apache.commons.lang.StringUtils; @@ -46,10 +47,10 @@ public class IdentifierHelper { return ret; } - public static Advice create(GremlinQueryComposer.Context context, - org.apache.atlas.query.Lookup lookup, - String identifier) { - Advice ia = new Advice(identifier); + public static IdentifierMetadata create(GremlinQueryComposer.Context context, + org.apache.atlas.query.Lookup lookup, + String identifier) { + IdentifierMetadata ia = new IdentifierMetadata(identifier); ia.update(lookup, context); return ia; } @@ -65,7 +66,7 @@ public class IdentifierHelper { try { return lookup.getQualifiedName(context, name); } catch (AtlasBaseException e) { - context.getErrorList().add(String.format("Error for %s.%s: %s", context.getActiveTypeName(), name, e.getMessage())); + context.error(e, AtlasErrorCode.INVALID_DSL_QUALIFIED_NAME, context.getActiveTypeName(), name); } return ""; @@ -100,24 +101,29 @@ public class IdentifierHelper { return rhs.equalsIgnoreCase("true") || rhs.equalsIgnoreCase("false"); } - public static class Advice { - private String raw; - private String actual; + public static String getFixedRegEx(String s) { + return s.replace("*", ".*").replace('?', '.'); + } + + + public static class IdentifierMetadata { + private String raw; + private String actual; private String[] parts; - private String typeName; - private String attributeName; - private boolean isPrimitive; - private String edgeLabel; - private boolean introduceType; - private boolean hasSubtypes; - private String subTypes; - private boolean isTrait; - private boolean newContext; - private boolean isAttribute; - private String qualifiedName; - private boolean isDate; - - public Advice(String s) { + private String typeName; + private String attributeName; + private boolean isPrimitive; + private String edgeLabel; + private boolean introduceType; + private boolean hasSubtypes; + private String subTypes; + private boolean isTrait; + private boolean newContext; + private boolean isAttribute; + private String qualifiedName; + private boolean isDate; + + public IdentifierMetadata(String s) { this.raw = removeQuotes(s); this.actual = IdentifierHelper.get(raw); } @@ -132,7 +138,7 @@ public class IdentifierHelper { updateParts(); updateTypeInfo(lookup, context); - isTrait = lookup.isTraitType(context); + setIsTrait(context, lookup, attributeName); updateEdgeInfo(lookup, context); introduceType = !isPrimitive() && !context.hasAlias(parts[0]); updateSubTypes(lookup, context); @@ -142,41 +148,45 @@ public class IdentifierHelper { } } + private void setIsTrait(GremlinQueryComposer.Context ctx, Lookup lookup, String s) { + isTrait = lookup.isTraitType(s); + } + private void updateSubTypes(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { - if(isTrait) { + if (isTrait) { return; } hasSubtypes = lookup.doesTypeHaveSubTypes(context); - if(hasSubtypes) { + if (hasSubtypes) { subTypes = lookup.getTypeAndSubTypes(context); } } private void updateEdgeInfo(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { - if(isPrimitive == false && isTrait == false) { + if (!isPrimitive && !isTrait && typeName != attributeName) { edgeLabel = lookup.getRelationshipEdgeLabel(context, attributeName); typeName = lookup.getTypeFromEdge(context, attributeName); } } private void updateTypeInfo(org.apache.atlas.query.Lookup lookup, GremlinQueryComposer.Context context) { - if(parts.length == 1) { + if (parts.length == 1) { typeName = context.hasAlias(parts[0]) ? - context.getTypeNameFromAlias(parts[0]) : - context.getActiveTypeName(); + context.getTypeNameFromAlias(parts[0]) : + context.getActiveTypeName(); qualifiedName = getDefaultQualifiedNameForSinglePartName(context, parts[0]); attributeName = parts[0]; } - if(parts.length == 2) { + if (parts.length == 2) { boolean isAttrOfActiveType = lookup.hasAttribute(context, parts[0]); - if(isAttrOfActiveType) { + if (isAttrOfActiveType) { attributeName = parts[0]; } else { typeName = context.hasAlias(parts[0]) ? - context.getTypeNameFromAlias(parts[0]) : - parts[0]; + context.getTypeNameFromAlias(parts[0]) : + parts[0]; attributeName = parts[1]; } @@ -190,7 +200,7 @@ public class IdentifierHelper { private String getDefaultQualifiedNameForSinglePartName(GremlinQueryComposer.Context context, String s) { String qn = context.getTypeNameFromAlias(s); - if(StringUtils.isEmpty(qn) && SelectClauseComposer.isKeyword(s)) { + if (StringUtils.isEmpty(qn) && SelectClauseComposer.isKeyword(s)) { return s; } @@ -198,22 +208,17 @@ public class IdentifierHelper { } private void setQualifiedName(Lookup lookup, GremlinQueryComposer.Context context, boolean isAttribute, String attrName) { - if(isAttribute) { + if (isAttribute) { qualifiedName = getQualifiedName(lookup, context, attrName); } } private String getQualifiedName(Lookup lookup, GremlinQueryComposer.Context context, String name) { - try { - return lookup.getQualifiedName(context, name); - } catch (AtlasBaseException e) { - context.getErrorList().add(String.format("Error for %s.%s: %s", context.getActiveTypeName(), name, e.getMessage())); - return ""; - } + return IdentifierHelper.getQualifiedName(lookup, context, name); } private void setIsDate(Lookup lookup, GremlinQueryComposer.Context context, boolean isPrimitive, String attrName) { - if(isPrimitive) { + if (isPrimitive) { isDate = lookup.isDate(context, attrName); } } @@ -246,7 +251,7 @@ public class IdentifierHelper { return typeName; } - public boolean getIntroduceType() { + public boolean isReferredType() { return introduceType; } @@ -277,5 +282,6 @@ public class IdentifierHelper { public String getRaw() { return raw; } + } } http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/Lookup.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/Lookup.java b/repository/src/main/java/org/apache/atlas/query/Lookup.java index 9a7d474..7a189fe 100644 --- a/repository/src/main/java/org/apache/atlas/query/Lookup.java +++ b/repository/src/main/java/org/apache/atlas/query/Lookup.java @@ -21,9 +21,6 @@ package org.apache.atlas.query; import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.type.AtlasType; -import java.util.Collection; -import java.util.List; - public interface Lookup { AtlasType getType(String typeName) throws AtlasBaseException; @@ -39,7 +36,7 @@ public interface Lookup { String getTypeAndSubTypes(GremlinQueryComposer.Context context); - boolean isTraitType(GremlinQueryComposer.Context context); + boolean isTraitType(String s); String getTypeFromEdge(GremlinQueryComposer.Context context, String item); http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java b/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java index eef3463..eec5216 100644 --- a/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java +++ b/repository/src/main/java/org/apache/atlas/query/RegistryBasedLookup.java @@ -20,20 +20,27 @@ package org.apache.atlas.query; import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.TypeCategory; -import org.apache.atlas.model.instance.AtlasObjectId; import org.apache.atlas.model.typedef.AtlasBaseTypeDef; +import org.apache.atlas.repository.Constants; import org.apache.atlas.type.*; import org.apache.commons.lang.StringUtils; -import java.util.ArrayList; -import java.util.List; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; class RegistryBasedLookup implements Lookup { - private final List<String> errorList; + private static final Set<String> SYSTEM_ATTRIBUTES = new HashSet<>( + Arrays.asList(Constants.GUID_PROPERTY_KEY, + Constants.MODIFIED_BY_KEY, + Constants.CREATED_BY_KEY, + Constants.STATE_PROPERTY_KEY, + Constants.TIMESTAMP_PROPERTY_KEY, + Constants.MODIFICATION_TIMESTAMP_PROPERTY_KEY)); + private final AtlasTypeRegistry typeRegistry; public RegistryBasedLookup(AtlasTypeRegistry typeRegistry) { - this.errorList = new ArrayList<>(); this.typeRegistry = typeRegistry; } @@ -49,7 +56,15 @@ class RegistryBasedLookup implements Lookup { return ""; } - return et.getQualifiedAttributeName(name); + if(isSystemAttribute(name)) { + return name; + } else { + return et.getQualifiedAttributeName(name); + } + } + + private boolean isSystemAttribute(String s) { + return SYSTEM_ATTRIBUTES.contains(s); } @Override @@ -59,6 +74,10 @@ class RegistryBasedLookup implements Lookup { return false; } + if(isSystemAttribute(attributeName)) { + return true; + } + AtlasType at = et.getAttributeType(attributeName); if(at == null) { return false; @@ -97,7 +116,8 @@ class RegistryBasedLookup implements Lookup { @Override public boolean hasAttribute(GremlinQueryComposer.Context context, String typeName) { - return (context.getActiveEntityType() != null) && context.getActiveEntityType().getAttribute(typeName) != null; + return (context.getActiveEntityType() != null) && + (isSystemAttribute(typeName) || context.getActiveEntityType().getAttribute(typeName) != null); } @Override @@ -123,9 +143,19 @@ class RegistryBasedLookup implements Lookup { } @Override - public boolean isTraitType(GremlinQueryComposer.Context context) { - return (context.getActiveType() != null && - context.getActiveType().getTypeCategory() == TypeCategory.CLASSIFICATION); + public boolean isTraitType(String typeName) { + AtlasType t = null; + try { + t = typeRegistry.getType(typeName); + } catch (AtlasBaseException e) { + return false; + } + + return isTraitType(t); + } + + private boolean isTraitType(AtlasType t) { + return (t != null && t.getTypeCategory() == TypeCategory.CLASSIFICATION); } @Override http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java ---------------------------------------------------------------------- diff --git a/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java b/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java index b7f6d3a..cdb460a 100644 --- a/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java +++ b/repository/src/main/java/org/apache/atlas/query/SelectClauseComposer.java @@ -41,9 +41,18 @@ class SelectClauseComposer { private int maxIdx = -1; private int minIdx = -1; private int aggCount = 0; + private int introducedTypesCount = 0; + private int primitiveTypeCount = 0; public SelectClauseComposer() {} + public static boolean isKeyword(String s) { + return COUNT_STR.equals(s) || + MIN_STR.equals(s) || + MAX_STR.equals(s) || + SUM_STR.equals(s); + } + public String[] getItems() { return items; } @@ -73,13 +82,6 @@ class SelectClauseComposer { return ret; } - public static boolean isKeyword(String s) { - return COUNT_STR.equals(s) || - MIN_STR.equals(s) || - MAX_STR.equals(s) || - SUM_STR.equals(s); - } - public String[] getAttributes() { return attributes; } @@ -164,7 +166,7 @@ class SelectClauseComposer { return assign(items[i], inline.get(s, clause.get(p1, p1))); } - private int getCountIdx() { + public int getCountIdx() { return countIdx; } @@ -173,7 +175,7 @@ class SelectClauseComposer { aggCount++; } - private int getSumIdx() { + public int getSumIdx() { return sumIdx; } @@ -182,7 +184,7 @@ class SelectClauseComposer { aggCount++; } - private int getMaxIdx() { + public int getMaxIdx() { return maxIdx; } @@ -191,7 +193,7 @@ class SelectClauseComposer { aggCount++; } - private int getMinIdx() { + public int getMinIdx() { return minIdx; } @@ -207,4 +209,32 @@ class SelectClauseComposer { .forEach(joiner::add); return joiner.toString(); } + + public boolean isAggregatorWithArgument(int i) { + return i == getMaxIdx() || i == getMinIdx() || i == getSumIdx(); + } + + public void incrementTypesIntroduced() { + introducedTypesCount++; + } + + public int getIntroducedTypesCount() { + return introducedTypesCount; + } + + public void incrementPrimitiveType() { + primitiveTypeCount++; + } + + public boolean hasMultipleReferredTypes() { + return getIntroducedTypesCount() > 1; + } + + public boolean hasMixedAttributes() { + return getIntroducedTypesCount() > 0 && getPrimitiveTypeCount() > 0; + } + + private int getPrimitiveTypeCount() { + return primitiveTypeCount; + } } http://git-wip-us.apache.org/repos/asf/atlas/blob/c35f82ca/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java ---------------------------------------------------------------------- diff --git a/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java b/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java index c44eea3..98cd5a9 100644 --- a/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java +++ b/repository/src/test/java/org/apache/atlas/query/DSLQueriesTest.java @@ -23,10 +23,13 @@ import org.apache.atlas.exception.AtlasBaseException; import org.apache.atlas.model.discovery.AtlasSearchResult; import org.apache.atlas.runner.LocalSolrRunner; import org.apache.commons.collections.CollectionUtils; -import org.testng.annotations.*; +import org.testng.annotations.AfterClass; +import org.testng.annotations.BeforeClass; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Guice; +import org.testng.annotations.Test; import javax.inject.Inject; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -39,6 +42,7 @@ import static org.testng.Assert.assertTrue; @Guice(modules = TestModules.TestOnlyModule.class) public class DSLQueriesTest extends BasicTestSetup { + private final int DEFAULT_LIMIT = 25; @Inject private EntityDiscoveryService discoveryService; @@ -61,7 +65,7 @@ public class DSLQueriesTest extends BasicTestSetup { {"Person where (birthday >= \"1975-01-01T02:35:58.440Z\" )", 2}, {"Person where (birthday <= \"1950-01-01T02:35:58.440Z\" )", 0}, {"Person where (birthday = \"1975-01-01T02:35:58.440Z\" )", 0}, - {"Person where (birthday != \"1975-01-01T02:35:58.440Z\" )", 0}, + {"Person where (birthday != \"1975-01-01T02:35:58.440Z\" )", 4}, {"Person where (hasPets = true)", 2}, {"Person where (hasPets = false)", 2}, @@ -74,7 +78,7 @@ public class DSLQueriesTest extends BasicTestSetup { {"Person where (numberOfCars < 2)", 3}, {"Person where (numberOfCars <= 2)", 4}, {"Person where (numberOfCars = 2)", 1}, - {"Person where (numberOfCars != 2)", 0}, + {"Person where (numberOfCars != 2)", 3}, {"Person where (houseNumber > 0)", 2}, {"Person where (houseNumber > 17)", 1}, @@ -82,7 +86,7 @@ public class DSLQueriesTest extends BasicTestSetup { {"Person where (houseNumber < 153)", 3}, {"Person where (houseNumber <= 153)", 4}, {"Person where (houseNumber = 17)", 1}, - {"Person where (houseNumber != 17)", 0}, + {"Person where (houseNumber != 17)", 3}, {"Person where (carMileage > 0)", 2}, {"Person where (carMileage > 13)", 1}, @@ -90,7 +94,7 @@ public class DSLQueriesTest extends BasicTestSetup { {"Person where (carMileage < 13364)", 3}, {"Person where (carMileage <= 13364)", 4}, {"Person where (carMileage = 13)", 1}, - {"Person where (carMileage != 13)", 0}, + {"Person where (carMileage != 13)", 3}, {"Person where (age > 36)", 1}, {"Person where (age > 49)", 1}, @@ -98,17 +102,17 @@ public class DSLQueriesTest extends BasicTestSetup { {"Person where (age < 50)", 3}, {"Person where (age <= 35)", 2}, {"Person where (age = 35)", 0}, - {"Person where (age != 35)", 0} + {"Person where (age != 35)", 4} }; } @Test(dataProvider = "comparisonQueriesProvider") public void comparison(String query, int expected) throws AtlasBaseException { - AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, 25, 0); - assertSearchResult(searchResult, expected); + AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, DEFAULT_LIMIT, 0); + assertSearchResult(searchResult, expected, query); - AtlasSearchResult searchResult2 = discoveryService.searchUsingDslQuery(query.replace("where", " "), 25, 0); - assertSearchResult(searchResult2, expected); + AtlasSearchResult searchResult2 = discoveryService.searchUsingDslQuery(query.replace("where", " "), DEFAULT_LIMIT, 0); + assertSearchResult(searchResult2, expected, query); } @DataProvider(name = "basicProvider") @@ -149,13 +153,29 @@ public class DSLQueriesTest extends BasicTestSetup { @Test(dataProvider = "basicProvider") public void basic(String query, int expected) throws AtlasBaseException { - queryAssert(query, expected); - queryAssert(query.replace("where", " "), expected); + queryAssert(query, expected, DEFAULT_LIMIT, 0); + queryAssert(query.replace("where", " "), expected, DEFAULT_LIMIT, 0); + } + + @DataProvider(name = "systemAttributesProvider") + private Object[][] systemAttributesQueries() { + return new Object[][]{ + {"hive_db has __state", 3}, + {"hive_db where hive_db has __state", 3}, + {"hive_db as d where d.__state = 'ACTIVE'", 3}, + {"hive_db select __guid", 3}, + {"hive_db where __state = 'ACTIVE' select name, __guid, __state", 3}, + }; + } + + @Test(dataProvider = "systemAttributesProvider") + public void systemAttributes(String query, int expected) throws AtlasBaseException { + queryAssert(query, expected, DEFAULT_LIMIT, 0); } - private void queryAssert(String query, int expected) throws AtlasBaseException { - AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, 25, 0); - assertSearchResult(searchResult, expected); + private void queryAssert(String query, final int expected, final int limit, final int offset) throws AtlasBaseException { + AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, limit, offset); + assertSearchResult(searchResult, expected, query); } @DataProvider(name = "limitProvider") @@ -172,12 +192,12 @@ public class DSLQueriesTest extends BasicTestSetup { @Test(dataProvider = "limitProvider") public void limit(String query, int expected, int limit, int offset) throws AtlasBaseException { - queryAssert(query, expected); - queryAssert(query.replace("where", " "), expected); + queryAssert(query, expected, limit, offset); + queryAssert(query.replace("where", " "), expected, limit, offset); } - @DataProvider(name = "syntaxVerifierProvider") - private Object[][] syntaxVerifierQueries() { + @DataProvider(name = "syntaxProvider") + private Object[][] syntaxQueries() { return new Object[][]{ {"hive_column limit 10 ", 10}, {"hive_column select hive_column.qualifiedName limit 10 ", 10}, @@ -264,10 +284,10 @@ public class DSLQueriesTest extends BasicTestSetup { }; } - @Test(dataProvider = "syntaxVerifierProvider") + @Test(dataProvider = "syntaxProvider") public void syntax(String query, int expected) throws AtlasBaseException { - queryAssert(query, expected); - queryAssert(query.replace("where", " "), expected); + queryAssert(query, expected, DEFAULT_LIMIT, 0); + queryAssert(query.replace("where", " "), expected, DEFAULT_LIMIT, 0); } @DataProvider(name = "orderByProvider") @@ -346,8 +366,8 @@ public class DSLQueriesTest extends BasicTestSetup { @Test(dataProvider = "orderByProvider") public void orderBy(String query, int expected, String orderBy, boolean ascending) throws AtlasBaseException { - queryAssert(query, expected); - queryAssert(query.replace("where", " "), expected); + queryAssert(query, expected, DEFAULT_LIMIT, 0); + queryAssert(query.replace("where", " "), expected, DEFAULT_LIMIT, 0); } @DataProvider(name = "likeQueriesProvider") @@ -365,8 +385,8 @@ public class DSLQueriesTest extends BasicTestSetup { @Test(dataProvider = "likeQueriesProvider") public void likeQueries(String query, int expected) throws AtlasBaseException { - queryAssert(query, expected); - queryAssert(query.replace("where", " "), expected); + queryAssert(query, expected, DEFAULT_LIMIT, 0); + queryAssert(query.replace("where", " "), expected, DEFAULT_LIMIT, 0); } @DataProvider(name = "minMaxCountProvider") @@ -468,16 +488,16 @@ public class DSLQueriesTest extends BasicTestSetup { new FieldValueValidator() .withFieldNames("'count'", "'sum'") .withExpectedValues(4, 86) }, -// { "from hive_db groupby (owner) select min(name) orderby name limit 2 ", -// new FieldValueValidator() -// .withFieldNames("min(name)") -// .withExpectedValues("Logging") -// .withExpectedValues("Reporting") }, -// { "from hive_db groupby (owner) select min(name) orderby name desc limit 2 ", -// new FieldValueValidator() -// .withFieldNames("min(name)") -// .withExpectedValues("Reporting") -// .withExpectedValues("Sales") } + { "from hive_db groupby (owner) select min(name) orderby name limit 2 ", + new FieldValueValidator() + .withFieldNames("min(name)") + .withExpectedValues("Logging") + .withExpectedValues("Reporting") }, + { "from hive_db groupby (owner) select min(name) orderby name desc limit 2 ", + new FieldValueValidator() + .withFieldNames("min(name)") + .withExpectedValues("Reporting") + .withExpectedValues("Sales") } }; } @@ -490,21 +510,32 @@ public class DSLQueriesTest extends BasicTestSetup { @DataProvider(name = "errorQueriesProvider") private Object[][] errorQueries() { return new Object[][]{ - {"`isa`"}, - {"PIII"}, - {"DBBB as d select d"}, + {"`isa`"}, // Tag doesn't exist in the test data + {"PIII"}, // same as above + {"DBBB as d select d"}, // same as above + {"hive_db has db"}, // same as above {"hive_table where (name = \"sales_fact\" and createTime >= \"2014-12-11\" ) select name as _col_0, createTime as _col_1 orderby name limit 0 offset 1"}, - {"hive_table as t, sd, hive_column as c where t.name=\"sales_fact\" select c.name as colName, c.dataType as colType"} + {"hive_table as t, sd, hive_column as c where t.name=\"sales_fact\" select c.name as colName, c.dataType as colType"}, + {"hive_table isa hive_db"}, // isa should be a trait/classification + {"hive_table isa FooTag"}, // FooTag doesn't exist + {"hive_table groupby(db.name)"}, // GroupBy on referred attribute is not supported + {"hive_table orderby(db.name)"}, // OrderBy on referred attribute is not supported + {"hive_table select db, columns"}, // Can't select multiple referred attributes/entity + {"hive_table select min(db.name), columns"}, // Can't do aggregation on referred attribute + {"hive_table select db.name, columns"}, // Can't select more than one referred attribute + {"hive_table select owner, columns"}, // Can't select a mix of immediate attribute and referred entity + {"hive_table select owner, db.name"}, // Same as above + {"hive_order"}, // From src should be an Entity or Classification }; } @Test(dataProvider = "errorQueriesProvider", expectedExceptions = { AtlasBaseException.class }) public void errorQueries(String query) throws AtlasBaseException { - discoveryService.searchUsingDslQuery(query, 25, 0); + discoveryService.searchUsingDslQuery(query, DEFAULT_LIMIT, 0); } private void queryAssert(String query, FieldValueValidator fv) throws AtlasBaseException { - AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, 25, 0); + AtlasSearchResult searchResult = discoveryService.searchUsingDslQuery(query, DEFAULT_LIMIT, 0); assertSearchResult(searchResult, fv); } @@ -521,17 +552,17 @@ public class DSLQueriesTest extends BasicTestSetup { assertEquals(searchResult.getAttributes().getValues().size(), expected.values.size()); } - private void assertSearchResult(AtlasSearchResult searchResult, int expected) { + private void assertSearchResult(AtlasSearchResult searchResult, int expected, String query) { assertNotNull(searchResult); if(expected == 0) { assertTrue(searchResult.getAttributes() == null || CollectionUtils.isEmpty(searchResult.getAttributes().getValues())); - assertNull(searchResult.getEntities()); + assertNull(searchResult.getEntities(), query); } else if(searchResult.getEntities() != null) { - assertEquals(searchResult.getEntities().size(), expected); + assertEquals(searchResult.getEntities().size(), expected, query); } else { assertNotNull(searchResult.getAttributes()); assertNotNull(searchResult.getAttributes().getValues()); - assertEquals(searchResult.getAttributes().getValues().size(), expected); + assertEquals(searchResult.getAttributes().getValues().size(), expected, query); } }
