fix: avoid mutating resource paths in ODataRequestCount (#1121)

Author: Louis-Stegra

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/expression/ODataResourcePath.java

@@ -1,6 +1,7 @@
 package com.sap.cloud.sdk.datamodel.odata.client.expression;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.stream.Collectors;
 
@@ -35,7 +36,20 @@ public final class ODataResourcePath
      */
     @Getter( AccessLevel.PUBLIC )
     @Nonnull
-    private final List<Tuple2<String, AbstractODataParameters>> segments = new ArrayList<>();
+    private final List<Tuple2<String, AbstractODataParameters>> segments;
+
+    /**
+     * Default constructor to create an empty resource path.
+     */
+    public ODataResourcePath()
+    {
+        this(Collections.emptyList());
+    }
+
+    private ODataResourcePath( @Nonnull final List<Tuple2<String, AbstractODataParameters>> segments )
+    {
+        this.segments = List.copyOf(segments);
+    }
 
     /**
      * Convenience method for {@code new ODataResourcePath().addSegment(segment)}. It creates a new resource path for
@@ -97,8 +111,9 @@ public ODataResourcePath addSegment( @Nonnull final String segment )
         ODataResourcePath
         addSegment( @Nonnull final String segment, @Nullable final AbstractODataParameters parameters )
     {
-        segments.add(Tuple.of(segment, parameters));
-        return this;
+        final var newSegments = new ArrayList<>(segments);
+        newSegments.add(Tuple.of(segment, parameters));
+        return new ODataResourcePath(newSegments);
     }
 
     /**
@@ -128,8 +143,9 @@ public ODataResourcePath addParameterToLastSegment( @Nonnull final AbstractOData
                         lastSegment._2());
             throw new IllegalStateException(msg);
         }
-        segments.set(segments.size() - 1, lastSegment.update2(parameters));
-        return this;
+        final var newSegments = new ArrayList<>(segments);
+        newSegments.set(newSegments.size() - 1, lastSegment.update2(parameters));
+        return new ODataResourcePath(newSegments);
     }
 
     /**

datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunction.java

@@ -171,9 +171,7 @@ private static ODataResourcePath appendResourcePathWithParameters(
         if( protocol.isEqualTo(ODataProtocol.V2) ) {
             return path;
         }
-        final ODataResourcePath appendedPath = new ODataResourcePath();
-        path.getSegments().forEach(s -> appendedPath.addSegment(s._1, s._2));
-        return appendedPath.addParameterToLastSegment(parameters);
+        return path.addParameterToLastSegment(parameters);
     }
 
     @Nullable

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataQueryReadByKeyTest.java

@@ -144,4 +144,16 @@ void testConstructorWithStructuredQuery()
         assertThat(actual).isEqualTo(expected);
         assertThat(actual.getQueryString()).isEqualTo(expected.getQueryString());
     }
+
+    @Test
+    void testConstructorWithStructuredQueryDoesNotMutateResourcePath()
+    {
+        final StructuredQuery structuredQuery =
+            StructuredQuery.onEntity(ENTITY_COLLECTION, ODataProtocol.V4).withInlineCount();
+        final ODataResourcePath resourcePath = ODataResourcePath.of(ENTITY_COLLECTION);
+
+        new ODataRequestReadByKey(SERVICE_PATH, resourcePath, ENTITY_KEY, structuredQuery);
+
+        assertThat(resourcePath.toString()).isEqualTo("/" + ENTITY_COLLECTION);
+    }
 }

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCountTest.java

@@ -92,4 +92,14 @@ void testConstructorWithStructuredQuery()
         assertThat(actual).isEqualTo(expected);
         assertThat(actual.getQueryString()).isEqualTo(expected.getQueryString());
     }
+
+    @Test
+    void testConstructorDoesNotMutateResourcePath()
+    {
+        final ODataResourcePath resourcePath = ODataResourcePath.of(ENTITY_NAME);
+
+        new ODataRequestCount(SERVICE_PATH, resourcePath, "", ODataProtocol.V4);
+
+        assertThat(resourcePath.toString()).isEqualTo("/" + ENTITY_NAME);
+    }
 }

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunctionTest.java

@@ -203,4 +203,27 @@ void testConstructorWithStructuredQuery()
             .hasParameter("$orderby", "name asc,ID asc")
             .hasParameter("$count", "true");
     }
+
+    @Test
+    void testConstructorWithStructuredQueryDoesNotMutateResourcePath()
+    {
+        final StructuredQuery structuredQuery = StructuredQuery.onEntity("Authors", ODataProtocol.V4).withInlineCount();
+        final ODataResourcePath functionPath = ODataResourcePath.of(ODATA_FUNCTION);
+
+        new ODataRequestFunction(ODATA_SERVICE_PATH, functionPath, structuredQuery);
+
+        assertThat(functionPath.toString()).isEqualTo("/" + ODATA_FUNCTION);
+    }
+
+    @Test
+    void testParameterHandlingDoesNotMutateResourcePath()
+    {
+        final ODataFunctionParameters parameters =
+            new ODataFunctionParameters(ODataProtocol.V4).addParameter("key", "val");
+        final ODataResourcePath functionPath = ODataResourcePath.of(ODATA_FUNCTION);
+
+        new ODataRequestFunction(ODATA_SERVICE_PATH, functionPath, parameters, null, ODataProtocol.V4);
+
+        assertThat(functionPath.toString()).isEqualTo("/" + ODATA_FUNCTION);
+    }
 }

datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestReadTest.java

@@ -306,4 +306,15 @@ void testConstructorWithStructuredQuery()
         assertThat(actual).isEqualTo(expected);
         assertThat(actual.getQueryString()).isEqualTo(expected.getQueryString());
     }
+
+    @Test
+    void testConstructorWithStructuredQueryDoesNotMutateResourcePath()
+    {
+        final StructuredQuery structuredQuery = StructuredQuery.onEntity("Authors", ODataProtocol.V4).withInlineCount();
+        final ODataResourcePath resourcePath = ODataResourcePath.of("Authors");
+
+        new ODataRequestRead("/some/service/path", resourcePath, structuredQuery);
+
+        assertThat(resourcePath.toString()).isEqualTo("/Authors");
+    }
 }

datamodel/odata-v4/odata-v4-core/src/main/java/com/sap/cloud/sdk/datamodel/odatav4/core/ServiceWithNavigableEntitiesImpl.java

@@ -65,17 +65,22 @@ public DeleteRequestBuilder<EntityT> delete()
         public <NavigationT extends VdmEntity<NavigationT>> NavigableEntityCollection<NavigationT> navigateTo(
             @Nonnull final NavigationProperty.Collection<EntityT, NavigationT> property )
         {
-            entityPath.addSegment(property.getFieldName());
-            return new EntityCollection<>(servicePath, entityPath, property.getItemType());
+            return new EntityCollection<>(
+                servicePath,
+                entityPath.addSegment(property.getFieldName()),
+                property.getItemType());
         }
 
         @Nonnull
         @Override
         public <NavigationT extends VdmEntity<NavigationT>> NavigableEntitySingle<NavigationT> navigateTo(
             @Nonnull final NavigationProperty.Single<EntityT, NavigationT> property )
         {
-            entityPath.addSegment(property.getFieldName());
-            return new EntitySingle<>(servicePath, entityPath, Option.none(), property.getItemType());
+            return new EntitySingle<>(
+                servicePath,
+                entityPath.addSegment(property.getFieldName()),
+                Option.none(),
+                property.getItemType());
         }
 
         @Nonnull
@@ -99,29 +104,34 @@ public <ResultT> CollectionValueFunctionRequestBuilder<ResultT> applyFunction(
         public <ResultT extends VdmEntity<ResultT>> NavigableEntitySingle<ResultT> withFunction(
             @Nonnull final BoundFunction.SingleToSingleEntity.Composable<EntityT, ResultT> function )
         {
-            entityPath.addSegment(function.getQualifiedName(), function.getParameters());
-            return new EntitySingle<>(servicePath, entityPath, Option.none(), function.getReturnType());
+            return new EntitySingle<>(
+                servicePath,
+                entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
+                Option.none(),
+                function.getReturnType());
         }
 
         @Override
         @Nonnull
         public <ResultT extends VdmEntity<ResultT>> NavigableEntityCollection<ResultT> withFunction(
             @Nonnull final BoundFunction.SingleToCollectionEntity.Composable<EntityT, ResultT> function )
         {
-            entityPath.addSegment(function.getQualifiedName(), function.getParameters());
-            return new EntityCollection<>(servicePath, entityPath, function.getReturnType());
+            return new EntityCollection<>(
+                servicePath,
+                entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
+                function.getReturnType());
         }
 
         @Override
         @Nonnull
         public <ResultT> SingleValueActionRequestBuilder<ResultT> applyAction(
             @Nonnull final BoundAction.SingleToSingle<EntityT, ResultT> action )
         {
-            entityPath.addSegment(action.getQualifiedName());
+            final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName());
             final SingleValueActionRequestBuilder<ResultT> requestBuilder =
                 new SingleValueActionRequestBuilder<>(
                     servicePath,
-                    entityPath,
+                    actionPath,
                     action.getParameters(),
                     action.getReturnType());
             maybeEntity
@@ -138,11 +148,11 @@ public <ResultT> SingleValueActionRequestBuilder<ResultT> applyAction(
         public <ResultT> CollectionValueActionRequestBuilder<ResultT> applyAction(
             @Nonnull final BoundAction.SingleToCollection<EntityT, ResultT> action )
         {
-            entityPath.addSegment(action.getQualifiedName());
+            final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName());
             final CollectionValueActionRequestBuilder<ResultT> requestBuilder =
                 new CollectionValueActionRequestBuilder<>(
                     servicePath,
-                    entityPath,
+                    actionPath,
                     action.getParameters(),
                     action.getReturnType());
             maybeEntity
@@ -170,8 +180,11 @@ static class EntityCollection<NavigationT extends VdmEntity<NavigationT>>
         public <EntityT extends VdmEntity<EntityT>> NavigableEntitySingle<EntityT> forEntity(
             @Nonnull final EntityT entity )
         {
-            entityPath.addParameterToLastSegment(entity.getKey());
-            return new EntitySingle<>(servicePath, entityPath, Option.of(entity), entity.getType());
+            return new EntitySingle<>(
+                servicePath,
+                entityPath.addParameterToLastSegment(entity.getKey()),
+                Option.of(entity),
+                entity.getType());
         }
 
         @Override
@@ -202,10 +215,9 @@ public CountRequestBuilder<NavigationT> count()
             NavigableEntitySingle<ResultT>
             withFunction( @Nonnull final BoundFunction.CollectionToSingleEntity.Composable<EntityT, ResultT> function )
         {
-            entityPath.addSegment(function.getQualifiedName(), function.getParameters());
             return new ServiceWithNavigableEntitiesImpl.EntitySingle<>(
                 servicePath,
-                entityPath,
+                entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
                 Option.none(),
                 function.getReturnType());
         }
@@ -218,10 +230,9 @@ public CountRequestBuilder<NavigationT> count()
             withFunction(
                 @Nonnull final BoundFunction.CollectionToCollectionEntity.Composable<EntityT, ResultT> function )
         {
-            entityPath.addSegment(function.getQualifiedName(), function.getParameters());
             return new ServiceWithNavigableEntitiesImpl.EntityCollection<>(
                 servicePath,
-                entityPath,
+                entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
                 function.getReturnType());
         }
 

datamodel/odata-v4/odata-v4-core/src/test/java/com/sap/cloud/sdk/datamodel/odatav4/core/NestedEntityTest.java

@@ -174,6 +174,19 @@ void testCountNestedTrip()
         assertThat(count.getRelativeUri()).hasToString("/TripPinServiceRW/People('russellwhyte')/Trips/$count");
     }
 
+    @Test
+    void testCountNestedTripToRequestIsStable()
+    {
+        final Person personByKey = Person.builder().userName("russellwhyte").build();
+        final CountRequestBuilder<Trip> countBuilder =
+            service.forEntity(personByKey).navigateTo(Person.TO_TRIPS).count();
+
+        assertThat(countBuilder.toRequest().getRelativeUri())
+            .hasToString("/TripPinServiceRW/People('russellwhyte')/Trips/$count");
+        assertThat(countBuilder.toRequest().getRelativeUri())
+            .hasToString("/TripPinServiceRW/People('russellwhyte')/Trips/$count");
+    }
+
     @Test
     void testGetByKeyNestedPlanItem()
     {

datamodel/odata/odata-core/src/main/java/com/sap/cloud/sdk/datamodel/odata/helper/FluentHelperCreate.java

@@ -86,8 +86,8 @@ public ODataRequestCreate toRequest()
         if( linkFromParentEntity != null && parentEntity != null ) {
             resourcePath =
                 ODataResourcePath
-                    .of(parentEntity.getEntityCollection(), ODataEntityKey.of(parentEntity.getKey(), ODataProtocol.V2));
-            resourcePath.addSegment(linkFromParentEntity.getFieldName());
+                    .of(parentEntity.getEntityCollection(), ODataEntityKey.of(parentEntity.getKey(), ODataProtocol.V2))
+                    .addSegment(linkFromParentEntity.getFieldName());
         } else {
             resourcePath = ODataResourcePath.of(getEntityCollection());
         }

release_notes.md

@@ -8,7 +8,7 @@
 
 ### 🔧 Compatibility Notes
 
-- 
+- `ODataResourcePath#addSegment(...)` and `addParameterToLastSegment(...)` now return a new path instance instead of mutating the existing one. Custom extensions that relied on in-place mutation need to reassign the returned path.
 
 ### ✨ New Functionality
 
@@ -20,4 +20,4 @@
 
 ### 🐛 Fixed Issues
 
-- 
+- Fixed stateful OData request path construction caused by shared `ODataResourcePath` instances being mutated when building count, read-by-key, and function requests.