Ensure RelationshipPropertyStore is never empty on construction

Author: s1ck

core/src/main/java/org/neo4j/gds/api/RelationshipPropertyStore.java

@@ -19,6 +19,7 @@
  */
 package org.neo4j.gds.api;
 
+import org.immutables.value.Value;
 import org.neo4j.gds.annotation.ValueClass;
 
 import java.util.Collection;
@@ -35,10 +36,6 @@ default RelationshipProperty get(String propertyKey) {
         return relationshipProperties().get(propertyKey);
     }
 
-    default boolean isEmpty() {
-        return relationshipProperties().isEmpty();
-    }
-
     default Set<String> keySet() {
         return relationshipProperties().keySet();
     }
@@ -51,8 +48,11 @@ default boolean containsKey(String propertyKey) {
         return relationshipProperties().containsKey(propertyKey);
     }
 
-    static RelationshipPropertyStore empty() {
-        return ImmutableRelationshipPropertyStore.of(Collections.emptyMap());
+    @Value.Check
+    default void validate() {
+        if (relationshipProperties().isEmpty()) {
+            throw new IllegalStateException("Relationship property store must not be empty.");
+        }
     }
 
     static Builder builder() {

core/src/main/java/org/neo4j/gds/core/loading/CSRGraphStore.java

@@ -403,7 +403,7 @@ public RelationshipProperty relationshipPropertyValues(RelationshipType relation
         return Optional.ofNullable(relationships.get(relationshipType))
             .flatMap(SingleTypeRelationshipImportResult::properties)
             .map(propertyStore -> propertyStore.get(propertyKey))
-            .orElseGet(() -> RelationshipPropertyStore.empty().get(propertyKey));
+            .orElseThrow(() -> new IllegalArgumentException("No relationship properties found for relationship type `" + relationshipType + "` and property key `" + propertyKey + "`."));
     }
 
     @Override
@@ -557,13 +557,12 @@ public CompositeRelationshipIterator getCompositeRelationshipIterator(
 
         var relationship = relationships.get(relationshipType);
         var adjacencyList = relationship.topology().adjacencyList();
-        var relationshipPropertyStore = relationship.properties().orElse(RelationshipPropertyStore.empty());
 
         var properties = propertyKeys.isEmpty()
             ? CSRCompositeRelationshipIterator.EMPTY_PROPERTIES
             : propertyKeys
                 .stream()
-                .map(relationshipPropertyStore::get)
+                .map(propertyKey -> relationshipPropertyValues(relationshipType, propertyKey))
                 .map(RelationshipProperty::values)
                 .map(Relationships.Properties::propertiesList)
                 .toArray(AdjacencyProperties[]::new);

core/src/main/java/org/neo4j/gds/core/loading/RelationshipImportResult.java

@@ -62,7 +62,7 @@ static RelationshipImportResult of(
             relationshipType,
             SingleTypeRelationshipImportResult.builder()
                 .topology(topology)
-                .properties(Optional.ofNullable(properties.get(relationshipType)).filter(p -> !p.isEmpty()))
+                .properties(Optional.ofNullable(properties.get(relationshipType)))
                 .direction(directions.get(relationshipType))
                 .build()
         ));

test-utils/src/main/java/org/neo4j/gds/gdl/GdlFactory.java

@@ -235,35 +235,39 @@ public CSRGraphStore build() {
         var relationshipImportResultBuilder = RelationshipImportResult.builder();
 
         relationships.forEach(loadResult -> {
-            var propertyStoreBuilder = loadResult
-                .properties()
-                .entrySet()
-                .stream()
-                .reduce(RelationshipPropertyStore.builder(), (builder, stringPropertiesEntry) -> {
-                    var propertyKey = stringPropertiesEntry.getKey();
-                    var properties = stringPropertiesEntry.getValue();
+            var singleTypeRelationshipImportBuilder = SingleTypeRelationshipImportResult.builder()
+                .direction(Direction.fromOrientation(graphProjectConfig().orientation()))
+                .topology(loadResult.topology());
+
+            if (!loadResult.properties().isEmpty()) {
+                var propertyStoreBuilder = loadResult
+                    .properties()
+                    .entrySet()
+                    .stream()
+                    .reduce(RelationshipPropertyStore.builder(), (builder, stringPropertiesEntry) -> {
+                        var propertyKey = stringPropertiesEntry.getKey();
+                        var properties = stringPropertiesEntry.getValue();
 
-                    builder.putIfAbsent(
-                        propertyKey,
-                        RelationshipProperty.of(
+                        builder.putIfAbsent(
                             propertyKey,
-                            NumberType.FLOATING_POINT,
-                            PropertyState.PERSISTENT,
-                            properties,
-                            DefaultValue.forDouble(),
-                            graphProjectConfig.aggregation()
-                        )
-                    );
-                    return builder;
-                }, (builder, __) -> builder);
+                            RelationshipProperty.of(
+                                propertyKey,
+                                NumberType.FLOATING_POINT,
+                                PropertyState.PERSISTENT,
+                                properties,
+                                DefaultValue.forDouble(),
+                                graphProjectConfig.aggregation()
+                            )
+                        );
+                        return builder;
+                    }, (builder, __) -> builder);
+
+                singleTypeRelationshipImportBuilder.properties(propertyStoreBuilder.build());
+            }
 
             relationshipImportResultBuilder.putImportResult(
                 loadResult.relationshipType(),
-                SingleTypeRelationshipImportResult.builder()
-                    .direction(Direction.fromOrientation(graphProjectConfig().orientation()))
-                    .topology(loadResult.topology())
-                    .properties(propertyStoreBuilder.build())
-                    .build()
+                singleTypeRelationshipImportBuilder.build()
             );
         });