Minor Bugfix: Double system query options result in server error

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/UriInfoImpl.java

@@ -53,6 +53,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 
 public class UriInfoImpl implements UriInfo {
 
@@ -62,9 +63,9 @@ public class UriInfoImpl implements UriInfo {
   private EdmEntityType entityTypeCast; // for $entity
 
   private List<CustomQueryOptionImpl> customQueryOptions = new ArrayList<CustomQueryOptionImpl>();
-  private HashMap<String, String> aliasToValue = new HashMap<String, String>();
+  private Map<String, String> aliasToValue = new HashMap<String, String>();
 
-  HashMap<SystemQueryOptionKind, SystemQueryOption> systemQueryOptions =
+  Map<SystemQueryOptionKind, SystemQueryOption> systemQueryOptions =
       new HashMap<SystemQueryOptionKind, SystemQueryOption>();
 
   private String fragment;
@@ -239,36 +240,36 @@ public class UriInfoImpl implements UriInfo {
     }
   }
 
-  public UriInfoImpl setSystemQueryOption(final SystemQueryOptionImpl systemOption) {
+  /** Adds system query option.
-
+   * @param systemOption the option to be added
-    if (systemOption.getKind() == SystemQueryOptionKind.EXPAND) {
+   * @return this object for method chaining
-      systemQueryOptions.put(SystemQueryOptionKind.EXPAND, systemOption);
+   * @throws ODataRuntimeException if an unsupported option is provided
-    } else if (systemOption.getKind() == SystemQueryOptionKind.FILTER) {
+   *                               or an option of this kind has been added before
-      systemQueryOptions.put(SystemQueryOptionKind.FILTER, systemOption);
+   */
-    } else if (systemOption.getKind() == SystemQueryOptionKind.FORMAT) {
+  public UriInfoImpl setSystemQueryOption(final SystemQueryOption systemOption) {
-      systemQueryOptions.put(SystemQueryOptionKind.FORMAT, systemOption);
+    final SystemQueryOptionKind kind = systemOption.getKind();
-    } else if (systemOption.getKind() == SystemQueryOptionKind.ID) {
+    switch (kind) {
-      systemQueryOptions.put(SystemQueryOptionKind.ID, systemOption);
+    case EXPAND:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.COUNT) {
+    case FILTER:
-      systemQueryOptions.put(SystemQueryOptionKind.COUNT, systemOption);
+    case FORMAT:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.ORDERBY) {
+    case ID:
-      systemQueryOptions.put(SystemQueryOptionKind.ORDERBY, systemOption);
+    case COUNT:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.SEARCH) {
+    case ORDERBY:
-      systemQueryOptions.put(SystemQueryOptionKind.SEARCH, systemOption);
+    case SEARCH:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.SELECT) {
+    case SELECT:
-      systemQueryOptions.put(SystemQueryOptionKind.SELECT, systemOption);
+    case SKIP:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.SKIP) {
+    case SKIPTOKEN:
-      systemQueryOptions.put(SystemQueryOptionKind.SKIP, systemOption);
+    case TOP:
-    } else if (systemOption.getKind() == SystemQueryOptionKind.SKIPTOKEN) {
+    case LEVELS:
-      systemQueryOptions.put(SystemQueryOptionKind.SKIPTOKEN, systemOption);
+      if (systemQueryOptions.containsKey(kind)) {
-    } else if (systemOption.getKind() == SystemQueryOptionKind.TOP) {
+        throw new ODataRuntimeException("Double System Query Option: " + systemOption.getName());
-      systemQueryOptions.put(SystemQueryOptionKind.TOP, systemOption);
+      } else {
-    } else if (systemOption.getKind() == SystemQueryOptionKind.LEVELS) {
+        systemQueryOptions.put(kind, systemOption);
-      systemQueryOptions.put(SystemQueryOptionKind.LEVELS, systemOption);
+      }
-    } else {
+      break;
+    default:
       throw new ODataRuntimeException("Unsupported System Query Option: " + systemOption.getName());
     }
-
     return this;
   }
 

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/Parser.java

@@ -27,12 +27,14 @@ import org.antlr.v4.runtime.RecognitionException;
 import org.antlr.v4.runtime.Token;
 import org.antlr.v4.runtime.atn.PredictionMode;
 import org.antlr.v4.runtime.misc.ParseCancellationException;
+import org.apache.olingo.commons.api.ODataRuntimeException;
 import org.apache.olingo.commons.api.edm.Edm;
 import org.apache.olingo.commons.api.format.ODataFormat;
 import org.apache.olingo.server.api.uri.UriInfo;
 import org.apache.olingo.server.api.uri.UriInfoKind;
 import org.apache.olingo.server.api.uri.UriResource;
 import org.apache.olingo.server.api.uri.UriResourcePartTyped;
+import org.apache.olingo.server.api.uri.queryoption.SystemQueryOption;
 import org.apache.olingo.server.api.uri.queryoption.SystemQueryOptionKind;
 import org.apache.olingo.server.core.uri.UriInfoImpl;
 import org.apache.olingo.server.core.uri.antlr.UriLexer;
@@ -161,118 +163,127 @@ public class Parser {
       }
 
       if (readQueryParameter) {
-        // second, read the simple systemQueryOptions and the Custom QueryOptions
+        // second, read the system query options and the custom query options
         for (RawUri.QueryOption option : uri.queryOptionListDecoded) {
-          if (!option.name.startsWith("$")) {
+          if (option.name.startsWith("$")) {
+            SystemQueryOption systemOption = null;
+            if (option.name.equals(SystemQueryOptionKind.FILTER.toString())) {
+              FilterExpressionEOFContext ctxFilterExpression =
+                  (FilterExpressionEOFContext) parseRule(option.value, ParserEntryRules.FilterExpression);
+
+              FilterOptionImpl filterOption =
+                  (FilterOptionImpl) uriParseTreeVisitor.visitFilterExpressionEOF(ctxFilterExpression);
+
+              systemOption = filterOption;
+
+            } else if (option.name.equals(SystemQueryOptionKind.FORMAT.toString())) {
+              FormatOptionImpl formatOption = new FormatOptionImpl();
+              formatOption.setName(option.name);
+              formatOption.setText(option.value);
+              if (option.value.equalsIgnoreCase(ODataFormat.JSON.name())
+                  || option.value.equalsIgnoreCase(ODataFormat.XML.name())
+                  || option.value.equalsIgnoreCase(ODataFormat.ATOM.name())
+                  || isFormatSyntaxValid(option)) {
+                formatOption.setFormat(option.value);
+              } else {
+                throw new UriParserSyntaxException("Illegal value of $format option!",
+                    UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION_FORMAT, option.value);
+              }
+              systemOption = formatOption;
+
+            } else if (option.name.equals(SystemQueryOptionKind.EXPAND.toString())) {
+              ExpandItemsEOFContext ctxExpandItems =
+                  (ExpandItemsEOFContext) parseRule(option.value, ParserEntryRules.ExpandItems);
+
+              ExpandOptionImpl expandOption =
+                  (ExpandOptionImpl) uriParseTreeVisitor.visitExpandItemsEOF(ctxExpandItems);
+
+              systemOption = expandOption;
+
+            } else if (option.name.equals(SystemQueryOptionKind.ID.toString())) {
+              IdOptionImpl idOption = new IdOptionImpl();
+              idOption.setName(option.name);
+              idOption.setText(option.value);
+              idOption.setValue(option.value);
+              systemOption = idOption;
+            } else if (option.name.equals(SystemQueryOptionKind.LEVELS.toString())) {
+              throw new UriParserSyntaxException("System query option '$levels' is allowed only inside '$expand'!",
+                  UriParserSyntaxException.MessageKeys.SYSTEM_QUERY_OPTION_LEVELS_NOT_ALLOWED_HERE);
+            } else if (option.name.equals(SystemQueryOptionKind.ORDERBY.toString())) {
+              OrderByEOFContext ctxOrderByExpression =
+                  (OrderByEOFContext) parseRule(option.value, ParserEntryRules.Orderby);
+
+              OrderByOptionImpl orderByOption =
+                  (OrderByOptionImpl) uriParseTreeVisitor.visitOrderByEOF(ctxOrderByExpression);
+
+              systemOption = orderByOption;
+            } else if (option.name.equals(SystemQueryOptionKind.SEARCH.toString())) {
+              throw new RuntimeException("System query option '$search' not implemented!");
+            } else if (option.name.equals(SystemQueryOptionKind.SELECT.toString())) {
+              SelectEOFContext ctxSelectEOF =
+                  (SelectEOFContext) parseRule(option.value, ParserEntryRules.Select);
+
+              SelectOptionImpl selectOption =
+                  (SelectOptionImpl) uriParseTreeVisitor.visitSelectEOF(ctxSelectEOF);
+
+              systemOption = selectOption;
+            } else if (option.name.equals(SystemQueryOptionKind.SKIP.toString())) {
+              SkipOptionImpl skipOption = new SkipOptionImpl();
+              skipOption.setName(option.name);
+              skipOption.setText(option.value);
+              try {
+                skipOption.setValue(Integer.parseInt(option.value));
+              } catch (final NumberFormatException e) {
+                throw new UriParserSyntaxException("Illegal value of $skip option!", e,
+                    UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
+                    option.name, option.value);
+              }
+              systemOption = skipOption;
+            } else if (option.name.equals(SystemQueryOptionKind.SKIPTOKEN.toString())) {
+              SkipTokenOptionImpl skipTokenOption = new SkipTokenOptionImpl();
+              skipTokenOption.setName(option.name);
+              skipTokenOption.setText(option.value);
+              skipTokenOption.setValue(option.value);
+              systemOption = skipTokenOption;
+            } else if (option.name.equals(SystemQueryOptionKind.TOP.toString())) {
+              TopOptionImpl topOption = new TopOptionImpl();
+              topOption.setName(option.name);
+              topOption.setText(option.value);
+              try {
+                topOption.setValue(Integer.parseInt(option.value));
+              } catch (final NumberFormatException e) {
+                throw new UriParserSyntaxException("Illegal value of $top option!", e,
+                    UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
+                    option.name, option.value);
+              }
+              systemOption = topOption;
+            } else if (option.name.equals(SystemQueryOptionKind.COUNT.toString())) {
+              CountOptionImpl inlineCountOption = new CountOptionImpl();
+              inlineCountOption.setName(option.name);
+              inlineCountOption.setText(option.value);
+              if (option.value.equals("true") || option.value.equals("false")) {
+                inlineCountOption.setValue(Boolean.parseBoolean(option.value));
+              } else {
+                throw new UriParserSyntaxException("Illegal value of $count option!",
+                    UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
+                    option.name, option.value);
+              }
+              systemOption = inlineCountOption;
+            } else {
+              throw new UriParserSyntaxException("Unknown system query option!",
+                  UriParserSyntaxException.MessageKeys.UNKNOWN_SYSTEM_QUERY_OPTION, option.name);
+            }
+            try {
+              context.contextUriInfo.setSystemQueryOption(systemOption);
+            } catch (final ODataRuntimeException e) {
+              throw new UriParserSyntaxException("Double system query option!", e,
+                  UriParserSyntaxException.MessageKeys.DOUBLE_SYSTEM_QUERY_OPTION, option.name);
+            }
+          } else {
             CustomQueryOptionImpl customOption = new CustomQueryOptionImpl();
             customOption.setName(option.name);
             customOption.setText(option.value);
             context.contextUriInfo.addCustomQueryOption(customOption);
-          } else if (option.name.equals(SystemQueryOptionKind.FILTER.toString())) {
-            FilterExpressionEOFContext ctxFilterExpression =
-                (FilterExpressionEOFContext) parseRule(option.value, ParserEntryRules.FilterExpression);
-
-            FilterOptionImpl filterOption =
-                (FilterOptionImpl) uriParseTreeVisitor.visitFilterExpressionEOF(ctxFilterExpression);
-
-            context.contextUriInfo.setSystemQueryOption(filterOption);
-
-          } else if (option.name.equals(SystemQueryOptionKind.FORMAT.toString())) {
-            FormatOptionImpl formatOption = new FormatOptionImpl();
-            formatOption.setName(option.name);
-            formatOption.setText(option.value);
-            if (option.value.equalsIgnoreCase(ODataFormat.JSON.name())
-                || option.value.equalsIgnoreCase(ODataFormat.XML.name())
-                || option.value.equalsIgnoreCase(ODataFormat.ATOM.name())
-                || isFormatSyntaxValid(option)) {
-              formatOption.setFormat(option.value);
-            } else {
-              throw new UriParserSyntaxException("Illegal value of $format option!",
-                  UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION_FORMAT, option.value);
-            }
-            context.contextUriInfo.setSystemQueryOption(formatOption);
-
-          } else if (option.name.equals(SystemQueryOptionKind.EXPAND.toString())) {
-            ExpandItemsEOFContext ctxExpandItems =
-                (ExpandItemsEOFContext) parseRule(option.value, ParserEntryRules.ExpandItems);
-
-            ExpandOptionImpl expandOption =
-                (ExpandOptionImpl) uriParseTreeVisitor.visitExpandItemsEOF(ctxExpandItems);
-
-            context.contextUriInfo.setSystemQueryOption(expandOption);
-
-          } else if (option.name.equals(SystemQueryOptionKind.ID.toString())) {
-            IdOptionImpl idOption = new IdOptionImpl();
-            idOption.setName(option.name);
-            idOption.setText(option.value);
-            idOption.setValue(option.value);
-            context.contextUriInfo.setSystemQueryOption(idOption);
-          } else if (option.name.equals(SystemQueryOptionKind.LEVELS.toString())) {
-            throw new UriParserSyntaxException("System query option '$levels' is allowed only inside '$expand'!",
-                UriParserSyntaxException.MessageKeys.SYSTEM_QUERY_OPTION_LEVELS_NOT_ALLOWED_HERE);
-          } else if (option.name.equals(SystemQueryOptionKind.ORDERBY.toString())) {
-            OrderByEOFContext ctxOrderByExpression =
-                (OrderByEOFContext) parseRule(option.value, ParserEntryRules.Orderby);
-
-            OrderByOptionImpl orderByOption =
-                (OrderByOptionImpl) uriParseTreeVisitor.visitOrderByEOF(ctxOrderByExpression);
-
-            context.contextUriInfo.setSystemQueryOption(orderByOption);
-          } else if (option.name.equals(SystemQueryOptionKind.SEARCH.toString())) {
-            throw new RuntimeException("System query option '$search' not implemented!");
-          } else if (option.name.equals(SystemQueryOptionKind.SELECT.toString())) {
-            SelectEOFContext ctxSelectEOF =
-                (SelectEOFContext) parseRule(option.value, ParserEntryRules.Select);
-
-            SelectOptionImpl selectOption =
-                (SelectOptionImpl) uriParseTreeVisitor.visitSelectEOF(ctxSelectEOF);
-
-            context.contextUriInfo.setSystemQueryOption(selectOption);
-          } else if (option.name.equals(SystemQueryOptionKind.SKIP.toString())) {
-            SkipOptionImpl skipOption = new SkipOptionImpl();
-            skipOption.setName(option.name);
-            skipOption.setText(option.value);
-            try {
-              skipOption.setValue(Integer.parseInt(option.value));
-            } catch (final NumberFormatException e) {
-              throw new UriParserSyntaxException("Illegal value of $skip option!", e,
-                  UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
-                  option.name, option.value);
-            }
-            context.contextUriInfo.setSystemQueryOption(skipOption);
-          } else if (option.name.equals(SystemQueryOptionKind.SKIPTOKEN.toString())) {
-            SkipTokenOptionImpl skipTokenOption = new SkipTokenOptionImpl();
-            skipTokenOption.setName(option.name);
-            skipTokenOption.setText(option.value);
-            skipTokenOption.setValue(option.value);
-            context.contextUriInfo.setSystemQueryOption(skipTokenOption);
-          } else if (option.name.equals(SystemQueryOptionKind.TOP.toString())) {
-            TopOptionImpl topOption = new TopOptionImpl();
-            topOption.setName(option.name);
-            topOption.setText(option.value);
-            try {
-              topOption.setValue(Integer.parseInt(option.value));
-            } catch (final NumberFormatException e) {
-              throw new UriParserSyntaxException("Illegal value of $top option!", e,
-                  UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
-                  option.name, option.value);
-            }
-            context.contextUriInfo.setSystemQueryOption(topOption);
-          } else if (option.name.equals(SystemQueryOptionKind.COUNT.toString())) {
-            CountOptionImpl inlineCountOption = new CountOptionImpl();
-            inlineCountOption.setName(option.name);
-            inlineCountOption.setText(option.value);
-            if (option.value.equals("true") || option.value.equals("false")) {
-              inlineCountOption.setValue(Boolean.parseBoolean(option.value));
-            } else {
-              throw new UriParserSyntaxException("Illegal value of $count option!",
-                  UriParserSyntaxException.MessageKeys.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
-                  option.name, option.value);
-            }
-            context.contextUriInfo.setSystemQueryOption(inlineCountOption);
-          } else {
-            throw new UriParserSyntaxException("Unknown system query option!",
-                UriParserSyntaxException.MessageKeys.UNKNOWN_SYSTEM_QUERY_OPTION, option.name);
           }
         }
       }

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/UriParserSyntaxException.java

@@ -25,6 +25,7 @@ public class UriParserSyntaxException extends UriParserException {
 
   public static enum MessageKeys implements MessageKey {
     /** parameter: query-option name */ UNKNOWN_SYSTEM_QUERY_OPTION,
+    /** parameter: query-option name */ DOUBLE_SYSTEM_QUERY_OPTION,
     /** parameters: query-option name, query-option value */ WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION,
     SYNTAX,
     SYSTEM_QUERY_OPTION_LEVELS_NOT_ALLOWED_HERE, 

lib/server-core/src/main/resources/server-core-exceptions-i18n.properties

@@ -25,6 +25,7 @@ ODataHandlerException.FUNCTIONALITY_NOT_IMPLEMENTED=The requested functionality
 ODataHandlerException.ODATA_VERSION_NOT_SUPPORTED=OData version '%1$s' is not supported.
 
 UriParserSyntaxException.UNKNOWN_SYSTEM_QUERY_OPTION=The system query option '%1$s' is not defined.
+UriParserSyntaxException.DOUBLE_SYSTEM_QUERY_OPTION=The system query option '%1$s' can be specified only once.
 UriParserSyntaxException.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION=The system query option '%1$s' has the not-allowed value '%2$s'.
 UriParserSyntaxException.WRONG_VALUE_FOR_SYSTEM_QUERY_OPTION_FORMAT=The system query option '$format' must be either 'json', 'xml', 'atom', or a valid content type; the value '%1$s' is neither.
 UriParserSyntaxException.SYNTAX=The URI is malformed.

lib/server-test/src/test/java/org/apache/olingo/server/core/uri/UriInfoImplTest.java

@@ -18,6 +18,7 @@
  */
 package org.apache.olingo.server.core.uri;
 
+import org.apache.olingo.commons.api.ODataRuntimeException;
 import org.apache.olingo.commons.api.edm.Edm;
 import org.apache.olingo.commons.api.edm.EdmEntityType;
 import org.apache.olingo.server.api.uri.UriInfoAll;
@@ -122,6 +123,13 @@ public class UriInfoImplTest {
     assertEquals(entitySet1, uriInfo.getLastResourcePart());
   }
 
+  @Test(expected = ODataRuntimeException.class)
+  public void doubleSystemQueryOptions() {
+    new UriInfoImpl()
+        .setSystemQueryOption(new FormatOptionImpl())
+        .setSystemQueryOption(new FormatOptionImpl());
+  }
+
   @Test
   public void testCustomQueryOption() {
     UriInfoImpl uriInfo = new UriInfoImpl();

lib/server-test/src/test/java/org/apache/olingo/server/core/uri/antlr/TestUriParserImpl.java

@@ -30,6 +30,7 @@ import org.apache.olingo.server.api.uri.queryoption.expression.MethodKind;
 import org.apache.olingo.server.core.edm.provider.EdmProviderImpl;
 import org.apache.olingo.server.core.uri.parser.UriParserException;
 import org.apache.olingo.server.core.uri.parser.UriParserSemanticException;
+import org.apache.olingo.server.core.uri.parser.UriParserSyntaxException;
 import org.apache.olingo.server.core.uri.testutil.EdmTechTestProvider;
 import org.apache.olingo.server.core.uri.testutil.FilterValidator;
 import org.apache.olingo.server.core.uri.testutil.ResourceValidator;
@@ -240,7 +241,7 @@ public class TestUriParserImpl {
         .goFilter().is("<<PropertyInt16> eq <123>>");
   }
 
-  @Test(expected = UriValidationException.class)
+  @Test(expected = UriParserSyntaxException.class)
   public void testEntityFailOnValidation2() throws Exception {
     // simple entity set; with qualifiedentityTypeName; with 2xformat(before and after), expand, filter
     testUri.run("$entity/olingo.odata.test1.ETTwoPrim?"

lib/server-test/src/test/java/org/apache/olingo/server/core/uri/validator/UriValidatorTest.java

@@ -314,6 +314,11 @@ public class UriValidatorTest {
     parseAndValidate("ESAllPrim?$skip=foo", HttpMethod.GET);
   }
 
+  @Test(expected = UriParserSyntaxException.class)
+  public void validateDoubleSystemOptions() throws Exception {
+    parseAndValidate("ESAllPrim?$skip=1&$skip=2", HttpMethod.GET);
+  }
+
   @Test(expected = UriValidationException.class)
   public void validateKeyPredicatesWrongKey() throws Exception {
     String uri = "ESTwoKeyNav(xxx=1, yyy='abc')";