stricter cardinality check in expression parser Change-Id: Ie48e84e73a9da37134f4062aee7bbbe50d605443
Signed-off-by: Christian Amend <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/olingo-odata4/repo Commit: http://git-wip-us.apache.org/repos/asf/olingo-odata4/commit/b76ebe95 Tree: http://git-wip-us.apache.org/repos/asf/olingo-odata4/tree/b76ebe95 Diff: http://git-wip-us.apache.org/repos/asf/olingo-odata4/diff/b76ebe95 Branch: refs/heads/OLINGO-573 Commit: b76ebe95d13cc528d80bda5ef5064cd2d925ab33 Parents: 9e232a2 Author: Klaus Straubinger <[email protected]> Authored: Fri Apr 10 15:25:12 2015 +0200 Committer: Christian Amend <[email protected]> Committed: Tue Apr 14 15:01:13 2015 +0200 ---------------------------------------------------------------------- .../core/uri/parser/UriParseTreeVisitor.java | 25 ++++++++------- .../expression/ExpressionVisitorImpl.java | 24 ++++++--------- .../core/uri/antlr/TestFullResourcePath.java | 22 +++++++++----- .../core/uri/testutil/FilterValidator.java | 32 +++++--------------- 4 files changed, 45 insertions(+), 58 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b76ebe95/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParseTreeVisitor.java ---------------------------------------------------------------------- diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParseTreeVisitor.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParseTreeVisitor.java index 5373fda..a94efbe 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParseTreeVisitor.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParseTreeVisitor.java @@ -366,14 +366,14 @@ public class UriParseTreeVisitor extends UriParserBaseVisitor<Object> { UriResource lastResourcePart = context.contextUriInfo.getLastResourcePart(); if (lastResourcePart == null) { - if (context.contextTypes.size() == 0) { - if(checkFirst && ctx.vNS == null){ + if (context.contextTypes.empty()) { + if (checkFirst && ctx.vNS == null) { throw wrap(new UriParserSemanticException( "Cannot find EntitySet, Singleton, ActionImport or FunctionImport with name '" + odi + "'.", UriParserSemanticException.MessageKeys.RESOURCE_NOT_FOUND, odi)); } - throw wrap(new UriParserSemanticException("Resource part '" + odi + "' can only applied on typed " - + "resource parts", + throw wrap(new UriParserSemanticException( + "Resource part '" + odi + "' can only applied on typed resource parts", UriParserSemanticException.MessageKeys.RESOURCE_PART_ONLY_FOR_TYPED_PARTS, odi)); } source = context.contextTypes.peek(); @@ -381,8 +381,9 @@ public class UriParseTreeVisitor extends UriParserBaseVisitor<Object> { source = getTypeInformation(lastResourcePart); if (source.type == null) { - throw wrap(new UriParserSemanticException("Resource part '" + odi + "' can only be applied on typed " - + "resource parts.", UriParserSemanticException.MessageKeys.RESOURCE_PART_ONLY_FOR_TYPED_PARTS, odi)); + throw wrap(new UriParserSemanticException( + "Resource part '" + odi + "' can only be applied on typed resource parts.", + UriParserSemanticException.MessageKeys.RESOURCE_PART_ONLY_FOR_TYPED_PARTS, odi)); } } @@ -400,12 +401,14 @@ public class UriParseTreeVisitor extends UriParserBaseVisitor<Object> { } if (!(source.type instanceof EdmStructuredType)) { - throw wrap(new UriParserSemanticException("Cannot parse '" + odi - + "'; previous path segment is not a structural type.", + throw wrap(new UriParserSemanticException( + "Cannot parse '" + odi + "'; previous path segment is not a structural type.", UriParserSemanticException.MessageKeys.RESOURCE_PART_MUST_BE_PRECEDED_BY_STRUCTURAL_TYPE, odi)); } - if (ctx.depth() <= 2 // path evaluation for the resource path + if ((ctx.depth() <= 2 // path evaluation for the resource path + || lastResourcePart instanceof UriResourceTypedImpl + || lastResourcePart instanceof UriResourceNavigationPropertyImpl) && source.isCollection) { throw wrap(new UriParserSemanticException("Property '" + odi + "' is not allowed after collection.", UriParserSemanticException.MessageKeys.PROPERTY_AFTER_COLLECTION, odi)); @@ -416,11 +419,11 @@ public class UriParseTreeVisitor extends UriParserBaseVisitor<Object> { EdmElement property = structType.getProperty(odi); if (property == null) { throw wrap(new UriParserSemanticException("Property '" + odi + "' not found in type '" - + structType.getNamespace() + "." + structType.getName() + "'", + + structType.getFullQualifiedName().getFullQualifiedNameAsString() + "'", ctx.depth() > 2 ? // path evaluation inside an expression or for the resource path? UriParserSemanticException.MessageKeys.EXPRESSION_PROPERTY_NOT_IN_TYPE : UriParserSemanticException.MessageKeys.PROPERTY_NOT_IN_TYPE, - structType.getFullQualifiedName().toString(), odi)); + structType.getFullQualifiedName().getFullQualifiedNameAsString(), odi)); } if (property instanceof EdmProperty) { http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b76ebe95/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/queryoptions/expression/ExpressionVisitorImpl.java ---------------------------------------------------------------------- diff --git a/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/queryoptions/expression/ExpressionVisitorImpl.java b/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/queryoptions/expression/ExpressionVisitorImpl.java index ab0eca3..9c035a8 100644 --- a/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/queryoptions/expression/ExpressionVisitorImpl.java +++ b/lib/server-tecsvc/src/main/java/org/apache/olingo/server/tecsvc/processor/queryoptions/expression/ExpressionVisitorImpl.java @@ -24,7 +24,6 @@ import java.util.Locale; import org.apache.olingo.commons.api.data.Entity; import org.apache.olingo.commons.api.data.Property; import org.apache.olingo.commons.api.edm.EdmBindingTarget; -import org.apache.olingo.commons.api.edm.EdmComplexType; import org.apache.olingo.commons.api.edm.EdmEnumType; import org.apache.olingo.commons.api.edm.EdmProperty; import org.apache.olingo.commons.api.edm.EdmType; @@ -32,7 +31,7 @@ import org.apache.olingo.commons.api.http.HttpStatusCode; import org.apache.olingo.server.api.ODataApplicationException; import org.apache.olingo.server.api.uri.UriInfoResource; import org.apache.olingo.server.api.uri.UriResource; -import org.apache.olingo.server.api.uri.UriResourcePartTyped; +import org.apache.olingo.server.api.uri.UriResourceProperty; import org.apache.olingo.server.api.uri.queryoption.expression.BinaryOperatorKind; import org.apache.olingo.server.api.uri.queryoption.expression.Expression; import org.apache.olingo.server.api.uri.queryoption.expression.ExpressionVisitException; @@ -102,7 +101,7 @@ public class ExpressionVisitorImpl implements ExpressionVisitor<VisitorOperand> case NOT: return unaryOperator.notOperation(); default: - // Can`t happen + // Can't happen. return throwNotImplemented(); } } @@ -180,22 +179,19 @@ public class ExpressionVisitorImpl implements ExpressionVisitor<VisitorOperand> final List<UriResource> uriResourceParts = member.getUriResourceParts(); // UriResourceParts contains at least one UriResource - Property currentProperty = entity.getProperty(uriResourceParts.get(0).toString()); - EdmType currentType = ((UriResourcePartTyped) uriResourceParts.get(0)).getType(); + if (!(uriResourceParts.get(0) instanceof UriResourceProperty)) { + return throwNotImplemented(); + } - EdmProperty currentEdmProperty = bindingTarget.getEntityType() - .getStructuralProperty(uriResourceParts.get(0).toString()); + EdmProperty currentEdmProperty = ((UriResourceProperty) uriResourceParts.get(0)).getProperty(); + Property currentProperty = entity.getProperty(currentEdmProperty.getName()); for (int i = 1; i < uriResourceParts.size(); i++) { - currentType = ((UriResourcePartTyped) uriResourceParts.get(i)).getType(); - if (currentProperty.isComplex()) { + currentEdmProperty = ((UriResourceProperty) uriResourceParts.get(i)).getProperty(); final List<Property> complex = currentProperty.asComplex().getValue(); - for (final Property innerProperty : complex) { - if (innerProperty.getName().equals(uriResourceParts.get(i).toString())) { - EdmComplexType edmComplexType = (EdmComplexType) currentEdmProperty.getType(); - currentEdmProperty = edmComplexType.getStructuralProperty(uriResourceParts.get(i).toString()); + if (innerProperty.getName().equals(currentEdmProperty.getName())) { currentProperty = innerProperty; break; } @@ -203,7 +199,7 @@ public class ExpressionVisitorImpl implements ExpressionVisitor<VisitorOperand> } } - return new TypedOperand(currentProperty.getValue(), currentType, currentEdmProperty); + return new TypedOperand(currentProperty.getValue(), currentEdmProperty.getType(), currentEdmProperty); } @Override http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b76ebe95/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java ---------------------------------------------------------------------- diff --git a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java index 416346c..88cd44e 100644 --- a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java +++ b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestFullResourcePath.java @@ -2950,21 +2950,27 @@ public class TestFullResourcePath { testFilter.runOnETTwoKeyNavEx("invalid") .isExSemantic(UriParserSemanticException.MessageKeys.EXPRESSION_PROPERTY_NOT_IN_TYPE); - testFilter.runOnETTwoKeyNavEx("PropertyComp") - .isExSemantic(UriParserSemanticException.MessageKeys.EXPRESSION_PROPERTY_NOT_IN_TYPE); + // TODO: This should throw an exception because the top node of the filter tree must be boolean. + //testFilter.runOnETTwoKeyNavEx("PropertyComp") + // .isExSemantic(UriParserSemanticException.MessageKeys.XYZ); testFilter.runOnETTwoKeyNavEx("PropertyComp/invalid") .isExSemantic(UriParserSemanticException.MessageKeys.EXPRESSION_PROPERTY_NOT_IN_TYPE); testFilter.runOnETTwoKeyNavEx("concat('a','b')/invalid").isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); testFilter.runOnETTwoKeyNavEx("PropertyComp/concat('a','b')") .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); - testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyInt16 eq '1'") - .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); - testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyComp/PropertyDate eq 1") - .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); - testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyComp/PropertyString eq 1") - .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); + // TODO: These should throw exceptions because the types are incompatible. + //testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyInt16 eq '1'") + // .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); + //testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyComp/PropertyDate eq 1") + // .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); + //testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyComp/PropertyString eq 1") + // .isExSyntax(UriParserSyntaxException.MessageKeys.SYNTAX); testFilter.runOnETTwoKeyNavEx("PropertyComp/PropertyInt64 eq 1") .isExSemantic(UriParserSemanticException.MessageKeys.EXPRESSION_PROPERTY_NOT_IN_TYPE); + testFilter.runOnETTwoKeyNavEx("NavPropertyETKeyNavMany/PropertyInt16 gt 42") + .isExSemantic(UriParserSemanticException.MessageKeys.PROPERTY_AFTER_COLLECTION); + testFilter.runOnETTwoKeyNavEx("NavPropertyETKeyNavMany/NavPropertyETTwoKeyNavOne eq null") + .isExSemantic(UriParserSemanticException.MessageKeys.PROPERTY_AFTER_COLLECTION); testFilter.runOnETAllPrim("PropertySByte eq PropertySByte") .is("<<PropertySByte> eq <PropertySByte>>") http://git-wip-us.apache.org/repos/asf/olingo-odata4/blob/b76ebe95/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/testutil/FilterValidator.java ---------------------------------------------------------------------- diff --git a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/testutil/FilterValidator.java b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/testutil/FilterValidator.java index 4d67fcd..daf8ed0 100644 --- a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/testutil/FilterValidator.java +++ b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/testutil/FilterValidator.java @@ -193,22 +193,13 @@ public class FilterValidator implements TestValidator { } public FilterValidator runUriEx(final String path, final String query) { - Parser parser = new Parser(); - UriInfo uriInfo = null; - + exception = null; try { - uriInfo = parser.parseUri(path, query, null, edm); - } catch (UriParserException e) { + new Parser().parseUri(path, query, null, edm); + fail("Expected exception not thrown."); + } catch (final UriParserException e) { exception = e; - return this; - } - - if (uriInfo.getKind() != UriInfoKind.resource) { - fail("Filtervalidator can only be used on resourcePaths"); } - - setFilter((FilterOptionImpl) uriInfo.getFilterOption()); - curExpression = filter.getExpression(); return this; } @@ -227,22 +218,13 @@ public class FilterValidator implements TestValidator { } public FilterValidator runUriOrderByEx(final String path, final String query) { - Parser parser = new Parser(); - UriInfo uriInfo = null; - + exception = null; try { - uriInfo = parser.parseUri(path, query, null, edm); + new Parser().parseUri(path, query, null, edm); fail("Expected exception not thrown."); - } catch (UriParserException e) { + } catch (final UriParserException e) { exception = e; - return this; - } - - if (uriInfo.getKind() != UriInfoKind.resource) { - fail("Filtervalidator can only be used on resourcePaths"); } - - setOrderBy((OrderByOptionImpl) uriInfo.getOrderByOption()); return this; }
