Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
4 changes: 2 additions & 2 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.
Loading