Skip to content

Commit ddb0316

Browse files
rpanackalrpanackal
authored
fix: [OData] Revert mutability of ODataResourcePath and resolve segment duplication (#1138)
1 parent 5a2e505 commit ddb0316

7 files changed

Lines changed: 46 additions & 40 deletions

File tree

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

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package com.sap.cloud.sdk.datamodel.odata.client.expression;
22

33
import java.util.ArrayList;
4-
import java.util.Collections;
54
import java.util.List;
65
import java.util.stream.Collectors;
76

87
import javax.annotation.Nonnull;
98
import javax.annotation.Nullable;
109

10+
import com.google.common.annotations.Beta;
1111
import com.sap.cloud.sdk.datamodel.odata.client.request.AbstractODataParameters;
1212
import com.sap.cloud.sdk.datamodel.odata.client.request.UriEncodingStrategy;
1313

@@ -36,20 +36,7 @@ public final class ODataResourcePath
3636
*/
3737
@Getter( AccessLevel.PUBLIC )
3838
@Nonnull
39-
private final List<Tuple2<String, AbstractODataParameters>> segments;
40-
41-
/**
42-
* Default constructor to create an empty resource path.
43-
*/
44-
public ODataResourcePath()
45-
{
46-
this(Collections.emptyList());
47-
}
48-
49-
private ODataResourcePath( @Nonnull final List<Tuple2<String, AbstractODataParameters>> segments )
50-
{
51-
this.segments = List.copyOf(segments);
52-
}
39+
private final List<Tuple2<String, AbstractODataParameters>> segments = new ArrayList<>();
5340

5441
/**
5542
* Convenience method for {@code new ODataResourcePath().addSegment(segment)}. It creates a new resource path for
@@ -111,9 +98,8 @@ public ODataResourcePath addSegment( @Nonnull final String segment )
11198
ODataResourcePath
11299
addSegment( @Nonnull final String segment, @Nullable final AbstractODataParameters parameters )
113100
{
114-
final var newSegments = new ArrayList<>(segments);
115-
newSegments.add(Tuple.of(segment, parameters));
116-
return new ODataResourcePath(newSegments);
101+
segments.add(Tuple.of(segment, parameters));
102+
return this;
117103
}
118104

119105
/**
@@ -143,9 +129,8 @@ public ODataResourcePath addParameterToLastSegment( @Nonnull final AbstractOData
143129
lastSegment._2());
144130
throw new IllegalStateException(msg);
145131
}
146-
final var newSegments = new ArrayList<>(segments);
147-
newSegments.set(newSegments.size() - 1, lastSegment.update2(parameters));
148-
return new ODataResourcePath(newSegments);
132+
segments.set(segments.size() - 1, lastSegment.update2(parameters));
133+
return this;
149134
}
150135

151136
/**
@@ -196,4 +181,22 @@ public String toString()
196181
.map(t -> t._1() + t._2())
197182
.collect(Collectors.joining("/"));
198183
}
184+
185+
/**
186+
* Creates a defensive copy of this resource path.
187+
*
188+
* @return A new {@link ODataResourcePath} with the same segments as this path.
189+
* @since 5.28.0
190+
*/
191+
@Beta
192+
@Nonnull
193+
public ODataResourcePath copy()
194+
{
195+
final var copy = new ODataResourcePath();
196+
for( final var segment : segments ) {
197+
copy.addSegment(segment._1(), segment._2());
198+
}
199+
return copy;
200+
}
201+
199202
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public ODataRequestCount(
5454
@Nullable final String encodedQuery,
5555
@Nonnull final ODataProtocol protocol )
5656
{
57-
super(servicePath, resourcePath.addSegment("$count"), encodedQuery, protocol);
57+
super(servicePath, resourcePath.copy().addSegment("$count"), encodedQuery, protocol);
5858
}
5959

6060
/**
@@ -74,7 +74,7 @@ public ODataRequestCount(
7474
{
7575
this(
7676
servicePath,
77-
resourcePath.addSegment(query.getEntityOrPropertyName()),
77+
resourcePath.copy().addSegment(query.getEntityOrPropertyName()),
7878
query.getEncodedQueryString(),
7979
query.getProtocol());
8080
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,10 @@ public ODataRequestFunction(
128128
@Nonnull final ODataResourcePath functionPath,
129129
@Nonnull final StructuredQuery structQuery )
130130
{
131-
super(servicePath, functionPath.addSegment(structQuery.getEntityOrPropertyName()), structQuery.getProtocol());
131+
super(
132+
servicePath,
133+
functionPath.copy().addSegment(structQuery.getEntityOrPropertyName()),
134+
structQuery.getProtocol());
132135
this.query = structQuery.getEncodedQueryString();
133136
}
134137

@@ -171,7 +174,7 @@ private static ODataResourcePath appendResourcePathWithParameters(
171174
if( protocol.isEqualTo(ODataProtocol.V2) ) {
172175
return path;
173176
}
174-
return path.addParameterToLastSegment(parameters);
177+
return path.copy().addParameterToLastSegment(parameters);
175178
}
176179

177180
@Nullable

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ public ODataRequestRead(
101101
{
102102
this(
103103
servicePath,
104-
entityPath.addSegment(query.getEntityOrPropertyName()),
104+
entityPath.copy().addSegment(query.getEntityOrPropertyName()),
105105
query.getEncodedQueryString(),
106106
query.getProtocol());
107107
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public ODataRequestReadByKey(
9696
{
9797
this(
9898
servicePath,
99-
entityPath.addParameterToLastSegment(entityKey),
99+
entityPath.copy().addParameterToLastSegment(entityKey),
100100
query.getEncodedQueryString(),
101101
query.getProtocol());
102102
}

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public <NavigationT extends VdmEntity<NavigationT>> NavigableEntityCollection<Na
6767
{
6868
return new EntityCollection<>(
6969
servicePath,
70-
entityPath.addSegment(property.getFieldName()),
70+
entityPath.copy().addSegment(property.getFieldName()),
7171
property.getItemType());
7272
}
7373

@@ -78,7 +78,7 @@ public <NavigationT extends VdmEntity<NavigationT>> NavigableEntitySingle<Naviga
7878
{
7979
return new EntitySingle<>(
8080
servicePath,
81-
entityPath.addSegment(property.getFieldName()),
81+
entityPath.copy().addSegment(property.getFieldName()),
8282
Option.none(),
8383
property.getItemType());
8484
}
@@ -106,7 +106,7 @@ public <ResultT extends VdmEntity<ResultT>> NavigableEntitySingle<ResultT> withF
106106
{
107107
return new EntitySingle<>(
108108
servicePath,
109-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
109+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
110110
Option.none(),
111111
function.getReturnType());
112112
}
@@ -118,7 +118,7 @@ public <ResultT extends VdmEntity<ResultT>> NavigableEntityCollection<ResultT> w
118118
{
119119
return new EntityCollection<>(
120120
servicePath,
121-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
121+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
122122
function.getReturnType());
123123
}
124124

@@ -127,7 +127,7 @@ public <ResultT extends VdmEntity<ResultT>> NavigableEntityCollection<ResultT> w
127127
public <ResultT> SingleValueActionRequestBuilder<ResultT> applyAction(
128128
@Nonnull final BoundAction.SingleToSingle<EntityT, ResultT> action )
129129
{
130-
final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName());
130+
final ODataResourcePath actionPath = entityPath.copy().addSegment(action.getQualifiedName());
131131
final SingleValueActionRequestBuilder<ResultT> requestBuilder =
132132
new SingleValueActionRequestBuilder<>(
133133
servicePath,
@@ -148,7 +148,7 @@ public <ResultT> SingleValueActionRequestBuilder<ResultT> applyAction(
148148
public <ResultT> CollectionValueActionRequestBuilder<ResultT> applyAction(
149149
@Nonnull final BoundAction.SingleToCollection<EntityT, ResultT> action )
150150
{
151-
final ODataResourcePath actionPath = entityPath.addSegment(action.getQualifiedName());
151+
final ODataResourcePath actionPath = entityPath.copy().addSegment(action.getQualifiedName());
152152
final CollectionValueActionRequestBuilder<ResultT> requestBuilder =
153153
new CollectionValueActionRequestBuilder<>(
154154
servicePath,
@@ -217,7 +217,7 @@ public CountRequestBuilder<NavigationT> count()
217217
{
218218
return new ServiceWithNavigableEntitiesImpl.EntitySingle<>(
219219
servicePath,
220-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
220+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
221221
Option.none(),
222222
function.getReturnType());
223223
}
@@ -232,7 +232,7 @@ public CountRequestBuilder<NavigationT> count()
232232
{
233233
return new ServiceWithNavigableEntitiesImpl.EntityCollection<>(
234234
servicePath,
235-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
235+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
236236
function.getReturnType());
237237
}
238238

@@ -243,7 +243,7 @@ public <EntityT extends VdmEntity<EntityT>, ResultT> CollectionValueActionReques
243243
{
244244
return new CollectionValueActionRequestBuilder<>(
245245
servicePath,
246-
entityPath.addSegment(action.getQualifiedName()),
246+
entityPath.copy().addSegment(action.getQualifiedName()),
247247
action.getParameters(),
248248
action.getReturnType());
249249
}
@@ -255,7 +255,7 @@ public <EntityT extends VdmEntity<EntityT>, ResultT> SingleValueActionRequestBui
255255
{
256256
return new SingleValueActionRequestBuilder<>(
257257
servicePath,
258-
entityPath.addSegment(action.getQualifiedName()),
258+
entityPath.copy().addSegment(action.getQualifiedName()),
259259
action.getParameters(),
260260
action.getReturnType());
261261
}
@@ -288,7 +288,7 @@ EntityT extends VdmEntity<EntityT>, ResultT> CollectionValueFunctionRequestBuild
288288
{
289289
return new SingleValueFunctionRequestBuilder<>(
290290
servicePath,
291-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
291+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
292292
function.getReturnType());
293293
}
294294

@@ -302,7 +302,7 @@ EntityT extends VdmEntity<EntityT>, ResultT> CollectionValueFunctionRequestBuild
302302
{
303303
return new CollectionValueFunctionRequestBuilder<>(
304304
servicePath,
305-
entityPath.addSegment(function.getQualifiedName(), function.getParameters()),
305+
entityPath.copy().addSegment(function.getQualifiedName(), function.getParameters()),
306306
function.getReturnType());
307307
}
308308
}

‎release_notes.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
### 🔧 Compatibility Notes
1010

11-
- `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.
1211
- [OpenAPI Apache Generator] Remove no args constructor in generated API clients.
1312

1413
### ✨ New Functionality
@@ -22,3 +21,4 @@
2221
### 🐛 Fixed Issues
2322

2423
- Fixed stateful OData request path construction caused by shared `ODataResourcePath` instances being mutated when building count, read-by-key, and function requests.
24+
- `ODataRequestRead`, `ODataRequestCount`, `ODataRequesReadByKey`, `ODataRequestFunction` and `ServiceWithNavigableEntitiesImpl` now create a defensive copy of the `ODataResourcePath` to prevent unintended side effects from shared mutable state.

0 commit comments

Comments
 (0)