Skip to content

Commit

Permalink
Fix deeply nested relations
Browse files Browse the repository at this point in the history
  • Loading branch information
@joostfarla
joostfarla committed Mar 5, 2024
1 parent 040f37a commit a43fc79
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 11 deletions.
2 changes: 1 addition & 1 deletion data/geo/model.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ objectTypes:
target: Address
cardinality: 0..*
inverseName: isAddressOf
inverseCardinality: 1
inverseCardinality: 1..*

Address:
attributes:
Expand Down
16 changes: 8 additions & 8 deletions engine/src/main/java/nl/geostandaarden/imx/orchestrate/engine/fetch/FetchPlanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import lombok.RequiredArgsConstructor;
import nl.geostandaarden.imx.orchestrate.engine.OrchestrateException;
Expand Down Expand Up @@ -287,15 +286,16 @@ private FetchOperation fetchSourceObject(ObjectTypeRef sourceTypeRef, Set<Path>
}
}

var identityPaths = targetType.getIdentityProperties()
.stream()
.map(Path::fromProperties)
.collect(Collectors.toSet());
// TODO: Disabled for now, since result mapper does handles this incorrectly
// var identityPaths = targetType.getIdentityProperties()
// .stream()
// .map(Path::fromProperties)
// .collect(Collectors.toSet());

// If only identity is selected, no next operation is needed
if (identityPaths.equals(nestedSourcePaths)) {
return;
}
// if (identityPaths.equals(nestedSourcePaths)) {
// return;
// }

nextOperations.add(NextOperation.builder()
.property(relation)
Expand Down
9 changes: 8 additions & 1 deletion engine/src/main/java/nl/geostandaarden/imx/orchestrate/engine/fetch/ObjectResultMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public ObjectResult map(ObjectResult objectResult, DataRequest request) {
.orElseThrow(() -> new OrchestrateException("Type mapping not found for source root: " + sourceType.getName()));
}

public ObjectResult map(ObjectResult objectResult, DataRequest request, ObjectType targetType, ObjectTypeMapping typeMapping) {
private ObjectResult map(ObjectResult objectResult, DataRequest request, ObjectType targetType, ObjectTypeMapping typeMapping) {
var properties = new HashMap<String, Object>();
var lineageBuilder = ObjectLineage.builder();

Expand Down Expand Up @@ -183,6 +183,13 @@ private Object mapRelation(Object value, DataRequest request) {
return map(nestedObjectResult, request);
}

if (value instanceof CollectionResult nestedCollectionResult) {
return nestedCollectionResult.getObjectResults()
.stream()
.map(result -> map(result, request))
.toList();
}

if (value instanceof List<?> nestedResultList) {
return nestedResultList.stream()
.flatMap(nestedResult -> {
Expand Down
152 changes: 152 additions & 0 deletions engine/src/test/java/nl/geostandaarden/imx/orchestrate/engine/OrchestrateEngineIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.util.Collections.emptyList;
import static org.assertj.core.api.Assertions.as;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.InstanceOfAssertFactories.LIST;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

Expand All @@ -13,6 +14,7 @@
import java.util.Map;
import nl.geostandaarden.imx.orchestrate.engine.exchange.CollectionRequest;
import nl.geostandaarden.imx.orchestrate.engine.exchange.ObjectRequest;
import nl.geostandaarden.imx.orchestrate.engine.exchange.ObjectResult;
import nl.geostandaarden.imx.orchestrate.engine.source.DataRepository;
import nl.geostandaarden.imx.orchestrate.ext.spatial.SpatialExtension;
import nl.geostandaarden.imx.orchestrate.model.ComponentRegistry;
Expand All @@ -28,6 +30,7 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.locationtech.jts.geom.Point;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import reactor.core.publisher.Flux;
Expand Down Expand Up @@ -314,4 +317,153 @@ void fetch_returnsMergedList_forMultipleSources() {
.assertNext(result -> assertThat(result.getObjectResults()).hasSize(2))
.verifyComplete();
}

@Test
void fetch_returnsAddressWithNestedBuilding_forDeepRelation() {
var targetModel = engine.getModelMapping()
.getTargetModel();

var request = ObjectRequest.builder(targetModel)
.objectType("Address")
.objectKey(Map.of("id", "A0001"))
.selectProperty("id")
.selectCollectionProperty("isAddressOf", builder -> builder
.selectProperty("id")
.selectProperty("geometry")
.build())
.build();

when(adrRepositoryMock.findOne(any(ObjectRequest.class)))
.thenAnswer(invocation -> {
var objectType = ((ObjectRequest) invocation.getArgument(0)).getObjectType();

return switch (objectType.getName()) {
case "Address" -> Mono.just(Map.of("id", "A0001"));
default -> throw new IllegalStateException();
};
});

when(cityRepositoryMock.findOne(any(ObjectRequest.class)))
.thenAnswer(invocation -> {
var objectType = ((ObjectRequest) invocation.getArgument(0)).getObjectType();

return switch (objectType.getName()) {
case "Building" -> Mono.just(Map.of("id", "BU0001", "geometry", Map.of("type", "Point", "coordinates", List.of(0, 0))));
default -> throw new IllegalStateException();
};
});

when(cityRepositoryMock.find(any(CollectionRequest.class)))
.thenAnswer(invocation -> {
var collectionRequest = ((CollectionRequest) invocation.getArgument(0));
var objectType = collectionRequest.getObjectType();
var filter = collectionRequest.getFilter();

if (!"BuildingPart".equals(objectType.getName()) || filter == null) {
throw new IllegalStateException();
}

if ("hasSubAddress".equals(filter.getPath().getFirstSegment())) {
return Flux.empty();
}

if ("hasMainAddress".equals(filter.getPath().getFirstSegment())) {
return Flux.just(Map.of("id", "BP0001", "isPartOf", List.of(Map.of("id", "BU0001"))));
}

throw new IllegalStateException();
});

var resultMono = engine.fetch(request);

StepVerifier.create(resultMono)
.assertNext(result -> {
assertThat(result).isNotNull();
assertThat(result.getProperties())
.containsEntry("id", "A0001")
.extractingByKey("isAddressOf", LIST)
.singleElement()
.isInstanceOfSatisfying(ObjectResult.class, nestedResult -> {
assertThat(nestedResult).isNotNull();
assertThat(nestedResult.getProperties())
.containsEntry("id", "BU0001")
.hasEntrySatisfying("geometry", geometry -> assertThat(geometry)
.isInstanceOf(Point.class));
});
})
.verifyComplete();
}

@Test
void fetch_returnsAddressWithNestedBuilding_forDeepRelationWithOnlyKeySelected() {
var targetModel = engine.getModelMapping()
.getTargetModel();

var request = ObjectRequest.builder(targetModel)
.objectType("Address")
.objectKey(Map.of("id", "A0001"))
.selectProperty("id")
.selectCollectionProperty("isAddressOf", builder -> builder
.selectProperty("id")
.build())
.build();

when(adrRepositoryMock.findOne(any(ObjectRequest.class)))
.thenAnswer(invocation -> {
var objectType = ((ObjectRequest) invocation.getArgument(0)).getObjectType();

return switch (objectType.getName()) {
case "Address" -> Mono.just(Map.of("id", "A0001"));
default -> throw new IllegalStateException();
};
});

when(cityRepositoryMock.findOne(any(ObjectRequest.class)))
.thenAnswer(invocation -> {
var objectType = ((ObjectRequest) invocation.getArgument(0)).getObjectType();

return switch (objectType.getName()) {
case "Building" -> Mono.just(Map.of("id", "BU0001"));
default -> throw new IllegalStateException();
};
});

when(cityRepositoryMock.find(any(CollectionRequest.class)))
.thenAnswer(invocation -> {
var collectionRequest = ((CollectionRequest) invocation.getArgument(0));
var objectType = collectionRequest.getObjectType();
var filter = collectionRequest.getFilter();

if (!"BuildingPart".equals(objectType.getName()) || filter == null) {
throw new IllegalStateException();
}

if ("hasSubAddress".equals(filter.getPath().getFirstSegment())) {
return Flux.empty();
}

if ("hasMainAddress".equals(filter.getPath().getFirstSegment())) {
return Flux.just(Map.of("id", "BP0001", "isPartOf", List.of(Map.of("id", "BU0001"))));
}

throw new IllegalStateException();
});

var resultMono = engine.fetch(request);

StepVerifier.create(resultMono)
.assertNext(result -> {
assertThat(result).isNotNull();
assertThat(result.getProperties())
.containsEntry("id", "A0001")
.extractingByKey("isAddressOf", LIST)
.singleElement()
.isInstanceOfSatisfying(ObjectResult.class, nestedResult -> {
assertThat(nestedResult).isNotNull();
assertThat(nestedResult.getProperties())
.containsEntry("id", "BU0001");
});
})
.verifyComplete();
}
}
2 changes: 1 addition & 1 deletion pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
<sonar.host.url>https://sonarcloud.io</sonar.host.url>
<sonar.coverage.jacoco.xmlReportPaths>${project.basedir}/../report-aggregate/target/site/jacoco-aggregate/jacoco.xml</sonar.coverage.jacoco.xmlReportPaths>
<maven-surefire-plugin.version>3.2.5</maven-surefire-plugin.version>
<maven-failsafe-plugin.version>3.2.3</maven-failsafe-plugin.version>
<maven-failsafe-plugin.version>3.2.5</maven-failsafe-plugin.version>
<maven-compiler-plugin.version>3.12.1</maven-compiler-plugin.version>
<maven-release-plugin.version>3.0.1</maven-release-plugin.version>
<!-- Dummy property to avoid JVM startup error in IDEs -->
Expand Down

0 comments on commit a43fc79

Please sign in to comment.