Skip to content

Commit

Permalink
Replace 3rd-party DOT Parser with own implementation
Browse files Browse the repository at this point in the history
Works around the issue reported in paypal/digraph-parser#3
  • Loading branch information
mtf90 committed Apr 4, 2020
1 parent 43f0693 commit 230a58f
Show file tree
Hide file tree
Showing 15 changed files with 837 additions and 118 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed

* We overhauled the handling of input and output streams for all (de-)serializers. Input and output streams are no longer closed automatically. This is to prevent asymmetric code where we would close a stream that we haven't opened. This is problematic in cases where e.g. `System.out` is passed as an output stream to simply print a serialized automaton and the `System.out` stream would be closed afterwards. Since input and output streams are usually opened in client-code, they should be closed in client-code as well. We suggest to simply wrap calls to the serializers in a try-with-resource block.
* Due to the DOT parsers rewrite (see **Fixed**), the attribute parser now receive a `Map<String, String>` instead of a `Map<String, Object>`.


### Removed
Expand All @@ -25,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

* Correctly enquote outputs containing whitespaces in `TAFWriter` ([#37](https://github.com/LearnLib/automatalib/issues/37), thanks to [Alexander Schieweck](https://github.com/aschieweck)).
* Fixed a bug in the `Graph` representation of `AbstractOneSEVPA`s ([#39](https://github.com/LearnLib/automatalib/pull/39), thanks to [DonatoClun](https://github.com/DonatoClun)).
* Replaced the 3rd-party DOT parser with our own implementation to fix the issue that multi-edges between nodes were not properly handled.


## [0.9.0](https://github.com/LearnLib/automatalib/releases/tag/automatalib-0.9.0) - 2020-02-05
Expand Down
7 changes: 7 additions & 0 deletions build-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,13 @@ limitations under the License.
<exclude>net/automatalib/util/automata/builders/MooreBuilder.class</exclude>
<exclude>net/automatalib/util/automata/builders/MooreBuilder$*.class</exclude>

<!-- generated parser for DOT serialization -->
<exclude>net/automatalib/serialization/dot/InternalDOTParser*.class</exclude>
<exclude>net/automatalib/serialization/dot/ParseException.class</exclude>
<exclude>net/automatalib/serialization/dot/SimpleCharStream.class</exclude>
<exclude>net/automatalib/serialization/dot/Token.class</exclude>
<exclude>net/automatalib/serialization/dot/TokenMgrError.class</exclude>

<!-- generated parser for TAF serialization -->
<exclude>net/automatalib/serialization/taf/parser/InternalTAFParser*.class</exclude>
<exclude>net/automatalib/serialization/taf/parser/ParseException.class</exclude>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ limitations under the License.
<Bug pattern="DM_DEFAULT_ENCODING,URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD,SBSC_USE_STRINGBUFFER_CONCATENATION,SF_SWITCH_NO_DEFAULT"/>
<!-- TODO: these modules/packages currently contains generated javaCC code that we cannot easily change -->
<Or>
<Class name="net.automatalib.serialization.dot.InternalDOTParser"/>
<Class name="net.automatalib.serialization.dot.InternalDOTParserConstants"/>
<Class name="net.automatalib.serialization.dot.InternalDOTParserTokenManager"/>
<Class name="net.automatalib.serialization.dot.ParseException"/>
<Class name="net.automatalib.serialization.dot.SimpleCharStream"/>
<Class name="net.automatalib.serialization.dot.Token"/>
<Class name="net.automatalib.serialization.dot.TokenMgrError"/>

<Class name="net.automatalib.serialization.taf.parser.InternalTAFParser"/>
<Class name="net.automatalib.serialization.taf.parser.InternalTAFParserConstants"/>
<Class name="net.automatalib.serialization.taf.parser.InternalTAFParserTokenManager"/>
Expand Down
6 changes: 0 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ limitations under the License.
<cacio.version>1.9</cacio.version>
<checkerframework.version>3.0.0</checkerframework.version>
<checkstyle.version>8.29</checkstyle.version>
<digraph-parser.version>1.0</digraph-parser.version>
<duzzt.version>0.1.0</duzzt.version>
<error-prone.version>9+181-r4173-1</error-prone.version> <!-- required by checkerframework, keep in sync -->
<guava.version>28.2-jre</guava.version>
Expand Down Expand Up @@ -437,11 +436,6 @@ limitations under the License.
<artifactId>checker-qual</artifactId>
<version>${checkerframework.version}</version>
</dependency>
<dependency>
<groupId>com.paypal.digraph</groupId>
<artifactId>digraph-parser</artifactId>
<version>${digraph-parser.version}</version>
</dependency>

<!-- Java 11 compat -->
<dependency>
Expand Down
15 changes: 10 additions & 5 deletions serialization/dot/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ limitations under the License.
<packaging>jar</packaging>

<name>AutomataLib :: Serialization :: DOT</name>
<description>Serializers for the DOT Format</description>
<description>(De-)Serializers for the DOT Format</description>

<dependencies>
<!-- internal -->
Expand All @@ -55,10 +55,6 @@ limitations under the License.
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</dependency>
<dependency>
<groupId>com.paypal.digraph</groupId>
<artifactId>digraph-parser</artifactId>
</dependency>

<!-- build -->
<dependency>
Expand All @@ -77,4 +73,13 @@ limitations under the License.
<artifactId>testng</artifactId>
</dependency>
</dependencies>

<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>javacc-maven-plugin</artifactId>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,16 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
import java.util.Collection;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;

import com.google.common.collect.Maps;
import com.paypal.digraph.parser.GraphEdge;
import com.paypal.digraph.parser.GraphNode;
import com.paypal.digraph.parser.GraphParser;
import com.paypal.digraph.parser.GraphParserException;
import net.automatalib.commons.util.IOUtil;
import net.automatalib.graphs.Graph;
import net.automatalib.graphs.MutableGraph;
import net.automatalib.serialization.FormatException;
import net.automatalib.serialization.ModelDeserializer;

/**
Expand All @@ -47,8 +44,8 @@
public class DOTGraphParser<NP, EP, G extends MutableGraph<?, ?, NP, EP>> implements ModelDeserializer<G> {

private final Supplier<G> creator;
private final Function<Map<String, Object>, NP> nodeParser;
private final Function<Map<String, Object>, EP> edgeParser;
private final Function<Map<String, String>, NP> nodeParser;
private final Function<Map<String, String>, EP> edgeParser;

/**
* Parser for (directed) {@link Graph}s with a custom graph instance and custom node and edge attributes.
Expand All @@ -61,8 +58,8 @@ public class DOTGraphParser<NP, EP, G extends MutableGraph<?, ?, NP, EP>> implem
* an edge parser that extracts from a property map of an edge the edge property
*/
public DOTGraphParser(Supplier<G> creator,
Function<Map<String, Object>, NP> nodeParser,
Function<Map<String, Object>, EP> edgeParser) {
Function<Map<String, String>, NP> nodeParser,
Function<Map<String, String>, EP> edgeParser) {
this.creator = creator;
this.nodeParser = nodeParser;
this.edgeParser = edgeParser;
Expand All @@ -71,33 +68,31 @@ public DOTGraphParser(Supplier<G> creator,
@Override
public G readModel(InputStream is) throws IOException {

final GraphParser gp;
try (Reader r = IOUtil.asUncompressedBufferedNonClosingUTF8Reader(is)) {
InternalDOTParser parser = new InternalDOTParser(r);
parser.parse();

try {
gp = new GraphParser(IOUtil.asUncompressedBufferedNonClosingInputStream(is));
} catch (GraphParserException gpe) {
throw new FormatException(gpe);
}

final G graph = creator.get();
final G graph = creator.get();

parseNodesAndEdges(gp, (MutableGraph<?, ?, NP, EP>) graph);
parseNodesAndEdges(parser, (MutableGraph<?, ?, NP, EP>) graph);

return graph;
return graph;
}
}

private <N> void parseNodesAndEdges(GraphParser gp, MutableGraph<N, ?, NP, EP> graph) {
final Map<String, N> stateMap = Maps.newHashMapWithExpectedSize(gp.getNodes().size());
private <N> void parseNodesAndEdges(InternalDOTParser parser, MutableGraph<N, ?, NP, EP> graph) {
final Collection<Node> nodes = parser.getNodes();
final Collection<Edge> edges = parser.getEdges();

final Map<String, N> stateMap = Maps.newHashMapWithExpectedSize(nodes.size());

for (GraphNode node : gp.getNodes().values()) {
final N n = graph.addNode(nodeParser.apply(node.getAttributes()));
stateMap.put(node.getId(), n);
for (Node node : nodes) {
final N n = graph.addNode(nodeParser.apply(node.attributes));
stateMap.put(node.id, n);
}

for (GraphEdge edge : gp.getEdges().values()) {
graph.connect(stateMap.get(edge.getNode1().getId()),
stateMap.get(edge.getNode2().getId()),
edgeParser.apply(edge.getAttributes()));
for (Edge edge : edges) {
graph.connect(stateMap.get(edge.src), stateMap.get(edge.tgt), edgeParser.apply(edge.attributes));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,18 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.Reader;
import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;

import com.google.common.collect.Maps;
import com.paypal.digraph.parser.GraphEdge;
import com.paypal.digraph.parser.GraphNode;
import com.paypal.digraph.parser.GraphParser;
import com.paypal.digraph.parser.GraphParserException;
import net.automatalib.automata.AutomatonCreator;
import net.automatalib.automata.MutableAutomaton;
import net.automatalib.commons.util.IOUtil;
import net.automatalib.commons.util.Pair;
import net.automatalib.serialization.FormatException;
import net.automatalib.serialization.InputModelData;
import net.automatalib.serialization.InputModelDeserializer;
import net.automatalib.words.Alphabet;
Expand All @@ -56,8 +52,8 @@ public class DOTMutableAutomatonParser<I, SP, TP, A extends MutableAutomaton<?,
implements InputModelDeserializer<I, A> {

private final AutomatonCreator<A, I> creator;
private final Function<Map<String, Object>, SP> nodeParser;
private final Function<Map<String, Object>, Pair<I, TP>> edgeParser;
private final Function<Map<String, String>, SP> nodeParser;
private final Function<Map<String, String>, Pair<I, TP>> edgeParser;
private final Collection<String> initialNodeIds;
private final boolean fakeInitialNodeIds;

Expand All @@ -81,8 +77,8 @@ public class DOTMutableAutomatonParser<I, SP, TP, A extends MutableAutomaton<?,
* matching the {@code initialNodeIds} will be used as initial nodes.
*/
public DOTMutableAutomatonParser(AutomatonCreator<A, I> creator,
Function<Map<String, Object>, SP> nodeParser,
Function<Map<String, Object>, Pair<I, TP>> edgeParser,
Function<Map<String, String>, SP> nodeParser,
Function<Map<String, String>, Pair<I, TP>> edgeParser,
Collection<String> initialNodeIds,
boolean fakeInitialNodeIds) {
this.creator = creator;
Expand All @@ -95,55 +91,54 @@ public DOTMutableAutomatonParser(AutomatonCreator<A, I> creator,
@Override
public InputModelData<I, A> readModel(InputStream is) throws IOException {

final GraphParser gp;
try (Reader r = IOUtil.asUncompressedBufferedNonClosingUTF8Reader(is)) {
InternalDOTParser parser = new InternalDOTParser(r);
parser.parse();

try {
gp = new GraphParser(IOUtil.asUncompressedBufferedNonClosingInputStream(is));
} catch (GraphParserException gpe) {
throw new FormatException(gpe);
}
assert parser.isDirected();

final Set<I> inputs = new HashSet<>();
final Set<I> inputs = new HashSet<>();

for (GraphEdge edge : gp.getEdges().values()) {
if (!fakeInitialNodeIds || !initialNodeIds.contains(edge.getNode1().getId())) {
inputs.add(edgeParser.apply(edge.getAttributes()).getFirst());
for (Edge edge : parser.getEdges()) {
if (!fakeInitialNodeIds || !initialNodeIds.contains(edge.src)) {
inputs.add(edgeParser.apply(edge.attributes).getFirst());
}
}
}

final Alphabet<I> alphabet = Alphabets.fromCollection(inputs);
final A automaton = creator.createAutomaton(alphabet, gp.getNodes().size());
final Alphabet<I> alphabet = Alphabets.fromCollection(inputs);
final A automaton = creator.createAutomaton(alphabet, parser.getNodes().size());

parseNodesAndEdges(gp, (MutableAutomaton<?, I, ?, SP, TP>) automaton);
parseNodesAndEdges(parser, (MutableAutomaton<?, I, ?, SP, TP>) automaton);

return new InputModelData<>(automaton, alphabet);
return new InputModelData<>(automaton, alphabet);
}
}

private <S> void parseNodesAndEdges(GraphParser gp, MutableAutomaton<S, I, ?, SP, TP> automaton) {
final Map<String, S> stateMap = Maps.newHashMapWithExpectedSize(gp.getNodes().size());
private <S> void parseNodesAndEdges(InternalDOTParser parser, MutableAutomaton<S, I, ?, SP, TP> automaton) {
final Map<String, S> stateMap = Maps.newHashMapWithExpectedSize(parser.getNodes().size());

for (GraphNode node : gp.getNodes().values()) {
for (Node node : parser.getNodes()) {
final S state;

if (fakeInitialNodeIds && initialNodeIds.contains(node.getId())) {
if (fakeInitialNodeIds && initialNodeIds.contains(node.id)) {
continue;
} else if (!fakeInitialNodeIds && initialNodeIds.contains(node.getId())) {
state = automaton.addInitialState(nodeParser.apply(node.getAttributes()));
} else if (!fakeInitialNodeIds && initialNodeIds.contains(node.id)) {
state = automaton.addInitialState(nodeParser.apply(node.attributes));
} else {
state = automaton.addState(nodeParser.apply(node.getAttributes()));
state = automaton.addState(nodeParser.apply(node.attributes));
}

stateMap.put(node.getId(), state);
stateMap.put(node.id, state);
}

for (GraphEdge edge : gp.getEdges().values()) {
if (fakeInitialNodeIds && initialNodeIds.contains(edge.getNode1().getId())) {
automaton.setInitial(stateMap.get(edge.getNode2().getId()), true);
for (Edge edge : parser.getEdges()) {
if (fakeInitialNodeIds && initialNodeIds.contains(edge.src)) {
automaton.setInitial(stateMap.get(edge.tgt), true);
} else {
final Pair<I, TP> property = edgeParser.apply(edge.getAttributes());
automaton.addTransition(stateMap.get(edge.getNode1().getId()),
final Pair<I, TP> property = edgeParser.apply(edge.attributes);
automaton.addTransition(stateMap.get(edge.src),
property.getFirst(),
stateMap.get(edge.getNode2().getId()),
stateMap.get(edge.tgt),
property.getSecond());
}
}
Expand Down
Loading

0 comments on commit 230a58f

Please sign in to comment.