Ticket #19624: 19624.1.patch

File 19624.1.patch, 13.9 KB (added by taylor.smock, 6 years ago)

Add tests, fix tests (due to input streams not supporting marks)

Line 
1Index: src/org/openstreetmap/josm/io/GeoJSONReader.java
2===================================================================
3--- src/org/openstreetmap/josm/io/GeoJSONReader.java (revision 16928)
4+++ src/org/openstreetmap/josm/io/GeoJSONReader.java (working copy)
5@@ -3,7 +3,13 @@
6
7 import static org.openstreetmap.josm.tools.I18n.tr;
8
9+import java.io.BufferedInputStream;
10+import java.io.BufferedReader;
11+import java.io.ByteArrayInputStream;
12+import java.io.IOException;
13 import java.io.InputStream;
14+import java.io.InputStreamReader;
15+import java.nio.charset.StandardCharsets;
16 import java.util.List;
17 import java.util.Map;
18 import java.util.Objects;
19@@ -51,6 +57,8 @@
20 private static final String PROPERTIES = "properties";
21 private static final String GEOMETRY = "geometry";
22 private static final String TYPE = "type";
23+ /** The record separator is 0x1E per RFC 7464 */
24+ private static final byte RECORD_SEPARATOR_BYTE = 0x1E;
25 private JsonParser parser;
26 private Projection projection = Projections.getProjectionByCode("EPSG:4326"); // WGS 84
27
28@@ -350,14 +358,55 @@
29 return tags;
30 }
31
32+ /**
33+ * Check if the inputstream follows RFC 7464
34+ * @param source The source to check (should be at the beginning)
35+ * @return {@code true} if the initial character is {@link GeoJSONReader#RECORD_SEPARATOR_BYTE}.
36+ */
37+ private static boolean isLineDelimited(InputStream source) {
38+ source.mark(2);
39+ try {
40+ int start = source.read();
41+ if (RECORD_SEPARATOR_BYTE == start) {
42+ return true;
43+ }
44+ source.reset();
45+ } catch (IOException e) {
46+ Logging.error(e);
47+ }
48+ return false;
49+ }
50+
51 @Override
52 protected DataSet doParseDataSet(InputStream source, ProgressMonitor progressMonitor) throws IllegalDataException {
53- setParser(Json.createParser(source));
54+ InputStream markSupported = source.markSupported() ? source : new BufferedInputStream(source);
55 ds.setUploadPolicy(UploadPolicy.DISCOURAGED);
56- try {
57- parse();
58- } catch (JsonParsingException e) {
59- throw new IllegalDataException(e);
60+ if (isLineDelimited(markSupported)) {
61+ BufferedReader reader = new BufferedReader(new InputStreamReader(markSupported));
62+ String line = null;
63+ String rs = new String(new byte[]{RECORD_SEPARATOR_BYTE}, StandardCharsets.US_ASCII);
64+ try {
65+ while ((line = reader.readLine()) != null) {
66+ line = line.replaceFirst(rs, "");
67+ try (InputStream is = new ByteArrayInputStream(line.getBytes())) {
68+ setParser(Json.createParser(is));
69+ parse();
70+ } catch (JsonParsingException e) {
71+ throw new IllegalDataException(e);
72+ } finally {
73+ parser.close();
74+ }
75+ }
76+ } catch (IOException e) {
77+ throw new IllegalDataException(e);
78+ }
79+ } else {
80+ setParser(Json.createParser(markSupported));
81+ try {
82+ parse();
83+ } catch (JsonParsingException e) {
84+ throw new IllegalDataException(e);
85+ }
86 }
87 return getDataSet();
88 }
89Index: test/data/geoLineByLine.json
90===================================================================
91--- test/data/geoLineByLine.json (nonexistent)
92+++ test/data/geoLineByLine.json (working copy)
93@@ -0,0 +1,4 @@
94+
95{"type": "FeatureCollection", "features": [{"type": "Feature", "geometry": {"type": "Point", "coordinates": [102.0, 0.5]}, "properties": {"propA": "valueA"}}]}
96+
97{"type": "FeatureCollection", "features": [{"type": "Feature", "geometry": {"type": "LineString", "coordinates": [[102.0, 0.5], [103.0, 1.0], [104.0, 0.0], [105.0, 1.0]]}, "properties": {"propB": "valueB", "propB2": 0.0}}]}
98+
99{"type": "FeatureCollection", "features": [{"type": "Feature", "geometry": {"type": "MultiPolygon", "coordinates": [[[[180.0, 40.0], [180.0, 50.0], [170.0, 50.0], [170.0, 40.0], [180.0, 40.0]]], [[[-170.0, 40.0], [-170.0, 50.0], [-180.0, 50.0], [-180.0, 40.0], [-170.0, 40.0]]]]}, "properties": {}}]}
100+
101{"type": "FeatureCollection", "features": [{"type": "Feature", "geometry": {"type": "Polygon", "coordinates": [[[100.0, 0.0], [101.0, 0.0], [101.0, 1.0], [100.0, 1.0], [100.0, 0.0]]]}, "properties": {"propD": "valueD", "propD2": {"this": "that"}, "propD3": true, "propD4": null}}]}
102Index: test/unit/org/openstreetmap/josm/io/GeoJSONReaderTest.java
103===================================================================
104--- test/unit/org/openstreetmap/josm/io/GeoJSONReaderTest.java (revision 16928)
105+++ test/unit/org/openstreetmap/josm/io/GeoJSONReaderTest.java (working copy)
106@@ -5,6 +5,7 @@
107 import static org.junit.Assert.assertFalse;
108 import static org.junit.Assert.assertNull;
109 import static org.junit.Assert.assertTrue;
110+import static org.junit.jupiter.api.Assertions.assertThrows;
111
112 import java.io.ByteArrayInputStream;
113 import java.io.InputStream;
114@@ -12,6 +13,7 @@
115 import java.nio.file.Files;
116 import java.nio.file.Paths;
117 import java.util.ArrayList;
118+import java.util.Collection;
119 import java.util.List;
120 import java.util.Optional;
121 import java.util.stream.IntStream;
122@@ -46,79 +48,99 @@
123 final List<OsmPrimitive> primitives = new ArrayList<>(new GeoJSONReader()
124 .doParseDataSet(in, null)
125 .getPrimitives(it -> true));
126- assertEquals(20, primitives.size());
127
128- final Node node1 = new Node(new LatLon(0.5, 102.0));
129- final Optional<OsmPrimitive> foundNode1 = primitives.stream()
130- .filter(it -> areEqualNodes(it, node1))
131- .findFirst();
132- assertTrue(foundNode1.isPresent());
133- assertEquals("valueA", foundNode1.get().get("propA"));
134+ assertExpectedGeoPrimitives(primitives);
135+ }
136+ }
137
138- final Way way1 = new Way();
139- way1.addNode(new Node(new LatLon(0.5, 102.0)));
140- way1.addNode(new Node(new LatLon(1, 103)));
141- way1.addNode(new Node(new LatLon(0, 104)));
142- way1.addNode(new Node(new LatLon(1, 105)));
143- final Optional<OsmPrimitive> foundWay1 = primitives.stream()
144- .filter(it -> areEqualWays(it, way1))
145- .findFirst();
146- assertTrue(foundWay1.isPresent());
147- assertEquals("valueB", foundWay1.get().get("propB"));
148- assertEquals("0.0", foundWay1.get().get("propB2"));
149- assertEquals(foundNode1.get(), ((Way) foundWay1.get()).firstNode());
150- assertEquals("valueA", ((Way) foundWay1.get()).firstNode().get("propA"));
151+ /**
152+ * Tests reading a GeoJSON file that is line by line separated, per RFC 7464
153+ * @throws Exception in case of an error
154+ */
155+ @Test
156+ public void testReadLineByLineGeoJSON() throws Exception {
157+ try (InputStream in = Files.newInputStream(Paths.get(TestUtils.getTestDataRoot(), "geoLineByLine.json"))) {
158+ final List<OsmPrimitive> primitives = new ArrayList<>(new GeoJSONReader()
159+ .doParseDataSet(in, null)
160+ .getPrimitives(it -> true));
161
162- final Way way2 = new Way();
163- way2.addNode(new Node(new LatLon(40, 180)));
164- way2.addNode(new Node(new LatLon(50, 180)));
165- way2.addNode(new Node(new LatLon(50, 170)));
166- way2.addNode(new Node(new LatLon(40, 170)));
167- way2.addNode(new Node(new LatLon(40, 180)));
168- final Optional<OsmPrimitive> foundWay2 = primitives.stream()
169- .filter(it -> areEqualWays(it, way2))
170- .findFirst();
171- assertTrue(foundWay2.isPresent());
172- assertEquals(
173- ((Way) foundWay2.get()).getNode(0),
174- ((Way) foundWay2.get()).getNode(((Way) foundWay2.get()).getNodesCount() - 1)
175- );
176+ assertExpectedGeoPrimitives(primitives);
177+ }
178+ }
179
180- final Way way3 = new Way();
181- way3.addNode(new Node(new LatLon(40, -170)));
182- way3.addNode(new Node(new LatLon(50, -170)));
183- way3.addNode(new Node(new LatLon(50, -180)));
184- way3.addNode(new Node(new LatLon(40, -180)));
185- way3.addNode(new Node(new LatLon(40, -170)));
186- final Optional<OsmPrimitive> foundWay3 = primitives.stream()
187- .filter(it -> areEqualWays(it, way3))
188- .findFirst();
189- assertTrue(foundWay3.isPresent());
190- assertEquals(
191- ((Way) foundWay3.get()).getNode(0),
192- ((Way) foundWay3.get()).getNode(((Way) foundWay3.get()).getNodesCount() - 1)
193- );
194+ private void assertExpectedGeoPrimitives(Collection<OsmPrimitive> primitives) {
195+ assertEquals(20, primitives.size());
196
197- final Way way4 = new Way();
198- way4.addNode(new Node(new LatLon(0, 100)));
199- way4.addNode(new Node(new LatLon(0, 101)));
200- way4.addNode(new Node(new LatLon(1, 101)));
201- way4.addNode(new Node(new LatLon(1, 100)));
202- way4.addNode(new Node(new LatLon(0, 100)));
203- final Optional<OsmPrimitive> foundWay4 = primitives.stream()
204- .filter(it -> areEqualWays(it, way4))
205- .findFirst();
206- assertTrue(foundWay4.isPresent());
207- assertEquals(
208- ((Way) foundWay4.get()).getNode(0),
209- ((Way) foundWay4.get()).getNode(((Way) foundWay4.get()).getNodesCount() - 1)
210- );
211- assertEquals("valueD", foundWay4.get().get("propD"));
212- assertFalse(foundWay4.get().hasTag("propD2"));
213- assertEquals("true", foundWay4.get().get("propD3"));
214- assertFalse(foundWay4.get().hasKey("propD4"));
215- assertNull(foundWay4.get().get("propD4"));
216- }
217+ final Node node1 = new Node(new LatLon(0.5, 102.0));
218+ final Optional<OsmPrimitive> foundNode1 = primitives.stream()
219+ .filter(it -> areEqualNodes(it, node1))
220+ .findFirst();
221+ assertTrue(foundNode1.isPresent());
222+ assertEquals("valueA", foundNode1.get().get("propA"));
223+
224+ final Way way1 = new Way();
225+ way1.addNode(new Node(new LatLon(0.5, 102.0)));
226+ way1.addNode(new Node(new LatLon(1, 103)));
227+ way1.addNode(new Node(new LatLon(0, 104)));
228+ way1.addNode(new Node(new LatLon(1, 105)));
229+ final Optional<OsmPrimitive> foundWay1 = primitives.stream()
230+ .filter(it -> areEqualWays(it, way1))
231+ .findFirst();
232+ assertTrue(foundWay1.isPresent());
233+ assertEquals("valueB", foundWay1.get().get("propB"));
234+ assertEquals("0.0", foundWay1.get().get("propB2"));
235+ assertEquals(foundNode1.get(), ((Way) foundWay1.get()).firstNode());
236+ assertEquals("valueA", ((Way) foundWay1.get()).firstNode().get("propA"));
237+
238+ final Way way2 = new Way();
239+ way2.addNode(new Node(new LatLon(40, 180)));
240+ way2.addNode(new Node(new LatLon(50, 180)));
241+ way2.addNode(new Node(new LatLon(50, 170)));
242+ way2.addNode(new Node(new LatLon(40, 170)));
243+ way2.addNode(new Node(new LatLon(40, 180)));
244+ final Optional<OsmPrimitive> foundWay2 = primitives.stream()
245+ .filter(it -> areEqualWays(it, way2))
246+ .findFirst();
247+ assertTrue(foundWay2.isPresent());
248+ assertEquals(
249+ ((Way) foundWay2.get()).getNode(0),
250+ ((Way) foundWay2.get()).getNode(((Way) foundWay2.get()).getNodesCount() - 1)
251+ );
252+
253+ final Way way3 = new Way();
254+ way3.addNode(new Node(new LatLon(40, -170)));
255+ way3.addNode(new Node(new LatLon(50, -170)));
256+ way3.addNode(new Node(new LatLon(50, -180)));
257+ way3.addNode(new Node(new LatLon(40, -180)));
258+ way3.addNode(new Node(new LatLon(40, -170)));
259+ final Optional<OsmPrimitive> foundWay3 = primitives.stream()
260+ .filter(it -> areEqualWays(it, way3))
261+ .findFirst();
262+ assertTrue(foundWay3.isPresent());
263+ assertEquals(
264+ ((Way) foundWay3.get()).getNode(0),
265+ ((Way) foundWay3.get()).getNode(((Way) foundWay3.get()).getNodesCount() - 1)
266+ );
267+
268+ final Way way4 = new Way();
269+ way4.addNode(new Node(new LatLon(0, 100)));
270+ way4.addNode(new Node(new LatLon(0, 101)));
271+ way4.addNode(new Node(new LatLon(1, 101)));
272+ way4.addNode(new Node(new LatLon(1, 100)));
273+ way4.addNode(new Node(new LatLon(0, 100)));
274+ final Optional<OsmPrimitive> foundWay4 = primitives.stream()
275+ .filter(it -> areEqualWays(it, way4))
276+ .findFirst();
277+ assertTrue(foundWay4.isPresent());
278+ assertEquals(
279+ ((Way) foundWay4.get()).getNode(0),
280+ ((Way) foundWay4.get()).getNode(((Way) foundWay4.get()).getNodesCount() - 1)
281+ );
282+ assertEquals("valueD", foundWay4.get().get("propD"));
283+ assertFalse(foundWay4.get().hasTag("propD2"));
284+ assertEquals("true", foundWay4.get().get("propD3"));
285+ assertFalse(foundWay4.get().hasKey("propD4"));
286+ assertNull(foundWay4.get().get("propD4"));
287 }
288
289 /**
290@@ -139,11 +161,9 @@
291
292 /**
293 * Test reading a JSON file which is not a proper GeoJSON (type missing).
294- * @throws IllegalDataException always
295 */
296- @Test(expected = IllegalDataException.class)
297- public void testReadGeoJsonWithoutType() throws IllegalDataException {
298- new GeoJSONReader().doParseDataSet(new ByteArrayInputStream("{}".getBytes(StandardCharsets.UTF_8)), null);
299+ public void testReadGeoJsonWithoutType() {
300+ assertThrows(IllegalDataException.class, () -> new GeoJSONReader().doParseDataSet(new ByteArrayInputStream("{}".getBytes(StandardCharsets.UTF_8)), null));
301 }
302
303 private static boolean areEqualNodes(final OsmPrimitive p1, final OsmPrimitive p2) {