Replace DataClass in NodeProjection

FlorentinD · s1ck · commit cb4496354274 · 2023-01-24T15:53:12.000+01:00
This also avoids accessing static base class members via derived types.
diff --git a/etc/spotbugs/spotbugs-exclude.xml b/etc/spotbugs/spotbugs-exclude.xml
@@ -697,7 +697,7 @@
         <Or>
             <Class name="org.neo4j.gds.core.utils.progress.EmptyTaskStore" />
             <Class name="org.neo4j.gds.core.utils.warnings.EmptyUserLogStore" />
-            <Class name="org.neo4j.gds.AbstractNodeProjection" />
+            <Class name="org.neo4j.gds.NodeProjection" />
         </Or>
         <Bug code="MS" />
     </Match>
diff --git a/graph-projection-api/src/main/java/org/neo4j/gds/NodeProjection.java b/graph-projection-api/src/main/java/org/neo4j/gds/NodeProjection.java
@@ -21,18 +21,18 @@
 
 import org.immutables.value.Value;
 import org.jetbrains.annotations.Nullable;
-import org.neo4j.gds.annotation.DataClass;
+import org.neo4j.gds.annotation.ValueClass;
 import org.neo4j.gds.core.ConfigKeyValidation;
 import org.neo4j.gds.utils.StringFormatting;
 
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
-@DataClass
-public abstract class AbstractNodeProjection extends ElementProjection {
+@ValueClass
+public abstract class NodeProjection extends ElementProjection {
 
-    private static final NodeProjection ALL = of(PROJECT_ALL);
+    private static final NodeProjection ALL = fromString(PROJECT_ALL);
 
     public abstract String label();
 
@@ -50,7 +50,7 @@ public boolean projectAll() {
     public static final String LABEL_KEY = "label";
 
     public static NodeProjection of(String label) {
-        return NodeProjection.of(label, PropertyMappings.of());
+        return ImmutableNodeProjection.of(label, PropertyMappings.of());
     }
 
     public static NodeProjection all() {
@@ -85,7 +85,7 @@ public static NodeProjection fromString(@Nullable String label) {
     public static NodeProjection fromMap(Map<String, Object> map, NodeLabel nodeLabel) {
         validateConfigKeys(map);
         String label = String.valueOf(map.getOrDefault(LABEL_KEY, nodeLabel.name));
-        return create(map, properties -> NodeProjection.of(label, properties));
+        return create(map, properties -> ImmutableNodeProjection.of(label, properties));
     }
 
     @Override
@@ -102,9 +102,9 @@ void writeToObject(Map<String, Object> value) {
     public NodeProjection withAdditionalPropertyMappings(PropertyMappings mappings) {
         PropertyMappings newMappings = properties().mergeWith(mappings);
         if (newMappings == properties()) {
-            return (NodeProjection) this;
+            return this;
         }
-        return ((NodeProjection) this).withProperties(newMappings);
+        return ((ImmutableNodeProjection) this).withProperties(newMappings);
     }
 
     public static Builder builder() {
@@ -116,7 +116,7 @@ private static void validateConfigKeys(Map<String, Object> map) {
     }
 
     @org.immutables.builder.Builder.AccessibleFields
-    public static final class Builder extends NodeProjection.Builder implements InlineProperties<Builder> {
+    public static final class Builder extends ImmutableNodeProjection.Builder implements InlineProperties<Builder> {
 
         private InlinePropertiesBuilder propertiesBuilder;
 
diff --git a/graph-projection-api/src/test/java/org/neo4j/gds/NodeProjectionsTest.java b/graph-projection-api/src/test/java/org/neo4j/gds/NodeProjectionsTest.java
@@ -39,9 +39,9 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.text.MatchesPattern.matchesPattern;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.neo4j.gds.AbstractNodeProjection.LABEL_KEY;
 import static org.neo4j.gds.ElementProjection.PROPERTIES_KEY;
 import static org.neo4j.gds.NodeLabel.ALL_NODES;
+import static org.neo4j.gds.NodeProjection.LABEL_KEY;
 
 class NodeProjectionsTest {
 
diff --git a/proc/catalog/src/test/java/org/neo4j/gds/catalog/GraphProjectProcTest.java b/proc/catalog/src/test/java/org/neo4j/gds/catalog/GraphProjectProcTest.java
@@ -82,8 +82,8 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.neo4j.gds.AbstractNodeProjection.LABEL_KEY;
 import static org.neo4j.gds.ElementProjection.PROPERTIES_KEY;
+import static org.neo4j.gds.NodeProjection.LABEL_KEY;
 import static org.neo4j.gds.RelationshipProjection.AGGREGATION_KEY;
 import static org.neo4j.gds.RelationshipProjection.INDEX_INVERSE_KEY;
 import static org.neo4j.gds.RelationshipProjection.ORIENTATION_KEY;
diff --git a/proc/test/src/main/java/org/neo4j/gds/AlgoBaseProcTest.java b/proc/test/src/main/java/org/neo4j/gds/AlgoBaseProcTest.java
@@ -323,7 +323,7 @@ default void testRunOnEmptyGraph() {
                 loadedGraphName,
                 NodeProjections.of(
                     Map.of(
-                        NodeLabel.of("X"), NodeProjection.of("X", PropertyMappings.of(propertyMappings))
+                        NodeLabel.of("X"), ImmutableNodeProjection.of("X", PropertyMappings.of(propertyMappings))
                     )
                 ),
                 relationshipProjections()
diff --git a/proc/test/src/main/java/org/neo4j/gds/ConfigurableSeedConfigTest.java b/proc/test/src/main/java/org/neo4j/gds/ConfigurableSeedConfigTest.java
@@ -85,7 +85,7 @@ default void testSeedPropertyValidation() {
         GraphProjectFromStoreConfig graphProjectConfig = ImmutableGraphProjectFromStoreConfig.of(
             "",
             graphName,
-            NodeProjections.single(NodeLabel.of("A"), NodeProjection.of("A", PropertyMappings.of(nodeProperties))),
+            NodeProjections.single(NodeLabel.of("A"), ImmutableNodeProjection.of("A", PropertyMappings.of(nodeProperties))),
             allRelationshipsProjection()
         );
 
diff --git a/proc/test/src/main/java/org/neo4j/gds/GraphProjectConfigSupport.java b/proc/test/src/main/java/org/neo4j/gds/GraphProjectConfigSupport.java
@@ -60,7 +60,7 @@ default GraphProjectFromStoreConfig withNameAndRelationshipProjections(
             graphName,
             AbstractNodeProjections.create(singletonMap(
                 ALL_NODES,
-                NodeProjection.of(PROJECT_ALL, PropertyMappings.of(propertyMappings))
+                ImmutableNodeProjection.of(PROJECT_ALL, PropertyMappings.of(propertyMappings))
             )),
             rels
         );
diff --git a/proc/test/src/main/java/org/neo4j/gds/MutateNodePropertyTest.java b/proc/test/src/main/java/org/neo4j/gds/MutateNodePropertyTest.java
@@ -60,11 +60,11 @@ default void testWriteBackGraphMutationOnFilteredGraph() {
         StoreLoaderBuilder storeLoaderBuilder = new StoreLoaderBuilder()
             .databaseService(graphDb())
             .graphName(graphName)
-            .addNodeProjection(NodeProjection.of(
+            .addNodeProjection(ImmutableNodeProjection.of(
                 "A",
                 PropertyMappings.of(nodeProperties().stream().map(PropertyMapping::of).collect(Collectors.toList()))
             ))
-            .addNodeProjection(NodeProjection.of(
+            .addNodeProjection(ImmutableNodeProjection.of(
                 "B",
                 PropertyMappings.of(nodeProperties().stream().map(PropertyMapping::of).collect(Collectors.toList()))
             ));
diff --git a/test-utils/src/main/java/org/neo4j/gds/GdsCypher.java b/test-utils/src/main/java/org/neo4j/gds/GdsCypher.java
@@ -817,8 +817,8 @@ private static MinimalObject toMinimalObject(
         ElementProjection projection,
         ElementIdentifier identifier
     ) {
-        if (projection instanceof AbstractNodeProjection) {
-            return toMinimalObject(((AbstractNodeProjection) projection), identifier);
+        if (projection instanceof NodeProjection) {
+            return toMinimalObject(((NodeProjection) projection), identifier);
         }
         if (projection instanceof RelationshipProjection) {
             return toMinimalObject(((RelationshipProjection) projection), identifier);
@@ -827,7 +827,7 @@ private static MinimalObject toMinimalObject(
     }
 
     private static MinimalObject toMinimalObject(
-        AbstractNodeProjection projection,
+        NodeProjection projection,
         ElementIdentifier identifier
     ) {
         MinimalObject properties = toMinimalObject(projection.properties(), false);
@@ -836,12 +836,12 @@ private static MinimalObject toMinimalObject(
         }
 
         Map<String, Object> value = new LinkedHashMap<>();
-        value.put(AbstractNodeProjection.LABEL_KEY, projection.label());
+        value.put(NodeProjection.LABEL_KEY, projection.label());
         properties.toObject().ifPresent(o -> value.put(PROPERTIES_KEY, o));
         return MinimalObject.map(value);
     }
 
-    private static boolean matchesLabel(String label, AbstractNodeProjection projection) {
+    private static boolean matchesLabel(String label, NodeProjection projection) {
         return Objects.equals(projection.label(), label);
     }
 
diff --git a/test-utils/src/main/java/org/neo4j/gds/GraphProjectConfigBuilders.java b/test-utils/src/main/java/org/neo4j/gds/GraphProjectConfigBuilders.java
@@ -70,7 +70,7 @@ public static GraphProjectFromStoreConfig storeConfig(
     ) {
         // Node projections
         Map<String, NodeProjection> tempNP = new LinkedHashMap<>();
-        nodeLabels.forEach(label -> tempNP.put(label, NodeProjection.of(label, PropertyMappings.of())));
+        nodeLabels.forEach(label -> tempNP.put(label, NodeProjection.of(label)));
         nodeProjections.forEach(np -> tempNP.put(np.label(), np));
         nodeProjectionsWithIdentifier.forEach(tempNP::put);
 

Original file line number	Diff line number	Diff line change
`@@ -323,7 +323,7 @@ default void testRunOnEmptyGraph() {`
`323`	`323`	`loadedGraphName,`
`324`	`324`	`NodeProjections.of(`
`325`	`325`	`Map.of(`
`326`		`- NodeLabel.of("X"), NodeProjection.of("X", PropertyMappings.of(propertyMappings))`
	`326`	`+ NodeLabel.of("X"), ImmutableNodeProjection.of("X", PropertyMappings.of(propertyMappings))`
`327`	`327`	`)`
`328`	`328`	`),`
`329`	`329`	`relationshipProjections()`
Original file line number	Diff line number	Diff line change
`@@ -817,8 +817,8 @@ private static MinimalObject toMinimalObject(`
`817`	`817`	`ElementProjection projection,`
`818`	`818`	`ElementIdentifier identifier`
`819`	`819`	`) {`
`820`		`- if (projection instanceof AbstractNodeProjection) {`
`821`		`- return toMinimalObject(((AbstractNodeProjection) projection), identifier);`
	`820`	`+ if (projection instanceof NodeProjection) {`
	`821`	`+ return toMinimalObject(((NodeProjection) projection), identifier);`
`822`	`822`	`}`
`823`	`823`	`if (projection instanceof RelationshipProjection) {`
`824`	`824`	`return toMinimalObject(((RelationshipProjection) projection), identifier);`
`@@ -827,7 +827,7 @@ private static MinimalObject toMinimalObject(`
`827`	`827`	`}`
`828`	`828`
`829`	`829`	`private static MinimalObject toMinimalObject(`
`830`		`- AbstractNodeProjection projection,`
	`830`	`+ NodeProjection projection,`
`831`	`831`	`ElementIdentifier identifier`
`832`	`832`	`) {`
`833`	`833`	`MinimalObject properties = toMinimalObject(projection.properties(), false);`
`@@ -836,12 +836,12 @@ private static MinimalObject toMinimalObject(`
`836`	`836`	`}`
`837`	`837`
`838`	`838`	`Map<String, Object> value = new LinkedHashMap<>();`
`839`		`- value.put(AbstractNodeProjection.LABEL_KEY, projection.label());`
	`839`	`+ value.put(NodeProjection.LABEL_KEY, projection.label());`
`840`	`840`	`properties.toObject().ifPresent(o -> value.put(PROPERTIES_KEY, o));`
`841`	`841`	`return MinimalObject.map(value);`
`842`	`842`	`}`
`843`	`843`
`844`		`- private static boolean matchesLabel(String label, AbstractNodeProjection projection) {`
	`844`	`+ private static boolean matchesLabel(String label, NodeProjection projection) {`
`845`	`845`	`return Objects.equals(projection.label(), label);`
`846`	`846`	`}`
`847`	`847`