Skip to content

Commit c2c8d78

Browse files
authored
Merge pull request #6611 from knutwalker/cagg-prepare1
Extract interface for CypherMapWrapper
2 parents aced8ed + 60ca30b commit c2c8d78

File tree

24 files changed

+610
-534
lines changed

24 files changed

+610
-534
lines changed

annotations/src/main/java/org/neo4j/gds/core/CypherMapAccess.java

Lines changed: 403 additions & 0 deletions
Large diffs are not rendered by default.

annotations/src/main/java/org/neo4j/gds/core/CypherMapWrapper.java

Lines changed: 29 additions & 404 deletions
Large diffs are not rendered by default.
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Neo4j is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU General Public License as published by
9+
* the Free Software Foundation, either version 3 of the License, or
10+
* (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU General Public License
18+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
19+
*/
20+
package org.neo4j.gds.core;
21+
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.Arguments;
24+
import org.junit.jupiter.params.provider.MethodSource;
25+
26+
import java.util.Locale;
27+
import java.util.stream.Stream;
28+
29+
import static org.junit.jupiter.api.Assertions.assertEquals;
30+
import static org.junit.jupiter.api.Assertions.assertThrows;
31+
32+
class CypherMapAccessTest {
33+
34+
@ParameterizedTest
35+
@MethodSource("positiveRangeValidationParameters")
36+
void shouldValidateDoubleRange(double value, double min, double max, boolean minInclusive, boolean maxInclusive) {
37+
assertEquals(value, CypherMapAccess.validateDoubleRange("value", value, min, max, minInclusive, maxInclusive));
38+
}
39+
40+
@ParameterizedTest
41+
@MethodSource("negativeRangeValidationParameters")
42+
void shouldThrowForInvalidDoubleRange(
43+
double value,
44+
double min,
45+
double max,
46+
boolean minInclusive,
47+
boolean maxInclusive
48+
) {
49+
IllegalArgumentException ex = assertThrows(
50+
IllegalArgumentException.class,
51+
() -> CypherMapAccess.validateDoubleRange("value", value, min, max, minInclusive, maxInclusive)
52+
);
53+
54+
assertEquals(String.format(
55+
Locale.ENGLISH,
56+
"Value for `value` was `%s`, but must be within the range %s%.2f, %.2f%s.",
57+
value,
58+
minInclusive ? "[" : "(",
59+
min,
60+
max,
61+
maxInclusive ? "]" : ")"
62+
), ex.getMessage());
63+
}
64+
65+
@ParameterizedTest
66+
@MethodSource("positiveRangeValidationParameters")
67+
void shouldValidateIntegerRange(int value, int min, int max, boolean minInclusive, boolean maxInclusive) {
68+
assertEquals(
69+
value,
70+
CypherMapAccess.validateIntegerRange("value", value, min, max, minInclusive, maxInclusive)
71+
);
72+
}
73+
74+
@ParameterizedTest
75+
@MethodSource("negativeRangeValidationParameters")
76+
void shouldThrowForInvalidIntegerRange(int value, int min, int max, boolean minInclusive, boolean maxInclusive) {
77+
IllegalArgumentException ex = assertThrows(
78+
IllegalArgumentException.class,
79+
() -> CypherMapAccess.validateIntegerRange("value", value, min, max, minInclusive, maxInclusive)
80+
);
81+
82+
assertEquals(String.format(
83+
Locale.ENGLISH,
84+
"Value for `value` was `%s`, but must be within the range %s%d, %d%s.",
85+
value,
86+
minInclusive ? "[" : "(",
87+
min,
88+
max,
89+
maxInclusive ? "]" : ")"
90+
), ex.getMessage());
91+
}
92+
93+
static Stream<Arguments> positiveRangeValidationParameters() {
94+
return Stream.of(
95+
Arguments.of(42, 42, 84, true, false),
96+
Arguments.of(84, 42, 84, false, true),
97+
Arguments.of(42, 42, 42, true, true)
98+
);
99+
}
100+
101+
static Stream<Arguments> negativeRangeValidationParameters() {
102+
return Stream.of(
103+
Arguments.of(42, 42, 84, false, false),
104+
Arguments.of(84, 42, 84, false, false),
105+
Arguments.of(42, 42, 42, false, false),
106+
107+
Arguments.of(21, 42, 84, true, false),
108+
Arguments.of(85, 42, 84, false, true)
109+
);
110+
}
111+
}

annotations/src/test/java/org/neo4j/gds/core/CypherMapWrapperTest.java

Lines changed: 0 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -128,65 +128,6 @@ void shouldFailOnLossyCasts() {
128128
assertTrue(doubleEx.getMessage().contains("must be of type `Long` but was `Double`"));
129129
}
130130

131-
@ParameterizedTest
132-
@MethodSource("positiveRangeValidationParameters")
133-
void shouldValidateDoubleRange(double value, double min, double max, boolean minInclusive, boolean maxInclusive) {
134-
assertEquals(value, CypherMapWrapper.validateDoubleRange("value", value, min, max, minInclusive, maxInclusive));
135-
}
136-
137-
@ParameterizedTest
138-
@MethodSource("negativeRangeValidationParameters")
139-
void shouldThrowForInvalidDoubleRange(
140-
double value,
141-
double min,
142-
double max,
143-
boolean minInclusive,
144-
boolean maxInclusive
145-
) {
146-
IllegalArgumentException ex = assertThrows(
147-
IllegalArgumentException.class,
148-
() -> CypherMapWrapper.validateDoubleRange("value", value, min, max, minInclusive, maxInclusive)
149-
);
150-
151-
assertEquals(String.format(
152-
Locale.ENGLISH,
153-
"Value for `value` was `%s`, but must be within the range %s%.2f, %.2f%s.",
154-
value,
155-
minInclusive ? "[" : "(",
156-
min,
157-
max,
158-
maxInclusive ? "]" : ")"
159-
), ex.getMessage());
160-
}
161-
162-
@ParameterizedTest
163-
@MethodSource("positiveRangeValidationParameters")
164-
void shouldValidateIntegerRange(int value, int min, int max, boolean minInclusive, boolean maxInclusive) {
165-
assertEquals(
166-
value,
167-
CypherMapWrapper.validateIntegerRange("value", value, min, max, minInclusive, maxInclusive)
168-
);
169-
}
170-
171-
@ParameterizedTest
172-
@MethodSource("negativeRangeValidationParameters")
173-
void shouldThrowForInvalidIntegerRange(int value, int min, int max, boolean minInclusive, boolean maxInclusive) {
174-
IllegalArgumentException ex = assertThrows(
175-
IllegalArgumentException.class,
176-
() -> CypherMapWrapper.validateIntegerRange("value", value, min, max, minInclusive, maxInclusive)
177-
);
178-
179-
assertEquals(String.format(
180-
Locale.ENGLISH,
181-
"Value for `value` was `%s`, but must be within the range %s%d, %d%s.",
182-
value,
183-
minInclusive ? "[" : "(",
184-
min,
185-
max,
186-
maxInclusive ? "]" : ")"
187-
), ex.getMessage());
188-
}
189-
190131
static Stream<Arguments> requiredParams() {
191132
return Stream.of(
192133
arguments(
@@ -328,25 +269,6 @@ void testMutexPairsConflictWithSecondPair(Map<String, Object> map, String expect
328269
assertEquals(expectedMessage, error.getMessage());
329270
}
330271

331-
static Stream<Arguments> positiveRangeValidationParameters() {
332-
return Stream.of(
333-
Arguments.of(42, 42, 84, true, false),
334-
Arguments.of(84, 42, 84, false, true),
335-
Arguments.of(42, 42, 42, true, true)
336-
);
337-
}
338-
339-
static Stream<Arguments> negativeRangeValidationParameters() {
340-
return Stream.of(
341-
Arguments.of(42, 42, 84, false, false),
342-
Arguments.of(84, 42, 84, false, false),
343-
Arguments.of(42, 42, 42, false, false),
344-
345-
Arguments.of(21, 42, 84, true, false),
346-
Arguments.of(85, 42, 84, false, true)
347-
);
348-
}
349-
350272
private static Map<String, Object> map(Object... objects) {
351273
Map<String, Object> map = new LinkedHashMap<>();
352274
for (int i = 0; i < objects.length; ) {

config-generator/src/main/java/org/neo4j/gds/proc/GenerateConfiguration.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import org.neo4j.gds.annotation.Configuration.ConvertWith;
3636
import org.neo4j.gds.annotation.Configuration.Parameter;
3737
import org.neo4j.gds.annotation.ValueClass;
38-
import org.neo4j.gds.core.CypherMapWrapper;
38+
import org.neo4j.gds.core.CypherMapAccess;
3939

4040
import javax.annotation.processing.Messager;
4141
import javax.lang.model.SourceVersion;
@@ -233,7 +233,7 @@ private MethodSpec defineConstructor(
233233

234234
if (requiredMapParameter) {
235235
constructorMethodBuilder.addParameter(
236-
TypeName.get(CypherMapWrapper.class).annotated(NOT_NULL),
236+
TypeName.get(CypherMapAccess.class).annotated(NOT_NULL),
237237
names.get(CONFIG_VAR)
238238
);
239239
}
@@ -321,7 +321,7 @@ private void addConfigFieldToConstructor(
321321
if (!isNullable) {
322322
codeBlock = CodeBlock.of(
323323
"$T.failOnNull($S, $L)",
324-
CypherMapWrapper.class,
324+
CypherMapAccess.class,
325325
definition.configKey(),
326326
codeBlock
327327
);
@@ -362,7 +362,7 @@ private void addValidationCode(MemberDefinition definition, CodeBlock.Builder fi
362362
.getAnnotation(Configuration.IntegerRange.class);
363363
validationConsumer.accept(CodeBlock.of(
364364
"$T.validateIntegerRange($S, $L, $L, $L, $L, $L)",
365-
CypherMapWrapper.class,
365+
CypherMapAccess.class,
366366
definition.configKey(),
367367
definition.fieldName(),
368368
elementUtils.getConstantExpression(range.min()),
@@ -379,7 +379,7 @@ private void addValidationCode(MemberDefinition definition, CodeBlock.Builder fi
379379
.getAnnotation(Configuration.LongRange.class);
380380
validationConsumer.accept(CodeBlock.of(
381381
"$T.validateLongRange($S, $L, $L, $L, $L, $L)",
382-
CypherMapWrapper.class,
382+
CypherMapAccess.class,
383383
definition.configKey(),
384384
definition.fieldName(),
385385
elementUtils.getConstantExpression(range.min()),
@@ -396,7 +396,7 @@ private void addValidationCode(MemberDefinition definition, CodeBlock.Builder fi
396396
.getAnnotation(Configuration.DoubleRange.class);
397397
validationConsumer.accept(CodeBlock.of(
398398
"$T.validateDoubleRange($S, $L, $L, $L, $L, $L)",
399-
CypherMapWrapper.class,
399+
CypherMapAccess.class,
400400
definition.configKey(),
401401
definition.fieldName(),
402402
elementUtils.getConstantExpression(range.min()),
@@ -423,7 +423,7 @@ private void addParameterToConstructor(
423423
paramType = paramType.annotated(NOT_NULL);
424424
valueProducer = CodeBlock.of(
425425
"$T.failOnNull($S, $N)",
426-
CypherMapWrapper.class,
426+
CypherMapAccess.class,
427427
definition.configKey(),
428428
definition.fieldName()
429429
);

config-generator/src/test/resources/expected/CollectingKeys.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.stream.Collectors;
2828
import javax.annotation.processing.Generated;
2929
import org.jetbrains.annotations.NotNull;
30+
import org.neo4j.gds.core.CypherMapAccess;
3031
import org.neo4j.gds.core.CypherMapWrapper;
3132

3233
@Generated("org.neo4j.gds.proc.ConfigurationProcessor")
@@ -37,7 +38,7 @@ public final class CollectingKeysConfig implements CollectingKeys {
3738

3839
private double baz;
3940

40-
public CollectingKeysConfig(int foo, @NotNull CypherMapWrapper config) {
41+
public CollectingKeysConfig(int foo, @NotNull CypherMapAccess config) {
4142
ArrayList<IllegalArgumentException> errors = new ArrayList<>();
4243
try {
4344
this.foo = foo;

config-generator/src/test/resources/expected/Conversions.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.stream.Collectors;
2626
import javax.annotation.processing.Generated;
2727
import org.jetbrains.annotations.NotNull;
28+
import org.neo4j.gds.core.CypherMapAccess;
2829
import org.neo4j.gds.core.CypherMapWrapper;
2930

3031
@Generated("org.neo4j.gds.proc.ConfigurationProcessor")
@@ -37,7 +38,7 @@ public final class ConversionsConfig implements Conversions.MyConversion {
3738

3839
private String referenceTypeAsResult;
3940

40-
public ConversionsConfig(@NotNull CypherMapWrapper config) {
41+
public ConversionsConfig(@NotNull CypherMapAccess config) {
4142
ArrayList<IllegalArgumentException> errors = new ArrayList<>();
4243
try {
4344
this.directMethod = Conversions.MyConversion.toInt(config.requireString("directMethod"));
@@ -55,7 +56,7 @@ public ConversionsConfig(@NotNull CypherMapWrapper config) {
5556
errors.add(e);
5657
}
5758
try {
58-
this.referenceTypeAsResult = CypherMapWrapper.failOnNull(
59+
this.referenceTypeAsResult = CypherMapAccess.failOnNull(
5960
"referenceTypeAsResult",
6061
Conversions.MyConversion.add42(config.requireString("referenceTypeAsResult"))
6162
);

config-generator/src/test/resources/expected/ConvertingParameters.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.stream.Collectors;
2626
import javax.annotation.processing.Generated;
2727
import org.jetbrains.annotations.NotNull;
28+
import org.neo4j.gds.core.CypherMapAccess;
2829
import org.neo4j.gds.core.CypherMapWrapper;
2930

3031
@Generated("org.neo4j.gds.proc.ConfigurationProcessor")
@@ -34,7 +35,7 @@ public final class ConvertingParametersConfig implements ConvertingParameters {
3435
public ConvertingParametersConfig(@NotNull String parametersAreSubjectToConversion) {
3536
ArrayList<IllegalArgumentException> errors = new ArrayList<>();
3637
try {
37-
this.parametersAreSubjectToConversion = ConvertingParameters.toInt(CypherMapWrapper.failOnNull(
38+
this.parametersAreSubjectToConversion = ConvertingParameters.toInt(CypherMapAccess.failOnNull(
3839
"parametersAreSubjectToConversion",
3940
parametersAreSubjectToConversion
4041
));

config-generator/src/test/resources/expected/DefaultValues.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.stream.Collectors;
2626
import javax.annotation.processing.Generated;
2727
import org.jetbrains.annotations.NotNull;
28+
import org.neo4j.gds.core.CypherMapAccess;
2829
import org.neo4j.gds.core.CypherMapWrapper;
2930

3031
@Generated("org.neo4j.gds.proc.ConfigurationProcessor")
@@ -33,15 +34,15 @@ public final class DefaultValuesConfig implements DefaultValues {
3334

3435
private String defaultString;
3536

36-
public DefaultValuesConfig(@NotNull CypherMapWrapper config) {
37+
public DefaultValuesConfig(@NotNull CypherMapAccess config) {
3738
ArrayList<IllegalArgumentException> errors = new ArrayList<>();
3839
try {
3940
this.defaultInt = config.getInt("defaultInt", DefaultValues.super.defaultInt());
4041
} catch (IllegalArgumentException e) {
4142
errors.add(e);
4243
}
4344
try {
44-
this.defaultString = CypherMapWrapper.failOnNull(
45+
this.defaultString = CypherMapAccess.failOnNull(
4546
"defaultString",
4647
config.getString("defaultString", DefaultValues.super.defaultString())
4748
);

0 commit comments

Comments
 (0)