[OLINGO-398] fix

ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractCollectionInvocationHandler.java

@@ -31,7 +31,9 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Future;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.tuple.Triple;
+import org.apache.olingo.client.api.uri.QueryOption;
 import org.apache.olingo.client.api.uri.URIFilter;
 import org.apache.olingo.client.api.uri.v4.URIBuilder;
 import org.apache.olingo.commons.api.domain.v4.ODataAnnotation;
@@ -315,16 +317,20 @@ public abstract class AbstractCollectionInvocationHandler<T extends Serializable
 
   public void expand(final String... expand) {
     if (this.uri != null) {
-      this.uri.expand(expand);
+      this.uri.replaceQueryOption(QueryOption.EXPAND, StringUtils.join(expand, ","));
     }
   }
 
   public void select(final String... select) {
     if (this.uri != null) {
-      this.uri.select(select);
+      this.uri.replaceQueryOption(QueryOption.SELECT, StringUtils.join(select, ","));
     }
   }
 
+  public URI getRequestURI() {
+    return this.uri == null ? null : this.uri.build();
+  }
+
   public void clearQueryOptions() {
     this.uri = this.baseURI == null ? null : getClient().newURIBuilder(baseURI.toASCIIString());
     this.nextPageURI = null;

ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/AbstractStructuredInvocationHandler.java

@@ -36,7 +36,9 @@ import java.util.Set;
 import java.util.concurrent.Callable;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.ArrayUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.olingo.client.api.uri.CommonURIBuilder;
+import org.apache.olingo.client.api.uri.QueryOption;
 import org.apache.olingo.client.api.uri.v4.URIBuilder;
 import org.apache.olingo.client.core.uri.URIUtils;
 import org.apache.olingo.commons.api.domain.CommonODataEntity;
@@ -621,11 +623,11 @@ public abstract class AbstractStructuredInvocationHandler extends AbstractInvoca
   }
 
   public void expand(final String... expand) {
-    this.uri.expand(expand);
+    this.uri.replaceQueryOption(QueryOption.EXPAND, StringUtils.join(expand, ","));
   }
 
   public void select(final String... select) {
-    this.uri.select(select);
+    this.uri.replaceQueryOption(QueryOption.SELECT, StringUtils.join(select, ","));
   }
 
   public void refs() {

ext/client-proxy/src/main/java/org/apache/olingo/ext/proxy/commons/EntitySetInvocationHandler.java

@@ -142,6 +142,8 @@ public class EntitySetInvocationHandler<
       LOG.debug("Object '{}({})' has been deleted", typeRef.getSimpleName(), uuid);
       return null;
     } else {
+      // clear query options
+      handler.clearQueryOptions();
       return (S) Proxy.newProxyInstance(
               Thread.currentThread().getContextClassLoader(),
               new Class<?>[] {typeRef},

fit/src/test/java/org/apache/olingo/fit/proxy/v4/APIBasicDesignTestITCase.java

@@ -26,6 +26,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.lang.reflect.Proxy;
 import java.math.BigDecimal;
 import java.sql.Timestamp;
 import java.util.Calendar;
@@ -39,6 +40,8 @@ import org.apache.olingo.commons.api.format.ContentType;
 import org.apache.olingo.ext.proxy.AbstractService;
 import org.apache.olingo.ext.proxy.api.EdmStreamValue;
 import org.apache.olingo.ext.proxy.api.PrimitiveCollection;
+import org.apache.olingo.ext.proxy.commons.AbstractCollectionInvocationHandler;
+import org.apache.olingo.ext.proxy.commons.EntitySetInvocationHandler;
 import org.junit.Test;
 
 //CHECKSTYLE:OFF (Maven checkstyle)
@@ -626,4 +629,29 @@ public class APIBasicDesignTestITCase extends AbstractTestITCase {
     }
     service.getContext().detachAll(); // avoid influences
   }
+
+  @Test
+  public void issueOLINGO398() {
+    AbstractCollectionInvocationHandler<?, ?> handler = AbstractCollectionInvocationHandler.class.cast(
+            Proxy.getInvocationHandler(container.getCustomers().getByKey(1).getOrders().
+            select("OrderID", "CustomerForOrder").
+            expand("CustomerForOrder").
+            top(1).
+            skip(2)));
+
+    assertEquals("http://localhost:9080/stub/StaticService/V40/Static.svc/Customers(1)/Orders?"
+            + "%24select=OrderID%2CCustomerForOrder&%24expand=CustomerForOrder&%24top=1&%24skip=2",
+            handler.getRequestURI().toASCIIString());
+
+    handler = AbstractCollectionInvocationHandler.class.cast(
+            Proxy.getInvocationHandler(container.getCustomers().getByKey(1).getOrders().
+            select("OrderID", "CustomerForOrder").
+            expand("CustomerForOrder").
+            top(1).
+            skip(2)));
+
+    assertEquals("http://localhost:9080/stub/StaticService/V40/Static.svc/Customers(1)/Orders?%24"
+            + "select=OrderID%2CCustomerForOrder&%24expand=CustomerForOrder&%24top=1&%24skip=2",
+            handler.getRequestURI().toASCIIString());
+  }
 }

lib/client-api/src/main/java/org/apache/olingo/client/api/uri/CommonURIBuilder.java

@@ -28,6 +28,8 @@ public interface CommonURIBuilder<UB extends CommonURIBuilder<?>> {
 
   /**
    * Adds the specified query option to the URI.
+   * <br />
+   * Concatenates value if the specified query option already exists.
    *
    * @param option query option.
    * @param value query option value.
@@ -36,13 +38,23 @@ public interface CommonURIBuilder<UB extends CommonURIBuilder<?>> {
   UB addQueryOption(QueryOption option, String value);
   
   /**
-   * Adds the specified (custom) query option to the URI.
+   * Adds/replaces the specified query option to the URI.
    *
    * @param option query option.
    * @param value query option value.
+   * @return current URIBuilder instance
+   */
+  UB replaceQueryOption(QueryOption option, String value);
+
+  /**
+   * Adds/Replaces the specified (custom) query option to the URI.
+   *
+   * @param option query option.
+   * @param value query option value.
+   * @param replace if <tt>true</tt> then replace existing one.
    * @return current URIBuilder instance.
    */
-  UB addQueryOption(String option, String value);
+  UB addQueryOption(String option, String value, boolean replace);
 
   /**
    * Adds the specified (custom) parameter alias to the URI.

lib/client-core/src/main/java/org/apache/olingo/client/core/uri/AbstractURIBuilder.java

@@ -62,7 +62,6 @@ public abstract class AbstractURIBuilder<UB extends CommonURIBuilder<?>> impleme
       return value;
     }
   }
-
   /**
    * Logger.
    */
@@ -102,13 +101,18 @@ public abstract class AbstractURIBuilder<UB extends CommonURIBuilder<?>> impleme
 
   @Override
   public UB addQueryOption(final QueryOption option, final String value) {
-    return addQueryOption(option.toString(), value);
+    return addQueryOption(option.toString(), value, false);
   }
 
   @Override
-  public UB addQueryOption(final String option, final String value) {
+  public UB replaceQueryOption(final QueryOption option, final String value) {
+    return addQueryOption(option.toString(), value, true);
+  }
+
+  @Override
+  public UB addQueryOption(final String option, final String value, final boolean replace) {
     final StringBuilder builder = new StringBuilder();
-    if (queryOptions.containsKey(option)) {
+    if (!replace && queryOptions.containsKey(option)) {
       builder.append(queryOptions.get(option)).append(',');
     }
     builder.append(value);
@@ -212,7 +216,7 @@ public abstract class AbstractURIBuilder<UB extends CommonURIBuilder<?>> impleme
 
   @Override
   public UB format(final String format) {
-    return addQueryOption(QueryOption.FORMAT, format);
+    return replaceQueryOption(QueryOption.FORMAT, format);
   }
 
   @Override
@@ -229,7 +233,7 @@ public abstract class AbstractURIBuilder<UB extends CommonURIBuilder<?>> impleme
 
   @Override
   public UB filter(final String filter) {
-    return addQueryOption(QueryOption.FILTER, filter);
+    return replaceQueryOption(QueryOption.FILTER, filter);
   }
 
   @Override
@@ -239,22 +243,22 @@ public abstract class AbstractURIBuilder<UB extends CommonURIBuilder<?>> impleme
 
   @Override
   public UB orderBy(final String order) {
-    return addQueryOption(QueryOption.ORDERBY, order);
+    return replaceQueryOption(QueryOption.ORDERBY, order);
   }
 
   @Override
   public UB top(final int top) {
-    return addQueryOption(QueryOption.TOP, String.valueOf(top));
+    return replaceQueryOption(QueryOption.TOP, String.valueOf(top));
   }
 
   @Override
   public UB skip(final int skip) {
-    return addQueryOption(QueryOption.SKIP, String.valueOf(skip));
+    return replaceQueryOption(QueryOption.SKIP, String.valueOf(skip));
   }
 
   @Override
   public UB skipToken(final String skipToken) {
-    return addQueryOption(QueryOption.SKIPTOKEN, skipToken);
+    return replaceQueryOption(QueryOption.SKIPTOKEN, skipToken);
   }
 
   protected abstract char getBoundOperationSeparator();