From 0ecf7e5728179dc3e70e485c39fe32953cf67626 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 17 Apr 2013 11:00:27 +0200 Subject: [PATCH 1/8] Add a couple more float to string conversion tests See #1811 --- tests/cpp_tests/conversions_test.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/cpp_tests/conversions_test.cpp b/tests/cpp_tests/conversions_test.cpp index 9144a7bb9..023d7f703 100644 --- a/tests/cpp_tests/conversions_test.cpp +++ b/tests/cpp_tests/conversions_test.cpp @@ -186,6 +186,14 @@ int main( int, char*[] ) BOOST_TEST_EQ( out, "1.00001" ); out.clear(); + to_string(out, double(67.65)); + BOOST_TEST_EQ( out, "67.65" ); + out.clear(); + + to_string(out, double(67.35)); + BOOST_TEST_EQ( out, "67.35" ); + out.clear(); + to_string(out, double(1234000000000000)); BOOST_TEST_EQ( out, "1.234e+15" ); out.clear(); From 0a68f66c35a42b92e47d6251906acd7ac490d19b Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 13:39:11 -0700 Subject: [PATCH 2/8] fixup postgis null comparision tests --- tests/python_tests/postgis_test.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/python_tests/postgis_test.py b/tests/python_tests/postgis_test.py index 71ea7b129..a5ed52ae2 100644 --- a/tests/python_tests/postgis_test.py +++ b/tests/python_tests/postgis_test.py @@ -556,7 +556,7 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[name] != 'name'").evaluate(feat),True) eq_(mapnik.Expression("[name] != ''").evaluate(feat),False) eq_(mapnik.Expression("[name] != null").evaluate(feat),True) - eq_[bool_field] + eq_(mapnik.Expression("[name] != true").evaluate(feat),True) eq_(mapnik.Expression("[name] != false").evaluate(feat),True) feat = fs.next() @@ -573,7 +573,7 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[name] != true").evaluate(feat),True) eq_(mapnik.Expression("[name] != false").evaluate(feat),True) - def test_null_comparision(): + def test_null_comparision2(): ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='test10', geometry_field='geom') fs = ds.featureset() @@ -586,8 +586,8 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[bool_field] = true").evaluate(feat),True) eq_(mapnik.Expression("[bool_field] = false").evaluate(feat),False) eq_(mapnik.Expression("[bool_field] != 'name'").evaluate(feat),True) - eq_(mapnik.Expression("[bool_field] != ''").evaluate(feat),True) - eq_(mapnik.Expression("[bool_field] != null").evaluate(feat),True) + eq_(mapnik.Expression("[bool_field] != ''").evaluate(feat),True) # in 2.1.x used to be False + eq_(mapnik.Expression("[bool_field] != null").evaluate(feat),True) # in 2.1.x used to be False eq_(mapnik.Expression("[bool_field] != true").evaluate(feat),False) eq_(mapnik.Expression("[bool_field] != false").evaluate(feat),True) @@ -601,7 +601,7 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[bool_field] = false").evaluate(feat),True) eq_(mapnik.Expression("[bool_field] != 'name'").evaluate(feat),True) eq_(mapnik.Expression("[bool_field] != ''").evaluate(feat),True) - eq_(mapnik.Expression("[bool_field] != null").evaluate(feat),True) + eq_(mapnik.Expression("[bool_field] != null").evaluate(feat),True) # in 2.1.x used to be False eq_(mapnik.Expression("[bool_field] != true").evaluate(feat),True) eq_(mapnik.Expression("[bool_field] != false").evaluate(feat),False) @@ -613,11 +613,11 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[bool_field] = null").evaluate(feat),True) eq_(mapnik.Expression("[bool_field] = true").evaluate(feat),False) eq_(mapnik.Expression("[bool_field] = false").evaluate(feat),False) - eq_(mapnik.Expression("[bool_field] != 'name'").evaluate(feat),True) - eq_(mapnik.Expression("[bool_field] != ''").evaluate(feat),True) + eq_(mapnik.Expression("[bool_field] != 'name'").evaluate(feat),True) # in 2.1.x used to be False + eq_(mapnik.Expression("[bool_field] != ''").evaluate(feat),True) # in 2.1.x used to be False eq_(mapnik.Expression("[bool_field] != null").evaluate(feat),False) - eq_(mapnik.Expression("[bool_field] != true").evaluate(feat),True) - eq_(mapnik.Expression("[bool_field] != false").evaluate(feat),True) + eq_(mapnik.Expression("[bool_field] != true").evaluate(feat),True) # in 2.1.x used to be False + eq_(mapnik.Expression("[bool_field] != false").evaluate(feat),True) # in 2.1.x used to be False atexit.register(postgis_takedown) From 20b643a22a60233367f0d0aaba0c2b966ffd308b Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 13:40:08 -0700 Subject: [PATCH 3/8] add missing exception translator for datasource exceptions by catching std::exception - closes #1816 --- bindings/python/mapnik_python.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bindings/python/mapnik_python.cpp b/bindings/python/mapnik_python.cpp index 7ed80d58d..a116e3a3a 100644 --- a/bindings/python/mapnik_python.cpp +++ b/bindings/python/mapnik_python.cpp @@ -348,6 +348,11 @@ void runtime_error_translator(std::runtime_error const & ex) PyErr_SetString(PyExc_RuntimeError, ex.what()); } +void standard_error_translator(std::exception const & ex) +{ + PyErr_SetString(PyExc_RuntimeError, ex.what()); +} + unsigned mapnik_version() { return MAPNIK_VERSION; @@ -424,6 +429,8 @@ BOOST_PYTHON_MODULE(_mapnik) register_exception_translator(&config_error_translator); register_exception_translator(&value_error_translator); register_exception_translator(&runtime_error_translator); + // catch the rest + register_exception_translator(&standard_error_translator); register_cairo(); export_query(); export_geometry(); From dac0e1406820a6894025a2ba51512e65299355b4 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 13:40:17 -0700 Subject: [PATCH 4/8] add test for #1816 --- tests/python_tests/postgis_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/python_tests/postgis_test.py b/tests/python_tests/postgis_test.py index a5ed52ae2..e8006a824 100644 --- a/tests/python_tests/postgis_test.py +++ b/tests/python_tests/postgis_test.py @@ -619,6 +619,13 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ eq_(mapnik.Expression("[bool_field] != true").evaluate(feat),True) # in 2.1.x used to be False eq_(mapnik.Expression("[bool_field] != false").evaluate(feat),True) # in 2.1.x used to be False + # https://github.com/mapnik/mapnik/issues/1816 + def test_exception_message_reporting(): + try: + ds = mapnik.PostGIS(dbname=MAPNIK_TEST_DBNAME,table='doesnotexist') + except Exception, e: + eq_(e.message != 'unidentifiable C++ exception', True) + atexit.register(postgis_takedown) if __name__ == "__main__": From 60c6592c4e27c9886a6acdfae52bdde7c24a4bf1 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 14:23:04 -0700 Subject: [PATCH 5/8] fix spelling of unknown --- include/mapnik/factory.hpp | 2 +- plugins/input/postgis/postgis_featureset.cpp | 2 +- src/wkb.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/mapnik/factory.hpp b/include/mapnik/factory.hpp index 012c5de84..ac44ca3e7 100644 --- a/include/mapnik/factory.hpp +++ b/include/mapnik/factory.hpp @@ -40,7 +40,7 @@ public: { const char* what() const throw() { - return "uknown object type"; + return "unknown object type"; } }; static product_type* on_unknown_type(const key_type&) diff --git a/plugins/input/postgis/postgis_featureset.cpp b/plugins/input/postgis/postgis_featureset.cpp index da4ce1e1f..9e5ce838d 100644 --- a/plugins/input/postgis/postgis_featureset.cpp +++ b/plugins/input/postgis/postgis_featureset.cpp @@ -199,7 +199,7 @@ feature_ptr postgis_featureset::next() default: { - MAPNIK_LOG_WARN(postgis) << "postgis_featureset: Uknown type_oid=" << oid; + MAPNIK_LOG_WARN(postgis) << "postgis_featureset: Unknown type_oid=" << oid; break; } diff --git a/src/wkb.cpp b/src/wkb.cpp index 93156bf22..ed8c627c3 100644 --- a/src/wkb.cpp +++ b/src/wkb.cpp @@ -442,7 +442,7 @@ private: case wkbMultiLineStringZ: s << "MultiLineStringZ"; break; case wkbMultiPolygonZ: s << "MultiPolygonZ"; break; case wkbGeometryCollectionZ: s << "GeometryCollectionZ"; break; - default: s << "wkbUknown(" << type << ")"; break; + default: s << "wkbUnknown(" << type << ")"; break; } return s.str(); From 19b777455e8c1f8691a4bed68314a17c3ba22f79 Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 15:28:48 -0700 Subject: [PATCH 6/8] also translate std::out_of_bounds => IndexError otherwise new std::exception => runtime catch will override - refs #1816 --- bindings/python/mapnik_python.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bindings/python/mapnik_python.cpp b/bindings/python/mapnik_python.cpp index a116e3a3a..4ef560c23 100644 --- a/bindings/python/mapnik_python.cpp +++ b/bindings/python/mapnik_python.cpp @@ -348,6 +348,11 @@ void runtime_error_translator(std::runtime_error const & ex) PyErr_SetString(PyExc_RuntimeError, ex.what()); } +void out_of_range_error_translator(std::out_of_range const & ex) +{ + PyErr_SetString(PyExc_IndexError, ex.what()); +} + void standard_error_translator(std::exception const & ex) { PyErr_SetString(PyExc_RuntimeError, ex.what()); @@ -426,11 +431,11 @@ BOOST_PYTHON_MODULE(_mapnik) using mapnik::save_map_to_string; using mapnik::render_grid; + register_exception_translator(&standard_error_translator); + register_exception_translator(&out_of_range_error_translator); register_exception_translator(&config_error_translator); register_exception_translator(&value_error_translator); register_exception_translator(&runtime_error_translator); - // catch the rest - register_exception_translator(&standard_error_translator); register_cairo(); export_query(); export_geometry(); From 8c2d31441340faffa6dbccc273d0a5615065d10f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 15:33:31 -0700 Subject: [PATCH 7/8] fix visual test reporting to show test that could not be run rather than just 0 --- tests/visual_tests/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/visual_tests/test.py b/tests/visual_tests/test.py index c28c4495e..2eaf44a76 100755 --- a/tests/visual_tests/test.py +++ b/tests/visual_tests/test.py @@ -159,7 +159,7 @@ class Reporting: print "\nVisual rendering: %s failed / %s passed" % (len(self.errors), self.passed) for idx, error in enumerate(self.errors): if error[0] == self.OTHER: - print str(idx+1) + ") \x1b[31mfailure to run test:\x1b[0m %s" % error[3] + print str(idx+1) + ") \x1b[31mfailure to run test:\x1b[0m %s" % error[2] elif error[0] == self.NOT_FOUND: if self.generate: print str(idx+1) + ") Generating reference image: '%s'" % error[2] From 83eb8f25959dbea1bd6c6d489f0d68c983b8b26f Mon Sep 17 00:00:00 2001 From: Dane Springmeyer Date: Wed, 17 Apr 2013 15:34:21 -0700 Subject: [PATCH 8/8] fix handling of null values for feature id in sqlite/postgis input - closes #1817 --- plugins/input/postgis/postgis_featureset.cpp | 9 +++++++- plugins/input/sqlite/sqlite_featureset.cpp | 7 +++++++ tests/python_tests/postgis_test.py | 22 ++++++++++++++++++++ tests/python_tests/sqlite_test.py | 21 +++++++++++++++++++ 4 files changed, 58 insertions(+), 1 deletion(-) diff --git a/plugins/input/postgis/postgis_featureset.cpp b/plugins/input/postgis/postgis_featureset.cpp index 9e5ce838d..6ca97d660 100644 --- a/plugins/input/postgis/postgis_featureset.cpp +++ b/plugins/input/postgis/postgis_featureset.cpp @@ -75,10 +75,17 @@ feature_ptr postgis_featureset::next() if (key_field_) { + std::string name = rs_->getFieldName(pos); + + // null feature id is not acceptable + if (rs_->isNull(pos)) + { + MAPNIK_LOG_WARN(postgis) << "postgis_featureset: null value encountered for key_field: " << name; + continue; + } // create feature with user driven id from attribute int oid = rs_->getTypeOID(pos); const char* buf = rs_->getValue(pos); - std::string name = rs_->getFieldName(pos); // validation happens of this type at initialization mapnik::value_integer val; diff --git a/plugins/input/sqlite/sqlite_featureset.cpp b/plugins/input/sqlite/sqlite_featureset.cpp index 9e11509c0..2f1735e47 100644 --- a/plugins/input/sqlite/sqlite_featureset.cpp +++ b/plugins/input/sqlite/sqlite_featureset.cpp @@ -71,6 +71,13 @@ feature_ptr sqlite_featureset::next() return feature_ptr(); } + // null feature id is not acceptable + if (rs_->column_type(1) == SQLITE_NULL) + { + MAPNIK_LOG_ERROR(postgis) << "sqlite_featureset: null value encountered for key_field"; + continue; + } + feature_ptr feature = feature_factory::create(ctx_,rs_->column_integer64(1)); if (!geometry_utils::from_wkb(feature->paths(), data, size, format_)) continue; diff --git a/tests/python_tests/postgis_test.py b/tests/python_tests/postgis_test.py index e8006a824..8e7bda160 100644 --- a/tests/python_tests/postgis_test.py +++ b/tests/python_tests/postgis_test.py @@ -626,6 +626,28 @@ if 'postgis' in mapnik.DatasourceCache.plugin_names() \ except Exception, e: eq_(e.message != 'unidentifiable C++ exception', True) + def test_null_id_field(): + opts = {'type':'postgis', + 'dbname':MAPNIK_TEST_DBNAME, + 'geometry_field':'geom', + 'table':"(select null::bigint as osm_id, GeomFromEWKT('SRID=4326;POINT(0 0)') as geom) as tmp"} + ds = mapnik.Datasource(**opts) + fs = ds.featureset() + feat = fs.next() + eq_(feat.id(),1L) + eq_(feat['osm_id'],None) + + @raises(StopIteration) + def test_null_key_field(): + opts = {'type':'postgis', + "key_field": 'osm_id', + 'dbname':MAPNIK_TEST_DBNAME, + 'geometry_field':'geom', + 'table':"(select null::bigint as osm_id, GeomFromEWKT('SRID=4326;POINT(0 0)') as geom) as tmp"} + ds = mapnik.Datasource(**opts) + fs = ds.featureset() + feat = fs.next() ## should throw since key_field is null: StopIteration: No more features. + atexit.register(postgis_takedown) if __name__ == "__main__": diff --git a/tests/python_tests/sqlite_test.py b/tests/python_tests/sqlite_test.py index b0c8f864e..b0e557307 100644 --- a/tests/python_tests/sqlite_test.py +++ b/tests/python_tests/sqlite_test.py @@ -379,6 +379,27 @@ if 'sqlite' in mapnik.DatasourceCache.plugin_names(): eq_(feat['OGC_FID'],2) eq_(feat['bigint'],922337203685477580) + + @raises(StopIteration) + def test_null_id_field(): + # form up an in-memory test db + wkb = '010100000000000000000000000000000000000000' + # note: the osm_id should be declared INTEGER PRIMARY KEY + # but in this case we intentionally do not make this a valid pkey + # otherwise sqlite would turn the null into a valid, serial id + ds = mapnik.SQLite(file=':memory:', + table='test1', + initdb=''' + create table test1 (osm_id INTEGER,geometry BLOB); + insert into test1 values (null,x'%s'); + ''' % wkb, + extent='-180,-60,180,60', + use_spatial_index=False, + key_field='osm_id' + ) + fs = ds.featureset() + feat = fs.next() ## should throw since key_field is null: StopIteration: No more features. + if __name__ == "__main__": setup() [eval(run)() for run in dir() if 'test_' in run]