[OLINGO-988] Prevent duplicate ExpandItems in ExpandTreeBuilder

lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/helper/ExpandTreeBuilder.java

@@ -19,13 +19,17 @@
 package org.apache.olingo.server.core.deserializer.helper;
 
 import org.apache.olingo.commons.api.edm.EdmNavigationProperty;
+import org.apache.olingo.server.api.uri.queryoption.ExpandOption;
 import org.apache.olingo.server.core.uri.UriInfoImpl;
 import org.apache.olingo.server.core.uri.UriResourceNavigationPropertyImpl;
 import org.apache.olingo.server.core.uri.queryoption.ExpandItemImpl;
 
 public abstract class ExpandTreeBuilder {
+  
   public abstract ExpandTreeBuilder expand(EdmNavigationProperty edmNavigationProperty);
 
+  public abstract ExpandOption build();
+  
   protected ExpandItemImpl buildExpandItem(final EdmNavigationProperty edmNavigationProperty) {
     return new ExpandItemImpl()
         .setResourcePath(new UriInfoImpl()

lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/helper/ExpandTreeBuilderImpl.java

@@ -18,6 +18,9 @@
  */
 package org.apache.olingo.server.core.deserializer.helper;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.olingo.commons.api.edm.EdmNavigationProperty;
 import org.apache.olingo.server.api.uri.queryoption.ExpandOption;
 import org.apache.olingo.server.core.uri.queryoption.ExpandItemImpl;
@@ -25,43 +28,41 @@ import org.apache.olingo.server.core.uri.queryoption.ExpandOptionImpl;
 
 public class ExpandTreeBuilderImpl extends ExpandTreeBuilder {
 
+  private final Map<String, ExpandTreeBuilder> childBuilderCache = new HashMap<String, ExpandTreeBuilder>();
+  private final ExpandItemImpl parentItem;
   private ExpandOptionImpl expandOption = null;
 
+  private ExpandTreeBuilderImpl(final ExpandItemImpl parentItem) {
+    this.parentItem = parentItem;
+  }
+  
+  
   @Override
   public ExpandTreeBuilder expand(final EdmNavigationProperty edmNavigationProperty) {
-    ExpandItemImpl expandItem = buildExpandItem(edmNavigationProperty);
-
     if (expandOption == null) {
       expandOption = new ExpandOptionImpl();
+      if(parentItem != null && parentItem.getExpandOption() == null){
+        parentItem.setSystemQueryOption(expandOption);
       }
+    }
+    
+    ExpandTreeBuilder builder = childBuilderCache.get(edmNavigationProperty.getName());
+    if(builder == null){
+      ExpandItemImpl expandItem = buildExpandItem(edmNavigationProperty);
       expandOption.addExpandItem(expandItem);
-
+      builder = new ExpandTreeBuilderImpl(expandItem);
-    return new ExpandTreeBuilderInner(expandItem);
+      childBuilderCache.put(edmNavigationProperty.getName(), builder);
     }
     
+    return builder;
+  }
+
+  @Override
   public ExpandOption build() {
     return expandOption;
   }
   
-  private class ExpandTreeBuilderInner extends ExpandTreeBuilder {
+  public static ExpandTreeBuilder create(){
-    private ExpandItemImpl parent;
+    return new ExpandTreeBuilderImpl(null);
-
-    public ExpandTreeBuilderInner(final ExpandItemImpl expandItem) {
-      parent = expandItem;
-    }
-
-    @Override
-    public ExpandTreeBuilder expand(final EdmNavigationProperty edmNavigationProperty) {
-      if (parent.getExpandOption() == null) {
-        final ExpandOptionImpl expandOption = new ExpandOptionImpl();
-        parent.setSystemQueryOption(expandOption);
-      }
-
-      final ExpandItemImpl expandItem = buildExpandItem(edmNavigationProperty);
-      ((ExpandOptionImpl) parent.getExpandOption()).addExpandItem(expandItem);
-
-      return new ExpandTreeBuilderInner(expandItem);
-    }
-
   }
 }

lib/server-core/src/main/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializer.java

@@ -134,7 +134,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
           throw new DeserializerException("Nested Arrays and primitive values are not allowed for an entity value.",
               DeserializerException.MessageKeys.INVALID_ENTITY);
         }
-        EdmEntityType derivedEdmEntityType = (EdmEntityType)getDerivedType(edmEntityType, arrayElement);
+        EdmEntityType derivedEdmEntityType = (EdmEntityType) getDerivedType(edmEntityType, arrayElement);
         entities.add(consumeEntityNode(derivedEdmEntityType, (ObjectNode) arrayElement, expandBuilder));
       }
       return entities;
@@ -149,9 +149,9 @@ public class ODataJsonDeserializer implements ODataDeserializer {
       throws DeserializerException {
     try {
       final ObjectNode tree = parseJsonTree(stream);
-      final ExpandTreeBuilderImpl expandBuilder = new ExpandTreeBuilderImpl();
+      final ExpandTreeBuilder expandBuilder = ExpandTreeBuilderImpl.create();
 
-      EdmEntityType derivedEdmEntityType = (EdmEntityType)getDerivedType(edmEntityType, tree);
+      EdmEntityType derivedEdmEntityType = (EdmEntityType) getDerivedType(edmEntityType, tree);
 
       return DeserializerResultImpl.with().entity(consumeEntityNode(derivedEdmEntityType, tree, expandBuilder))
           .expandOption(expandBuilder.build())
@@ -384,9 +384,9 @@ public class ODataJsonDeserializer implements ODataDeserializer {
       final EdmNavigationProperty edmNavigationProperty) throws DeserializerException {
     Link link = new Link();
     link.setTitle(navigationPropertyName);
-    final ExpandTreeBuilder childExpandBuilder = (expandBuilder != null) ?
+    final ExpandTreeBuilder childExpandBuilder = (expandBuilder != null) ? expandBuilder.expand(edmNavigationProperty)
-        expandBuilder.expand(edmNavigationProperty) : null;
+        : null;
-        EdmEntityType derivedEdmEntityType = (EdmEntityType)getDerivedType(
+    EdmEntityType derivedEdmEntityType = (EdmEntityType) getDerivedType(
         edmNavigationProperty.getType(), jsonNode);
     if (jsonNode.isArray() && edmNavigationProperty.isCollection()) {
       link.setType(Constants.ENTITY_SET_NAVIGATION_LINK_TYPE);
@@ -398,8 +398,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
         && !edmNavigationProperty.isCollection()) {
       link.setType(Constants.ENTITY_NAVIGATION_LINK_TYPE);
       if (!jsonNode.isNull()) {
-            Entity inlineEntity = consumeEntityNode(derivedEdmEntityType, (ObjectNode) jsonNode,
+        Entity inlineEntity = consumeEntityNode(derivedEdmEntityType, (ObjectNode) jsonNode, childExpandBuilder);
-                childExpandBuilder);
         link.setInlineEntity(inlineEntity);
       }
     } else {
@@ -537,9 +536,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
             isNullable, maxLength, precision, scale, isUnicode, mapping, arrayElement);
         valueArray.add(value);
       }
-      property.setValue(type.getKind() == EdmTypeKind.ENUM ?
+      property.setValue(type.getKind() == EdmTypeKind.ENUM ? ValueType.COLLECTION_ENUM : ValueType.COLLECTION_PRIMITIVE,
-          ValueType.COLLECTION_ENUM :
-          ValueType.COLLECTION_PRIMITIVE,
           valueArray);
       break;
     case COMPLEX:
@@ -631,14 +628,10 @@ public class ODataJsonDeserializer implements ODataDeserializer {
    */
   private Class<?> getJavaClassForPrimitiveType(final EdmMapping mapping, final EdmPrimitiveType type) {
     final EdmPrimitiveType edmPrimitiveType =
-        type.getKind() == EdmTypeKind.ENUM ?
+        type.getKind() == EdmTypeKind.ENUM ? ((EdmEnumType) type).getUnderlyingType() : type
-            ((EdmEnumType) type).getUnderlyingType() :
+            .getKind() == EdmTypeKind.DEFINITION ? ((EdmTypeDefinition) type).getUnderlyingType() : type;
-            type.getKind() == EdmTypeKind.DEFINITION ?
+    return mapping == null || mapping.getMappedJavaClass() == null ? edmPrimitiveType.getDefaultType() : mapping
-                ((EdmTypeDefinition) type).getUnderlyingType() :
+        .getMappedJavaClass();
-                type;
-                  return mapping == null || mapping.getMappedJavaClass() == null ?
-                      edmPrimitiveType.getDefaultType() :
-                        mapping.getMappedJavaClass();
   }
 
   /**
@@ -841,7 +834,7 @@ public class ODataJsonDeserializer implements ODataDeserializer {
         }
 
         EdmStructuredType currentEdmType = null;
-        if(edmType instanceof EdmEntityType) {
+        if (edmType instanceof EdmEntityType) {
           currentEdmType = serviceMetadata.getEdm()
               .getEntityType(new FullQualifiedName(odataType));
         } else {

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataDeserializerDeepInsertTest.java

@@ -30,11 +30,63 @@ import org.apache.olingo.commons.api.data.Entity;
 import org.apache.olingo.commons.api.data.Link;
 import org.apache.olingo.commons.api.format.ContentType;
 import org.apache.olingo.server.api.deserializer.DeserializerException;
+import org.apache.olingo.server.api.deserializer.DeserializerResult;
+import org.apache.olingo.server.api.uri.queryoption.ExpandItem;
+import org.apache.olingo.server.api.uri.queryoption.ExpandOption;
 import org.apache.olingo.server.core.deserializer.AbstractODataDeserializerTest;
 import org.junit.Test;
 
 public class ODataDeserializerDeepInsertTest extends AbstractODataDeserializerTest {
 
+  @Test
+  public void unbalancedESAllPrim() throws Exception {
+    final DeserializerResult result = deserializeWithResult("UnbalancedESAllPrimFeed.json");
+    ExpandOption root = result.getExpandTree();
+    assertEquals(1, root.getExpandItems().size());
+
+    ExpandItem etTwoPrimManyLevel = root.getExpandItems().get(0);
+    assertEquals("NavPropertyETTwoPrimMany", etTwoPrimManyLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertEquals(1, etTwoPrimManyLevel.getExpandOption().getExpandItems().size());
+
+    ExpandItem etAllPrimOneLevel = etTwoPrimManyLevel.getExpandOption().getExpandItems().get(0);
+    assertEquals("NavPropertyETAllPrimOne", etAllPrimOneLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertEquals(1, etAllPrimOneLevel.getExpandOption().getExpandItems().size());
+
+    ExpandItem etTwoPrimOneLevel = etAllPrimOneLevel.getExpandOption().getExpandItems().get(0);
+    assertEquals("NavPropertyETTwoPrimOne", etTwoPrimOneLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertNull(etTwoPrimOneLevel.getExpandOption());
+  }
+
+  @Test
+  public void unbalancedESAllPrim2() throws Exception {
+    final DeserializerResult result = deserializeWithResult("UnbalancedESAllPrimFeed2.json");
+    ExpandOption root = result.getExpandTree();
+    assertEquals(1, root.getExpandItems().size());
+
+    ExpandItem etTwoPrimManyLevel = root.getExpandItems().get(0);
+    assertEquals("NavPropertyETTwoPrimMany", etTwoPrimManyLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertEquals(1, etTwoPrimManyLevel.getExpandOption().getExpandItems().size());
+
+    ExpandItem etAllPrimOneLevel = etTwoPrimManyLevel.getExpandOption().getExpandItems().get(0);
+    assertEquals("NavPropertyETAllPrimOne", etAllPrimOneLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertEquals(2, etAllPrimOneLevel.getExpandOption().getExpandItems().size());
+
+    ExpandItem etTwoPrimOneLevel = etAllPrimOneLevel.getExpandOption().getExpandItems().get(0);
+    assertEquals("NavPropertyETTwoPrimMany", etTwoPrimOneLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertNull(etTwoPrimOneLevel.getExpandOption());
+
+    etTwoPrimOneLevel = etAllPrimOneLevel.getExpandOption().getExpandItems().get(1);
+    assertEquals("NavPropertyETTwoPrimOne", etTwoPrimOneLevel.getResourcePath().getUriResourceParts().get(0)
+        .getSegmentValue());
+    assertNull(etTwoPrimOneLevel.getExpandOption());
+  }
+
   @Test
   public void esAllPrimExpandedToOne() throws Exception {
     final Entity entity = deserialize("EntityESAllPrimExpandedNavPropertyETTwoPrimOne.json");
@@ -152,4 +204,10 @@ public class ODataDeserializerDeepInsertTest extends AbstractODataDeserializerTe
     return ODataJsonDeserializerEntityTest.deserialize(getFileAsStream(resourceName),
         "ETAllPrim", ContentType.JSON);
   }
+
+  private DeserializerResult deserializeWithResult(final String resourceName) throws IOException,
+      DeserializerException {
+    return ODataJsonDeserializerEntityTest.deserializeWithResult(getFileAsStream(resourceName),
+        "ETAllPrim", ContentType.JSON);
+  }
 }

lib/server-test/src/test/java/org/apache/olingo/server/core/deserializer/json/ODataJsonDeserializerEntityTest.java

@@ -49,6 +49,7 @@ import org.apache.olingo.commons.api.format.ContentType;
 import org.apache.olingo.commons.core.edm.primitivetype.EdmDate;
 import org.apache.olingo.server.api.OData;
 import org.apache.olingo.server.api.deserializer.DeserializerException;
+import org.apache.olingo.server.api.deserializer.DeserializerResult;
 import org.apache.olingo.server.api.deserializer.ODataDeserializer;
 import org.apache.olingo.server.core.deserializer.AbstractODataDeserializerTest;
 import org.junit.Assert;
@@ -1348,6 +1349,12 @@ public class ODataJsonDeserializerEntityTest extends AbstractODataDeserializerTe
         .getEntity();
   }
   
+  protected static DeserializerResult deserializeWithResult(final InputStream stream, final String entityTypeName,
+      final ContentType contentType) throws DeserializerException {
+    return OData.newInstance().createDeserializer(contentType, metadata)
+        .entity(stream, edm.getEntityType(new FullQualifiedName(NAMESPACE, entityTypeName)));
+  }
+
   private static Entity deserialize(final String entityString, final String entityTypeName,
       final ContentType contentType) throws DeserializerException {
     return deserialize(new ByteArrayInputStream(entityString.getBytes()), entityTypeName, contentType);

lib/server-test/src/test/resources/UnbalancedESAllPrimFeed.json

@@ -0,0 +1,23 @@
+{
+  "@odata.context": "$metadata#ESAllPrim\/$entity",
+  "@odata.metadataEtag": "W\/\"4efd6576-89c0-487c-8d6c-584e2acbae16\"",
+  "PropertyInt16": 1,
+  "NavPropertyETTwoPrimMany": [
+    {
+      "PropertyInt16": 2,
+      "NavPropertyETAllPrimOne": {
+        "PropertyInt16": 3
+      }
+    },
+    {
+      "PropertyInt16": 2,
+      "NavPropertyETAllPrimOne": {
+        "PropertyInt16": 3,
+        "NavPropertyETTwoPrimOne": {
+          "PropertyInt16": 32766,
+          "PropertyString": "Innermost Entry"
+        }
+      }
+    }
+  ]
+}

lib/server-test/src/test/resources/UnbalancedESAllPrimFeed2.json

@@ -0,0 +1,24 @@
+{
+  "@odata.context": "$metadata#ESAllPrim\/$entity",
+  "@odata.metadataEtag": "W\/\"4efd6576-89c0-487c-8d6c-584e2acbae16\"",
+  "PropertyInt16": 1,
+  "NavPropertyETTwoPrimMany": [
+    {
+      "PropertyInt16": 2,
+      "NavPropertyETAllPrimOne": {
+          "PropertyInt16": 3,
+          "NavPropertyETTwoPrimMany": []
+        } 
+    },
+    {
+      "PropertyInt16": 2,
+      "NavPropertyETAllPrimOne": {
+          "PropertyInt16": 3,
+          "NavPropertyETTwoPrimOne": {
+            "PropertyInt16": 32766,
+            "PropertyString": "Innermost Entry"
+            }           
+        }      
+    }
+  ]
+}