Skip to content

Commit

Permalink
Support stable incident edge order for directed Immutable[Value]Graphs.
Browse files Browse the repository at this point in the history
RELNOTES=n/a

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=284109953
  • Loading branch information
nymanjens authored and cgdecker committed Dec 6, 2019
1 parent b54558e commit ac12249
Show file tree
Hide file tree
Showing 14 changed files with 158 additions and 122 deletions.
Expand Up @@ -25,7 +25,6 @@
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.HashSet;
import java.util.Set;
import org.junit.After;
Expand Down Expand Up @@ -81,25 +80,18 @@ public abstract class AbstractGraphTest {
* A proxy method that adds the node {@code n} to the graph being tested. In case of Immutable
* graph implementations, this method should replace {@link #graph} with a new graph that includes
* this node.
*
* @return {@code true} iff the graph was modified as a result of this call
*/
@CanIgnoreReturnValue
abstract boolean addNode(Integer n);
abstract void addNode(Integer n);

/**
* A proxy method that adds the edge {@code e} to the graph being tested. In case of Immutable
* graph implementations, this method should replace {@link #graph} with a new graph that includes
* this edge.
*
* @return {@code true} iff the graph was modified as a result of this call
*/
@CanIgnoreReturnValue
abstract boolean putEdge(Integer n1, Integer n2);
abstract void putEdge(Integer n1, Integer n2);

@CanIgnoreReturnValue
final boolean putEdge(EndpointPair<Integer> endpoints) {
return putEdge(endpoints.nodeU(), endpoints.nodeV());
final void putEdge(EndpointPair<Integer> endpoints) {
putEdge(endpoints.nodeU(), endpoints.nodeV());
}

final boolean graphIsMutable() {
Expand Down
Expand Up @@ -16,7 +16,6 @@

package com.google.common.graph;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.Collection;
import org.junit.runner.RunWith;
Expand All @@ -34,12 +33,14 @@ public static Collection<Object[]> parameters() {
new Object[][] {
{false, ElementOrder.unordered()},
{true, ElementOrder.unordered()},
// TODO(b/142723300): Add ElementOrder.stable() once it is supported
{false, ElementOrder.stable()},
{true, ElementOrder.stable()}
});
}

private final boolean allowsSelfLoops;
private final ElementOrder<Integer> incidentEdgeOrder;
private ImmutableGraph.Builder<Integer> graphBuilder;

public StandardImmutableDirectedGraphTest(
boolean allowsSelfLoops, ElementOrder<Integer> incidentEdgeOrder) {
Expand All @@ -59,28 +60,23 @@ ElementOrder<Integer> incidentEdgeOrder() {

@Override
public Graph<Integer> createGraph() {
return GraphBuilder.directed()
.allowsSelfLoops(allowsSelfLoops())
.incidentEdgeOrder(incidentEdgeOrder)
.immutable()
.build();
graphBuilder =
GraphBuilder.directed()
.allowsSelfLoops(allowsSelfLoops())
.incidentEdgeOrder(incidentEdgeOrder)
.immutable();
return graphBuilder.build();
}

@CanIgnoreReturnValue
@Override
final boolean addNode(Integer n) {
MutableGraph<Integer> mutableGraph = Graphs.copyOf(graph);
boolean somethingChanged = mutableGraph.addNode(n);
graph = ImmutableGraph.copyOf(mutableGraph);
return somethingChanged;
final void addNode(Integer n) {
graphBuilder.addNode(n);
graph = graphBuilder.build();
}

@CanIgnoreReturnValue
@Override
final boolean putEdge(Integer n1, Integer n2) {
MutableGraph<Integer> mutableGraph = Graphs.copyOf(graph);
boolean somethingChanged = mutableGraph.putEdge(n1, n2);
graph = ImmutableGraph.copyOf(mutableGraph);
return somethingChanged;
final void putEdge(Integer n1, Integer n2) {
graphBuilder.putEdge(n1, n2);
graph = graphBuilder.build();
}
}
Expand Up @@ -16,7 +16,6 @@

package com.google.common.graph;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.Collection;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -66,15 +65,13 @@ public MutableGraph<Integer> createGraph() {
.build();
}

@CanIgnoreReturnValue
@Override
final boolean addNode(Integer n) {
return graphAsMutableGraph.addNode(n);
final void addNode(Integer n) {
graphAsMutableGraph.addNode(n);
}

@CanIgnoreReturnValue
@Override
final boolean putEdge(Integer n1, Integer n2) {
return graphAsMutableGraph.putEdge(n1, n2);
final void putEdge(Integer n1, Integer n2) {
graphAsMutableGraph.putEdge(n1, n2);
}
}
Expand Up @@ -16,7 +16,6 @@

package com.google.common.graph;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.Collection;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -52,15 +51,13 @@ public MutableGraph<Integer> createGraph() {
return GraphBuilder.undirected().allowsSelfLoops(allowsSelfLoops()).build();
}

@CanIgnoreReturnValue
@Override
final boolean addNode(Integer n) {
return graphAsMutableGraph.addNode(n);
final void addNode(Integer n) {
graphAsMutableGraph.addNode(n);
}

@CanIgnoreReturnValue
@Override
final boolean putEdge(Integer n1, Integer n2) {
return graphAsMutableGraph.putEdge(n1, n2);
final void putEdge(Integer n1, Integer n2) {
graphAsMutableGraph.putEdge(n1, n2);
}
}
Expand Up @@ -16,6 +16,7 @@

package com.google.common.graph;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.graph.GraphConstants.INNER_CAPACITY;
Expand All @@ -25,7 +26,7 @@

import com.google.common.base.Function;
import com.google.common.collect.AbstractIterator;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterators;
import com.google.common.collect.UnmodifiableIterator;
import java.util.AbstractSet;
Expand Down Expand Up @@ -175,21 +176,55 @@ static <N, V> DirectedGraphConnections<N, V> of(ElementOrder<N> incidentEdgeOrde
}

static <N, V> DirectedGraphConnections<N, V> ofImmutable(
Set<N> predecessors, Map<N, V> successorValues) {
N thisNode, Iterable<EndpointPair<N>> incidentEdges, Function<N, V> successorNodeToValueFn) {
Map<N, Object> adjacentNodeValues = new HashMap<>();
adjacentNodeValues.putAll(successorValues);
for (N predecessor : predecessors) {
Object value = adjacentNodeValues.put(predecessor, PRED);
if (value != null) {
adjacentNodeValues.put(predecessor, new PredAndSucc(value));
ImmutableList.Builder<NodeConnection<N>> orderedNodeConnectionsBuilder =
ImmutableList.builder();
int predecessorCount = 0;
int successorCount = 0;

for (EndpointPair<N> incidentEdge : incidentEdges) {
if (incidentEdge.nodeU().equals(thisNode) && incidentEdge.nodeV().equals(thisNode)) {
// incidentEdge is a self-loop

adjacentNodeValues.put(thisNode, new PredAndSucc(successorNodeToValueFn.apply(thisNode)));

orderedNodeConnectionsBuilder.add(new NodeConnection.Pred<>(thisNode));
orderedNodeConnectionsBuilder.add(new NodeConnection.Succ<>(thisNode));
predecessorCount++;
successorCount++;
} else if (incidentEdge.nodeV().equals(thisNode)) { // incidentEdge is an inEdge
N predecessor = incidentEdge.nodeU();

Object existingValue = adjacentNodeValues.put(predecessor, PRED);
if (existingValue != null) {
adjacentNodeValues.put(predecessor, new PredAndSucc(existingValue));
}

orderedNodeConnectionsBuilder.add(new NodeConnection.Pred<>(predecessor));
predecessorCount++;
} else { // incidentEdge is an outEdge
checkArgument(incidentEdge.nodeU().equals(thisNode));

N successor = incidentEdge.nodeV();
V value = successorNodeToValueFn.apply(successor);

Object existingValue = adjacentNodeValues.put(successor, value);
if (existingValue != null) {
checkArgument(existingValue == PRED);
adjacentNodeValues.put(successor, new PredAndSucc(value));
}

orderedNodeConnectionsBuilder.add(new NodeConnection.Succ<>(successor));
successorCount++;
}
}

return new DirectedGraphConnections<>(
/* adjacentNodeValues = */ ImmutableMap.copyOf(adjacentNodeValues),
// TODO(b/142723300): Pass in an ImmutableList here with the ordered node connections
/* orderedNodeConnections = */ null,
/* predecessorCount = */ predecessors.size(),
/* successorCount = */ successorValues.size());
adjacentNodeValues,
orderedNodeConnectionsBuilder.build(),
predecessorCount,
successorCount);
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions android/guava/src/com/google/common/graph/ImmutableGraph.java
Expand Up @@ -84,11 +84,12 @@ private static <N> ImmutableMap<N, GraphConnections<N, Presence>> getNodeConnect
return nodeConnections.build();
}

@SuppressWarnings("unchecked")
private static <N> GraphConnections<N, Presence> connectionsOf(Graph<N> graph, N node) {
Function<Object, Presence> edgeValueFn = Functions.constant(Presence.EDGE_EXISTS);
Function<N, Presence> edgeValueFn =
(Function<N, Presence>) Functions.constant(Presence.EDGE_EXISTS);
return graph.isDirected()
? DirectedGraphConnections.ofImmutable(
graph.predecessors(node), Maps.asMap(graph.successors(node), edgeValueFn))
? DirectedGraphConnections.ofImmutable(node, graph.incidentEdges(node), edgeValueFn)
: UndirectedGraphConnections.ofImmutable(
Maps.asMap(graph.adjacentNodes(node), edgeValueFn));
}
Expand Down
Expand Up @@ -94,7 +94,7 @@ public V apply(N successorNode) {
};
return graph.isDirected()
? DirectedGraphConnections.ofImmutable(
graph.predecessors(node), Maps.asMap(graph.successors(node), successorNodeToValueFn))
node, graph.incidentEdges(node), successorNodeToValueFn)
: UndirectedGraphConnections.ofImmutable(
Maps.asMap(graph.adjacentNodes(node), successorNodeToValueFn));
}
Expand Down
16 changes: 4 additions & 12 deletions guava-tests/test/com/google/common/graph/AbstractGraphTest.java
Expand Up @@ -25,7 +25,6 @@
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.HashSet;
import java.util.Set;
import org.junit.After;
Expand Down Expand Up @@ -81,25 +80,18 @@ public abstract class AbstractGraphTest {
* A proxy method that adds the node {@code n} to the graph being tested. In case of Immutable
* graph implementations, this method should replace {@link #graph} with a new graph that includes
* this node.
*
* @return {@code true} iff the graph was modified as a result of this call
*/
@CanIgnoreReturnValue
abstract boolean addNode(Integer n);
abstract void addNode(Integer n);

/**
* A proxy method that adds the edge {@code e} to the graph being tested. In case of Immutable
* graph implementations, this method should replace {@link #graph} with a new graph that includes
* this edge.
*
* @return {@code true} iff the graph was modified as a result of this call
*/
@CanIgnoreReturnValue
abstract boolean putEdge(Integer n1, Integer n2);
abstract void putEdge(Integer n1, Integer n2);

@CanIgnoreReturnValue
final boolean putEdge(EndpointPair<Integer> endpoints) {
return putEdge(endpoints.nodeU(), endpoints.nodeV());
final void putEdge(EndpointPair<Integer> endpoints) {
putEdge(endpoints.nodeU(), endpoints.nodeV());
}

final boolean graphIsMutable() {
Expand Down
Expand Up @@ -16,7 +16,6 @@

package com.google.common.graph;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.Collection;
import org.junit.runner.RunWith;
Expand All @@ -34,12 +33,14 @@ public static Collection<Object[]> parameters() {
new Object[][] {
{false, ElementOrder.unordered()},
{true, ElementOrder.unordered()},
// TODO(b/142723300): Add ElementOrder.stable() once it is supported
{false, ElementOrder.stable()},
{true, ElementOrder.stable()}
});
}

private final boolean allowsSelfLoops;
private final ElementOrder<Integer> incidentEdgeOrder;
private ImmutableGraph.Builder<Integer> graphBuilder;

public StandardImmutableDirectedGraphTest(
boolean allowsSelfLoops, ElementOrder<Integer> incidentEdgeOrder) {
Expand All @@ -59,28 +60,23 @@ ElementOrder<Integer> incidentEdgeOrder() {

@Override
public Graph<Integer> createGraph() {
return GraphBuilder.directed()
.allowsSelfLoops(allowsSelfLoops())
.incidentEdgeOrder(incidentEdgeOrder)
.immutable()
.build();
graphBuilder =
GraphBuilder.directed()
.allowsSelfLoops(allowsSelfLoops())
.incidentEdgeOrder(incidentEdgeOrder)
.immutable();
return graphBuilder.build();
}

@CanIgnoreReturnValue
@Override
final boolean addNode(Integer n) {
MutableGraph<Integer> mutableGraph = Graphs.copyOf(graph);
boolean somethingChanged = mutableGraph.addNode(n);
graph = ImmutableGraph.copyOf(mutableGraph);
return somethingChanged;
final void addNode(Integer n) {
graphBuilder.addNode(n);
graph = graphBuilder.build();
}

@CanIgnoreReturnValue
@Override
final boolean putEdge(Integer n1, Integer n2) {
MutableGraph<Integer> mutableGraph = Graphs.copyOf(graph);
boolean somethingChanged = mutableGraph.putEdge(n1, n2);
graph = ImmutableGraph.copyOf(mutableGraph);
return somethingChanged;
final void putEdge(Integer n1, Integer n2) {
graphBuilder.putEdge(n1, n2);
graph = graphBuilder.build();
}
}
Expand Up @@ -16,7 +16,6 @@

package com.google.common.graph;

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.util.Arrays;
import java.util.Collection;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -66,15 +65,13 @@ public MutableGraph<Integer> createGraph() {
.build();
}

@CanIgnoreReturnValue
@Override
final boolean addNode(Integer n) {
return graphAsMutableGraph.addNode(n);
final void addNode(Integer n) {
graphAsMutableGraph.addNode(n);
}

@CanIgnoreReturnValue
@Override
final boolean putEdge(Integer n1, Integer n2) {
return graphAsMutableGraph.putEdge(n1, n2);
final void putEdge(Integer n1, Integer n2) {
graphAsMutableGraph.putEdge(n1, n2);
}
}

0 comments on commit ac12249

Please sign in to comment.