Merge pull request #1592 from joto/fix-parse-style-file

lonvia · web-flow · commit 0a1a458eb91c · 2021-12-14T16:00:19.000+01:00
Fix parsing problems in style file reader
diff --git a/src/taginfo.cpp b/src/taginfo.cpp
@@ -84,10 +84,6 @@ static unsigned get_tag_type(std::string const &tag)
 
 bool read_style_file(std::string const &filename, export_list *exlist)
 {
-    char osmtype[24];
-    char tag[64];
-    char datatype[24];
-    char flags[128];
     bool enable_way_area = true;
 
     FILE *const in = std::fopen(filename.c_str(), "rt");
@@ -110,6 +106,10 @@ bool read_style_file(std::string const &filename, export_list *exlist)
         }
 
         //grab the expected fields for this row
+        char osmtype[24] = {0};
+        char tag[64] = {0};
+        char datatype[24] = {0};
+        char flags[128] = {0};
         int const fields = std::sscanf(buffer, "%23s %63s %23s %127s", osmtype,
                                        tag, datatype, flags);
         if (fields <= 0) { /* Blank line */
@@ -124,7 +124,7 @@ bool read_style_file(std::string const &filename, export_list *exlist)
         }
 
         //place to keep info about this tag
-        taginfo temp;
+        taginfo temp{};
         temp.name = tag;
         temp.type = datatype;
         temp.flags = parse_tag_flags(flags, lineno);
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
@@ -56,6 +56,7 @@ set_test(test-output-pgsql-area)
 set_test(test-output-pgsql-hstore-match-only)
 set_test(test-output-pgsql-int4)
 set_test(test-output-pgsql-schema)
+set_test(test-output-pgsql-style-file)
 set_test(test-output-pgsql-tablespace LABELS Tablespace)
 set_test(test-output-pgsql-validgeom)
 set_test(test-output-pgsql-z_order)
diff --git a/tests/style/comments.style b/tests/style/comments.style
@@ -0,0 +1,3 @@
+# This is a comment
+
+# Also comment
diff --git a/tests/style/data-types.style b/tests/style/data-types.style
@@ -0,0 +1,4 @@
+# OsmType  Tag         DataType  Flags
+node,way   name        text      linear
+way        width       real      linear
+node,way   population  integer   polygon
diff --git a/tests/style/empty.style b/tests/style/empty.style
diff --git a/tests/style/invalid-data-type.style b/tests/style/invalid-data-type.style
@@ -0,0 +1,2 @@
+# OsmType  Tag         DataType  Flags
+way        highway     foo       linear
diff --git a/tests/style/invalid-osm-type.style b/tests/style/invalid-osm-type.style
@@ -0,0 +1,2 @@
+# OsmType  Tag         DataType  Flags
+foo        bar         text      linear
diff --git a/tests/style/missing.style b/tests/style/missing.style
@@ -0,0 +1,5 @@
+# OsmType  Tag    DataType     Flags
+node       node1  text         linear
+node       node2  text
+way        way1   text         polygon
+way        way2   text
diff --git a/tests/style/node.style b/tests/style/node.style
@@ -0,0 +1 @@
+node access text linear
diff --git a/tests/style/valid.style b/tests/style/valid.style
@@ -0,0 +1,12 @@
+# OsmType  Tag               DataType     Flags
+node       node_text_linear  text         linear
+way        way_text_linear   text         linear
+node,way   both_text_linear  text         linear
+
+node       node_text_polygon text         polygon
+way        way_text_polygon  text         polygon
+node,way   both_text_polygon text         polygon
+
+node,way   both_nocolumn     text         nocolumn
+
+node,way   both_delete       text         delete
diff --git a/tests/style/way-area.style b/tests/style/way-area.style
@@ -0,0 +1,3 @@
+# OsmType  Tag       DataType  Flags
+node,way   wetland   text      polygon,nocolumn
+way        way_area  real
diff --git a/tests/test-output-pgsql-style-file.cpp b/tests/test-output-pgsql-style-file.cpp
@@ -0,0 +1,231 @@
+/**
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This file is part of osm2pgsql (https://osm2pgsql.org/).
+ *
+ * Copyright (C) 2006-2021 by the osm2pgsql developer community.
+ * For a full list of authors see the git log.
+ */
+
+#include <catch.hpp>
+
+#include "common-import.hpp"
+#include "common-options.hpp"
+
+TEST_CASE("Parse default style file")
+{
+    export_list exlist;
+    auto const enable_way_area =
+        read_style_file(OSM2PGSQLDATA_DIR "default.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 98);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 104);
+
+}
+
+TEST_CASE("Parse empty style file")
+{
+    export_list exlist;
+
+    REQUIRE_THROWS_WITH(
+        read_style_file(OSM2PGSQLDATA_DIR "tests/style/empty.style",
+                        &exlist),
+        "Unable to parse any valid columns from the style file. Aborting.");
+}
+
+TEST_CASE("Parse style file with invalid osm type")
+{
+    export_list exlist;
+
+    REQUIRE_THROWS(read_style_file(
+        OSM2PGSQLDATA_DIR "tests/style/invalid-osm-type.style", &exlist));
+}
+
+TEST_CASE("Parse style file with comments only")
+{
+    export_list exlist;
+
+    REQUIRE_THROWS_WITH(
+        read_style_file(OSM2PGSQLDATA_DIR "tests/style/comments.style",
+                        &exlist),
+        "Unable to parse any valid columns from the style file. Aborting.");
+}
+
+TEST_CASE("Parse style file with single node entry")
+{
+    export_list exlist;
+    auto const enable_way_area =
+        read_style_file(OSM2PGSQLDATA_DIR "tests/style/node.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 1);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 0);
+
+    auto const &ex = exlist.get(osmium::item_type::node).front();
+    REQUIRE(ex.name == "access");
+    REQUIRE(ex.type == "text");
+    REQUIRE(ex.flags == column_flags::FLAG_LINEAR);
+    REQUIRE(ex.column_type() == ColumnType::TEXT);
+}
+
+TEST_CASE("Parse style file with a few valid entries")
+{
+    export_list exlist;
+    auto const enable_way_area =
+        read_style_file(OSM2PGSQLDATA_DIR "tests/style/valid.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 6);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 6);
+
+    auto const &nodes = exlist.get(osmium::item_type::node);
+    auto const &ways = exlist.get(osmium::item_type::way);
+
+    for (auto const &node : nodes) {
+        REQUIRE(node.type == "text");
+        REQUIRE(node.column_type() == ColumnType::TEXT);
+    }
+
+    for (auto const &way : ways) {
+        REQUIRE(way.type == "text");
+        REQUIRE(way.column_type() == ColumnType::TEXT);
+    }
+
+    REQUIRE(nodes[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(nodes[1].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(nodes[2].flags == column_flags::FLAG_POLYGON);
+    REQUIRE(nodes[3].flags == column_flags::FLAG_POLYGON);
+    REQUIRE(nodes[4].flags == column_flags::FLAG_NOCOLUMN);
+    REQUIRE(nodes[5].flags == column_flags::FLAG_DELETE);
+    REQUIRE(ways[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(ways[1].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(ways[2].flags == column_flags::FLAG_POLYGON);
+    REQUIRE(ways[3].flags == column_flags::FLAG_POLYGON);
+    REQUIRE(ways[4].flags == column_flags::FLAG_NOCOLUMN);
+    REQUIRE(ways[5].flags == column_flags::FLAG_DELETE);
+}
+
+TEST_CASE("Parse style file with missing fields")
+{
+    export_list exlist;
+    auto const enable_way_area =
+        read_style_file(OSM2PGSQLDATA_DIR "tests/style/missing.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 2);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 2);
+
+    auto const &nodes = exlist.get(osmium::item_type::node);
+    auto const &ways = exlist.get(osmium::item_type::way);
+
+    for (auto const &node : nodes) {
+        REQUIRE(node.type == "text");
+        REQUIRE(node.column_type() == ColumnType::TEXT);
+    }
+    REQUIRE(nodes[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(nodes[1].flags == 0);
+
+    for (auto const &way : ways) {
+        REQUIRE(way.type == "text");
+        REQUIRE(way.column_type() == ColumnType::TEXT);
+    }
+    REQUIRE(ways[0].flags == column_flags::FLAG_POLYGON);
+    REQUIRE(ways[1].flags == 0);
+}
+
+TEST_CASE("Parse style file with way_area")
+{
+    export_list exlist;
+    auto const enable_way_area = read_style_file(
+        OSM2PGSQLDATA_DIR "tests/style/way-area.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 1);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 2);
+
+    auto const &nodes = exlist.get(osmium::item_type::node);
+    auto const &ways = exlist.get(osmium::item_type::way);
+
+    REQUIRE(nodes[0].type == "text");
+    REQUIRE(nodes[0].flags ==
+            (column_flags::FLAG_POLYGON | column_flags::FLAG_NOCOLUMN));
+    REQUIRE(nodes[0].column_type() == ColumnType::TEXT);
+
+    REQUIRE(ways[0].type == "text");
+    REQUIRE(ways[0].flags ==
+            (column_flags::FLAG_POLYGON | column_flags::FLAG_NOCOLUMN));
+    REQUIRE(ways[0].column_type() == ColumnType::TEXT);
+
+    REQUIRE(ways[1].type == "real");
+    REQUIRE(ways[1].flags == 0);
+    REQUIRE(ways[1].column_type() ==
+            ColumnType::TEXT); // Special case for way_area!
+}
+
+TEST_CASE("Parse style file with different data types")
+{
+    export_list exlist;
+    auto const enable_way_area = read_style_file(
+        OSM2PGSQLDATA_DIR "tests/style/data-types.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 2);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 3);
+
+    auto const &nodes = exlist.get(osmium::item_type::node);
+    auto const &ways = exlist.get(osmium::item_type::way);
+
+    REQUIRE(nodes[0].name == "name");
+    REQUIRE(nodes[0].type == "text");
+    REQUIRE(nodes[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(nodes[0].column_type() == ColumnType::TEXT);
+
+    REQUIRE(nodes[1].name == "population");
+    REQUIRE(nodes[1].type == "integer");
+    REQUIRE(nodes[1].flags ==
+            (column_flags::FLAG_POLYGON | column_flags::FLAG_INT_TYPE));
+    REQUIRE(nodes[1].column_type() == ColumnType::INT);
+
+    REQUIRE(ways[0].name == "name");
+    REQUIRE(ways[0].type == "text");
+    REQUIRE(ways[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(ways[0].column_type() == ColumnType::TEXT);
+
+    REQUIRE(ways[1].name == "width");
+    REQUIRE(ways[1].type == "real");
+    REQUIRE(ways[1].flags ==
+            (column_flags::FLAG_LINEAR | column_flags::FLAG_REAL_TYPE));
+    REQUIRE(ways[1].column_type() == ColumnType::REAL);
+
+    REQUIRE(ways[2].name == "population");
+    REQUIRE(ways[2].type == "integer");
+    REQUIRE(ways[2].flags ==
+            (column_flags::FLAG_POLYGON | column_flags::FLAG_INT_TYPE));
+    REQUIRE(ways[2].column_type() == ColumnType::INT);
+}
+
+TEST_CASE("Parse style file with invalid data types")
+{
+    export_list exlist;
+    auto const enable_way_area = read_style_file(
+        OSM2PGSQLDATA_DIR "tests/style/invalid-data-type.style", &exlist);
+
+    REQUIRE(enable_way_area);
+
+    REQUIRE(exlist.get(osmium::item_type::node).size() == 0);
+    REQUIRE(exlist.get(osmium::item_type::way).size() == 1);
+
+    auto const &ways = exlist.get(osmium::item_type::way);
+
+    REQUIRE(ways[0].name == "highway");
+    REQUIRE(ways[0].type == "foo");
+    REQUIRE(ways[0].flags == column_flags::FLAG_LINEAR);
+    REQUIRE(ways[0].column_type() == ColumnType::TEXT);
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+# This is a comment`
	`2`	`+`
	`3`	`+# Also comment`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+# OsmType Tag DataType Flags`
	`2`	`+way highway foo linear`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+# OsmType Tag DataType Flags`
	`2`	`+foo bar text linear`