From 8e0f821430acaa5b77c5c2c05380e3d29f18e410 Mon Sep 17 00:00:00 2001 From: jrtom Date: Wed, 27 Nov 2019 15:46:42 -0800 Subject: [PATCH] AbstractNetwork: fix bug in AbstractNetwork.hasEdgeConnecting() causing it to throw if either endpoint was not in the graph. RELNOTES=Fix bug in AbstractNetwork.hasEdgeConnecting() causing it to throw if either endpoint was not in the graph. Originally reported as GitHub issue #3721. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=282846559 --- .../google/common/graph/AbstractNetworkTest.java | 16 ++++++++++++++++ .../com/google/common/graph/AbstractNetwork.java | 6 ++++-- .../google/common/graph/AbstractNetworkTest.java | 16 ++++++++++++++++ .../com/google/common/graph/AbstractNetwork.java | 6 ++++-- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/android/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java b/android/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java index 71aacaece567..ca16ed33b5bf 100644 --- a/android/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java +++ b/android/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java @@ -533,6 +533,22 @@ public void edgesConnecting_nodesNotInGraph() { } } + @Test + public void hasEdgeConnecting_disconnectedNodes() { + addNode(N1); + addNode(N2); + assertThat(network.hasEdgeConnecting(N1, N2)).isFalse(); + } + + @Test + public void hasEdgesConnecting_nodesNotInGraph() { + addNode(N1); + addNode(N2); + assertThat(network.hasEdgeConnecting(N1, NODE_NOT_IN_GRAPH)).isFalse(); + assertThat(network.hasEdgeConnecting(NODE_NOT_IN_GRAPH, N2)).isFalse(); + assertThat(network.hasEdgeConnecting(NODE_NOT_IN_GRAPH, NODE_NOT_IN_GRAPH)).isFalse(); + } + @Test public void inEdges_noInEdges() { addNode(N1); diff --git a/android/guava/src/com/google/common/graph/AbstractNetwork.java b/android/guava/src/com/google/common/graph/AbstractNetwork.java index a1c11ced8b59..654997fc58a4 100644 --- a/android/guava/src/com/google/common/graph/AbstractNetwork.java +++ b/android/guava/src/com/google/common/graph/AbstractNetwork.java @@ -209,7 +209,9 @@ public E edgeConnectingOrNull(EndpointPair endpoints) { @Override public boolean hasEdgeConnecting(N nodeU, N nodeV) { - return !edgesConnecting(nodeU, nodeV).isEmpty(); + checkNotNull(nodeU); + checkNotNull(nodeV); + return nodes().contains(nodeU) && successors(nodeU).contains(nodeV); } @Override @@ -218,7 +220,7 @@ public boolean hasEdgeConnecting(EndpointPair endpoints) { if (!isOrderingCompatible(endpoints)) { return false; } - return !edgesConnecting(endpoints.nodeU(), endpoints.nodeV()).isEmpty(); + return hasEdgeConnecting(endpoints.nodeU(), endpoints.nodeV()); } /** diff --git a/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java b/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java index 2d61f8e9a688..5f5c598e19e0 100644 --- a/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java +++ b/guava-tests/test/com/google/common/graph/AbstractNetworkTest.java @@ -540,6 +540,22 @@ public void edgesConnecting_nodesNotInGraph() { } } + @Test + public void hasEdgeConnecting_disconnectedNodes() { + addNode(N1); + addNode(N2); + assertThat(network.hasEdgeConnecting(N1, N2)).isFalse(); + } + + @Test + public void hasEdgesConnecting_nodesNotInGraph() { + addNode(N1); + addNode(N2); + assertThat(network.hasEdgeConnecting(N1, NODE_NOT_IN_GRAPH)).isFalse(); + assertThat(network.hasEdgeConnecting(NODE_NOT_IN_GRAPH, N2)).isFalse(); + assertThat(network.hasEdgeConnecting(NODE_NOT_IN_GRAPH, NODE_NOT_IN_GRAPH)).isFalse(); + } + @Test public void inEdges_noInEdges() { addNode(N1); diff --git a/guava/src/com/google/common/graph/AbstractNetwork.java b/guava/src/com/google/common/graph/AbstractNetwork.java index 904e2aa1c824..7d6adcf88231 100644 --- a/guava/src/com/google/common/graph/AbstractNetwork.java +++ b/guava/src/com/google/common/graph/AbstractNetwork.java @@ -219,7 +219,9 @@ public Optional edgeConnecting(EndpointPair endpoints) { @Override public boolean hasEdgeConnecting(N nodeU, N nodeV) { - return !edgesConnecting(nodeU, nodeV).isEmpty(); + checkNotNull(nodeU); + checkNotNull(nodeV); + return nodes().contains(nodeU) && successors(nodeU).contains(nodeV); } @Override @@ -228,7 +230,7 @@ public boolean hasEdgeConnecting(EndpointPair endpoints) { if (!isOrderingCompatible(endpoints)) { return false; } - return !edgesConnecting(endpoints.nodeU(), endpoints.nodeV()).isEmpty(); + return hasEdgeConnecting(endpoints.nodeU(), endpoints.nodeV()); } /**