Skip to content

Commit

Permalink
roll back fix of behavior of Graph/ValueGraph views for a node when t…
Browse files Browse the repository at this point in the history
…hat node is removed from the graph (breaking internal tests)

RELNOTES=roll back fix of behavior of Graph/ValueGraph views for a node when that node is removed from the graph (breaking internal tests)
PiperOrigin-RevId: 571958083
  • Loading branch information
java-team-github-bot authored and Google Java Core Libraries committed Oct 9, 2023
1 parent a78bea4 commit 0a30cc3
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,10 @@ public void removeNode_nodeNotPresent() {
public void removeNode_queryAfterRemoval() {
assume().that(graphIsMutable()).isTrue();

putEdge(N1, N2);
putEdge(N2, N1);
Set<Integer> n1AdjacentNodes = graph.adjacentNodes(N1);
Set<Integer> n2AdjacentNodes = graph.adjacentNodes(N2);
addNode(N1);
@SuppressWarnings("unused")
Set<Integer> unused = graph.adjacentNodes(N1); // ensure cache (if any) is populated
assertThat(graphAsMutableGraph.removeNode(N1)).isTrue();
assertThat(n1AdjacentNodes).isEmpty();
assertThat(n2AdjacentNodes).isEmpty();
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> graph.adjacentNodes(N1));
assertNodeNotInGraphErrorMessage(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,11 @@ public void removeNode_nodeNotPresent() {
public void removeNode_queryAfterRemoval() {
assume().that(graphIsMutable()).isTrue();

addEdge(N1, N2, E12);
Set<Integer> n1AdjacentNodes = networkAsMutableNetwork.adjacentNodes(N1);
Set<Integer> n2AdjacentNodes = networkAsMutableNetwork.adjacentNodes(N2);
addNode(N1);
@SuppressWarnings("unused")
Set<Integer> unused =
networkAsMutableNetwork.adjacentNodes(N1); // ensure cache (if any) is populated
assertTrue(networkAsMutableNetwork.removeNode(N1));
assertThat(n1AdjacentNodes).isEmpty();
assertThat(n2AdjacentNodes).isEmpty();
IllegalArgumentException e =
assertThrows(
IllegalArgumentException.class, () -> networkAsMutableNetwork.adjacentNodes(N1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static com.google.common.graph.Graphs.checkPositive;
import static java.util.Objects.requireNonNull;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import javax.annotation.CheckForNull;

Expand Down Expand Up @@ -137,21 +136,17 @@ public boolean removeNode(N node) {
}
}

for (N successor : ImmutableList.copyOf(connections.successors())) {
for (N successor : connections.successors()) {
// requireNonNull is safe because the node is a successor.
requireNonNull(nodeConnections.getWithoutCaching(successor)).removePredecessor(node);
requireNonNull(connections.removeSuccessor(successor));
--edgeCount;
}
if (isDirected()) { // In undirected graphs, the successor and predecessor sets are equal.
// Since views are returned, we need to copy the predecessors that will be removed.
// Thus we avoid modifying the underlying view while iterating over it.
for (N predecessor : ImmutableList.copyOf(connections.predecessors())) {
for (N predecessor : connections.predecessors()) {
// requireNonNull is safe because the node is a predecessor.
checkState(
requireNonNull(nodeConnections.getWithoutCaching(predecessor)).removeSuccessor(node)
!= null);
connections.removePredecessor(predecessor);
--edgeCount;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,13 +385,10 @@ public void removeNode_nodeNotPresent() {
public void removeNode_queryAfterRemoval() {
assume().that(graphIsMutable()).isTrue();

putEdge(N1, N2);
putEdge(N2, N1);
Set<Integer> n1AdjacentNodes = graph.adjacentNodes(N1);
Set<Integer> n2AdjacentNodes = graph.adjacentNodes(N2);
addNode(N1);
@SuppressWarnings("unused")
Set<Integer> unused = graph.adjacentNodes(N1); // ensure cache (if any) is populated
assertThat(graphAsMutableGraph.removeNode(N1)).isTrue();
assertThat(n1AdjacentNodes).isEmpty();
assertThat(n2AdjacentNodes).isEmpty();
IllegalArgumentException e =
assertThrows(IllegalArgumentException.class, () -> graph.adjacentNodes(N1));
assertNodeNotInGraphErrorMessage(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -677,12 +677,11 @@ public void removeNode_nodeNotPresent() {
public void removeNode_queryAfterRemoval() {
assume().that(graphIsMutable()).isTrue();

addEdge(N1, N2, E12);
Set<Integer> n1AdjacentNodes = networkAsMutableNetwork.adjacentNodes(N1);
Set<Integer> n2AdjacentNodes = networkAsMutableNetwork.adjacentNodes(N2);
addNode(N1);
@SuppressWarnings("unused")
Set<Integer> unused =
networkAsMutableNetwork.adjacentNodes(N1); // ensure cache (if any) is populated
assertTrue(networkAsMutableNetwork.removeNode(N1));
assertThat(n1AdjacentNodes).isEmpty();
assertThat(n2AdjacentNodes).isEmpty();
IllegalArgumentException e =
assertThrows(
IllegalArgumentException.class, () -> networkAsMutableNetwork.adjacentNodes(N1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import static com.google.common.graph.Graphs.checkPositive;
import static java.util.Objects.requireNonNull;

import com.google.common.collect.ImmutableList;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import javax.annotation.CheckForNull;

Expand Down Expand Up @@ -137,21 +136,17 @@ public boolean removeNode(N node) {
}
}

for (N successor : ImmutableList.copyOf(connections.successors())) {
for (N successor : connections.successors()) {
// requireNonNull is safe because the node is a successor.
requireNonNull(nodeConnections.getWithoutCaching(successor)).removePredecessor(node);
requireNonNull(connections.removeSuccessor(successor));
--edgeCount;
}
if (isDirected()) { // In undirected graphs, the successor and predecessor sets are equal.
// Since views are returned, we need to copy the predecessors that will be removed.
// Thus we avoid modifying the underlying view while iterating over it.
for (N predecessor : ImmutableList.copyOf(connections.predecessors())) {
for (N predecessor : connections.predecessors()) {
// requireNonNull is safe because the node is a predecessor.
checkState(
requireNonNull(nodeConnections.getWithoutCaching(predecessor)).removeSuccessor(node)
!= null);
connections.removePredecessor(predecessor);
--edgeCount;
}
}
Expand Down

0 comments on commit 0a30cc3

Please sign in to comment.