Include type in error message

And extract checking/throwing code

Author: jjaderberg

algo/src/main/java/org/neo4j/gds/similarity/knn/metrics/NullCheckingNodeProperties.java

@@ -19,6 +19,7 @@
  */
 package org.neo4j.gds.similarity.knn.metrics;
 
+import org.jetbrains.annotations.Nullable;
 import org.neo4j.gds.api.IdMap;
 import org.neo4j.gds.api.NodeProperties;
 import org.neo4j.gds.api.nodeproperties.ValueType;
@@ -41,52 +42,28 @@ public class NullCheckingNodeProperties implements NodeProperties {
     @Override
     public double[] doubleArrayValue(long nodeId) {
         var value = properties.doubleArrayValue(nodeId);
-        if (value == null) {
-            throw new IllegalArgumentException(formatWithLocale(
-                "Missing node property `%s` for node with id `%s`.",
-                name,
-                idMap.toOriginalNodeId(nodeId)
-            ));
-        }
+        check(nodeId, value);
         return value;
     }
 
     @Override
     public float[] floatArrayValue(long nodeId) {
         var value = properties.floatArrayValue(nodeId);
-        if (value == null) {
-            throw new IllegalArgumentException(formatWithLocale(
-                "Missing node property `%s` for node with id `%s`.",
-                name,
-                idMap.toOriginalNodeId(nodeId)
-            ));
-        }
+        check(nodeId, value);
         return value;
     }
 
     @Override
     public long[] longArrayValue(long nodeId) {
         var value = properties.longArrayValue(nodeId);
-        if (value == null) {
-            throw new IllegalArgumentException(formatWithLocale(
-                "Missing node property `%s` for node with id `%s`.",
-                name,
-                idMap.toOriginalNodeId(nodeId)
-            ));
-        }
+        check(nodeId, value);
         return value;
     }
 
     @Override
     public Object getObject(long nodeId) {
         var value = properties.getObject(nodeId);
-        if (value == null) {
-            throw new IllegalArgumentException(formatWithLocale(
-                "Missing node property `%s` for node with id `%s`.",
-                name,
-                idMap.toOriginalNodeId(nodeId)
-            ));
-        }
+        check(nodeId, value);
         return value;
     }
 
@@ -98,18 +75,23 @@ public ValueType valueType() {
     @Override
     public Value value(long nodeId) {
         var value = properties.value(nodeId);
-        if (value == null) {
-            throw new IllegalArgumentException(formatWithLocale(
-                "Missing node property `%s` for node with id `%s`.",
-                name,
-                idMap.toOriginalNodeId(nodeId)
-            ));
-        }
+        check(nodeId, value);
         return value;
     }
 
     @Override
     public long size() {
         return properties.size();
     }
+
+    private void check(long nodeId, @Nullable Object value) {
+        if (value == null) {
+            throw new IllegalArgumentException(formatWithLocale(
+                "Missing `%s` node property `%s` for node with id `%s`.",
+                properties.valueType().cypherName(),
+                name,
+                idMap.toOriginalNodeId(nodeId)
+            ));
+        }
+    }
 }

algo/src/test/java/org/neo4j/gds/similarity/knn/metrics/NullCheckingNodePropertiesTest.java

@@ -82,7 +82,7 @@ void shouldThrowUsefullyForNullDoubleArrayValues() {
         var nonNullProps = new NullCheckingNodeProperties(nullProps, "propertyName", graph);
         assertThatThrownBy(() -> nonNullProps.doubleArrayValue(1))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `propertyName` for node with id `1`.");
+            .hasMessageContaining("Missing `List of Float` node property `propertyName` for node with id `1`.");
     }
 
     @Test
@@ -91,7 +91,7 @@ void shouldThrowUsefullyForNullFloatArrayValues() {
         var nonNullProps = new NullCheckingNodeProperties(nullProps, "propertyName", graph);
         assertThatThrownBy(() -> nonNullProps.floatArrayValue(1))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `propertyName` for node with id `1`.");
+            .hasMessageContaining("Missing `List of Float` node property `propertyName` for node with id `1`.");
     }
 
     @Test
@@ -100,6 +100,6 @@ void shouldThrowUsefullyForNullLongArrayValues() {
         var nonNullProps = new NullCheckingNodeProperties(nullProps, "propertyName", graph);
         assertThatThrownBy(() -> nonNullProps.longArrayValue(1))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `propertyName` for node with id `1`.");
+            .hasMessageContaining("Missing `List of Integer` node property `propertyName` for node with id `1`.");
     }
 }

algo/src/test/java/org/neo4j/gds/similarity/knn/metrics/SimilarityComputerTest.java

@@ -238,7 +238,7 @@ void doubleArraySimilarityComputerHandlesNullProperties() {
         );
         assertThatThrownBy(() -> sim.similarity(0, 1))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `doubleArrayProperty` for node with id");
+            .hasMessageContaining("Missing `List of Float` node property `doubleArrayProperty` for node with id");
     }
 
     @Test
@@ -252,7 +252,7 @@ void floatArraySimilarityComputerHandlesNullProperties() {
         );
         assertThatThrownBy(() -> sim.similarity(0, 1))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `floatArrayProperty` for node with id");
+            .hasMessageContaining("Missing `List of Float` node property `floatArrayProperty` for node with id");
     }
 
     @Test
@@ -277,7 +277,7 @@ public long size() {
                 props,
                 SimilarityMetric.defaultMetricForType(ValueType.LONG_ARRAY)
             )).isInstanceOf(IllegalArgumentException.class)
-            .hasMessageContaining("Missing node property `longArrayProperty` for node with id");
+            .hasMessageContaining("Missing `List of Integer` node property `longArrayProperty` for node with id");
     }
 
     static Stream<SimilarityComputer> nonFiniteSimilarities() {