diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c3c00ba9..9840af1ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Released: xx-xx-xx - Added support for quantising small (less than 3 pixel) images (ref #3466) - Added support for natural logarithm function in expressions (ref #3475) - Improved logic determining if certain compiler features are available e.g `inheriting constructors` (MSVC) +- GeoJSON - corrected quoting in `stringgifird` objects (ref #3491) ## 3.0.11 diff --git a/bootstrap.sh b/bootstrap.sh index 62eb1ecf6..113d60f8c 100755 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -30,7 +30,6 @@ function install() { MASON_PLATFORM_ID=$(mason env MASON_PLATFORM_ID) if [[ ! -d ./mason_packages/${MASON_PLATFORM_ID}/${1}/${2} ]]; then mason install $1 $2 - mason link $1 $2 if [[ ${3:-false} != false ]]; then LA_FILE=$(mason prefix $1 $2)/lib/$3.la if [[ -f ${LA_FILE} ]]; then @@ -40,6 +39,7 @@ function install() { fi fi fi + mason link $1 $2 } ICU_VERSION="55.1" @@ -60,7 +60,7 @@ function install_mason_deps() { install protobuf 2.6.1 & # technically protobuf is not a mapnik core dep, but installing # here by default helps make mapnik-vector-tile builds easier - install webp 0.4.2 libwebp & + install webp 0.5.0 libwebp & install gdal 1.11.2 libgdal & install boost 1.61.0 & install boost_libsystem 1.61.0 & diff --git a/demo/c++/rundemo.cpp b/demo/c++/rundemo.cpp index c20c288cf..ff7907d2e 100644 --- a/demo/c++/rundemo.cpp +++ b/demo/c++/rundemo.cpp @@ -55,7 +55,7 @@ int main ( int, char** ) try { std::cout << " running demo ... \n"; datasource_cache::instance().register_datasources("plugins/input/"); - freetype_engine::register_font("fonts/dejavu-fonts-ttf-2.35/ttf/DejaVuSans.ttf"); + freetype_engine::register_font("fonts/dejavu-fonts-ttf-2.37/ttf/DejaVuSans.ttf"); Map m(800,600); m.set_background(parse_color("white")); diff --git a/include/mapnik/feature_kv_iterator.hpp b/include/mapnik/feature_kv_iterator.hpp index 77b292fd5..1def2e7c2 100644 --- a/include/mapnik/feature_kv_iterator.hpp +++ b/include/mapnik/feature_kv_iterator.hpp @@ -46,12 +46,11 @@ class feature_impl; class MAPNIK_DECL feature_kv_iterator : public boost::iterator_facade const, + std::tuple const, boost::forward_traversal_tag> { public: using value_type = std::tuple; - feature_kv_iterator (feature_impl const& f, bool begin = false); private: friend class boost::iterator_core_access; diff --git a/include/mapnik/feature_layer_desc.hpp b/include/mapnik/feature_layer_desc.hpp index 9d1186a7c..cd69d831f 100644 --- a/include/mapnik/feature_layer_desc.hpp +++ b/include/mapnik/feature_layer_desc.hpp @@ -94,12 +94,21 @@ public: { return extra_params_; } + bool has_name(std::string const& name) const { auto result = std::find_if(std::begin(descriptors_), std::end(descriptors_), [&name](attribute_descriptor const& desc) { return name == desc.get_name();}); return result != std::end(descriptors_); } + void order_by_name() + { + std::sort(std::begin(descriptors_), std::end(descriptors_), + [](attribute_descriptor const& d0, attribute_descriptor const& d1) + { + return d0.get_name() < d1.get_name(); + }); + } private: std::string name_; std::string encoding_; diff --git a/include/mapnik/featureset.hpp b/include/mapnik/featureset.hpp index 6886d8755..a6ad4e85d 100644 --- a/include/mapnik/featureset.hpp +++ b/include/mapnik/featureset.hpp @@ -41,21 +41,25 @@ struct MAPNIK_DECL Featureset : private util::noncopyable virtual ~Featureset() {} }; - -struct MAPNIK_DECL empty_featureset final : Featureset +struct MAPNIK_DECL invalid_featureset final : Featureset { feature_ptr next() { return feature_ptr(); } - ~empty_featureset() {} + ~invalid_featureset() {} }; using featureset_ptr = std::shared_ptr; -inline featureset_ptr make_empty_featureset() +inline featureset_ptr make_invalid_featureset() { - return std::make_shared(); + return std::make_shared(); +} + +inline bool is_valid(featureset_ptr const& ptr) +{ + return (dynamic_cast(ptr.get()) == nullptr) ? true : false; } } diff --git a/plugins/input/csv/csv_datasource.cpp b/plugins/input/csv/csv_datasource.cpp index 4a246b404..322940528 100644 --- a/plugins/input/csv/csv_datasource.cpp +++ b/plugins/input/csv/csv_datasource.cpp @@ -431,7 +431,7 @@ mapnik::featureset_ptr csv_datasource::features(mapnik::query const& q) const return std::make_shared(filename_, filter, locator_, separator_, quote_, headers_, ctx_); } } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } mapnik::featureset_ptr csv_datasource::features_at_point(mapnik::coord2d const& pt, double tol) const diff --git a/plugins/input/geojson/geojson_datasource.cpp b/plugins/input/geojson/geojson_datasource.cpp index d2ebc2d54..90270873d 100644 --- a/plugins/input/geojson/geojson_datasource.cpp +++ b/plugins/input/geojson/geojson_datasource.cpp @@ -116,7 +116,8 @@ geojson_datasource::geojson_datasource(parameters const& params) inline_string_(), extent_(), features_(), - tree_(nullptr) + tree_(nullptr), + num_features_to_query_(*params.get("num_features_to_query",5)) { boost::optional inline_string = params.get("inline"); if (inline_string) @@ -233,7 +234,7 @@ void geojson_datasource::initialise_disk_index(std::string const& filename) mapnik::util::file file(filename_); if (!file) throw mapnik::datasource_exception("GeoJSON Plugin: could not open: '" + filename_ + "'"); - + mapnik::context_ptr ctx = std::make_shared(); for (auto const& pos : positions) { std::fseek(file.get(), pos.first, SEEK_SET); @@ -242,8 +243,7 @@ void geojson_datasource::initialise_disk_index(std::string const& filename) std::fread(record.data(), pos.second, 1, file.get()); auto const* start = record.data(); auto const* end = start + record.size(); - mapnik::context_ptr ctx = std::make_shared(); - mapnik::feature_ptr feature(mapnik::feature_factory::create(ctx,1)); + mapnik::feature_ptr feature(mapnik::feature_factory::create(ctx, -1)); using namespace boost::spirit; standard::space_type space; if (!boost::spirit::qi::phrase_parse(start, end, @@ -254,6 +254,7 @@ void geojson_datasource::initialise_disk_index(std::string const& filename) } initialise_descriptor(feature); } + desc_.order_by_name(); } template @@ -314,6 +315,7 @@ void geojson_datasource::initialise_index(Iterator start, Iterator end) tree_ = std::make_unique(boxes); // calculate total extent std::size_t feature_count = 0; + mapnik::context_ptr ctx = std::make_shared(); for (auto const& item : boxes) { auto const& box = std::get<0>(item); @@ -326,7 +328,6 @@ void geojson_datasource::initialise_index(Iterator start, Iterator end) // NOTE: this doesn't yield correct answer for geoJSON in general, just an indication Iterator itr2 = start + geometry_index.first; Iterator end2 = itr2 + geometry_index.second; - mapnik::context_ptr ctx = std::make_shared(); mapnik::feature_ptr feature(mapnik::feature_factory::create(ctx,-1)); // temp feature if (!boost::spirit::qi::phrase_parse(itr2, end2, (geojson_datasource_static_feature_grammar)(boost::phoenix::ref(*feature)), space) @@ -339,6 +340,7 @@ void geojson_datasource::initialise_index(Iterator start, Iterator end) } } } + desc_.order_by_name(); } template @@ -355,7 +357,7 @@ void geojson_datasource::parse_geojson(Iterator start, Iterator end) try { bool result = boost::spirit::qi::phrase_parse(itr, end, (geojson_datasource_static_fc_grammar) - (boost::phoenix::ref(ctx),boost::phoenix::ref(start_id), boost::phoenix::ref(callback)), + (boost::phoenix::ref(ctx), boost::phoenix::ref(start_id), boost::phoenix::ref(callback)), space); if (!result || itr != end) { @@ -457,7 +459,7 @@ boost::optional geojson_datasource::get_geometry_ mapnik::util::file file(filename_); if (!file) throw mapnik::datasource_exception("GeoJSON Plugin: could not open: '" + filename_ + "'"); - + mapnik::context_ptr ctx = std::make_shared(); for (auto const& pos : positions) { std::fseek(file.get(), pos.first, SEEK_SET); @@ -466,7 +468,6 @@ boost::optional geojson_datasource::get_geometry_ std::fread(record.data(), pos.second, 1, file.get()); auto const* start = record.data(); auto const* end = start + record.size(); - mapnik::context_ptr ctx = std::make_shared(); mapnik::feature_ptr feature(mapnik::feature_factory::create(ctx, -1)); // temp feature using namespace boost::spirit; standard::space_type space; @@ -588,7 +589,7 @@ mapnik::featureset_ptr geojson_datasource::features(mapnik::query const& q) cons } // otherwise return an empty featureset - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } mapnik::featureset_ptr geojson_datasource::features_at_point(mapnik::coord2d const& pt, double tol) const diff --git a/plugins/input/geojson/geojson_datasource.hpp b/plugins/input/geojson/geojson_datasource.hpp index a50bd9567..5ba36a66b 100644 --- a/plugins/input/geojson/geojson_datasource.hpp +++ b/plugins/input/geojson/geojson_datasource.hpp @@ -104,7 +104,7 @@ private: std::unique_ptr tree_; bool cache_features_ = true; bool has_disk_index_ = false; - const std::size_t num_features_to_query_ = 5; + const std::size_t num_features_to_query_; }; diff --git a/plugins/input/ogr/ogr_datasource.cpp b/plugins/input/ogr/ogr_datasource.cpp index 2943962fc..706a376e3 100644 --- a/plugins/input/ogr/ogr_datasource.cpp +++ b/plugins/input/ogr/ogr_datasource.cpp @@ -560,7 +560,7 @@ featureset_ptr ogr_datasource::features(query const& q) const } } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } featureset_ptr ogr_datasource::features_at_point(coord2d const& pt, double tol) const @@ -603,5 +603,5 @@ featureset_ptr ogr_datasource::features_at_point(coord2d const& pt, double tol) } } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } diff --git a/plugins/input/pgraster/pgraster_datasource.cpp b/plugins/input/pgraster/pgraster_datasource.cpp index ced1810b5..e6eddf758 100644 --- a/plugins/input/pgraster/pgraster_datasource.cpp +++ b/plugins/input/pgraster/pgraster_datasource.cpp @@ -998,7 +998,7 @@ featureset_ptr pgraster_datasource::features_with_context(query const& q,process } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } @@ -1011,7 +1011,7 @@ featureset_ptr pgraster_datasource::features_at_point(coord2d const& pt, double if (pool) { shared_ptr conn = pool->borrowObject(); - if (!conn) return mapnik::make_empty_featureset(); + if (!conn) return mapnik::make_invalid_featureset(); if (conn->isOK()) { @@ -1082,7 +1082,7 @@ featureset_ptr pgraster_datasource::features_at_point(coord2d const& pt, double } } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } box2d pgraster_datasource::envelope() const diff --git a/plugins/input/postgis/postgis_datasource.cpp b/plugins/input/postgis/postgis_datasource.cpp index 2653e5757..10ce521d0 100644 --- a/plugins/input/postgis/postgis_datasource.cpp +++ b/plugins/input/postgis/postgis_datasource.cpp @@ -942,7 +942,7 @@ featureset_ptr postgis_datasource::features_with_context(query const& q,processo } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } @@ -955,7 +955,7 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t if (pool) { shared_ptr conn = pool->borrowObject(); - if (!conn) return mapnik::make_empty_featureset(); + if (!conn) return mapnik::make_invalid_featureset(); if (conn->isOK()) { @@ -1030,7 +1030,7 @@ featureset_ptr postgis_datasource::features_at_point(coord2d const& pt, double t } } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } box2d postgis_datasource::envelope() const diff --git a/plugins/input/raster/raster_datasource.cpp b/plugins/input/raster/raster_datasource.cpp index 38a6aa94c..e059247cd 100644 --- a/plugins/input/raster/raster_datasource.cpp +++ b/plugins/input/raster/raster_datasource.cpp @@ -224,5 +224,5 @@ featureset_ptr raster_datasource::features_at_point(coord2d const&, double tol) { MAPNIK_LOG_WARN(raster) << "raster_datasource: feature_at_point not supported"; - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } diff --git a/plugins/input/sqlite/sqlite_datasource.cpp b/plugins/input/sqlite/sqlite_datasource.cpp index 37e44117b..28add6453 100644 --- a/plugins/input/sqlite/sqlite_datasource.cpp +++ b/plugins/input/sqlite/sqlite_datasource.cpp @@ -551,7 +551,7 @@ featureset_ptr sqlite_datasource::features(query const& q) const using_subquery_); } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } featureset_ptr sqlite_datasource::features_at_point(coord2d const& pt, double tol) const @@ -631,5 +631,5 @@ featureset_ptr sqlite_datasource::features_at_point(coord2d const& pt, double to using_subquery_); } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } diff --git a/plugins/input/topojson/topojson_datasource.cpp b/plugins/input/topojson/topojson_datasource.cpp index e6cab26d8..2cfad1fb8 100644 --- a/plugins/input/topojson/topojson_datasource.cpp +++ b/plugins/input/topojson/topojson_datasource.cpp @@ -284,7 +284,7 @@ mapnik::featureset_ptr topojson_datasource::features(mapnik::query const& q) con } } // otherwise return an empty featureset pointer - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } mapnik::featureset_ptr topojson_datasource::features_at_point(mapnik::coord2d const& pt, double tol) const diff --git a/src/map.cpp b/src/map.cpp index 5b6a83c1b..dc3d53040 100644 --- a/src/map.cpp +++ b/src/map.cpp @@ -746,7 +746,7 @@ featureset_ptr Map::query_point(unsigned index, double x, double y) const else s << " (map has no layers)"; throw std::out_of_range(s.str()); } - return mapnik::make_empty_featureset(); + return mapnik::make_invalid_featureset(); } featureset_ptr Map::query_map_point(unsigned index, double x, double y) const diff --git a/src/svg/svg_parser.cpp b/src/svg/svg_parser.cpp index 2483bae56..827f557fc 100644 --- a/src/svg/svg_parser.cpp +++ b/src/svg/svg_parser.cpp @@ -111,7 +111,6 @@ mapnik::color parse_color(T & error_messages, const char* str) } catch (mapnik::config_error const& ex) { - error_messages.emplace_back(ex.what()); } return c; diff --git a/test/data b/test/data index ca5716fce..fb1529e22 160000 --- a/test/data +++ b/test/data @@ -1 +1 @@ -Subproject commit ca5716fcef3ed33b319d3047f7d8190bf6ed6081 +Subproject commit fb1529e225b36f8a3077ad23f7005951a07c8a7e diff --git a/test/unit/datasource/geojson.cpp b/test/unit/datasource/geojson.cpp index 60052db8b..bcc9fa2e6 100644 --- a/test/unit/datasource/geojson.cpp +++ b/test/unit/datasource/geojson.cpp @@ -96,6 +96,45 @@ TEST_CASE("geojson") { } } + SECTION("GeoJSON an empty FeatureCollection") + { + for (auto cache_features : {true, false}) + { + mapnik::parameters params; + params["type"] = "geojson"; + params["file"] = "./test/data/json/empty_featurecollection.json"; + params["cache_features"] = cache_features; + auto ds = mapnik::datasource_cache::instance().create(params); + CHECK(ds != nullptr); + auto fs = all_features(ds); + REQUIRE(!mapnik::is_valid(fs)); + while (auto f = fs->next()) + { + CHECK(false); // shouldn't get here + } + } + } + + SECTION("GeoJSON attribute descriptors are alphabetically ordered") + { + for (auto cache_features : {true, false}) + { + mapnik::parameters params; + params["type"] = "geojson"; + params["file"] = "./test/data/json/properties.json"; + params["cache_features"] = cache_features; + auto ds = mapnik::datasource_cache::instance().create(params); + CHECK(ds != nullptr); + std::vector expected_names = {"a", "b", "c", "d", "e"}; + auto fields = ds->get_descriptor().get_descriptors(); + std::size_t index = 0; + for (auto const& field : fields) + { + REQUIRE(field.get_name() == expected_names[index++]); + } + } + } + SECTION("GeoJSON invalid Point") { for (auto cache_features : {true, false}) @@ -285,8 +324,6 @@ TEST_CASE("geojson") { } auto features = ds->features(query); auto features2 = ds->features_at_point(ds->envelope().center(),0); - REQUIRE(features != nullptr); - REQUIRE(features2 != nullptr); auto feature = features->next(); auto feature2 = features2->next(); REQUIRE(feature != nullptr); @@ -404,7 +441,6 @@ TEST_CASE("geojson") { query.add_property_name(field.get_name()); } auto features = ds->features(query); - REQUIRE(features != nullptr); auto feature = features->next(); REQUIRE(feature != nullptr); REQUIRE(feature->envelope() == mapnik::box2d(123,456,123,456)); @@ -693,7 +729,6 @@ TEST_CASE("geojson") { REQUIRE_FIELD_NAMES(fields, names); auto fs = all_features(ds); - REQUIRE(bool(fs)); std::initializer_list attrs = { attr{"name", tr.transcode("Test")}, attr{"NOM_FR", tr.transcode("Québec")},