Skip to content

Commit 47b8aa2

Browse files
knutwalkerknutwalker
committed
Refactor NodeLabelTokens to not rely on two different meanings of null
1 parent 6a39685 commit 47b8aa2

File tree

4 files changed

+145
-55
lines changed

4 files changed

+145
-55
lines changed

‎core/src/main/java/org/neo4j/gds/core/loading/LazyIdMapBuilder.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,12 @@
1919
*/
2020
package org.neo4j.gds.core.loading;
2121

22-
import org.jetbrains.annotations.NotNull;
23-
import org.jetbrains.annotations.Nullable;
2422
import org.neo4j.gds.annotation.ValueClass;
2523
import org.neo4j.gds.api.IdMap;
2624
import org.neo4j.gds.api.PartialIdMap;
2725
import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
2826
import org.neo4j.gds.core.loading.construction.GraphFactory;
2927
import org.neo4j.gds.core.loading.construction.NodeLabelToken;
30-
import org.neo4j.gds.core.loading.construction.NodeLabelTokens;
3128
import org.neo4j.gds.core.loading.construction.NodesBuilder;
3229
import org.neo4j.gds.core.loading.construction.PropertyValues;
3330
import org.neo4j.gds.core.utils.paged.ShardedLongLongMap;
@@ -54,7 +51,7 @@ public LazyIdMapBuilder(int concurrency, boolean hasLabelInformation, boolean ha
5451
.build();
5552
}
5653

57-
public long addNode(long nodeId, @Nullable NodeLabelToken nodeLabels) {
54+
public long addNode(long nodeId, NodeLabelToken nodeLabels) {
5855
var intermediateId = this.intermediateIdMapBuilder.toMappedNodeId(nodeId);
5956

6057
// deduplication
@@ -65,9 +62,6 @@ public long addNode(long nodeId, @Nullable NodeLabelToken nodeLabels) {
6562
intermediateId = this.intermediateIdMapBuilder.addNode(nodeId);
6663

6764
isEmpty.lazySet(false);
68-
if (nodeLabels == null) {
69-
nodeLabels = NodeLabelTokens.empty();
70-
}
7165
this.nodesBuilder.addNode(intermediateId, nodeLabels);
7266

7367
return intermediateId;
@@ -76,7 +70,7 @@ public long addNode(long nodeId, @Nullable NodeLabelToken nodeLabels) {
7670
public long addNodeWithProperties(
7771
long nodeId,
7872
PropertyValues properties,
79-
@NotNull NodeLabelToken nodeLabels
73+
NodeLabelToken nodeLabels
8074
) {
8175
var intermediateId = this.intermediateIdMapBuilder.toMappedNodeId(nodeId);
8276

‎core/src/main/java/org/neo4j/gds/core/loading/construction/NodeLabelToken.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,21 @@
2424

2525
public interface NodeLabelToken {
2626

27+
/**
28+
* @return `true` iff no label information was provided at all.
29+
*/
30+
boolean isMissing();
31+
32+
/**
33+
* @return `true` iff the provided label information could not be mapped to an internal type
34+
* because it was provided as a wrong type.
35+
*/
36+
boolean isInvalid();
37+
38+
/**
39+
* @return `true` if the provided label information does not actually contain any labels or
40+
* if no label information was provided at all.
41+
*/
2742
boolean isEmpty();
2843

2944
int size();

‎core/src/main/java/org/neo4j/gds/core/loading/construction/NodeLabelTokens.java‎

Lines changed: 114 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.neo4j.gds.core.loading.construction;
2121

2222
import org.eclipse.collections.api.block.function.primitive.ObjectIntToObjectFunction;
23-
import org.jetbrains.annotations.Contract;
2423
import org.jetbrains.annotations.NotNull;
2524
import org.jetbrains.annotations.Nullable;
2625
import org.neo4j.gds.NodeLabel;
@@ -40,32 +39,19 @@
4039
public final class NodeLabelTokens {
4140
public static @NotNull NodeLabelToken of(@Nullable Object nodeLabels) {
4241
var nodeLabelToken = ofNullable(nodeLabels);
43-
if (nodeLabelToken == null) {
42+
if (nodeLabelToken.isInvalid()) {
4443
throw new IllegalArgumentException(formatWithLocale(
4544
"Could not represent a value of type[%s] as nodeLabels: %s",
46-
nodeLabels.getClass(),
45+
nodeLabels != null ? nodeLabels.getClass() : "null",
4746
nodeLabels
4847
));
4948
}
5049
return nodeLabelToken;
5150
}
5251

53-
public static NodeLabelToken of(TextValue textValue) {
54-
return new Singleton<>(textValue, tv -> NodeLabelTokens.labelOf(tv.stringValue()));
55-
}
56-
57-
public static NodeLabelToken of(TextArray textArray) {
58-
return new Sequence<>(textArray, TextArray::stringValue);
59-
}
60-
61-
public static @NotNull NodeLabelToken of(SequenceValue nodeLabels) {
62-
return new Sequence<>(nodeLabels, (s, i) -> ((TextValue) s.value(i)).stringValue());
63-
}
64-
65-
@Contract("null -> !null")
66-
public static @Nullable NodeLabelToken ofNullable(@Nullable Object nodeLabels) {
52+
public static NodeLabelToken ofNullable(@Nullable Object nodeLabels) {
6753
if (nodeLabels == null) {
68-
return empty();
54+
return missing();
6955
}
7056

7157
if (nodeLabels instanceof NodeLabel[]) {
@@ -88,8 +74,28 @@ public static NodeLabelToken of(TextArray textArray) {
8874
return ofSingleton(nodeLabels);
8975
}
9076

77+
public static @NotNull NodeLabelToken missing() {
78+
return Missing.INSTANCE;
79+
}
80+
81+
public static @NotNull NodeLabelToken invalid() {
82+
return Invalid.INSTANCE;
83+
}
84+
9185
public static @NotNull NodeLabelToken empty() {
92-
return Null.INSTANCE;
86+
return Empty.INSTANCE;
87+
}
88+
89+
public static NodeLabelToken of(TextValue textValue) {
90+
return new Singleton<>(textValue, tv -> NodeLabelTokens.labelOf(tv.stringValue()));
91+
}
92+
93+
public static NodeLabelToken of(TextArray textArray) {
94+
return new Sequence<>(textArray, TextArray::stringValue);
95+
}
96+
97+
public static @NotNull NodeLabelToken of(SequenceValue nodeLabels) {
98+
return new Sequence<>(nodeLabels, (s, i) -> ((TextValue) s.value(i)).stringValue());
9399
}
94100

95101
public static @NotNull NodeLabelToken ofNodeLabels(NodeLabel... nodeLabels) {
@@ -104,7 +110,7 @@ public static NodeLabelToken of(TextArray textArray) {
104110
return new Singleton<>(nodeLabel, Function.identity());
105111
}
106112

107-
private static @Nullable NodeLabelToken ofList(List<?> nodeLabels) {
113+
private static NodeLabelToken ofList(List<?> nodeLabels) {
108114
if (nodeLabels.isEmpty()) {
109115
return empty();
110116
}
@@ -128,10 +134,10 @@ public static NodeLabelToken of(TextArray textArray) {
128134
return new JavaList<>((List<Label>) nodeLabels, NodeLabelTokens::labelOf);
129135
}
130136

131-
return null;
137+
return invalid();
132138
}
133139

134-
private static @Nullable NodeLabelToken ofSingleton(Object nodeLabel) {
140+
private static NodeLabelToken ofSingleton(Object nodeLabel) {
135141
if (nodeLabel instanceof NodeLabel) {
136142
return new Singleton<>((NodeLabel) nodeLabel, Function.identity());
137143
}
@@ -144,7 +150,7 @@ public static NodeLabelToken of(TextArray textArray) {
144150
return new Singleton<>((Label) nodeLabel, NodeLabelTokens::labelOf);
145151
}
146152

147-
return null;
153+
return invalid();
148154
}
149155

150156
private static NodeLabel labelOf(String label) {
@@ -155,7 +161,87 @@ private static NodeLabel labelOf(Label label) {
155161
return new NodeLabel(label.name());
156162
}
157163

158-
private enum Null implements NodeLabelToken {
164+
private enum Missing implements NodeLabelToken {
165+
INSTANCE;
166+
167+
@Override
168+
public boolean isMissing() {
169+
return true;
170+
}
171+
172+
@Override
173+
public boolean isEmpty() {
174+
return true;
175+
}
176+
177+
@Override
178+
public boolean isInvalid() {
179+
return false;
180+
}
181+
182+
@Override
183+
public int size() {
184+
return 0;
185+
}
186+
187+
@Override
188+
public @NotNull NodeLabel get(int index) {
189+
throw new NoSuchElementException();
190+
}
191+
192+
@Override
193+
public String[] getStrings() {
194+
return new String[0];
195+
}
196+
}
197+
198+
private enum Invalid implements NodeLabelToken {
199+
INSTANCE;
200+
201+
@Override
202+
public boolean isMissing() {
203+
return false;
204+
}
205+
206+
@Override
207+
public boolean isEmpty() {
208+
return true;
209+
}
210+
211+
@Override
212+
public boolean isInvalid() {
213+
return true;
214+
}
215+
216+
@Override
217+
public int size() {
218+
return 0;
219+
}
220+
221+
@Override
222+
public @NotNull NodeLabel get(int index) {
223+
throw new NoSuchElementException();
224+
}
225+
226+
@Override
227+
public String[] getStrings() {
228+
return new String[0];
229+
}
230+
}
231+
232+
private interface ValidNodeLabelToken extends NodeLabelToken {
233+
@Override
234+
default boolean isMissing() {
235+
return false;
236+
}
237+
238+
@Override
239+
default boolean isInvalid() {
240+
return false;
241+
}
242+
}
243+
244+
private enum Empty implements ValidNodeLabelToken {
159245
INSTANCE;
160246

161247
@Override
@@ -179,7 +265,7 @@ public String[] getStrings() {
179265
}
180266
}
181267

182-
private static final class Array<T> implements NodeLabelToken {
268+
private static final class Array<T> implements ValidNodeLabelToken {
183269
private final @NotNull T[] nodeLabels;
184270

185271
private final Function<T, NodeLabel> toNodeLabel;
@@ -223,7 +309,7 @@ public int hashCode() {
223309
}
224310
}
225311

226-
private static final class JavaList<T> implements NodeLabelToken {
312+
private static final class JavaList<T> implements ValidNodeLabelToken {
227313
private final @NotNull List<T> nodeLabels;
228314

229315
private final Function<T, NodeLabel> toNodeLabel;
@@ -267,7 +353,7 @@ public int hashCode() {
267353
}
268354
}
269355

270-
private static final class Sequence<T extends SequenceValue> implements NodeLabelToken {
356+
private static final class Sequence<T extends SequenceValue> implements ValidNodeLabelToken {
271357
private final T sequence;
272358
private final ObjectIntToObjectFunction<T, String> toString;
273359

@@ -312,7 +398,7 @@ public int hashCode() {
312398
}
313399
}
314400

315-
private static final class Singleton<T> implements NodeLabelToken {
401+
private static final class Singleton<T> implements ValidNodeLabelToken {
316402

317403
private final NodeLabel item;
318404

‎cypher-aggregation/src/main/java/org/neo4j/gds/projection/CypherAggregation.java‎

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ public void update(
154154

155155
Map<String, Value> sourceNodePropertyValues = null;
156156
Map<String, Value> targetNodePropertyValues = null;
157-
@Nullable NodeLabelToken sourceNodeLabels = null;
158-
@Nullable NodeLabelToken targetNodeLabels = null;
157+
NodeLabelToken sourceNodeLabels = NodeLabelTokens.missing();
158+
NodeLabelToken targetNodeLabels = NodeLabelTokens.missing();
159159

160160
if (nodesConfig != null) {
161161
sourceNodePropertyValues = propertiesConfig("sourceNodeProperties", nodesConfig);
@@ -265,8 +265,8 @@ private void initGraphName(String graphName) {
265265
private void initIdMapBuilder(
266266
@Nullable Map<String, Value> sourceNodePropertyValues,
267267
@Nullable Map<String, Value> targetNodePropertyValues,
268-
@Nullable NodeLabelToken sourceNodeLabels,
269-
@Nullable NodeLabelToken targetNodeLabels
268+
NodeLabelToken sourceNodeLabels,
269+
NodeLabelToken targetNodeLabels
270270
) {
271271
if (this.idMapBuilder == null) {
272272
initObjectUnderLock(() -> this.idMapBuilder, () -> {
@@ -297,14 +297,14 @@ private void initObjectUnderLock(Supplier<Object> s, Runnable code) {
297297

298298
@NotNull
299299
private LazyIdMapBuilder newIdMapBuilder(
300-
@Nullable NodeLabelToken sourceNodeLabels,
300+
NodeLabelToken sourceNodeLabels,
301301
@Nullable Map<String, Value> sourceNodeProperties,
302-
@Nullable NodeLabelToken targetNodeLabels,
302+
NodeLabelToken targetNodeLabels,
303303
@Nullable Map<String, Value> targetNodeProperties
304304
) {
305305
assert this.config != null;
306306

307-
boolean hasLabelInformation = !(sourceNodeLabels == null && targetNodeLabels == null);
307+
boolean hasLabelInformation = !(sourceNodeLabels.isMissing() && targetNodeLabels.isMissing());
308308
boolean hasProperties = !(sourceNodeProperties == null && targetNodeProperties == null);
309309
return new LazyIdMapBuilder(config.readConcurrency(), hasLabelInformation, hasProperties);
310310
}
@@ -346,28 +346,26 @@ private NodeLabelToken tryLabelsConfig(
346346
@Nullable Object nodeLabels,
347347
String nodeLabelKey
348348
) {
349-
if (nodeLabels == null || Boolean.FALSE.equals(nodeLabels)) {
349+
if (Boolean.FALSE.equals(nodeLabels)) {
350350
return NodeLabelTokens.empty();
351351
}
352352

353-
NodeLabelToken nodeLabelToken;
354-
355353
if (Boolean.TRUE.equals(nodeLabels)) {
356354
if (!(node instanceof Node)) {
357355
throw new IllegalArgumentException(
358356
"Using `true` to load all labels does only work if the node is a Neo4j node object"
359357
);
360358
}
361-
nodeLabelToken = NodeLabelTokens.ofNullable(((Node) node).getLabels());
362-
} else {
363-
nodeLabelToken = NodeLabelTokens.ofNullable(nodeLabels);
359+
nodeLabels = ((Node) node).getLabels();
364360
}
365361

366-
if (nodeLabelToken == null) {
362+
var nodeLabelToken = NodeLabelTokens.ofNullable(nodeLabels);
363+
364+
if (nodeLabelToken.isInvalid()) {
367365
throw new IllegalArgumentException(formatWithLocale(
368366
"The value of `%s` must be either a `List of Strings`, a `String`, or a `Boolean`, but was `%s`.",
369367
nodeLabelKey,
370-
nodeLabels.getClass().getSimpleName()
368+
nodeLabels != null ? nodeLabels.getClass().getSimpleName() : "null"
371369
));
372370
}
373371

@@ -507,15 +505,12 @@ private IllegalArgumentException invalidNodeType(@Nullable Object node) {
507505
*/
508506
private long loadNode(
509507
@Nullable Object node,
510-
@Nullable NodeLabelToken nodeLabels,
508+
NodeLabelToken nodeLabels,
511509
@Nullable Map<String, Value> nodeProperties
512510
) {
513511
assert this.idMapBuilder != null;
514512

515513
var originalNodeId = extractNodeId(node);
516-
if (nodeLabels == null) {
517-
nodeLabels = NodeLabelTokens.empty();
518-
}
519514

520515
return nodeProperties == null
521516
? this.idMapBuilder.addNode(originalNodeId, nodeLabels)

0 commit comments

Comments
 (0)