Replace DataClass in PropertyMappings

FlorentinD · s1ck · commit e9dc7c791679 · 2023-01-24T15:53:27.000+01:00
This removes the last usage of the DataClass
diff --git a/algo-common/src/main/java/org/neo4j/gds/scaling/ScalePropertiesBaseConfig.java b/algo-common/src/main/java/org/neo4j/gds/scaling/ScalePropertiesBaseConfig.java
@@ -27,7 +27,7 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import static org.neo4j.gds.AbstractPropertyMappings.fromObject;
+import static org.neo4j.gds.PropertyMappings.fromObject;
 
 @ValueClass
 @SuppressWarnings("immutables:subtype")
diff --git a/annotations/src/main/java/org/neo4j/gds/annotation/DataClass.java b/annotations/src/main/java/org/neo4j/gds/annotation/DataClass.java
diff --git a/annotations/src/main/resources/META-INF/annotations/org.immutables.value.immutable b/annotations/src/main/resources/META-INF/annotations/org.immutables.value.immutable
@@ -1,2 +1 @@
-org.neo4j.gds.annotation.DataClass
 org.neo4j.gds.annotation.ValueClass
diff --git a/core/src/main/java/org/neo4j/gds/config/GraphProjectFromStoreConfig.java b/core/src/main/java/org/neo4j/gds/config/GraphProjectFromStoreConfig.java
@@ -63,16 +63,16 @@ public interface GraphProjectFromStoreConfig extends GraphProjectConfig {
 
     @Value.Default
     @Value.Parameter(false)
-    @Configuration.ConvertWith(method = "org.neo4j.gds.AbstractPropertyMappings#fromObject")
-    @Configuration.ToMapValue("org.neo4j.gds.AbstractPropertyMappings#toObject")
+    @Configuration.ConvertWith(method = "org.neo4j.gds.PropertyMappings#fromObject")
+    @Configuration.ToMapValue("org.neo4j.gds.PropertyMappings#toObject")
     default PropertyMappings nodeProperties() {
         return PropertyMappings.of();
     }
 
     @Value.Default
     @Value.Parameter(false)
-    @Configuration.ConvertWith(method = "org.neo4j.gds.AbstractPropertyMappings#fromObject")
-    @Configuration.ToMapValue("org.neo4j.gds.AbstractPropertyMappings#toObject")
+    @Configuration.ConvertWith(method = "org.neo4j.gds.PropertyMappings#fromObject")
+    @Configuration.ToMapValue("org.neo4j.gds.PropertyMappings#toObject")
     default PropertyMappings relationshipProperties() {
         return PropertyMappings.of();
     }
diff --git a/core/src/main/java/org/neo4j/gds/core/loading/CypherRelationshipLoader.java b/core/src/main/java/org/neo4j/gds/core/loading/CypherRelationshipLoader.java
@@ -23,6 +23,7 @@
 import org.eclipse.collections.impl.map.mutable.primitive.ObjectDoubleHashMap;
 import org.eclipse.collections.impl.map.mutable.primitive.ObjectIntHashMap;
 import org.immutables.value.Value;
+import org.neo4j.gds.ImmutablePropertyMappings;
 import org.neo4j.gds.Orientation;
 import org.neo4j.gds.PropertyMapping;
 import org.neo4j.gds.PropertyMappings;
@@ -123,7 +124,7 @@ BatchLoadResult loadSingleBatch(InternalTransaction tx, int bufferSize) {
                 ))
                 .collect(Collectors.toList());
 
-            initFromPropertyMappings(PropertyMappings.of(propertyMappings));
+            initFromPropertyMappings(ImmutablePropertyMappings.of(propertyMappings));
 
             initializedFromResult = true;
         }
diff --git a/graph-projection-api/src/main/java/org/neo4j/gds/ElementProjection.java b/graph-projection-api/src/main/java/org/neo4j/gds/ElementProjection.java
@@ -102,7 +102,7 @@ default Self addProperty(
         ) {
             inlineBuilder()
                 .propertiesBuilder()
-                .addMapping(propertyKey, neoPropertyKey, defaultValue, aggregation);
+                .addMapping(PropertyMapping.of(propertyKey, neoPropertyKey, defaultValue, aggregation));
             return (Self) this;
         }
 
@@ -126,7 +126,7 @@ default void buildProperties() {
     static final class InlinePropertiesBuilder {
         private final Supplier<PropertyMappings> getProperties;
         private final Consumer<PropertyMappings> setProperties;
-        private AbstractPropertyMappings.Builder propertiesBuilder;
+        private PropertyMappings.Builder propertiesBuilder;
 
         InlinePropertiesBuilder(
             Supplier<PropertyMappings> getProperties,
@@ -151,9 +151,9 @@ private void build() {
             }
         }
 
-        private AbstractPropertyMappings.Builder propertiesBuilder() {
+        private PropertyMappings.Builder propertiesBuilder() {
             if (propertiesBuilder == null) {
-                propertiesBuilder = AbstractPropertyMappings.builder();
+                propertiesBuilder = PropertyMappings.builder();
                 PropertyMappings properties = getProperties.get();
                 if (properties != null) {
                     propertiesBuilder.from(properties);
diff --git a/graph-projection-api/src/main/java/org/neo4j/gds/PropertyMappings.java b/graph-projection-api/src/main/java/org/neo4j/gds/PropertyMappings.java
@@ -19,12 +19,10 @@
  */
 package org.neo4j.gds;
 
-import org.eclipse.collections.api.tuple.primitive.IntObjectPair;
 import org.immutables.builder.Builder.AccessibleFields;
 import org.immutables.value.Value;
-import org.neo4j.gds.annotation.DataClass;
+import org.neo4j.gds.annotation.ValueClass;
 import org.neo4j.gds.core.Aggregation;
-import org.neo4j.gds.core.utils.CollectionUtil;
 
 import java.util.Arrays;
 import java.util.Iterator;
@@ -40,27 +38,27 @@
 import static java.util.Collections.singletonMap;
 import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
 
-@DataClass
+@ValueClass
 @Value.Immutable(singleton = true)
-public abstract class AbstractPropertyMappings implements Iterable<PropertyMapping> {
+public abstract class PropertyMappings implements Iterable<PropertyMapping> {
 
     public abstract List<PropertyMapping> mappings();
 
     public static PropertyMappings of(PropertyMapping... mappings) {
         if (mappings == null) {
-            return PropertyMappings.of();
+            return ImmutablePropertyMappings.of();
         }
-        return PropertyMappings.of(Arrays.asList(mappings));
+        return ImmutablePropertyMappings.of(Arrays.asList(mappings));
     }
 
     public static PropertyMappings fromObject(Object relPropertyMapping) {
         return fromObject(relPropertyMapping, Aggregation.DEFAULT);
     }
 
     public static PropertyMappings fromObject(Object relPropertyMapping, Aggregation defaultAggregation) {
-        if (relPropertyMapping instanceof PropertyMappings) {
-            PropertyMappings properties = (PropertyMappings) relPropertyMapping;
-            return PropertyMappings.builder().from(properties).withDefaultAggregation(defaultAggregation).build();
+        if (relPropertyMapping instanceof ImmutablePropertyMappings) {
+            ImmutablePropertyMappings properties = (ImmutablePropertyMappings) relPropertyMapping;
+            return ImmutablePropertyMappings.builder().from(properties).withDefaultAggregation(defaultAggregation).build();
         }
         if (relPropertyMapping instanceof String) {
             String propertyMapping = (String) relPropertyMapping;
@@ -95,7 +93,7 @@ public static PropertyMappings fromObject(Object relPropertyMapping, Aggregation
         }
     }
 
-    public static Map<String, Object> toObject(AbstractPropertyMappings propertyMappings) {
+    public static Map<String, Object> toObject(PropertyMappings propertyMappings) {
         return propertyMappings.toObject(true);
     }
 
@@ -112,10 +110,6 @@ public Iterator<PropertyMapping> iterator() {
         return mappings().iterator();
     }
 
-    public Stream<IntObjectPair<PropertyMapping>> enumerate() {
-        return CollectionUtil.enumerate(mappings());
-    }
-
     public boolean hasMappings() {
         return !mappings().isEmpty();
     }
@@ -144,7 +138,7 @@ public PropertyMappings mergeWith(PropertyMappings other) {
             return other;
         }
         if (!other.hasMappings()) {
-            return PropertyMappings.copyOf(this);
+            return ImmutablePropertyMappings.copyOf(this);
         }
         Builder builder = PropertyMappings.builder();
         builder.addMappings(Stream.concat(stream(), other.stream()).distinct());
@@ -168,7 +162,7 @@ public static Builder builder() {
     }
 
     @AccessibleFields
-    public static final class Builder extends PropertyMappings.Builder {
+    public static final class Builder extends ImmutablePropertyMappings.Builder {
 
         private Aggregation aggregation;
 
diff --git a/io/core/src/main/java/org/neo4j/gds/core/io/GraphStoreExporterBaseConfig.java b/io/core/src/main/java/org/neo4j/gds/core/io/GraphStoreExporterBaseConfig.java
@@ -51,8 +51,8 @@ default int batchSize() {
 
     @Value.Default
     @Value.Parameter(false)
-    @Configuration.ConvertWith(method = "org.neo4j.gds.AbstractPropertyMappings#fromObject")
-    @Configuration.ToMapValue("org.neo4j.gds.AbstractPropertyMappings#toObject")
+    @Configuration.ConvertWith(method = "org.neo4j.gds.PropertyMappings#fromObject")
+    @Configuration.ToMapValue("org.neo4j.gds.PropertyMappings#toObject")
     default org.neo4j.gds.PropertyMappings additionalNodeProperties() {
         return org.neo4j.gds.PropertyMappings.of();
     }
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,
                 ImmutableNodeProjections.of(
                     Map.of(
-                        NodeLabel.of("X"), ImmutableNodeProjection.of("X", PropertyMappings.of(propertyMappings))
+                        NodeLabel.of("X"), ImmutableNodeProjection.of("X", ImmutablePropertyMappings.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,10 @@ default void testSeedPropertyValidation() {
         GraphProjectFromStoreConfig graphProjectConfig = ImmutableGraphProjectFromStoreConfig.of(
             "",
             graphName,
-            NodeProjections.single(NodeLabel.of("A"), ImmutableNodeProjection.of("A", PropertyMappings.of(nodeProperties))),
+            NodeProjections.single(
+                NodeLabel.of("A"),
+                ImmutableNodeProjection.of("A", ImmutablePropertyMappings.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,
             NodeProjections.create(singletonMap(
                 ALL_NODES,
-                ImmutableNodeProjection.of(PROJECT_ALL, PropertyMappings.of(propertyMappings))
+                ImmutableNodeProjection.of(PROJECT_ALL, ImmutablePropertyMappings.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
@@ -30,7 +30,6 @@
 import java.lang.reflect.InvocationTargetException;
 import java.util.Collections;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -62,11 +61,11 @@ default void testWriteBackGraphMutationOnFilteredGraph() {
             .graphName(graphName)
             .addNodeProjection(ImmutableNodeProjection.of(
                 "A",
-                PropertyMappings.of(nodeProperties().stream().map(PropertyMapping::of).collect(Collectors.toList()))
+                PropertyMappings.fromObject(nodeProperties())
             ))
             .addNodeProjection(ImmutableNodeProjection.of(
                 "B",
-                PropertyMappings.of(nodeProperties().stream().map(PropertyMapping::of).collect(Collectors.toList()))
+                PropertyMappings.fromObject(nodeProperties())
             ));
 
 
diff --git a/proc/test/src/main/java/org/neo4j/gds/MutatePropertyProcTest.java b/proc/test/src/main/java/org/neo4j/gds/MutatePropertyProcTest.java
@@ -103,7 +103,7 @@ default void testGraphMutationOnFilteredGraph() {
             .orElse(Orientation.NATURAL);
         GraphStore graphStore = new TestNativeGraphLoader(graphDb())
             .withLabels("A", "B")
-            .withNodeProperties(PropertyMappings.of(nodeProperties()
+            .withNodeProperties(ImmutablePropertyMappings.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
@@ -662,8 +662,8 @@ static GraphProjectFromStoreConfig inlineGraphProjectConfig(
             .graphName(graphName.orElse(""))
             .nodeProjections(NodeProjections.create(nodeProjections))
             .relationshipProjections(ImmutableRelationshipProjections.builder().putAllProjections(relProjections).build())
-            .nodeProperties(PropertyMappings.of(nodeProperties))
-            .relationshipProperties(PropertyMappings.of(relProperties))
+            .nodeProperties(ImmutablePropertyMappings.of(nodeProperties))
+            .relationshipProperties(ImmutablePropertyMappings.of(relProperties))
             .build();
     }
 
@@ -881,7 +881,7 @@ private static boolean matchesType(String type, RelationshipProjection projectio
     }
 
     private static MinimalObject toMinimalObject(
-        AbstractPropertyMappings propertyMappings,
+        PropertyMappings propertyMappings,
         boolean includeAggregation
     ) {
         List<PropertyMapping> mappings = propertyMappings.mappings();
diff --git a/test-utils/src/main/java/org/neo4j/gds/GraphProjectConfigBuilders.java b/test-utils/src/main/java/org/neo4j/gds/GraphProjectConfigBuilders.java
@@ -126,7 +126,7 @@ public static GraphProjectFromStoreConfig storeConfig(
             .graphName(graphName.orElse(""))
             .nodeProjections(np)
             .relationshipProjections(rp)
-            .nodeProperties(PropertyMappings.of(nodeProperties))
+            .nodeProperties(ImmutablePropertyMappings.of(nodeProperties))
             .relationshipProperties(relationshipPropertyMappings)
             .readConcurrency(concurrency.orElse(ConcurrencyConfig.DEFAULT_CONCURRENCY))
             .jobId(jobId.orElse(new JobId()))

Original file line number	Diff line number	Diff line change
`@@ -1,2 +1 @@`
`1`		`-org.neo4j.gds.annotation.DataClass`
`2`	`1`	`org.neo4j.gds.annotation.ValueClass`
Original file line number	Diff line number	Diff line change
`@@ -323,7 +323,7 @@ default void testRunOnEmptyGraph() {`
`323`	`323`	`loadedGraphName,`
`324`	`324`	`ImmutableNodeProjections.of(`
`325`	`325`	`Map.of(`
`326`		`- NodeLabel.of("X"), ImmutableNodeProjection.of("X", PropertyMappings.of(propertyMappings))`
	`326`	`+ NodeLabel.of("X"), ImmutableNodeProjection.of("X", ImmutablePropertyMappings.of(propertyMappings))`
`327`	`327`	`)`
`328`	`328`	`),`
`329`	`329`	`relationshipProjections()`