Skip to content

Commit 76260d9

Browse files
java-team-github-botGoogle Java Core Libraries
authored and
Google Java Core Libraries
committedMay 25, 2022
Fix issue #5843
RELNOTES=Fix issue #5843 PiperOrigin-RevId: 450827341
1 parent 7731825 commit 76260d9

14 files changed

+171
-57
lines changed
 

‎android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
166166
@Test
167167
public void hasEdgeConnecting_mismatch() {
168168
putEdge(N1, N2);
169-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
170-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
169+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
170+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
171171
}
172172

173173
@Test

‎android/guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java

+33-12
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.common.graph;
1818

19+
import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
1920
import static com.google.common.truth.Truth.assertThat;
2021
import static com.google.common.truth.TruthJUnit.assume;
2122
import static org.junit.Assert.assertTrue;
@@ -195,15 +196,30 @@ public void successors_checkReturnedSetMutability() {
195196
@Test
196197
public void edges_containsOrderMismatch() {
197198
addEdge(N1, N2, E12);
198-
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
199-
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
199+
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
200+
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
201+
}
202+
203+
@Test
204+
public void edgesConnecting_orderMismatch() {
205+
addEdge(N1, N2, E12);
206+
try {
207+
Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
208+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
209+
} catch (IllegalArgumentException e) {
210+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
211+
}
200212
}
201213

202214
@Test
203215
public void edgeConnectingOrNull_orderMismatch() {
204216
addEdge(N1, N2, E12);
205-
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
206-
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
217+
try {
218+
String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
219+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
220+
} catch (IllegalArgumentException e) {
221+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
222+
}
207223
}
208224

209225
@Test
@@ -418,7 +434,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
418434
networkAsMutableNetwork.addEdge(N4, N5, E12);
419435
fail(ERROR_ADDED_EXISTING_EDGE);
420436
} catch (IllegalArgumentException e) {
421-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
437+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
422438
}
423439
}
424440

@@ -432,13 +448,13 @@ public void addEdge_parallelEdge_notAllowed() {
432448
networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
433449
fail(ERROR_ADDED_PARALLEL_EDGE);
434450
} catch (IllegalArgumentException e) {
435-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
451+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
436452
}
437453
try {
438454
networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
439455
fail(ERROR_ADDED_PARALLEL_EDGE);
440456
} catch (IllegalArgumentException e) {
441-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
457+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
442458
}
443459
}
444460

@@ -458,7 +474,12 @@ public void addEdge_orderMismatch() {
458474
assume().that(graphIsMutable()).isTrue();
459475

460476
EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
461-
assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
477+
try {
478+
networkAsMutableNetwork.addEdge(endpoints, E12);
479+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
480+
} catch (IllegalArgumentException e) {
481+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
482+
}
462483
}
463484

464485
@Test
@@ -527,20 +548,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
527548
networkAsMutableNetwork.addEdge(N1, N2, E11);
528549
fail("Reusing an existing self-loop edge to connect different nodes succeeded");
529550
} catch (IllegalArgumentException e) {
530-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
551+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
531552
}
532553
try {
533554
networkAsMutableNetwork.addEdge(N2, N2, E11);
534555
fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
535556
} catch (IllegalArgumentException e) {
536-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
557+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
537558
}
538559
addEdge(N1, N2, E12);
539560
try {
540561
networkAsMutableNetwork.addEdge(N1, N1, E12);
541562
fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
542563
} catch (IllegalArgumentException e) {
543-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
564+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
544565
}
545566
}
546567

@@ -555,7 +576,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
555576
networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
556577
fail("Adding a parallel self-loop edge succeeded");
557578
} catch (IllegalArgumentException e) {
558-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
579+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
559580
}
560581
}
561582

‎android/guava-tests/test/com/google/common/graph/EndpointPairTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
214214
assertThat(edges).contains(EndpointPair.unordered(N1, N2));
215215
assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)
216216

217-
// ordered endpoints OK for undirected graph (because ordering is irrelevant)
218-
assertThat(edges).contains(EndpointPair.ordered(N1, N2));
217+
// ordered endpoints not compatible with undirected graph
218+
assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));
219219

220220
assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
221221
assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph

‎android/guava-tests/test/com/google/common/graph/ValueGraphTest.java

+34-6
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ public void hasEdgeConnecting_undirected_backwards() {
173173
public void hasEdgeConnecting_undirected_mismatch() {
174174
graph = ValueGraphBuilder.undirected().build();
175175
graph.putEdgeValue(1, 2, "A");
176-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isTrue();
177-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isTrue();
176+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isFalse();
177+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isFalse();
178178
}
179179

180180
@Test
@@ -223,8 +223,19 @@ public void edgeValueOrDefault_undirected_backwards() {
223223
public void edgeValueOrDefault_undirected_mismatch() {
224224
graph = ValueGraphBuilder.undirected().build();
225225
graph.putEdgeValue(1, 2, "A");
226-
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
227-
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
226+
// Check that edgeValueOrDefault() throws on each possible ordering of an ordered EndpointPair
227+
try {
228+
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(1, 2), "default");
229+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
230+
} catch (IllegalArgumentException e) {
231+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
232+
}
233+
try {
234+
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default");
235+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
236+
} catch (IllegalArgumentException e) {
237+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
238+
}
228239
}
229240

230241
@Test
@@ -251,7 +262,12 @@ public void putEdgeValue_directed_orderMismatch() {
251262
@Test
252263
public void putEdgeValue_undirected_orderMismatch() {
253264
graph = ValueGraphBuilder.undirected().build();
254-
assertThat(graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant")).isNull();
265+
try {
266+
graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant");
267+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
268+
} catch (IllegalArgumentException e) {
269+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
270+
}
255271
}
256272

257273
@Test
@@ -311,7 +327,19 @@ public void removeEdge_directed_orderMismatch() {
311327
public void removeEdge_undirected_orderMismatch() {
312328
graph = ValueGraphBuilder.undirected().build();
313329
graph.putEdgeValue(1, 2, "1-2");
314-
assertThat(graph.removeEdge(EndpointPair.ordered(1, 2))).isEqualTo("1-2");
330+
// Check that removeEdge() throws on each possible ordering of an ordered EndpointPair
331+
try {
332+
graph.removeEdge(EndpointPair.ordered(1, 2));
333+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
334+
} catch (IllegalArgumentException e) {
335+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
336+
}
337+
try {
338+
graph.removeEdge(EndpointPair.ordered(2, 1));
339+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
340+
} catch (IllegalArgumentException e) {
341+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
342+
}
315343
}
316344

317345
@Test

‎android/guava/src/com/google/common/graph/AbstractBaseGraph.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,11 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
177177
checkArgument(isOrderingCompatible(endpoints), ENDPOINTS_MISMATCH);
178178
}
179179

180+
/**
181+
* Returns {@code true} iff {@code endpoints}' ordering is compatible with the directionality of
182+
* this graph.
183+
*/
180184
protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
181-
return endpoints.isOrdered() || !this.isDirected();
185+
return endpoints.isOrdered() == this.isDirected();
182186
}
183187
}

‎android/guava/src/com/google/common/graph/AbstractNetwork.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
241241
}
242242

243243
protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
244-
return endpoints.isOrdered() || !this.isDirected();
244+
return endpoints.isOrdered() == this.isDirected();
245245
}
246246

247247
@Override

‎android/guava/src/com/google/common/graph/GraphConstants.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ private GraphConstants() {}
5252
+ "adjacentNode(node) if you already have a node, or nodeU()/nodeV() if you don't.";
5353
static final String EDGE_ALREADY_EXISTS = "Edge %s already exists in the graph.";
5454
static final String ENDPOINTS_MISMATCH =
55-
"Mismatch: unordered endpoints cannot be used with directed graphs";
55+
"Mismatch: endpoints' ordering is not compatible with directionality of the graph";
5656

5757
/** Singleton edge value for {@link Graph} implementations backed by {@link ValueGraph}s. */
5858
enum Presence {

‎guava-tests/test/com/google/common/graph/AbstractStandardUndirectedGraphTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ public void hasEdgeConnecting_correct() {
166166
@Test
167167
public void hasEdgeConnecting_mismatch() {
168168
putEdge(N1, N2);
169-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isTrue();
170-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isTrue();
169+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N1, N2))).isFalse();
170+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(N2, N1))).isFalse();
171171
}
172172

173173
@Test

‎guava-tests/test/com/google/common/graph/AbstractStandardUndirectedNetworkTest.java

+35-17
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@
1616

1717
package com.google.common.graph;
1818

19+
import static com.google.common.graph.GraphConstants.ENDPOINTS_MISMATCH;
1920
import static com.google.common.truth.Truth.assertThat;
20-
import static com.google.common.truth.Truth8.assertThat;
2121
import static com.google.common.truth.TruthJUnit.assume;
2222
import static org.junit.Assert.assertTrue;
2323
import static org.junit.Assert.fail;
2424

2525
import com.google.common.collect.ImmutableSet;
2626
import com.google.common.testing.EqualsTester;
27+
import java.util.Optional;
2728
import java.util.Set;
2829
import org.junit.After;
2930
import org.junit.Test;
@@ -196,29 +197,41 @@ public void successors_checkReturnedSetMutability() {
196197
@Test
197198
public void edges_containsOrderMismatch() {
198199
addEdge(N1, N2, E12);
199-
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N2N1);
200-
assertThat(network.asGraph().edges()).contains(ENDPOINTS_N1N2);
200+
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N2N1);
201+
assertThat(network.asGraph().edges()).doesNotContain(ENDPOINTS_N1N2);
201202
}
202203

203204
@Test
204205
public void edgesConnecting_orderMismatch() {
205206
addEdge(N1, N2, E12);
206-
assertThat(network.edgesConnecting(ENDPOINTS_N2N1)).containsExactly(E12);
207-
assertThat(network.edgesConnecting(ENDPOINTS_N1N2)).containsExactly(E12);
207+
try {
208+
Set<String> unused = network.edgesConnecting(ENDPOINTS_N1N2);
209+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
210+
} catch (IllegalArgumentException e) {
211+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
212+
}
208213
}
209214

210215
@Test
211216
public void edgeConnecting_orderMismatch() {
212217
addEdge(N1, N2, E12);
213-
assertThat(network.edgeConnecting(ENDPOINTS_N2N1)).hasValue(E12);
214-
assertThat(network.edgeConnecting(ENDPOINTS_N1N2)).hasValue(E12);
218+
try {
219+
Optional<String> unused = network.edgeConnecting(ENDPOINTS_N1N2);
220+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
221+
} catch (IllegalArgumentException e) {
222+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
223+
}
215224
}
216225

217226
@Test
218227
public void edgeConnectingOrNull_orderMismatch() {
219228
addEdge(N1, N2, E12);
220-
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N2N1)).isEqualTo(E12);
221-
assertThat(network.edgeConnectingOrNull(ENDPOINTS_N1N2)).isEqualTo(E12);
229+
try {
230+
String unused = network.edgeConnectingOrNull(ENDPOINTS_N1N2);
231+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
232+
} catch (IllegalArgumentException e) {
233+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
234+
}
222235
}
223236

224237
@Test
@@ -433,7 +446,7 @@ public void addEdge_existingEdgeBetweenDifferentNodes() {
433446
networkAsMutableNetwork.addEdge(N4, N5, E12);
434447
fail(ERROR_ADDED_EXISTING_EDGE);
435448
} catch (IllegalArgumentException e) {
436-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
449+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
437450
}
438451
}
439452

@@ -447,13 +460,13 @@ public void addEdge_parallelEdge_notAllowed() {
447460
networkAsMutableNetwork.addEdge(N1, N2, EDGE_NOT_IN_GRAPH);
448461
fail(ERROR_ADDED_PARALLEL_EDGE);
449462
} catch (IllegalArgumentException e) {
450-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
463+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
451464
}
452465
try {
453466
networkAsMutableNetwork.addEdge(N2, N1, EDGE_NOT_IN_GRAPH);
454467
fail(ERROR_ADDED_PARALLEL_EDGE);
455468
} catch (IllegalArgumentException e) {
456-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
469+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
457470
}
458471
}
459472

@@ -473,7 +486,12 @@ public void addEdge_orderMismatch() {
473486
assume().that(graphIsMutable()).isTrue();
474487

475488
EndpointPair<Integer> endpoints = EndpointPair.ordered(N1, N2);
476-
assertThat(networkAsMutableNetwork.addEdge(endpoints, E12)).isTrue();
489+
try {
490+
networkAsMutableNetwork.addEdge(endpoints, E12);
491+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
492+
} catch (IllegalArgumentException e) {
493+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
494+
}
477495
}
478496

479497
@Test
@@ -542,20 +560,20 @@ public void addEdge_existingEdgeBetweenDifferentNodes_selfLoops() {
542560
networkAsMutableNetwork.addEdge(N1, N2, E11);
543561
fail("Reusing an existing self-loop edge to connect different nodes succeeded");
544562
} catch (IllegalArgumentException e) {
545-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
563+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
546564
}
547565
try {
548566
networkAsMutableNetwork.addEdge(N2, N2, E11);
549567
fail("Reusing an existing self-loop edge to make a different self-loop edge succeeded");
550568
} catch (IllegalArgumentException e) {
551-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
569+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
552570
}
553571
addEdge(N1, N2, E12);
554572
try {
555573
networkAsMutableNetwork.addEdge(N1, N1, E12);
556574
fail("Reusing an existing edge to add a self-loop edge between different nodes succeeded");
557575
} catch (IllegalArgumentException e) {
558-
assertThat(e.getMessage()).contains(ERROR_REUSE_EDGE);
576+
assertThat(e).hasMessageThat().contains(ERROR_REUSE_EDGE);
559577
}
560578
}
561579

@@ -570,7 +588,7 @@ public void addEdge_parallelSelfLoopEdge_notAllowed() {
570588
networkAsMutableNetwork.addEdge(N1, N1, EDGE_NOT_IN_GRAPH);
571589
fail("Adding a parallel self-loop edge succeeded");
572590
} catch (IllegalArgumentException e) {
573-
assertThat(e.getMessage()).contains(ERROR_PARALLEL_EDGE);
591+
assertThat(e).hasMessageThat().contains(ERROR_PARALLEL_EDGE);
574592
}
575593
}
576594

‎guava-tests/test/com/google/common/graph/EndpointPairTest.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -214,8 +214,8 @@ public void endpointPair_undirected_contains() {
214214
assertThat(edges).contains(EndpointPair.unordered(N1, N2));
215215
assertThat(edges).contains(EndpointPair.unordered(N2, N1)); // equal to unordered(N1, N2)
216216

217-
// ordered endpoints OK for undirected graph (because ordering is irrelevant)
218-
assertThat(edges).contains(EndpointPair.ordered(N1, N2));
217+
// ordered endpoints not compatible with undirected graph
218+
assertThat(edges).doesNotContain(EndpointPair.ordered(N1, N2));
219219

220220
assertThat(edges).doesNotContain(EndpointPair.unordered(N2, N2)); // edge not present
221221
assertThat(edges).doesNotContain(EndpointPair.unordered(N3, N4)); // nodes not in graph

‎guava-tests/test/com/google/common/graph/ValueGraphTest.java

+47-8
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ public void hasEdgeConnecting_undirected_backwards() {
175175
public void hasEdgeConnecting_undirected_mismatch() {
176176
graph = ValueGraphBuilder.undirected().build();
177177
graph.putEdgeValue(1, 2, "A");
178-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isTrue();
179-
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isTrue();
178+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(1, 2))).isFalse();
179+
assertThat(graph.hasEdgeConnecting(EndpointPair.ordered(2, 1))).isFalse();
180180
}
181181

182182
@Test
@@ -224,8 +224,19 @@ public void edgeValue_undirected_backwards() {
224224
public void edgeValue_undirected_mismatch() {
225225
graph = ValueGraphBuilder.undirected().build();
226226
graph.putEdgeValue(1, 2, "A");
227-
assertThat(graph.edgeValue(EndpointPair.ordered(1, 2))).hasValue("A");
228-
assertThat(graph.edgeValue(EndpointPair.ordered(2, 1))).hasValue("A");
227+
// Check that edgeValue() throws on each possible ordering of an ordered EndpointPair
228+
try {
229+
Optional<String> unused = graph.edgeValue(EndpointPair.ordered(1, 2));
230+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
231+
} catch (IllegalArgumentException e) {
232+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
233+
}
234+
try {
235+
Optional<String> unused = graph.edgeValue(EndpointPair.ordered(2, 1));
236+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
237+
} catch (IllegalArgumentException e) {
238+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
239+
}
229240
}
230241

231242
@Test
@@ -274,8 +285,19 @@ public void edgeValueOrDefault_undirected_backwards() {
274285
public void edgeValueOrDefault_undirected_mismatch() {
275286
graph = ValueGraphBuilder.undirected().build();
276287
graph.putEdgeValue(1, 2, "A");
277-
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
278-
assertThat(graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default")).isEqualTo("A");
288+
// Check that edgeValueOrDefault() throws on each possible ordering of an ordered EndpointPair
289+
try {
290+
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(1, 2), "default");
291+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
292+
} catch (IllegalArgumentException e) {
293+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
294+
}
295+
try {
296+
String unused = graph.edgeValueOrDefault(EndpointPair.ordered(2, 1), "default");
297+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
298+
} catch (IllegalArgumentException e) {
299+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
300+
}
279301
}
280302

281303
@Test
@@ -302,7 +324,12 @@ public void putEdgeValue_directed_orderMismatch() {
302324
@Test
303325
public void putEdgeValue_undirected_orderMismatch() {
304326
graph = ValueGraphBuilder.undirected().build();
305-
assertThat(graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant")).isNull();
327+
try {
328+
graph.putEdgeValue(EndpointPair.ordered(1, 2), "irrelevant");
329+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
330+
} catch (IllegalArgumentException e) {
331+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
332+
}
306333
}
307334

308335
@Test
@@ -362,7 +389,19 @@ public void removeEdge_directed_orderMismatch() {
362389
public void removeEdge_undirected_orderMismatch() {
363390
graph = ValueGraphBuilder.undirected().build();
364391
graph.putEdgeValue(1, 2, "1-2");
365-
assertThat(graph.removeEdge(EndpointPair.ordered(1, 2))).isEqualTo("1-2");
392+
// Check that removeEdge() throws on each possible ordering of an ordered EndpointPair
393+
try {
394+
graph.removeEdge(EndpointPair.ordered(1, 2));
395+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
396+
} catch (IllegalArgumentException e) {
397+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
398+
}
399+
try {
400+
graph.removeEdge(EndpointPair.ordered(2, 1));
401+
fail("Expected IllegalArgumentException: " + ENDPOINTS_MISMATCH);
402+
} catch (IllegalArgumentException e) {
403+
assertThat(e).hasMessageThat().contains(ENDPOINTS_MISMATCH);
404+
}
366405
}
367406

368407
@Test

‎guava/src/com/google/common/graph/AbstractBaseGraph.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,11 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
177177
checkArgument(isOrderingCompatible(endpoints), ENDPOINTS_MISMATCH);
178178
}
179179

180+
/**
181+
* Returns {@code true} iff {@code endpoints}' ordering is compatible with the directionality of
182+
* this graph.
183+
*/
180184
protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
181-
return endpoints.isOrdered() || !this.isDirected();
185+
return endpoints.isOrdered() == this.isDirected();
182186
}
183187
}

‎guava/src/com/google/common/graph/AbstractNetwork.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ protected final void validateEndpoints(EndpointPair<?> endpoints) {
253253
}
254254

255255
protected final boolean isOrderingCompatible(EndpointPair<?> endpoints) {
256-
return endpoints.isOrdered() || !this.isDirected();
256+
return endpoints.isOrdered() == this.isDirected();
257257
}
258258

259259
@Override

‎guava/src/com/google/common/graph/GraphConstants.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ private GraphConstants() {}
5252
+ "adjacentNode(node) if you already have a node, or nodeU()/nodeV() if you don't.";
5353
static final String EDGE_ALREADY_EXISTS = "Edge %s already exists in the graph.";
5454
static final String ENDPOINTS_MISMATCH =
55-
"Mismatch: unordered endpoints cannot be used with directed graphs";
55+
"Mismatch: endpoints' ordering is not compatible with directionality of the graph";
5656

5757
/** Singleton edge value for {@link Graph} implementations backed by {@link ValueGraph}s. */
5858
enum Presence {

0 commit comments

Comments
 (0)
Please sign in to comment.