Skip to content

Commit 007f82b

Browse files
vnickolovvnickolov
committed
Handle null remainingSizes in Bridges result mapping
Fixes #354
1 parent fd96d72 commit 007f82b

File tree

9 files changed

+269
-35
lines changed

9 files changed

+269
-35
lines changed

‎algo/src/main/java/org/neo4j/gds/bridges/Bridge.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
*/
2020
package org.neo4j.gds.bridges;
2121

22-
public record Bridge(long from, long to,long[] remainingSizes) {
22+
import org.jetbrains.annotations.Nullable;
23+
24+
public record Bridge(long from, long to, long @Nullable [] remainingSizes) {
2325

2426
static Bridge create(long from, long to, long[] remainingSizes){
2527
return new Bridge(Math.min(from,to), Math.max(from,to), remainingSizes);

‎proc/centrality/src/integrationTest/java/org/neo4j/gds/bridges/BridgesStreamProcTest.java

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
import org.neo4j.gds.extension.Neo4jGraph;
3131

3232
import static org.assertj.core.api.Assertions.assertThat;
33+
import static org.assertj.core.api.Assertions.assertThatNoException;
3334

34-
class BridgesStreamProcTest extends BaseProcTest {
35+
class BridgesStreamProcTest extends BaseProcTest {
3536

3637
@Neo4jGraph
3738
private static final String DB_CYPHER =
@@ -66,27 +67,34 @@ void setup() throws Exception {
6667
}
6768

6869
@Test
69-
void shouldStreamBackResults(){
70-
var query = GdsCypher.call(DEFAULT_GRAPH_NAME)
71-
.algo("gds.bridges")
72-
.streamMode()
73-
.yields();
70+
void shouldStreamBackResults() {
71+
var query = "CALL gds.bridges.stream('graph')";
7472

75-
var expectedNode1 = idFunction.of("a");
76-
var expectedNode2 = idFunction.of("b");
73+
var expectedNode1 = idFunction.of("a");
74+
var expectedNode2 = idFunction.of("b");
7775

78-
var rowCount = runQueryWithRowConsumer(query, (resultRow) -> {
79-
var fromId = resultRow.getNumber("from");
80-
var toId = resultRow.getNumber("to");
76+
var rowCount = runQueryWithRowConsumer(
77+
query, (resultRow) -> {
78+
var fromId = resultRow.getNumber("from");
79+
var toId = resultRow.getNumber("to");
8180

82-
assertThat(fromId.longValue()).isNotEqualTo(toId.longValue());
83-
assertThat(fromId.longValue()).isIn(expectedNode1, expectedNode2);
84-
assertThat(toId.longValue()).isIn(expectedNode1, expectedNode2);
81+
assertThat(fromId.longValue()).isNotEqualTo(toId.longValue());
82+
assertThat(fromId.longValue()).isIn(expectedNode1, expectedNode2);
83+
assertThat(toId.longValue()).isIn(expectedNode1, expectedNode2);
8584

86-
assertThat(resultRow.get("remainingSizes")).isNotNull();
85+
assertThat(resultRow.get("remainingSizes")).isNotNull();
8786

88-
});
89-
assertThat(rowCount).isEqualTo(1l);
90-
}
87+
}
88+
);
89+
90+
assertThat(rowCount).isEqualTo(1L);
91+
}
92+
93+
@Test
94+
void shouldNotFailWhenRemainingSizesIsNotYielded() {
95+
assertThatNoException().isThrownBy(
96+
() -> runQuery("CALL gds.bridges.stream('graph') YIELD from, to")
97+
);
98+
}
9199

92100
}

‎procedures/algorithms-facade/build.gradle

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ dependencies {
4646
testImplementation openGds.mockito.junit.jupiter
4747
testImplementation openGds.assertj.core
4848

49+
testImplementation project(':test-utils')
50+
4951
testImplementation group: 'org.neo4j', name: 'neo4j-graphdb-api', version: ver.'neo4j'
5052
testImplementation group: 'org.neo4j', name: 'neo4j-kernel-api', version: ver.'neo4j'
5153

‎procedures/algorithms-facade/src/main/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesResultBuilderForStreamMode.java renamed to ‎procedures/algorithms-facade/src/main/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesResultBuilderForStreamModeWithComponentSizes.java

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,27 +28,25 @@
2828
import java.util.Optional;
2929
import java.util.stream.Stream;
3030

31-
class BridgesResultBuilderForStreamMode implements StreamResultBuilder<BridgeResult,BridgesStreamResult> {
32-
31+
class BridgesResultBuilderForStreamModeWithComponentSizes implements StreamResultBuilder<BridgeResult, BridgesStreamResult> {
3332

3433
@Override
3534
public Stream<BridgesStreamResult> build(
3635
Graph graph,
3736
GraphStore graphStore,
3837
Optional<BridgeResult> result
3938
) {
40-
if (result.isEmpty()) return Stream.empty();
41-
42-
var bridges = result.get().bridges();
39+
if (result.isEmpty()) {
40+
return Stream.empty();
41+
}
4342

44-
return bridges
43+
return result.get()
44+
.bridges()
4545
.stream()
46-
.map( b ->
47-
new BridgesStreamResult(
48-
graph.toOriginalNodeId(b.from()),
49-
graph.toOriginalNodeId(b.to()),
50-
List.of(b.remainingSizes()[0],b.remainingSizes()[1])
51-
)
52-
);
46+
.map(bridge -> new BridgesStreamResult(
47+
graph.toOriginalNodeId(bridge.from()),
48+
graph.toOriginalNodeId(bridge.to()),
49+
List.of(bridge.remainingSizes()[0], bridge.remainingSizes()[1])
50+
));
5351
}
5452
}

‎procedures/algorithms-facade/src/main/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesResultBuilderForStreamModeWithoutComponentSizes.java

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
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.procedures.algorithms.centrality;
21+
22+
import org.neo4j.gds.api.Graph;
23+
import org.neo4j.gds.api.GraphStore;
24+
import org.neo4j.gds.applications.algorithms.machinery.StreamResultBuilder;
25+
import org.neo4j.gds.bridges.BridgeResult;
26+
27+
import java.util.Optional;
28+
import java.util.stream.Stream;
29+
30+
class BridgesResultBuilderForStreamModeWithoutComponentSizes implements StreamResultBuilder<BridgeResult, BridgesStreamResult> {
31+
32+
@Override
33+
public Stream<BridgesStreamResult> build(
34+
Graph graph,
35+
GraphStore graphStore,
36+
Optional<BridgeResult> result
37+
) {
38+
if (result.isEmpty()) {
39+
return Stream.empty();
40+
}
41+
42+
return result.get()
43+
.bridges()
44+
.stream()
45+
.map(bridge -> new BridgesStreamResult(
46+
graph.toOriginalNodeId(bridge.from()),
47+
graph.toOriginalNodeId(bridge.to()),
48+
null
49+
));
50+
}
51+
}

‎procedures/algorithms-facade/src/main/java/org/neo4j/gds/procedures/algorithms/centrality/LocalCentralityProcedureFacade.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -659,7 +659,11 @@ public Stream<BridgesStreamResult> bridgesStream(
659659
String graphName,
660660
Map<String, Object> configuration
661661
) {
662-
var resultBuilder = new BridgesResultBuilderForStreamMode();
662+
var shouldComputeComponents = procedureReturnColumns.contains("remainingSizes");
663+
var resultBuilder = shouldComputeComponents
664+
? new BridgesResultBuilderForStreamModeWithComponentSizes()
665+
: new BridgesResultBuilderForStreamModeWithoutComponentSizes();
666+
663667
var parsedConfiguration = configurationParser.parseConfiguration(
664668
configuration,
665669
BridgesStreamConfig::of
@@ -669,7 +673,7 @@ public Stream<BridgesStreamResult> bridgesStream(
669673
GraphName.parse(graphName),
670674
parsedConfiguration,
671675
resultBuilder,
672-
procedureReturnColumns.contains("remainingSizes")
676+
shouldComputeComponents
673677
);
674678
}
675679

‎procedures/algorithms-facade/src/test/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesResultBuilderForStreamModeWithComponentSizesTest.java

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
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.procedures.algorithms.centrality;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.gds.api.Graph;
24+
import org.neo4j.gds.api.GraphStore;
25+
import org.neo4j.gds.bridges.Bridge;
26+
import org.neo4j.gds.bridges.BridgeResult;
27+
28+
import java.util.List;
29+
import java.util.Optional;
30+
31+
import static org.assertj.core.api.Assertions.assertThat;
32+
import static org.assertj.core.api.Assertions.assertThatNoException;
33+
import static org.mockito.ArgumentMatchers.anyLong;
34+
import static org.mockito.Mockito.mock;
35+
import static org.mockito.Mockito.when;
36+
37+
class BridgesResultBuilderForStreamModeWithComponentSizesTest {
38+
39+
@Test
40+
void shouldMapCorrectly() {
41+
var graphMock = mock(Graph.class);
42+
when(graphMock.toOriginalNodeId(anyLong())).thenReturn(0L, 1L);
43+
44+
var bridgeResult = new BridgeResult(
45+
List.of(
46+
new Bridge(0, 1, new long[]{5, 19})
47+
)
48+
);
49+
50+
var bridgesResultStream = new BridgesResultBuilderForStreamModeWithComponentSizes().build(
51+
graphMock,
52+
mock(GraphStore.class),
53+
Optional.of(bridgeResult)
54+
);
55+
56+
assertThatNoException()
57+
.isThrownBy(() -> assertThat(bridgesResultStream)
58+
.containsExactly(new BridgesStreamResult(0, 1, List.of(5L, 19L))));
59+
}
60+
61+
@Test
62+
void shouldReturnEmptyStreamWhenTheAlgorithmResultIsEmpty() {
63+
var bridgesResultStream = new BridgesResultBuilderForStreamModeWithComponentSizes().build(
64+
mock(Graph.class),
65+
mock(GraphStore.class),
66+
Optional.empty()
67+
);
68+
69+
assertThat(bridgesResultStream).isEmpty();
70+
}
71+
72+
}

‎procedures/algorithms-facade/src/test/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesResultBuilderForStreamModeWithoutComponentSizesTest.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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.procedures.algorithms.centrality;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.gds.api.Graph;
24+
import org.neo4j.gds.api.GraphStore;
25+
import org.neo4j.gds.bridges.Bridge;
26+
import org.neo4j.gds.bridges.BridgeResult;
27+
28+
import java.util.List;
29+
import java.util.Optional;
30+
31+
import static org.assertj.core.api.Assertions.assertThat;
32+
import static org.assertj.core.api.Assertions.assertThatNoException;
33+
import static org.mockito.ArgumentMatchers.anyLong;
34+
import static org.mockito.Mockito.mock;
35+
import static org.mockito.Mockito.when;
36+
37+
class BridgesResultBuilderForStreamModeWithoutComponentSizesTest {
38+
39+
@Test
40+
void shouldMapCorrectly() {
41+
var graphMock = mock(Graph.class);
42+
when(graphMock.toOriginalNodeId(anyLong())).thenReturn(0L, 1L);
43+
44+
var bridgeResult = new BridgeResult(
45+
List.of(
46+
new Bridge(0, 1, new long[] {5, 19})
47+
)
48+
);
49+
50+
var bridgesResultStream = new BridgesResultBuilderForStreamModeWithoutComponentSizes().build(
51+
graphMock,
52+
mock(GraphStore.class),
53+
Optional.of(bridgeResult)
54+
);
55+
56+
assertThatNoException()
57+
.isThrownBy(() -> assertThat(bridgesResultStream)
58+
.containsExactly(new BridgesStreamResult(0, 1, null)));
59+
}
60+
61+
@Test
62+
void shouldNotThrowExceptionWhenRemainingSizesIsNull() {
63+
var graphMock = mock(Graph.class);
64+
when(graphMock.toOriginalNodeId(anyLong())).thenReturn(0L, 1L);
65+
66+
var bridgeResult = new BridgeResult(
67+
List.of(
68+
new Bridge(0, 1, null)
69+
)
70+
);
71+
72+
var bridgesResultStream = new BridgesResultBuilderForStreamModeWithoutComponentSizes().build(
73+
graphMock,
74+
mock(GraphStore.class),
75+
Optional.of(bridgeResult)
76+
);
77+
78+
assertThatNoException()
79+
// need to consume the stream to trigger processing the results
80+
.isThrownBy(() -> assertThat(bridgesResultStream)
81+
.containsExactly(new BridgesStreamResult(0, 1, null)));
82+
}
83+
84+
@Test
85+
void shouldReturnEmptyStreamWhenTheAlgorithmResultIsEmpty() {
86+
var bridgesResultStream = new BridgesResultBuilderForStreamModeWithoutComponentSizes().build(
87+
mock(Graph.class),
88+
mock(GraphStore.class),
89+
Optional.empty()
90+
);
91+
92+
assertThat(bridgesResultStream).isEmpty();
93+
}
94+
95+
}

‎procedures/facade-api/centrality-facade-api/src/main/java/org/neo4j/gds/procedures/algorithms/centrality/BridgesStreamResult.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
*/
2020
package org.neo4j.gds.procedures.algorithms.centrality;
2121

22+
import org.jetbrains.annotations.Nullable;
23+
2224
import java.util.List;
2325

24-
public record BridgesStreamResult(long from, long to, List<Long> remainingSizes) {
26+
public record BridgesStreamResult(long from, long to, @Nullable List<Long> remainingSizes) {
2527
}

0 commit comments

Comments
 (0)