From b9fa920c3d1dc2f805ef69526072b73dc74e6104 Mon Sep 17 00:00:00 2001 From: Paul Norman Date: Wed, 1 Apr 2015 10:40:13 -0700 Subject: [PATCH] Always resize the node_cache in geometry-processor There was a bug where node_cache was not properly resized when the current way was smaller than previous ways or a way was missing nodes (i.e. incomplete extract). This fixes #274 and adds a testcase. This entire section of code should be rewritten to use std::vector everywhere. --- .gitignore | 1 + Makefile.am | 4 + geometry-processor.cpp | 11 +- tests/test-output-multi-line-storage.cpp | 111 ++++++++++++++++++ tests/test_output_multi_line_storage.osm | 35 ++++++ tests/test_output_multi_line_trivial.lua | 10 ++ .../test_output_multi_line_trivial.style.json | 14 +++ 7 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 tests/test-output-multi-line-storage.cpp create mode 100644 tests/test_output_multi_line_storage.osm create mode 100644 tests/test_output_multi_line_trivial.lua create mode 100644 tests/test_output_multi_line_trivial.style.json diff --git a/.gitignore b/.gitignore index 3f51b71b7..7b8dd0ce3 100644 --- a/.gitignore +++ b/.gitignore @@ -48,6 +48,7 @@ tests/test-middle-pgsql tests/test-pgsql-escape tests/test-parse-options tests/test-output-multi-line +tests/test-output-multi-line-storage tests/test-output-multi-point tests/test-output-multi-point-multi-table tests/test-output-multi-polygon diff --git a/Makefile.am b/Makefile.am index 7e9f4d28f..2816bafbb 100644 --- a/Makefile.am +++ b/Makefile.am @@ -81,6 +81,7 @@ check_PROGRAMS = \ tests/test-middle-ram \ tests/test-middle-pgsql \ tests/test-output-multi-line \ + tests/test-output-multi-line-storage \ tests/test-output-multi-point \ tests/test-output-multi-point-multi-table \ tests/test-output-multi-polygon \ @@ -97,6 +98,8 @@ tests_test_middle_pgsql_SOURCES = tests/test-middle-pgsql.cpp tests/middle-tests tests_test_middle_pgsql_LDADD = libosm2pgsql.la tests_test_output_multi_line_SOURCES = tests/test-output-multi-line.cpp tests/common-pg.cpp tests_test_output_multi_line_LDADD = libosm2pgsql.la +tests_test_output_multi_line_storage_SOURCES = tests/test-output-multi-line-storage.cpp tests/common-pg.cpp +tests_test_output_multi_line_storage_LDADD = libosm2pgsql.la tests_test_output_multi_point_SOURCES = tests/test-output-multi-point.cpp tests/common-pg.cpp tests_test_output_multi_point_LDADD = libosm2pgsql.la tests_test_output_multi_point_multi_table_SOURCES = tests/test-output-multi-point-multi-table.cpp tests/common-pg.cpp @@ -159,6 +162,7 @@ tests_test_parse_xml2_LDADD += $(GLOBAL_LDFLAGS) tests_test_middle_ram_LDADD += $(GLOBAL_LDFLAGS) tests_test_middle_pgsql_LDADD += $(GLOBAL_LDFLAGS) tests_test_output_multi_line_LDADD += $(GLOBAL_LDFLAGS) +tests_test_output_multi_line_storage_LDADD += $(GLOBAL_LDFLAGS) tests_test_output_multi_point_LDADD += $(GLOBAL_LDFLAGS) tests_test_output_multi_point_multi_table_LDADD += $(GLOBAL_LDFLAGS) tests_test_output_multi_polygon_LDADD += $(GLOBAL_LDFLAGS) diff --git a/geometry-processor.cpp b/geometry-processor.cpp index 16e302bae..7b7e85654 100644 --- a/geometry-processor.cpp +++ b/geometry-processor.cpp @@ -75,11 +75,12 @@ way_helper::~way_helper() } size_t way_helper::set(const osmid_t *node_ids, size_t node_count, const middle_query_t *mid) { - //if we don't have enough space already get more - if(node_cache.size() < node_count) - node_cache.resize(node_count); - //get the node data - mid->nodes_get_list(&node_cache.front(), node_ids, node_count); + // other parts of the code assume that the node cache is the size of the way + // TODO: Fix this, and use std::vector everywhere + node_cache.resize(node_count); + // get the node data, and resize the node cache in case there were missing nodes + node_cache.resize(mid->nodes_get_list(&node_cache.front(), node_ids, node_count)); + // equivalent to returning node_count for complete ways, different for partial extractsx return node_cache.size(); } diff --git a/tests/test-output-multi-line-storage.cpp b/tests/test-output-multi-line-storage.cpp new file mode 100644 index 000000000..2864bb303 --- /dev/null +++ b/tests/test-output-multi-line-storage.cpp @@ -0,0 +1,111 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +#include "osmtypes.hpp" +#include "middle.hpp" +#include "output-multi.hpp" +#include "options.hpp" +#include "middle-pgsql.hpp" +#include "taginfo_impl.hpp" +#include "parse.hpp" + +#include +#include +#include + +#include +#include + +#include "tests/middle-tests.hpp" +#include "tests/common-pg.hpp" + +void check_count(pg::conn_ptr &conn, int expected, const std::string &query) { + pg::result_ptr res = conn->exec(query); + + int ntuples = PQntuples(res->get()); + if (ntuples != 1) { + throw std::runtime_error((boost::format("Expected only one tuple from a query " + "to check COUNT(*), but got %1%. Query " + "was: %2%.") + % ntuples % query).str()); + } + + std::string numstr = PQgetvalue(res->get(), 0, 0); + int count = boost::lexical_cast(numstr); + + if (count != expected) { + throw std::runtime_error((boost::format("Expected %1%, but got %2%, when running " + "query: %3%.") + % expected % count % query).str()); + } +} + +int main(int argc, char *argv[]) { + boost::scoped_ptr db; + + try { + db.reset(new pg::tempdb); + } catch (const std::exception &e) { + std::cerr << "Unable to setup database: " << e.what() << "\n"; + return 77; // <-- code to skip this test. + } + + try { + options_t options; + options.conninfo = db->conninfo().c_str(); + options.num_procs = 1; + options.slim = 1; + + options.projection.reset(new reprojection(PROJ_LATLONG)); + + options.output_backend = "multi"; + options.style = "tests/test_output_multi_line_trivial.style.json"; + + //setup the front (input) + parse_delegate_t parser(options.extra_attributes, options.bbox, options.projection); + + //setup the middle + boost::shared_ptr middle = middle_t::create_middle(options.slim); + + //setup the backend (output) + std::vector > outputs = output_t::create_outputs(middle.get(), options); + + //let osmdata orchestrate between the middle and the outs + osmdata_t osmdata(middle, outputs); + + osmdata.start(); + + if (parser.streamFile("libxml2", "tests/test_output_multi_line_storage.osm", options.sanitize, &osmdata) != 0) { + throw std::runtime_error("Unable to read input file `tests/test_output_multi_line_storage.osm'."); + } + + osmdata.stop(); + + // start a new connection to run tests on + pg::conn_ptr test_conn = pg::conn::connect(db->conninfo()); + + check_count(test_conn, 1, "select count(*) from pg_catalog.pg_class where relname = 'test_line'"); + check_count(test_conn, 3, "select count(*) from test_line"); + + //check that we have the number of vertexes in each linestring + check_count(test_conn, 3, "SELECT ST_NumPoints(way) FROM test_line WHERE osm_id = 1"); + check_count(test_conn, 2, "SELECT ST_NumPoints(way) FROM test_line WHERE osm_id = 2"); + check_count(test_conn, 2, "SELECT ST_NumPoints(way) FROM test_line WHERE osm_id = 3"); + + return 0; + + } catch (const std::exception &e) { + std::cerr << "ERROR: " << e.what() << std::endl; + + } catch (...) { + std::cerr << "UNKNOWN ERROR" << std::endl; + } + + return 1; +} diff --git a/tests/test_output_multi_line_storage.osm b/tests/test_output_multi_line_storage.osm new file mode 100644 index 000000000..425598e5a --- /dev/null +++ b/tests/test_output_multi_line_storage.osm @@ -0,0 +1,35 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/test_output_multi_line_trivial.lua b/tests/test_output_multi_line_trivial.lua new file mode 100644 index 000000000..7c3f0da7e --- /dev/null +++ b/tests/test_output_multi_line_trivial.lua @@ -0,0 +1,10 @@ +function drop_all (...) + return 1, {} +end + +-- A generic way to process ways, given a function which determines if tags are interesting +-- Takes an optional function to process tags. Always says it's a polygon if there's matching tags +function test_ways (kv, num_keys) + tags = {} + return 0, tags, 1, 0 +end diff --git a/tests/test_output_multi_line_trivial.style.json b/tests/test_output_multi_line_trivial.style.json new file mode 100644 index 000000000..05d7a32ee --- /dev/null +++ b/tests/test_output_multi_line_trivial.style.json @@ -0,0 +1,14 @@ +[ + { + "name": "test_line", + "type": "line", + "tagtransform": "tests/test_output_multi_line_trivial.lua", + "tagtransform-node-function": "drop_all", + "tagtransform-way-function": "test_ways", + "tagtransform-relation-function": "drop_all", + "tagtransform-relation-member-function": "drop_all", + "tags": [ + {"name": "foo", "type": "text"} + ] + } +]