From d379052edd74eeefa279c455538a34e18a78cd8a Mon Sep 17 00:00:00 2001
From: Ronja Quensel <72978761+ronjaquensel@users.noreply.github.com>
Date: Wed, 25 Sep 2024 12:21:20 +0200
Subject: [PATCH] fix: only return dataset with valid offers (#4490)

* fix: only return dataset with valid offers

* test: add additional test
---
 .../catalog/DatasetResolverImpl.java          |  9 ++++
 .../catalog/DatasetResolverImplTest.java      | 44 +++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/core/control-plane/control-plane-catalog/src/main/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImpl.java b/core/control-plane/control-plane-catalog/src/main/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImpl.java
index 45378cff31f..847fd3544b8 100644
--- a/core/control-plane/control-plane-catalog/src/main/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImpl.java
+++ b/core/control-plane/control-plane-catalog/src/main/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImpl.java
@@ -62,6 +62,10 @@ public DatasetResolverImpl(ContractDefinitionResolver contractDefinitionResolver
     @NotNull
     public Stream<Dataset> query(ParticipantAgent agent, QuerySpec querySpec) {
         var contractDefinitions = contractDefinitionResolver.definitionsFor(agent).toList();
+        if (contractDefinitions.isEmpty()) {
+            return Stream.empty();
+        }
+        
         var assetsQuery = QuerySpec.Builder.newInstance().offset(0).limit(MAX_VALUE).filter(querySpec.getFilterExpression()).build();
         return assetIndex.queryAssets(assetsQuery)
                 .map(asset -> toDataset(contractDefinitions, asset))
@@ -73,9 +77,14 @@ public Stream<Dataset> query(ParticipantAgent agent, QuerySpec querySpec) {
     @Override
     public Dataset getById(ParticipantAgent agent, String id) {
         var contractDefinitions = contractDefinitionResolver.definitionsFor(agent).toList();
+        if (contractDefinitions.isEmpty()) {
+            return null;
+        }
+        
         return Optional.of(id)
                 .map(assetIndex::findById)
                 .map(asset -> toDataset(contractDefinitions, asset))
+                .filter(Dataset::hasOffers)
                 .orElse(null);
     }
 
diff --git a/core/control-plane/control-plane-catalog/src/test/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImplTest.java b/core/control-plane/control-plane-catalog/src/test/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImplTest.java
index ab3f5e7d128..9eba1ddc6df 100644
--- a/core/control-plane/control-plane-catalog/src/test/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImplTest.java
+++ b/core/control-plane/control-plane-catalog/src/test/java/org/eclipse/edc/connector/controlplane/catalog/DatasetResolverImplTest.java
@@ -56,6 +56,7 @@
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.isA;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -98,6 +99,16 @@ void search_shouldReturnOneDatasetPerAsset() {
             assertThat(dataset.getProperties()).contains(entry("key", "value"));
         });
     }
+    
+    @Test
+    void query_shouldNotQueryAssets_whenNoValidContractDefinition() {
+        when(contractDefinitionResolver.definitionsFor(any())).thenReturn(Stream.empty());
+        
+        var datasets = datasetResolver.query(createParticipantAgent(), QuerySpec.none());
+        
+        assertThat(datasets).isNotNull().isEmpty();
+        verify(assetIndex, never()).queryAssets(any());
+    }
 
     @Test
     void query_shouldReturnNoDataset_whenPolicyNotFound() {
@@ -295,6 +306,39 @@ void getById_shouldReturnNull_whenAssetNotFound() {
 
         assertThat(dataset).isNull();
     }
+    
+    @Test
+    void getById_shouldReturnNull_whenNoValidContractDefinition() {
+        var participantAgent = createParticipantAgent();
+        
+        when(contractDefinitionResolver.definitionsFor(any())).thenReturn(Stream.empty());
+        
+        var dataset = datasetResolver.getById(participantAgent, "datasetId");
+        
+        assertThat(dataset).isNull();
+        verify(assetIndex, never()).findById(any());
+    }
+    
+    @Test
+    void getById_shouldReturnNull_whenNoValidContractDefinitionForAsset() {
+        var assetId = "assetId";
+        var participantAgent = createParticipantAgent();
+        
+        when(contractDefinitionResolver.definitionsFor(any())).thenReturn(Stream.of(
+                contractDefinitionBuilder("definition")
+                        .assetsSelectorCriterion(Criterion.Builder.newInstance()
+                                .operandRight(EDC_NAMESPACE + "id")
+                                .operator("=")
+                                .operandLeft("a-different-asset")
+                                .build())
+                        .build()
+        ));
+        when(assetIndex.findById(any())).thenReturn(createAsset(assetId).build());
+        
+        var dataset = datasetResolver.getById(participantAgent, assetId);
+        
+        assertThat(dataset).isNull();
+    }
 
     private ContractDefinition.Builder contractDefinitionBuilder(String id) {
         return ContractDefinition.Builder.newInstance()