Skip to content

Commit 6f517e9

Browse files
IoannisPanagiotasIoannisPanagiotas
committed
Move Validation out of algorithm and change exception being thrown
1 parent fd96d72 commit 6f517e9

File tree

16 files changed

+251
-26
lines changed

16 files changed

+251
-26
lines changed

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,8 @@
2525
import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
2626
import org.neo4j.gds.similarity.knn.KnnNodePropertySpec;
2727
import org.neo4j.gds.similarity.knn.metrics.LongArrayPropertySimilarityComputer.SortedLongArrayPropertyValues;
28-
import org.neo4j.gds.utils.StringJoining;
2928

3029
import java.util.List;
31-
import java.util.Objects;
3230

3331
import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
3432

@@ -52,14 +50,7 @@ static SimilarityComputer ofProperties(Graph graph, List<KnnNodePropertySpec> kn
5250

5351
static SimilarityComputer ofProperty(Graph graph, KnnNodePropertySpec knnNodePropertySpec) {
5452
var propertyName = knnNodePropertySpec.name();
55-
var nodeProperties = Objects.requireNonNull(
56-
graph.nodeProperties(propertyName),
57-
() -> formatWithLocale(
58-
"The property `%s` has not been loaded. Available properties: %s",
59-
propertyName,
60-
StringJoining.join(graph.availableNodeProperties())
61-
)
62-
);
53+
var nodeProperties = graph.nodeProperties(propertyName);
6354

6455
if (knnNodePropertySpec.metric() == SimilarityMetric.DEFAULT) {
6556
knnNodePropertySpec.setMetric(SimilarityMetric.defaultMetricForType(nodeProperties.valueType()));

‎applications/algorithms/similarity/build.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@ dependencies {
1414
implementation project(":memory-usage")
1515
implementation project(":progress-tracking")
1616
implementation project(":similarity-configs")
17+
implementation project(':string-formatting')
1718
implementation project(":termination")
1819

1920
// this is needed because NodeSimilarity internally calls Wcc
2021
implementation project(':community-algorithms')
2122
implementation project(":community-configs")
23+
24+
testImplementation project(':test-utils')
25+
2226
}

‎applications/algorithms/similarity/src/main/java/org/neo4j/gds/applications/algorithms/similarity/KnnHook.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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.applications.algorithms.similarity;
21+
22+
import org.neo4j.gds.api.Graph;
23+
import org.neo4j.gds.api.GraphStore;
24+
import org.neo4j.gds.core.loading.PostLoadValidationHook;
25+
import org.neo4j.gds.similarity.knn.KnnNodePropertySpec;
26+
import org.neo4j.gds.utils.StringJoining;
27+
28+
import java.util.List;
29+
30+
import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
31+
32+
public class KnnHook implements PostLoadValidationHook {
33+
34+
private List<KnnNodePropertySpec> knnNodeProperties;
35+
36+
public KnnHook(List<KnnNodePropertySpec> knnNodeProperties) {this.knnNodeProperties = knnNodeProperties;}
37+
38+
@Override
39+
public void onGraphStoreLoaded(GraphStore graphStore) {
40+
//ignore me
41+
}
42+
43+
@Override
44+
public void onGraphLoaded(Graph graph) {
45+
for (var property : knnNodeProperties){
46+
var propertyName = property.name();
47+
if (graph.nodeProperties(propertyName) == null){
48+
throw new IllegalArgumentException(
49+
formatWithLocale("The property `%s` has not been loaded. Available properties: %s",
50+
propertyName,
51+
StringJoining.join(graph.availableNodeProperties()))
52+
);
53+
}
54+
55+
}
56+
}
57+
}

‎applications/algorithms/similarity/src/main/java/org/neo4j/gds/applications/algorithms/similarity/SimilarityAlgorithmsMutateModeBusinessFacade.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import org.neo4j.gds.similarity.nodesim.NodeSimilarityMutateConfig;
3434
import org.neo4j.gds.similarity.nodesim.NodeSimilarityResult;
3535

36+
import java.util.List;
3637
import java.util.Map;
38+
import java.util.Optional;
3739

3840
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredKNN;
3941
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredNodeSimilarity;
@@ -70,15 +72,19 @@ public <RESULT> RESULT filteredKnn(
7072
shouldComputeSimilarityDistribution
7173
);
7274

73-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInMutateMode(
75+
return algorithmProcessingTemplateConvenience.processAlgorithmInMutateMode(
76+
Optional.empty(),
7477
graphName,
7578
configuration,
79+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
80+
Optional.empty(),
7681
FilteredKNN,
7782
() -> estimationFacade.filteredKnn(configuration),
7883
(graph, __) -> similarityAlgorithms.filteredKnn(graph, configuration),
7984
mutateStep,
8085
resultBuilder
8186
);
87+
8288
}
8389

8490
public <RESULT> RESULT filteredNodeSimilarity(
@@ -115,9 +121,12 @@ public <RESULT> RESULT knn(
115121
shouldComputeSimilarityDistribution
116122
);
117123

118-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInMutateMode(
124+
return algorithmProcessingTemplateConvenience.processAlgorithmInMutateMode(
125+
Optional.empty(),
119126
graphName,
120127
configuration,
128+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
129+
Optional.empty(),
121130
KNN,
122131
() -> estimationFacade.knn(configuration),
123132
(graph, __) -> similarityAlgorithms.knn(graph, configuration),

‎applications/algorithms/similarity/src/main/java/org/neo4j/gds/applications/algorithms/similarity/SimilarityAlgorithmsStatsModeBusinessFacade.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
import org.neo4j.gds.similarity.nodesim.NodeSimilarityResult;
3131
import org.neo4j.gds.similarity.nodesim.NodeSimilarityStatsConfig;
3232

33+
import java.util.List;
34+
import java.util.Optional;
35+
3336
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredKNN;
3437
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredNodeSimilarity;
3538
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.KNN;
@@ -55,13 +58,16 @@ public <RESULT> RESULT filteredKnn(
5558
FilteredKnnStatsConfig configuration,
5659
StatsResultBuilder<FilteredKnnResult, RESULT> resultBuilder
5760
) {
58-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInStatsMode(
61+
return algorithmProcessingTemplateConvenience.processAlgorithmInStatsMode(
5962
graphName,
6063
configuration,
6164
FilteredKNN,
6265
() -> estimationFacade.filteredKnn(configuration),
6366
(graph, __) -> similarityAlgorithms.filteredKnn(graph, configuration),
64-
resultBuilder
67+
resultBuilder,
68+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
69+
Optional.empty(),
70+
Optional.empty()
6571
);
6672
}
6773

@@ -85,13 +91,16 @@ public <RESULT> RESULT knn(
8591
KnnStatsConfig configuration,
8692
StatsResultBuilder<KnnResult, RESULT> resultBuilder
8793
) {
88-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInStatsMode(
94+
return algorithmProcessingTemplateConvenience.processAlgorithmInStatsMode(
8995
graphName,
9096
configuration,
9197
KNN,
9298
() -> estimationFacade.knn(configuration),
9399
(graph, __) -> similarityAlgorithms.knn(graph, configuration),
94-
resultBuilder
100+
resultBuilder,
101+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
102+
Optional.empty(),
103+
Optional.empty()
95104
);
96105
}
97106

‎applications/algorithms/similarity/src/main/java/org/neo4j/gds/applications/algorithms/similarity/SimilarityAlgorithmsStreamModeBusinessFacade.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import org.neo4j.gds.similarity.nodesim.NodeSimilarityResult;
3131
import org.neo4j.gds.similarity.nodesim.NodeSimilarityStreamConfig;
3232

33+
import java.util.List;
34+
import java.util.Optional;
3335
import java.util.stream.Stream;
3436

3537
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredKNN;
@@ -57,13 +59,16 @@ public <RESULT> Stream<RESULT> filteredKnn(
5759
FilteredKnnStreamConfig configuration,
5860
StreamResultBuilder<FilteredKnnResult, RESULT> resultBuilder
5961
) {
60-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInStreamMode(
62+
return algorithmProcessingTemplateConvenience.processAlgorithmInStreamMode(
6163
graphName,
6264
configuration,
6365
FilteredKNN,
6466
() -> estimationFacade.filteredKnn(configuration),
6567
(graph, __) -> similarityAlgorithms.filteredKnn(graph, configuration),
66-
resultBuilder
68+
resultBuilder,
69+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
70+
Optional.empty(),
71+
Optional.empty()
6772
);
6873
}
6974

@@ -87,13 +92,16 @@ public <RESULT> Stream<RESULT> knn(
8792
KnnStreamConfig configuration,
8893
StreamResultBuilder<KnnResult, RESULT> resultBuilder
8994
) {
90-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInStreamMode(
95+
return algorithmProcessingTemplateConvenience.processAlgorithmInStreamMode(
9196
graphName,
9297
configuration,
9398
KNN,
9499
() -> estimationFacade.knn(configuration),
95100
(graph, __) -> similarityAlgorithms.knn(graph, configuration),
96-
resultBuilder
101+
resultBuilder,
102+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
103+
Optional.empty(),
104+
Optional.empty()
97105
);
98106
}
99107

‎applications/algorithms/similarity/src/main/java/org/neo4j/gds/applications/algorithms/similarity/SimilarityAlgorithmsWriteModeBusinessFacade.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@
3333
import org.neo4j.gds.similarity.nodesim.NodeSimilarityResult;
3434
import org.neo4j.gds.similarity.nodesim.NodeSimilarityWriteConfig;
3535

36+
import java.util.List;
3637
import java.util.Map;
38+
import java.util.Optional;
3739

3840
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredKNN;
3941
import static org.neo4j.gds.applications.algorithms.machinery.AlgorithmLabel.FilteredNodeSimilarity;
@@ -70,14 +72,18 @@ public <RESULT> RESULT filteredKnn(
7072
writeRelationshipService
7173
);
7274

73-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInWriteMode(
75+
return algorithmProcessingTemplateConvenience.processAlgorithmInWriteMode(
76+
Optional.empty(),
7477
graphName,
7578
configuration,
79+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
80+
Optional.empty(),
7681
FilteredKNN,
7782
() -> estimationFacade.filteredKnn(configuration),
7883
(graph, __) -> similarityAlgorithms.filteredKnn(graph, configuration),
7984
writeStep,
8085
resultBuilder
86+
8187
);
8288
}
8389

@@ -116,9 +122,12 @@ public <RESULT> RESULT knn(
116122
writeRelationshipService
117123
);
118124

119-
return algorithmProcessingTemplateConvenience.processRegularAlgorithmInWriteMode(
125+
return algorithmProcessingTemplateConvenience.processAlgorithmInWriteMode(
126+
Optional.empty(),
120127
graphName,
121128
configuration,
129+
Optional.of(List.of(new KnnHook(configuration.nodeProperties()))),
130+
Optional.empty(),
122131
KNN,
123132
() -> estimationFacade.knn(configuration),
124133
(graph, __) -> similarityAlgorithms.knn(graph, configuration),

‎applications/algorithms/similarity/src/test/java/org/neo4j/gds/applications/algorithms/similarity/KnnHookTest.java

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
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.applications.algorithms.similarity;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.gds.api.Graph;
24+
import org.neo4j.gds.api.properties.nodes.LongNodePropertyValues;
25+
import org.neo4j.gds.similarity.knn.KnnNodePropertySpec;
26+
27+
import java.util.List;
28+
import java.util.Set;
29+
30+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
31+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
32+
import static org.mockito.Mockito.mock;
33+
import static org.mockito.Mockito.when;
34+
35+
class KnnHookTest {
36+
37+
@Test
38+
void shouldNotThrowForExistingProperty(){
39+
var graph = mock(Graph.class);
40+
var nodeProp = mock(LongNodePropertyValues.class);
41+
when(graph.nodeProperties("foo")).thenReturn(nodeProp);
42+
43+
var knnSpec = mock(KnnNodePropertySpec.class);
44+
when(knnSpec.name()).thenReturn("foo");
45+
46+
var hook = new KnnHook(List.of(knnSpec));
47+
48+
assertDoesNotThrow( () -> hook.onGraphLoaded(graph));
49+
}
50+
51+
@Test
52+
void shouldThrowForNonExistingProperty(){
53+
var graph = mock(Graph.class);
54+
var nodeProp = mock(LongNodePropertyValues.class);
55+
when(graph.nodeProperties("foo")).thenReturn(nodeProp);
56+
when(graph.nodeProperties("bar")).thenReturn(null);
57+
when(graph.availableNodeProperties()).thenReturn(Set.of("foo"));
58+
59+
var knnSpec1 = mock(KnnNodePropertySpec.class);
60+
when(knnSpec1.name()).thenReturn("foo");
61+
var knnSpec2 = mock(KnnNodePropertySpec.class);
62+
when(knnSpec2.name()).thenReturn("bar");
63+
64+
var hook = new KnnHook(List.of(knnSpec1, knnSpec2));
65+
66+
assertThatThrownBy(() -> hook.onGraphLoaded(graph))
67+
.hasMessageContaining("The property `bar` has not been loaded. Available properties: ['foo']");
68+
}
69+
70+
}

‎proc/similarity/src/test/java/org/neo4j/gds/similarity/filteredknn/FilteredKnnMutateProcTest.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import java.util.List;
3737

3838
import static org.assertj.core.api.Assertions.assertThat;
39+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
3940
import static org.assertj.core.api.InstanceOfAssertFactories.DOUBLE;
4041
import static org.assertj.core.api.InstanceOfAssertFactories.LONG;
4142
import static org.assertj.core.api.InstanceOfAssertFactories.MAP;
@@ -282,4 +283,11 @@ void shouldMutateWithFilteredNodesAndMultipleProperties() {
282283
);
283284
}
284285

286+
@Test
287+
void shouldIllegalArgumentExceptionThrowForNullProperty(){
288+
var query = "CALL gds.knn.filtered.mutate('filteredKnnGraph', {nodeProperties: ['foo'], mutateProperty: 'foo', mutateRelationshipType: 'bar'}) YIELD *";
289+
assertThatThrownBy(() -> runQuery(query))
290+
.hasMessageContaining("Caused by: java.lang.IllegalArgumentException");
291+
}
292+
285293
}

0 commit comments

Comments
 (0)