You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@olingo.apache.org by ra...@apache.org on 2021/09/17 07:50:33 UTC

[olingo-odata4] branch master updated: [OLINGO-1526]Allow aggregates in concat to have multiple identical aliases

This is an automated email from the ASF dual-hosted git repository.

ramyav pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/olingo-odata4.git


The following commit(s) were added to refs/heads/master by this push:
     new bc6a299  [OLINGO-1526]Allow aggregates in concat to have multiple identical aliases
bc6a299 is described below

commit bc6a299a1f940165ca5633a9a805a9b176f2bab2
Author: ramya vasanth <ra...@sap.com>
AuthorDate: Fri Sep 17 13:20:19 2021 +0530

    [OLINGO-1526]Allow aggregates in concat to have multiple identical aliases
---
 .../uri/queryoption/apply/AggregateExpression.java | 13 ++++++
 .../olingo/server/core/uri/parser/ApplyParser.java | 51 ++++++++++++----------
 .../queryoption/apply/AggregateExpressionImpl.java | 13 ++++++
 .../queryoption/apply/DynamicStructuredType.java   |  4 ++
 .../server/core/uri/parser/ApplyParserTest.java    | 13 ++++++
 5 files changed, 72 insertions(+), 22 deletions(-)

diff --git a/lib/server-api/src/main/java/org/apache/olingo/server/api/uri/queryoption/apply/AggregateExpression.java b/lib/server-api/src/main/java/org/apache/olingo/server/api/uri/queryoption/apply/AggregateExpression.java
index 9947596..44c3b8d 100644
--- a/lib/server-api/src/main/java/org/apache/olingo/server/api/uri/queryoption/apply/AggregateExpression.java
+++ b/lib/server-api/src/main/java/org/apache/olingo/server/api/uri/queryoption/apply/AggregateExpression.java
@@ -19,6 +19,7 @@
 package org.apache.olingo.server.api.uri.queryoption.apply;
 
 import java.util.List;
+import java.util.Set;
 
 import org.apache.olingo.commons.api.edm.FullQualifiedName;
 import org.apache.olingo.server.api.uri.UriResource;
@@ -77,4 +78,16 @@ public interface AggregateExpression extends Expression {
    * @return a (potentially empty) list of aggregate expressions (but never <code>null</code>)
    */
   List<AggregateExpression> getFrom();
+  
+  /**
+   * Gets the dynamic properties for aggregation expression.
+   * @return the set of properties
+   */
+  Set<String> getDynamicProperties();
+
+  /**
+   * Adds the dynamic property for aggregation expression.
+   * @param name an identifier
+   */
+  void addDynamicProperty(String name);
 }
diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/ApplyParser.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/ApplyParser.java
index 6cfe85d..71404d2 100644
--- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/ApplyParser.java
+++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/ApplyParser.java
@@ -21,8 +21,10 @@ package org.apache.olingo.server.core.uri.parser;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.olingo.commons.api.edm.Edm;
 import org.apache.olingo.commons.api.edm.EdmElement;
@@ -221,21 +223,33 @@ public class ApplyParser {
   private Aggregate parseAggregateTrafo(EdmStructuredType referencedType)
       throws UriParserException, UriValidationException {
     AggregateImpl aggregate = new AggregateImpl();
+    Set<String> dynamicProps = new HashSet<>();
     do {
-    	aggregate.addExpression(parseAggregateExpr(referencedType, Requirement.REQUIRED));
+    	AggregateExpression aggregateExpr = parseAggregateExpr(referencedType, dynamicProps, Requirement.REQUIRED);
+        aggregate.addExpression(aggregateExpr);
+        dynamicProps.addAll(aggregateExpr.getDynamicProperties());
     } while (tokenizer.next(TokenKind.COMMA));
+    dynamicProps.forEach(dp -> addPropertyToRefType(referencedType, dp));
     ParserHelper.requireNext(tokenizer, TokenKind.CLOSE);
     return aggregate;
   }
   
+  private void addPropertyToRefType(EdmStructuredType referencedType, String alias) {
+      ((DynamicStructuredType) referencedType).addProperty(
+              createDynamicProperty(alias,
+                  // The OData standard mandates Edm.Decimal (with no decimals), although counts are always integer.
+                  odata.createPrimitiveTypeInstance(EdmPrimitiveTypeKind.Decimal)));
+  }
+  
   public AggregateExpression parseAggregateMethodCallExpr(UriTokenizer tokenizer, EdmStructuredType referringType)
 	      throws UriParserException, UriValidationException {
 	    this.tokenizer = tokenizer;
 
-	    return parseAggregateExpr(referringType, Requirement.FORBIDDEN);
+	    return parseAggregateExpr(referringType, Collections.emptySet(), Requirement.FORBIDDEN);
 	  }
 
-  private AggregateExpression parseAggregateExpr(EdmStructuredType referencedType, Requirement aliasRequired)
+  private AggregateExpression parseAggregateExpr(EdmStructuredType referencedType, Set<String> dynamicProps,
+		  Requirement aliasRequired)
       throws UriParserException, UriValidationException {
 	    AggregateExpressionImpl aggregateExpression = new AggregateExpressionImpl();
 	    tokenizer.saveState();
@@ -254,19 +268,16 @@ public class ApplyParser {
 	      aggregateExpression.setPath(uriInfo);
 	      DynamicStructuredType inlineType = new DynamicStructuredType((EdmStructuredType)
 	          ParserHelper.getTypeInformation((UriResourcePartTyped) lastResourcePart));
-	      aggregateExpression.setInlineAggregateExpression(parseAggregateExpr(inlineType, aliasRequired));
+	      aggregateExpression.setInlineAggregateExpression(parseAggregateExpr(inlineType, 
+	    		  dynamicProps, aliasRequired));
 	      ParserHelper.requireNext(tokenizer, TokenKind.CLOSE);
 	    } else if (tokenizer.next(TokenKind.COUNT)) {
 	      uriInfo.addResourcePart(new UriResourceCountImpl());
 	      aggregateExpression.setPath(uriInfo);
-	      final String alias = parseAsAlias(referencedType, aliasRequired);
+	      final String alias = parseAsAlias(referencedType, dynamicProps, aliasRequired);
 	      if (alias != null) {
 	        aggregateExpression.setAlias(alias);
-	        ((DynamicStructuredType) referencedType).addProperty(
-	            createDynamicProperty(alias,
-	                // The OData standard mandates Edm.Decimal (with no decimals), 
-	            	// although counts are always integer.
-	                odata.createPrimitiveTypeInstance(EdmPrimitiveTypeKind.Decimal)));
+	        aggregateExpression.addDynamicProperty(alias);
 	      }
 	    } else {
 	      // No legitimate continuation of a path prefix has been found.
@@ -286,14 +297,10 @@ public class ApplyParser {
 				
 				return aggregateExpression;
 		      }
-	      final String alias = parseAsAlias(referencedType, aliasRequired);
+	      final String alias = parseAsAlias(referencedType, dynamicProps, aliasRequired);
 	      if(alias != null) {
 	        aggregateExpression.setAlias(alias);
-	        ((DynamicStructuredType) referencedType).addProperty(
-	            createDynamicProperty(alias,
-	                // Determine the type for standard methods; there is no way to do this for custom methods.
-	                getTypeForAggregateMethod(aggregateExpression.getStandardMethod(),
-	                    ExpressionParser.getType(expression))));
+	        aggregateExpression.addDynamicProperty(alias);
 	      }
 	      parseAggregateFrom(aggregateExpression, referencedType);
 	    }
@@ -311,10 +318,10 @@ public class ApplyParser {
     // allowed and have no type.
     uriInfo.addResourcePart(new UriResourcePrimitivePropertyImpl(createDynamicProperty(customAggregate, null)));
     aggregateExpression.setPath(uriInfo);
-    final String alias = parseAsAlias(referencedType, Requirement.OPTIONAL);
+    final String alias = parseAsAlias(referencedType, aggregateExpression.getDynamicProperties(), Requirement.OPTIONAL);
     if (alias != null) {
       aggregateExpression.setAlias(alias);
-      ((DynamicStructuredType) referencedType).addProperty(createDynamicProperty(alias, null));
+      aggregateExpression.addDynamicProperty(alias);
     }
     parseAggregateFrom(aggregateExpression, referencedType);
   }
@@ -371,8 +378,8 @@ public class ApplyParser {
 	    REQUIRED, OPTIONAL, FORBIDDEN
 	  }
 
-  private String parseAsAlias(final EdmStructuredType referencedType, final Requirement requirement)
-      throws UriParserException {
+  private String parseAsAlias(final EdmStructuredType referencedType, Set<String> dynamicProps,
+	      final Requirement requirement) throws UriParserException {
 	    if (tokenizer.next(TokenKind.AsOperator)) {
 	        if(requirement == Requirement.FORBIDDEN) {
 	          throw new UriParserSyntaxException("Unexpected as alias found.", 
@@ -380,7 +387,7 @@ public class ApplyParser {
 	        }
 	        ParserHelper.requireNext(tokenizer, TokenKind.ODataIdentifier);
 	        final String name = tokenizer.getText();
-	        if (referencedType.getProperty(name) != null) {
+	        if (((DynamicStructuredType)referencedType).hasStaticProperty(name) || dynamicProps.contains(name)) {
 	          throw new UriParserSemanticException("Alias '" + name + "' is already a property.",
 	              UriParserSemanticException.MessageKeys.IS_PROPERTY, name);
 	        }
@@ -417,7 +424,7 @@ public class ApplyParser {
         throw new UriParserSemanticException("Compute expressions must return primitive values.",
             UriParserSemanticException.MessageKeys.ONLY_FOR_PRIMITIVE_TYPES, "compute");
       }
-      final String alias = parseAsAlias(referencedType, Requirement.REQUIRED);
+      final String alias = parseAsAlias(referencedType, Collections.emptySet(), Requirement.REQUIRED);
       ((DynamicStructuredType) referencedType).addProperty(createDynamicProperty(alias, expressionType));
       compute.addExpression(new ComputeExpressionImpl()
           .setExpression(expression)
diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/AggregateExpressionImpl.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/AggregateExpressionImpl.java
index f7f6fd9..63baa1c 100644
--- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/AggregateExpressionImpl.java
+++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/AggregateExpressionImpl.java
@@ -20,7 +20,9 @@ package org.apache.olingo.server.core.uri.queryoption.apply;
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.olingo.commons.api.edm.FullQualifiedName;
 import org.apache.olingo.server.api.ODataApplicationException;
@@ -43,6 +45,7 @@ public class AggregateExpressionImpl implements AggregateExpression {
   private String alias;
   private AggregateExpression inlineAggregateExpression;
   private List<AggregateExpression> from = new ArrayList<>();
+  private Set<String> dynamicProperties = new HashSet<>();
 
   @Override
   public List<UriResource> getPath() {
@@ -119,4 +122,14 @@ public class AggregateExpressionImpl implements AggregateExpression {
     // TODO Auto-generated method stub
     return null;
   }
+  
+  @Override
+  public Set<String> getDynamicProperties() {
+    return Collections.unmodifiableSet(dynamicProperties);
+  }
+
+  @Override
+  public void addDynamicProperty(String name) {
+    dynamicProperties.add(name);
+  }
 }
diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/DynamicStructuredType.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/DynamicStructuredType.java
index 37a796a..8678c06 100644
--- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/DynamicStructuredType.java
+++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/queryoption/apply/DynamicStructuredType.java
@@ -59,6 +59,10 @@ public class DynamicStructuredType implements EdmStructuredType, Cloneable {
         properties == null ? null : properties.get(name) :
         property;
   }
+  
+  public boolean hasStaticProperty(final String name) {
+	  return startType.getProperty(name) != null;
+  }
 
   @Override
   public List<String> getPropertyNames() {
diff --git a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/ApplyParserTest.java b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/ApplyParserTest.java
index 8307c47..b8c95a9 100644
--- a/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/ApplyParserTest.java
+++ b/lib/server-test/src/test/java/org/apache/olingo/server/core/uri/parser/ApplyParserTest.java
@@ -364,6 +364,19 @@ public class ApplyParserTest {
     parseEx("ESTwoKeyNav", "Namespace1_Alias.BFCESTwoKeyNavRTTwoKeyNav()")
         .isExSemantic(UriParserSemanticException.MessageKeys.FUNCTION_MUST_USE_COLLECTIONS);
   }
+  
+  @Test
+  public void concat2aggregatesSameAlias() throws Exception {
+    parse("ESTwoKeyNav",
+        "concat(aggregate(PropertyInt16 with sum as s),aggregate(PropertyInt16 with max as s))")
+        .is(Concat.class).goConcat(0).goAggregate(0).goUp().goUp().goConcat(1).goAggregate(0);
+  }
+
+  @Test
+  public void aggregate2expressionsSameAlias() throws Exception {
+    parseEx("ESTwoKeyNav", "aggregate(PropertyInt16 with sum as s,PropertyInt16 with max as s)")
+    .isExSemantic(UriParserSemanticException.MessageKeys.IS_PROPERTY);
+  }
 
   @Test
   public void groupBy() throws Exception {