Merge branch 'master' into spirit-x3

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
 

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 &

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"));

include/mapnik/feature_kv_iterator.hpp

@@ -46,12 +46,11 @@ class feature_impl;
 
 class MAPNIK_DECL feature_kv_iterator :
         public boost::iterator_facade<feature_kv_iterator,
-                                      std::tuple<std::string , value> const,
+                                      std::tuple<std::string, value> const,
                                       boost::forward_traversal_tag>
 {
 public:
     using value_type = std::tuple<std::string,value>;
-
     feature_kv_iterator (feature_impl const& f, bool begin = false);
 private:
     friend class boost::iterator_core_access;

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_;

include/mapnik/featureset.hpp

@@ -41,21 +41,25 @@ struct MAPNIK_DECL Featureset : private util::noncopyable
     virtual ~Featureset() {}
 };
 
-
+struct MAPNIK_DECL invalid_featureset final : Featureset
-struct MAPNIK_DECL empty_featureset final : Featureset
 {
     feature_ptr next()
     {
         return feature_ptr();
     }
-    ~empty_featureset() {}
+    ~invalid_featureset() {}
 };
 
 using featureset_ptr = std::shared_ptr<Featureset>;
 
-inline featureset_ptr make_empty_featureset()
+inline featureset_ptr make_invalid_featureset()
 {
-    return std::make_shared<empty_featureset>();
+    return std::make_shared<invalid_featureset>();
+}
+
+inline bool is_valid(featureset_ptr const& ptr)
+{
+    return (dynamic_cast<invalid_featureset*>(ptr.get()) == nullptr) ? true : false;
 }
 
 }

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<csv_index_featureset>(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

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<mapnik::value_integer>("num_features_to_query",5))
 {
     boost::optional<std::string> inline_string = params.get<std::string>("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<mapnik::context_type>();
     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::context_type>();
+        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 <typename Iterator>
@@ -314,6 +315,7 @@ void geojson_datasource::initialise_index(Iterator start, Iterator end)
         tree_ = std::make_unique<spatial_index_type>(boxes);
         // calculate total extent
         std::size_t feature_count = 0;
+        mapnik::context_ptr ctx = std::make_shared<mapnik::context_type>();
         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::context_type>();
                 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 <typename Iterator>
@@ -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<mapnik::datasource_geometry_t> 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<mapnik::context_type>();
         for (auto const& pos : positions)
         {
             std::fseek(file.get(), pos.first, SEEK_SET);
@@ -466,7 +468,6 @@ boost::optional<mapnik::datasource_geometry_t> 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::context_type>();
             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

plugins/input/geojson/geojson_datasource.hpp

@@ -104,7 +104,7 @@ private:
     std::unique_ptr<spatial_index_type> 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_;
 };
 
 

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();
 }

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<Connection> 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<double> pgraster_datasource::envelope() const

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<Connection> 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<double> postgis_datasource::envelope() const

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();
 }

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();
 }

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

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

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;

test/data

@@ -1 +1 @@
-Subproject commit ca5716fcef3ed33b319d3047f7d8190bf6ed6081
+Subproject commit fb1529e225b36f8a3077ad23f7005951a07c8a7e

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<std::string> 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<double>(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<attr> attrs = {
                         attr{"name", tr.transcode("Test")},
                         attr{"NOM_FR", tr.transcode("Québec")},