From 37f9bd0b4903e25f667625e755f980868663ee2f Mon Sep 17 00:00:00 2001 From: Louis Klein Date: Sat, 14 Mar 2026 15:45:38 +0100 Subject: [PATCH 1/2] fix: avoid mutating resource paths in ODataRequestCount --- .../client/request/ODataRequestCount.java | 20 +++++++++++++++++-- .../client/request/ODataRequestCountTest.java | 10 ++++++++++ .../odatav4/core/NestedEntityTest.java | 13 ++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java index d30668c0b..893700cf5 100644 --- a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java +++ b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java @@ -54,7 +54,7 @@ public ODataRequestCount( @Nullable final String encodedQuery, @Nonnull final ODataProtocol protocol ) { - super(servicePath, resourcePath.addSegment("$count"), encodedQuery, protocol); + super(servicePath, appendCountResourcePath(resourcePath), encodedQuery, protocol); } /** @@ -74,8 +74,24 @@ public ODataRequestCount( { this( servicePath, - resourcePath.addSegment(query.getEntityOrPropertyName()), + appendResourcePath(resourcePath, query.getEntityOrPropertyName()), query.getEncodedQueryString(), query.getProtocol()); } + + @Nonnull + private static ODataResourcePath appendCountResourcePath( @Nonnull final ODataResourcePath resourcePath ) + { + return appendResourcePath(resourcePath, "$count"); + } + + @Nonnull + private static + ODataResourcePath + appendResourcePath( @Nonnull final ODataResourcePath resourcePath, @Nonnull final String segment ) + { + final ODataResourcePath appendedPath = new ODataResourcePath(); + resourcePath.getSegments().forEach(s -> appendedPath.addSegment(s._1, s._2)); + return appendedPath.addSegment(segment); + } } diff --git a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCountTest.java b/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCountTest.java index e432a9627..7eab1070a 100644 --- a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCountTest.java +++ b/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); + } } diff --git a/datamodel/odata-v4/odata-v4-core/src/test/java/com/sap/cloud/sdk/datamodel/odatav4/core/NestedEntityTest.java b/datamodel/odata-v4/odata-v4-core/src/test/java/com/sap/cloud/sdk/datamodel/odatav4/core/NestedEntityTest.java index e3b0b96e3..060cb9e4a 100644 --- a/datamodel/odata-v4/odata-v4-core/src/test/java/com/sap/cloud/sdk/datamodel/odatav4/core/NestedEntityTest.java +++ b/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 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() { From f38c3c403af0a08253af1c2704aa8320635f77be Mon Sep 17 00:00:00 2001 From: Louis Klein Date: Mon, 16 Mar 2026 14:21:27 +0100 Subject: [PATCH 2/2] refactor: make ODataResourcePath immutable --- .../client/expression/ODataResourcePath.java | 26 ++++++++-- .../client/request/ODataRequestCount.java | 20 +------- .../client/request/ODataRequestFunction.java | 4 +- .../request/ODataQueryReadByKeyTest.java | 12 +++++ .../request/ODataRequestFunctionTest.java | 23 +++++++++ .../client/request/ODataRequestReadTest.java | 11 +++++ .../ServiceWithNavigableEntitiesImpl.java | 47 ++++++++++++------- .../odata/helper/FluentHelperCreate.java | 4 +- release_notes.md | 4 +- 9 files changed, 103 insertions(+), 48 deletions(-) diff --git a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/expression/ODataResourcePath.java b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/expression/ODataResourcePath.java index 8a9e6e280..9556d0b77 100644 --- a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/expression/ODataResourcePath.java +++ b/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> segments = new ArrayList<>(); + private final List> segments; + + /** + * Default constructor to create an empty resource path. + */ + public ODataResourcePath() + { + this(Collections.emptyList()); + } + + private ODataResourcePath( @Nonnull final List> 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); } /** diff --git a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java index 893700cf5..d30668c0b 100644 --- a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java +++ b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestCount.java @@ -54,7 +54,7 @@ public ODataRequestCount( @Nullable final String encodedQuery, @Nonnull final ODataProtocol protocol ) { - super(servicePath, appendCountResourcePath(resourcePath), encodedQuery, protocol); + super(servicePath, resourcePath.addSegment("$count"), encodedQuery, protocol); } /** @@ -74,24 +74,8 @@ public ODataRequestCount( { this( servicePath, - appendResourcePath(resourcePath, query.getEntityOrPropertyName()), + resourcePath.addSegment(query.getEntityOrPropertyName()), query.getEncodedQueryString(), query.getProtocol()); } - - @Nonnull - private static ODataResourcePath appendCountResourcePath( @Nonnull final ODataResourcePath resourcePath ) - { - return appendResourcePath(resourcePath, "$count"); - } - - @Nonnull - private static - ODataResourcePath - appendResourcePath( @Nonnull final ODataResourcePath resourcePath, @Nonnull final String segment ) - { - final ODataResourcePath appendedPath = new ODataResourcePath(); - resourcePath.getSegments().forEach(s -> appendedPath.addSegment(s._1, s._2)); - return appendedPath.addSegment(segment); - } } diff --git a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunction.java b/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunction.java index 60ccb7f6e..960821ac9 100644 --- a/datamodel/odata-client/src/main/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunction.java +++ b/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 diff --git a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataQueryReadByKeyTest.java b/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataQueryReadByKeyTest.java index b12b5b651..1d485e429 100644 --- a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataQueryReadByKeyTest.java +++ b/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); + } } diff --git a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunctionTest.java b/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunctionTest.java index e0ed44830..0bc7f1bf0 100644 --- a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestFunctionTest.java +++ b/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); + } } diff --git a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestReadTest.java b/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestReadTest.java index 8a32f7446..2b1cd903c 100644 --- a/datamodel/odata-client/src/test/java/com/sap/cloud/sdk/datamodel/odata/client/request/ODataRequestReadTest.java +++ b/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"); + } } diff --git a/datamodel/odata-v4/odata-v4-core/src/main/java/com/sap/cloud/sdk/datamodel/odatav4/core/ServiceWithNavigableEntitiesImpl.java b/datamodel/odata-v4/odata-v4-core/src/main/java/com/sap/cloud/sdk/datamodel/odatav4/core/ServiceWithNavigableEntitiesImpl.java index 7437a82d6..b38f7a3a6 100644 --- a/datamodel/odata-v4/odata-v4-core/src/main/java/com/sap/cloud/sdk/datamodel/odatav4/core/ServiceWithNavigableEntitiesImpl.java +++ b/datamodel/odata-v4/odata-v4-core/src/main/java/com/sap/cloud/sdk/datamodel/odatav4/core/ServiceWithNavigableEntitiesImpl.java @@ -65,8 +65,10 @@ public DeleteRequestBuilder delete() public > NavigableEntityCollection navigateTo( @Nonnull final NavigationProperty.Collection property ) { - entityPath.addSegment(property.getFieldName()); - return new EntityCollection<>(servicePath, entityPath, property.getItemType()); + return new EntityCollection<>( + servicePath, + entityPath.addSegment(property.getFieldName()), + property.getItemType()); } @Nonnull @@ -74,8 +76,11 @@ public > NavigableEntityCollection> NavigableEntitySingle navigateTo( @Nonnull final NavigationProperty.Single 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,8 +104,11 @@ public CollectionValueFunctionRequestBuilder applyFunction( public > NavigableEntitySingle withFunction( @Nonnull final BoundFunction.SingleToSingleEntity.Composable 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 @@ -108,8 +116,10 @@ public > NavigableEntitySingle withF public > NavigableEntityCollection withFunction( @Nonnull final BoundFunction.SingleToCollectionEntity.Composable 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 @@ -117,11 +127,11 @@ public > NavigableEntityCollection w public SingleValueActionRequestBuilder applyAction( @Nonnull final BoundAction.SingleToSingle action ) { - entityPath.addSegment(action.getQualifiedName()); + final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName()); final SingleValueActionRequestBuilder requestBuilder = new SingleValueActionRequestBuilder<>( servicePath, - entityPath, + actionPath, action.getParameters(), action.getReturnType()); maybeEntity @@ -138,11 +148,11 @@ public SingleValueActionRequestBuilder applyAction( public CollectionValueActionRequestBuilder applyAction( @Nonnull final BoundAction.SingleToCollection action ) { - entityPath.addSegment(action.getQualifiedName()); + final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName()); final CollectionValueActionRequestBuilder requestBuilder = new CollectionValueActionRequestBuilder<>( servicePath, - entityPath, + actionPath, action.getParameters(), action.getReturnType()); maybeEntity @@ -170,8 +180,11 @@ static class EntityCollection> public > NavigableEntitySingle 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 count() NavigableEntitySingle withFunction( @Nonnull final BoundFunction.CollectionToSingleEntity.Composable 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 count() withFunction( @Nonnull final BoundFunction.CollectionToCollectionEntity.Composable function ) { - entityPath.addSegment(function.getQualifiedName(), function.getParameters()); return new ServiceWithNavigableEntitiesImpl.EntityCollection<>( servicePath, - entityPath, + entityPath.addSegment(function.getQualifiedName(), function.getParameters()), function.getReturnType()); } diff --git a/datamodel/odata/odata-core/src/main/java/com/sap/cloud/sdk/datamodel/odata/helper/FluentHelperCreate.java b/datamodel/odata/odata-core/src/main/java/com/sap/cloud/sdk/datamodel/odata/helper/FluentHelperCreate.java index c457d4294..67a90ca1f 100644 --- a/datamodel/odata/odata-core/src/main/java/com/sap/cloud/sdk/datamodel/odata/helper/FluentHelperCreate.java +++ b/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()); } diff --git a/release_notes.md b/release_notes.md index 3112514a9..38c4d0168 100644 --- a/release_notes.md +++ b/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.