Fix issue #5843

RELNOTES=Fix issue #5843 PiperOrigin-RevId: 450827341
google · May 25, 2022 · 76260d9 · 76260d9
1 parent 7731825
commit 76260d9
Show file tree

Hide file tree

Showing 14 changed files with 171 additions and 57 deletions.
diff --git a/android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java b/android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java
@@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
   @Test
   public void hasEdgeConnecting_mismatch() {
     putEdge(N1, N2);
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
   }
 
   @Test

diff --git a/android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java b/android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java
@@ -16,6 +16,7 @@
 
 package com.google.common.graph;
 
+import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 import static org.junit.Assert.assertTrue;
@@ -195,15 +196,30 @@ public void successors_checkReturnedSetMutability() {
   @Test
   public void edges_containsOrderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
-    assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
+    assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
+    assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
+  }
+
+  @Test
+  public void edgesConnecting_orderMismatch() {
+    addEdge(N1, N2, E12);
+    try {
+      Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
   public void edgeConnectingOrNull_orderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
-    assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
+    try {
+      String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -418,7 +434,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
       networkAsMutableNetwork.addEdge(N4, N5, E12);
       fail(ERROR_ADDED_EXISTING_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
   }
 
@@ -432,13 +448,13 @@ public void addEdge_parallelEdge_notAllowed() {
       networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
       fail(ERROR_ADDED_PARALLEL_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
     try {
       networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
       fail(ERROR_ADDED_PARALLEL_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
   }
 
@@ -458,7 +474,12 @@ public void addEdge_orderMismatch() {
     assume().that(graphIsMutable()).isTrue();
 
     EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
-    assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
+    try {
+      networkAsMutableNetwork.addEdge(endpoints, E12);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -527,20 +548,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
       networkAsMutableNetwork.addEdge(N1, N2, E11);
       fail("Reusing an existing self-loop edge to connect different nodes succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
     try {
       networkAsMutableNetwork.addEdge(N2, N2, E11);
       fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
     addEdge(N1, N2, E12);
     try {
       networkAsMutableNetwork.addEdge(N1, N1, E12);
       fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
   }
 
@@ -555,7 +576,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
       networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
       fail("Adding a parallel self-loop edge succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
   }
 

diff --git a/android/guava-tests/test/com/google/common/graph/EndpointPairTest.java b/android/guava-tests/test/com/google/common/graph/EndpointPairTest.java
@@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
     assertThat(edges).contains(EndpointPair.unordered(N1, N2));
     assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)
 
-    // ordered endpoints OK for undirected graph (because ordering is irrelevant)
-    assertThat(edges).contains(EndpointPair.ordered(N1, N2));
+    // ordered endpoints not compatible with undirected graph
+    assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));
 
     assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
     assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph

diff --git a/android/guava-tests/test/com/google/common/graph/ValueGraphTest.java b/android/guava-tests/test/com/google/common/graph/ValueGraphTest.java
@@ -173,8 +173,8 @@ public void hasEdgeConnecting_undirected_backwards() {
   public void hasEdgeConnecting_undirected_mismatch() {
     graph = ValueGraphBuilder.undirected().build();
     graph.putEdgeValue(1, 2, "A");
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isTrue();
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isTrue();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isFalse();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isFalse();
   }
 
   @Test
@@ -223,8 +223,19 @@ public void edgeValueOrDefault_undirected_backwards() {
   public void edgeValueOrDefault_undirected_mismatch() {
     graph = ValueGraphBuilder.undirected().build();
     graph.putEdgeValue(1, 2, "A");
-    assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
-    assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
+    // Check that edgeValueOrDefault() throws on each possible ordering of an ordered EndpointPair
+    try {
+      String unused = graph.edgeValueOrDefault(EndpointPair.ordered(1, 2), "default");
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
+    try {
+      String unused = graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default");
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -251,7 +262,12 @@ public void putEdgeValue_directed_orderMismatch() {
   @Test
   public void putEdgeValue_undirected_orderMismatch() {
     graph = ValueGraphBuilder.undirected().build();
-    assertThat(graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant")).isNull();
+    try {
+      graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant");
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -311,7 +327,19 @@ public void removeEdge_directed_orderMismatch() {
   public void removeEdge_undirected_orderMismatch() {
     graph = ValueGraphBuilder.undirected().build();
     graph.putEdgeValue(1, 2, "1-2");
-    assertThat(graph.removeEdge(EndpointPair.ordered(1, 2))).isEqualTo("1-2");
+    // Check that removeEdge() throws on each possible ordering of an ordered EndpointPair
+    try {
+      graph.removeEdge(EndpointPair.ordered(1, 2));
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
+    try {
+      graph.removeEdge(EndpointPair.ordered(2, 1));
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test

diff --git a/android/guava/src/com/google/common/graph/AbstractBaseGraph.java b/android/guava/src/com/google/common/graph/AbstractBaseGraph.java
@@ -177,7 +177,11 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
     checkArgument(isOrderingCompatible(endpoints), ENDPOINTS_MISMATCH);
   }
 
+  /**
+   * Returns {@code true} iff {@code endpoints}' ordering is compatible with the directionality of
+   * this graph.
+   */
   protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
-    return endpoints.isOrdered() || !this.isDirected();
+    return endpoints.isOrdered() == this.isDirected();
   }
 }
diff --git a/android/guava/src/com/google/common/graph/AbstractNetwork.java b/android/guava/src/com/google/common/graph/AbstractNetwork.java
@@ -241,7 +241,7 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
   }
 
   protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
-    return endpoints.isOrdered() || !this.isDirected();
+    return endpoints.isOrdered() == this.isDirected();
   }
 
   @Override

diff --git a/android/guava/src/com/google/common/graph/GraphConstants.java b/android/guava/src/com/google/common/graph/GraphConstants.java
@@ -52,7 +52,7 @@ private GraphConstants() {}
           + "adjacentNode(node) if you already have a node, or nodeU()/nodeV() if you don't.";
   static final String EDGE_ALREADY_EXISTS = "Edge %s already exists in the graph.";
   static final String ENDPOINTS_MISMATCH =
-      "Mismatch: unordered endpoints cannot be used with directed graphs";
+      "Mismatch: endpoints' ordering is not compatible with directionality of the graph";
 
   /** Singleton edge value for {@link Graph} implementations backed by {@link ValueGraph}s. */
   enum Presence {

diff --git a/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java b/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java
@@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
   @Test
   public void hasEdgeConnecting_mismatch() {
     putEdge(N1, N2);
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
-    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
+    assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
   }
 
   @Test

diff --git a/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java b/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java
@@ -16,14 +16,15 @@
 
 package com.google.common.graph;
 
+import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.common.truth.Truth8.assertThat;
 import static com.google.common.truth.TruthJUnit.assume;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.testing.EqualsTester;
+import java.util.Optional;
 import java.util.Set;
 import org.junit.After;
 import org.junit.Test;
@@ -196,29 +197,41 @@ public void successors_checkReturnedSetMutability() {
   @Test
   public void edges_containsOrderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
-    assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
+    assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
+    assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
   }
 
   @Test
   public void edgesConnecting_orderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.edgesConnecting(ENDPOINTS_N2N1)).containsExactly(E12);
-    assertThat(network.edgesConnecting(ENDPOINTS_N1N2)).containsExactly(E12);
+    try {
+      Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
   public void edgeConnecting_orderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.edgeConnecting(ENDPOINTS_N2N1)).hasValue(E12);
-    assertThat(network.edgeConnecting(ENDPOINTS_N1N2)).hasValue(E12);
+    try {
+      Optional<String> unused = network.edgeConnecting(ENDPOINTS_N1N2);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
   public void edgeConnectingOrNull_orderMismatch() {
     addEdge(N1, N2, E12);
-    assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
-    assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
+    try {
+      String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -433,7 +446,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
       networkAsMutableNetwork.addEdge(N4, N5, E12);
       fail(ERROR_ADDED_EXISTING_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
   }
 
@@ -447,13 +460,13 @@ public void addEdge_parallelEdge_notAllowed() {
       networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
       fail(ERROR_ADDED_PARALLEL_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
     try {
       networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
       fail(ERROR_ADDED_PARALLEL_EDGE);
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
   }
 
@@ -473,7 +486,12 @@ public void addEdge_orderMismatch() {
     assume().that(graphIsMutable()).isTrue();
 
     EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
-    assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
+    try {
+      networkAsMutableNetwork.addEdge(endpoints, E12);
+      fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
+    }
   }
 
   @Test
@@ -542,20 +560,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
       networkAsMutableNetwork.addEdge(N1, N2, E11);
       fail("Reusing an existing self-loop edge to connect different nodes succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
     try {
       networkAsMutableNetwork.addEdge(N2, N2, E11);
       fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
     addEdge(N1, N2, E12);
     try {
       networkAsMutableNetwork.addEdge(N1, N1, E12);
       fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
     }
   }
 
@@ -570,7 +588,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
       networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
       fail("Adding a parallel self-loop edge succeeded");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
+      assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
     }
   }
 

diff --git a/guava-tests/test/com/google/common/graph/EndpointPairTest.java b/guava-tests/test/com/google/common/graph/EndpointPairTest.java
@@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
     assertThat(edges).contains(EndpointPair.unordered(N1, N2));
     assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)
 
-    // ordered endpoints OK for undirected graph (because ordering is irrelevant)
-    assertThat(edges).contains(EndpointPair.ordered(N1, N2));
+    // ordered endpoints not compatible with undirected graph
+    assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));
 
     assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
     assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph