use QuerySpec instead of AssetSelectorExpression

eclipse-edc · Sep 26, 2022 · 54efc3e · 54efc3e
1 parent bef3932
commit 54efc3e
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 26 deletions.
diff --git a/...src/main/java/org/eclipse/dataspaceconnector/contract/offer/ContractOfferServiceImpl.java b/...src/main/java/org/eclipse/dataspaceconnector/contract/offer/ContractOfferServiceImpl.java
@@ -26,13 +26,15 @@
 import org.eclipse.dataspaceconnector.spi.contract.offer.ContractOfferService;
 import org.eclipse.dataspaceconnector.spi.message.Range;
 import org.eclipse.dataspaceconnector.spi.policy.store.PolicyDefinitionStore;
+import org.eclipse.dataspaceconnector.spi.query.QuerySpec;
 import org.eclipse.dataspaceconnector.spi.types.domain.asset.Asset;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.offer.ContractDefinition;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.offer.ContractOffer;
 import org.jetbrains.annotations.NotNull;
 
 import java.net.URI;
 import java.util.Optional;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
@@ -55,17 +57,21 @@ public ContractOfferServiceImpl(ParticipantAgentService agentService, ContractDe
     @NotNull
     public Stream<ContractOffer> queryContractOffers(ContractOfferQuery query, Range range) {
         var agent = agentService.createFor(query.getClaimToken());
-
-        return definitionService.definitionsFor(agent)
+        var offers = definitionService.definitionsFor(agent)
                 .flatMap(definition -> {
-                    var assets = assetIndex.queryAssets(definition.getSelectorExpression());
+                    var spec = QuerySpec.Builder.newInstance().filter(definition.getSelectorExpression().getCriteria())
+                            .build();
                     return Optional.of(definition.getContractPolicyId())
                             .map(policyStore::findById)
-                            .map(policy -> assets.map(asset -> createContractOffer(definition, policy.getPolicy(), asset)))
+                            .map(policy -> assetIndex.queryAssets(spec).map(asset -> createContractOffer(definition, policy.getPolicy(), asset)))
                             .orElseGet(Stream::empty);
                 })
+                // todo: change this: take limit/offset from query
                 .skip(range.getFrom())
-                .limit(range.getTo() - range.getFrom());
+                .limit(range.getTo() - range.getFrom())
+                .collect(Collectors.toList());
+        return offers.stream();
+
     }
 
     @NotNull

diff --git a/...test/java/org/eclipse/dataspaceconnector/contract/offer/ContractOfferServiceImplTest.java b/...test/java/org/eclipse/dataspaceconnector/contract/offer/ContractOfferServiceImplTest.java
@@ -27,20 +27,21 @@
 import org.eclipse.dataspaceconnector.spi.message.Range;
 import org.eclipse.dataspaceconnector.spi.policy.PolicyDefinition;
 import org.eclipse.dataspaceconnector.spi.policy.store.PolicyDefinitionStore;
+import org.eclipse.dataspaceconnector.spi.query.QuerySpec;
 import org.eclipse.dataspaceconnector.spi.types.domain.asset.Asset;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.offer.ContractDefinition;
 import org.eclipse.dataspaceconnector.spi.types.domain.contract.offer.ContractOffer;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Collections.emptyMap;
 import static java.util.stream.IntStream.range;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.isA;
+import static org.mockito.Mockito.atLeastOnce;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -55,14 +56,6 @@ class ContractOfferServiceImplTest {
 
     private ContractOfferService contractOfferService;
 
-    private static ContractDefinition.Builder getContractDefBuilder(String id) {
-        return ContractDefinition.Builder.newInstance()
-                .id(id)
-                .accessPolicyId("access")
-                .contractPolicyId("contract")
-                .selectorExpression(AssetSelectorExpression.SELECT_ALL);
-    }
-
     @BeforeEach
     void setUp() {
         contractOfferService = new ContractOfferServiceImpl(agentService, contractDefinitionService, assetIndex, policyStore);
@@ -76,15 +69,15 @@ void shouldGetContractOffers() {
         when(agentService.createFor(isA(ClaimToken.class))).thenReturn(new ParticipantAgent(emptyMap(), emptyMap()));
         when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenReturn(Stream.of(contractDefinition));
         var assetStream = Stream.of(Asset.Builder.newInstance().build(), Asset.Builder.newInstance().build());
-        when(assetIndex.queryAssets(isA(AssetSelectorExpression.class))).thenReturn(assetStream);
+        when(assetIndex.queryAssets(isA(QuerySpec.class))).thenReturn(assetStream);
         when(policyStore.findById(any())).thenReturn(PolicyDefinition.Builder.newInstance().policy(Policy.Builder.newInstance().build()).build());
 
         var query = ContractOfferQuery.builder().claimToken(ClaimToken.Builder.newInstance().build()).build();
 
         assertThat(contractOfferService.queryContractOffers(query, DEFAULT_RANGE)).hasSize(2);
         verify(agentService).createFor(isA(ClaimToken.class));
         verify(contractDefinitionService).definitionsFor(isA(ParticipantAgent.class));
-        verify(assetIndex).queryAssets(isA(AssetSelectorExpression.class));
+        verify(assetIndex).queryAssets(isA(QuerySpec.class));
         verify(policyStore).findById("contract");
     }
 
@@ -94,7 +87,7 @@ void shouldNotGetContractOfferIfPolicyIsNotFound() {
                 .build();
         when(agentService.createFor(isA(ClaimToken.class))).thenReturn(new ParticipantAgent(emptyMap(), emptyMap()));
         when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenReturn(Stream.of(contractDefinition));
-        when(assetIndex.queryAssets(isA(AssetSelectorExpression.class))).thenReturn(Stream.of(Asset.Builder.newInstance().build()));
+        when(assetIndex.queryAssets(isA(QuerySpec.class))).thenReturn(Stream.of(Asset.Builder.newInstance().build()));
         when(policyStore.findById(any())).thenReturn(null);
 
         var query = ContractOfferQuery.builder().claimToken(ClaimToken.Builder.newInstance().build()).build();
@@ -105,27 +98,92 @@ void shouldNotGetContractOfferIfPolicyIsNotFound() {
     }
 
     @Test
-    void queryContractOffers_ensureCorrectRange() {
-        var definitions = range(0, 1000).mapToObj(i -> getContractDefBuilder(String.valueOf(i)).contractPolicyId("contract" + i).accessPolicyId("access" + i).build()).collect(Collectors.toList());
-        var assets = range(0, 50_000_000).mapToObj(i -> createAsset("asset" + i).build());
+    void shouldLimitResult() {
+        var contractDefinition = range(0, 100).mapToObj(i -> getContractDefBuilder(String.valueOf(i))
+                .build());
+
         when(agentService.createFor(isA(ClaimToken.class))).thenReturn(new ParticipantAgent(emptyMap(), emptyMap()));
-        when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenReturn(definitions.stream());
-        when(assetIndex.queryAssets(isA(AssetSelectorExpression.class))).thenReturn(assets);
+        when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenAnswer(i -> contractDefinition);
+
+        when(assetIndex.queryAssets(isA(QuerySpec.class))).thenAnswer(inv -> range(0, 50_000_000).mapToObj(i -> createAsset("asset" + i).build()));
         when(policyStore.findById(any())).thenReturn(PolicyDefinition.Builder.newInstance().policy(Policy.Builder.newInstance().build()).build());
 
         var query = ContractOfferQuery.builder().claimToken(ClaimToken.Builder.newInstance().build()).build();
-        var result = contractOfferService.queryContractOffers(query, new Range(200, 500));
 
-        assertThat(result).hasSize(300)
+        var from = 20;
+        var to = 50;
+
+        assertThat(contractOfferService.queryContractOffers(query, new Range(from, to))).hasSize(to - from)
                 .extracting(ContractOffer::getAsset)
                 .extracting(Asset::getId)
                 .allSatisfy(id -> {
                     var idNumber = Integer.valueOf(id.replace("asset", ""));
-                    assertThat(idNumber).isStrictlyBetween(199, 500);
+                    assertThat(idNumber).isStrictlyBetween(from - 1, to);
                 });
+        verify(agentService).createFor(isA(ClaimToken.class));
+        verify(contractDefinitionService).definitionsFor(isA(ParticipantAgent.class));
+        verify(assetIndex, atLeastOnce()).queryAssets(isA(QuerySpec.class));
+        verify(policyStore).findById("contract");
+    }
+
+    @Test
+    void shouldLimitResult_insufficientAssets() {
+        var contractDefinition = range(0, 4).mapToObj(i -> getContractDefBuilder(String.valueOf(i))
+                .build());
+
+        when(agentService.createFor(isA(ClaimToken.class))).thenReturn(new ParticipantAgent(emptyMap(), emptyMap()));
+        when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenAnswer(i -> contractDefinition);
+
+        when(assetIndex.queryAssets(isA(QuerySpec.class))).thenAnswer(inv -> range(0, 10).mapToObj(i -> createAsset("asset" + i).build()));
+        when(policyStore.findById(any())).thenReturn(PolicyDefinition.Builder.newInstance().policy(Policy.Builder.newInstance().build()).build());
+
+        var query = ContractOfferQuery.builder().claimToken(ClaimToken.Builder.newInstance().build()).build();
+
+        var from = 20;
+        var to = 50;
+
+        // 4 definitions, 10 assets each = 40 offers total -> offset 20 ==> result = 20
+        assertThat(contractOfferService.queryContractOffers(query, new Range(from, to))).hasSize(20);
+        verify(agentService).createFor(isA(ClaimToken.class));
+        verify(contractDefinitionService).definitionsFor(isA(ParticipantAgent.class));
+        verify(assetIndex, atLeastOnce()).queryAssets(isA(QuerySpec.class));
+        verify(policyStore, atLeastOnce()).findById("contract");
+    }
+
+    @Test
+    void shouldLimitResult_pageOffsetLargerThanNumAssets() {
+        var contractDefinition = range(0, 2).mapToObj(i -> getContractDefBuilder(String.valueOf(i))
+                .build());
+
+        when(agentService.createFor(isA(ClaimToken.class))).thenReturn(new ParticipantAgent(emptyMap(), emptyMap()));
+        when(contractDefinitionService.definitionsFor(isA(ParticipantAgent.class))).thenAnswer(i -> contractDefinition);
+
+        when(assetIndex.queryAssets(isA(QuerySpec.class))).thenAnswer(inv -> range(0, 10).mapToObj(i -> createAsset("asset" + i).build()));
+        when(policyStore.findById(any())).thenReturn(PolicyDefinition.Builder.newInstance().policy(Policy.Builder.newInstance().build()).build());
+
+        var query = ContractOfferQuery.builder().claimToken(ClaimToken.Builder.newInstance().build()).build();
+
+        var from = 25;
+        var to = 50;
+
+        // 2 definitions, 10 assets each = 20 offers total -> offset of 25 is outside
+        assertThat(contractOfferService.queryContractOffers(query, new Range(from, to))).isEmpty();
+        verify(agentService).createFor(isA(ClaimToken.class));
+        verify(contractDefinitionService).definitionsFor(isA(ParticipantAgent.class));
+        verify(assetIndex, atLeastOnce()).queryAssets(isA(QuerySpec.class));
+        verify(policyStore, atLeastOnce()).findById("contract");
+    }
+
+    private ContractDefinition.Builder getContractDefBuilder(String id) {
+        return ContractDefinition.Builder.newInstance()
+                .id(id)
+                .accessPolicyId("access")
+                .contractPolicyId("contract")
+                .selectorExpression(AssetSelectorExpression.SELECT_ALL);
     }
 
     private Asset.Builder createAsset(String id) {
         return Asset.Builder.newInstance().id(id).name("test asset " + id);
     }
+
 }
diff --git a/...on/iam/decentralized-identity/identity-common-test/bin/testFixtures/edc-test-keystore.jks b/...on/iam/decentralized-identity/identity-common-test/bin/testFixtures/edc-test-keystore.jks
diff --git a/...mmon/iam/decentralized-identity/identity-common-test/bin/testFixtures/readme-keystore.txt b/...mmon/iam/decentralized-identity/identity-common-test/bin/testFixtures/readme-keystore.txt