+ code: avoid exposing unsafe static methods in datasource_cache ( #1451

)

+ python: remove redundent 'instance' method (mapnik.DatasourceCache)
+ python: reflect plugin_directories method
+ tests: update python usage

TODO: consider using similar approach in FontEngine etc..
TODO: consider returning reference from singleton::instance() to
      safeguard from accidental deleting a 'singleton' pointer

bindings/python/mapnik/__init__.py

@@ -54,7 +54,7 @@ def bootstrap_env():
     The settings file should be a python file with an 'env' variable
     that declares a dictionary of key:value pairs to push into the
     global process environment, if not already set, like:
-    
+
         env = {'ICU_DATA':'/usr/local/share/icu/'}
     """
     if os.path.exists(os.path.join(os.path.dirname(__file__),'mapnik_settings.py')):
@@ -136,52 +136,52 @@ def __repr__(self):
 
     def forward(self, projection):
         """
-        Projects the point from the geographic coordinate 
-        space  into the cartesian space. The x component is 
-        considered to be longitude, the y component the 
+        Projects the point from the geographic coordinate
+        space  into the cartesian space. The x component is
+        considered to be longitude, the y component the
         latitude.
 
-        Returns the easting (x) and northing (y) as a 
+        Returns the easting (x) and northing (y) as a
         coordinate pair.
 
-        Example: Project the geographic coordinates of the 
+        Example: Project the geographic coordinates of the
                  city center of Stuttgart into the local
-                 map projection (GK Zone 3/DHDN, EPSG 31467)  
-        >>> p = Projection('+init=epsg:31467') 
+                 map projection (GK Zone 3/DHDN, EPSG 31467)
+        >>> p = Projection('+init=epsg:31467')
         >>> Coord(9.1, 48.7).forward(p)
         Coord(3507360.12813,5395719.2749)
         """
         return forward_(self, projection)
 
     def inverse(self, projection):
         """
-        Projects the point from the cartesian space 
-        into the geographic space. The x component is 
-        considered to be the easting, the y component 
+        Projects the point from the cartesian space
+        into the geographic space. The x component is
+        considered to be the easting, the y component
         to be the northing.
 
-        Returns the longitude (x) and latitude (y) as a 
+        Returns the longitude (x) and latitude (y) as a
         coordinate pair.
 
-        Example: Project the cartesian coordinates of the 
+        Example: Project the cartesian coordinates of the
                  city center of Stuttgart in the local
                  map projection (GK Zone 3/DHDN, EPSG 31467)
                  into geographic coordinates:
-        >>> p = Projection('+init=epsg:31467') 
+        >>> p = Projection('+init=epsg:31467')
         >>> Coord(3507360.12813,5395719.2749).inverse(p)
         Coord(9.1, 48.7)
         """
         return inverse_(self, projection)
 
 class _Box2d(Box2d,_injector):
     """
-    Represents a spatial envelope (i.e. bounding box). 
+    Represents a spatial envelope (i.e. bounding box).
 
 
     Following operators are defined for Box2d:
 
     Addition:
-    e1 + e2 is equvalent to e1.expand_to_include(e2) but yields 
+    e1 + e2 is equvalent to e1.expand_to_include(e2) but yields
     a new envelope instead of modifying e1
 
     Subtraction:
@@ -191,7 +191,7 @@ class _Box2d(Box2d,_injector):
     Multiplication and division change the width and height of the envelope
     by the given factor without modifying its center..
 
-    That is, e1 * x is equivalent to: 
+    That is, e1 * x is equivalent to:
            e1.width(x * e1.width())
            e1.height(x * e1.height()),
     except that a new envelope is created instead of modifying e1.
@@ -207,8 +207,8 @@ def __repr__(self):
 
     def forward(self, projection):
         """
-        Projects the envelope from the geographic space 
-        into the cartesian space by projecting its corner 
+        Projects the envelope from the geographic space
+        into the cartesian space by projecting its corner
         points.
 
         See also:
@@ -218,8 +218,8 @@ def forward(self, projection):
 
     def inverse(self, projection):
         """
-        Projects the envelope from the cartesian space 
-        into the geographic space by projecting its corner 
+        Projects the envelope from the cartesian space
+        into the geographic space by projecting its corner
         points.
 
         See also:
@@ -234,7 +234,7 @@ def __repr__(self):
 
     def forward(self,obj):
         """
-        Projects the given object (Box2d or Coord) 
+        Projects the given object (Box2d or Coord)
         from the geographic space into the cartesian space.
 
         See also:
@@ -245,7 +245,7 @@ def forward(self,obj):
 
     def inverse(self,obj):
         """
-        Projects the given object (Box2d or Coord) 
+        Projects the given object (Box2d or Coord)
         from the cartesian space into the geographic space.
 
         See also:
@@ -331,7 +331,7 @@ def Shapefile(**keywords):
       encoding -- file encoding (default 'utf-8')
 
     >>> from mapnik import Shapefile, Layer
-    >>> shp = Shapefile(base='/home/mapnik/data',file='world_borders') 
+    >>> shp = Shapefile(base='/home/mapnik/data',file='world_borders')
     >>> lyr = Layer('Shapefile Layer')
     >>> lyr.datasource = shp
 
@@ -346,7 +346,7 @@ def PostGIS(**keywords):
       dbname -- database name to connect to
       table -- table name or subselect query
 
-      *Note: if using subselects for the 'table' value consider also 
+      *Note: if using subselects for the 'table' value consider also
        passing the 'geometry_field' and 'srid' and 'extent_from_subquery'
        options and/or specifying the 'geometry_table' option.
 
@@ -405,7 +405,7 @@ def Raster(**keywords):
       tile_stride -- if an image is in tiles, what's the increment between rows/cols (default 1)
 
     >>> from mapnik import Raster, Layer
-    >>> raster = Raster(base='/home/mapnik/data',file='elevation.tif',lox=-122.8,loy=48.5,hix=-122.7,hiy=48.6) 
+    >>> raster = Raster(base='/home/mapnik/data',file='elevation.tif',lox=-122.8,loy=48.5,hix=-122.7,hiy=48.6)
     >>> lyr = Layer('Tiff Layer')
     >>> lyr.datasource = raster
 
@@ -479,7 +479,7 @@ def Ogr(**keywords):
       encoding -- file encoding (default 'utf-8')
 
     >>> from mapnik import Ogr, Layer
-    >>> datasource = Ogr(base='/home/mapnik/data',file='rivers.geojson',layer='OGRGeoJSON') 
+    >>> datasource = Ogr(base='/home/mapnik/data',file='rivers.geojson',layer='OGRGeoJSON')
     >>> lyr = Layer('OGR Layer from GeoJSON file')
     >>> lyr.datasource = datasource
 
@@ -507,7 +507,7 @@ def SQLite(**keywords):
       use_spatial_index -- boolean, instruct sqlite plugin to use Rtree spatial index (default True)
 
     >>> from mapnik import SQLite, Layer
-    >>> sqlite = SQLite(base='/home/mapnik/data',file='osm.db',table='osm',extent='-20037508,-19929239,20037508,19929239') 
+    >>> sqlite = SQLite(base='/home/mapnik/data',file='osm.db',table='osm',extent='-20037508,-19929239,20037508,19929239')
     >>> lyr = Layer('SQLite Layer')
     >>> lyr.datasource = sqlite
 
@@ -527,7 +527,7 @@ def Rasterlite(**keywords):
       extent -- manually specified data extent (comma delimited string, default None)
 
     >>> from mapnik import Rasterlite, Layer
-    >>> rasterlite = Rasterlite(base='/home/mapnik/data',file='osm.db',table='osm',extent='-20037508,-19929239,20037508,19929239') 
+    >>> rasterlite = Rasterlite(base='/home/mapnik/data',file='osm.db',table='osm',extent='-20037508,-19929239,20037508,19929239')
     >>> lyr = Layer('Rasterlite Layer')
     >>> lyr.datasource = rasterlite
 
@@ -547,7 +547,7 @@ def Osm(**keywords):
       bbox -- data bounding box for fetching data (default None)
 
     >>> from mapnik import Osm, Layer
-    >>> datasource = Osm(file='test.osm') 
+    >>> datasource = Osm(file='test.osm')
     >>> lyr = Layer('Osm Layer')
     >>> lyr.datasource = datasource
 
@@ -569,7 +569,7 @@ def Kismet(**keywords):
       extent -- manually specified data extent (comma delimited string, default None)
 
     >>> from mapnik import Kismet, Layer
-    >>> datasource = Kismet(host='localhost',port=2501,extent='-179,-85,179,85') 
+    >>> datasource = Kismet(host='localhost',port=2501,extent='-179,-85,179,85')
     >>> lyr = Layer('Kismet Server Layer')
     >>> lyr.datasource = datasource
 
@@ -587,7 +587,7 @@ def Geos(**keywords):
       extent -- manually specified data extent (comma delimited string, default None)
 
     >>> from mapnik import Geos, Layer
-    >>> datasource = Geos(wkt='MULTIPOINT(100 100, 50 50, 0 0)') 
+    >>> datasource = Geos(wkt='MULTIPOINT(100 100, 50 50, 0 0)')
     >>> lyr = Layer('GEOS Layer from WKT string')
     >>> lyr.datasource = datasource
 
@@ -621,7 +621,7 @@ def __init__(self, envelope=None, geometry_type=None, data_type=None):
 
     def features(self, query):
         """Return an iterable which yields instances of Feature for features within the passed query.
-        
+
         Required arguments:
           query -- a Query instance specifying the region for which features should be returned
         """
@@ -1122,7 +1122,7 @@ def mapnik_version_from_string(version_string):
 
 def register_plugins(path=inputpluginspath):
     """Register plugins located by specified path"""
-    DatasourceCache.instance().register_datasources(path)
+    DatasourceCache.register_datasources(path)
 
 def register_fonts(path=fontscollectionpath,valid_extensions=['.ttf','.otf','.ttc','.pfa','.pfb','.ttc','.dfont']):
     """Recursively register fonts using path argument as base directory"""

bindings/python/mapnik_datasource.cpp

@@ -81,7 +81,7 @@ boost::shared_ptr<mapnik::datasource> create_datasource(const dict& d)
         }
     }
 
-    return mapnik::datasource_cache::create(params, bind);
+    return mapnik::datasource_cache::instance()->create(params, bind);
 }
 
 boost::python::dict describe(boost::shared_ptr<mapnik::datasource> const& ds)

bindings/python/mapnik_datasource_cache.cpp

@@ -23,25 +23,76 @@
 #include <boost/python.hpp>
 #include <mapnik/datasource_cache.hpp>
 
+namespace  {
+
+using namespace boost::python;
+
+boost::shared_ptr<mapnik::datasource> create_datasource(const dict& d)
+{
+    bool bind=true;
+    mapnik::parameters params;
+    boost::python::list keys=d.keys();
+    for (int i=0; i<len(keys); ++i)
+    {
+        std::string key = extract<std::string>(keys[i]);
+        object obj = d[key];
+
+        if (key == "bind")
+        {
+            bind = extract<bool>(obj)();
+            continue;
+        }
+
+        extract<std::string> ex0(obj);
+        extract<int> ex1(obj);
+        extract<double> ex2(obj);
+
+        if (ex0.check())
+        {
+            params[key] = ex0();
+        }
+        else if (ex1.check())
+        {
+            params[key] = ex1();
+        }
+        else if (ex2.check())
+        {
+            params[key] = ex2();
+        }
+    }
+
+    return mapnik::datasource_cache::instance()->create(params, bind);
+}
+
+void register_datasources(std::string const& path)
+{
+    mapnik::datasource_cache::instance()->register_datasources(path);
+}
+
+std::vector<std::string> plugin_names()
+{
+    return mapnik::datasource_cache::instance()->plugin_names();
+}
+
+std::string plugin_directories()
+{
+    return mapnik::datasource_cache::instance()->plugin_directories();
+}
+
+}
+
 void export_datasource_cache()
 {
     using mapnik::datasource_cache;
-    using mapnik::singleton;
-    using mapnik::CreateStatic;
-    using namespace boost::python;
-    class_<singleton<datasource_cache,CreateStatic>,boost::noncopyable>("Singleton",no_init)
-        .def("instance",&singleton<datasource_cache,CreateStatic>::instance,
-             return_value_policy<reference_existing_object>())
-        .staticmethod("instance")
-        ;
-
-    class_<datasource_cache,bases<singleton<datasource_cache,CreateStatic> >,
-        boost::noncopyable>("DatasourceCache",no_init)
-        .def("create",&datasource_cache::create)
+    class_<datasource_cache,
+           boost::noncopyable>("DatasourceCache",no_init)
+        .def("create",&create_datasource)
         .staticmethod("create")
-        .def("register_datasources",&datasource_cache::register_datasources)
+        .def("register_datasources",&register_datasources)
         .staticmethod("register_datasources")
-        .def("plugin_names",&datasource_cache::plugin_names)
+        .def("plugin_names",&plugin_names)
         .staticmethod("plugin_names")
+        .def("plugin_directories",&plugin_directories)
+        .staticmethod("plugin_directories")
         ;
 }

include/mapnik/datasource_cache.hpp

@@ -36,28 +36,28 @@
 // stl
 #include <map>
 
-namespace mapnik {
-class MAPNIK_DECL datasource_cache :
-        public singleton <datasource_cache,CreateStatic>,
-        private boost::noncopyable
+namespace mapnik { namespace detail {
+class MAPNIK_DECL datasource_cache_impl
 {
-    friend class CreateStatic<datasource_cache>;
-private:
-    datasource_cache();
-    ~datasource_cache();
-    datasource_cache(const datasource_cache&);
-    datasource_cache& operator=(const datasource_cache&);
-    static std::map<std::string,boost::shared_ptr<PluginInfo> > plugins_;
-    static bool registered_;
-    static bool insert(std::string const&  name,const lt_dlhandle module);
-    static std::vector<std::string> plugin_directories_;
 public:
-    static std::vector<std::string> plugin_names();
-    static std::string plugin_directories();
-    static void register_datasources(std::string const& path);
-    static bool register_datasource(std::string const& path);
-    static boost::shared_ptr<datasource> create(parameters const& params, bool bind=true);
+    datasource_cache_impl();
+    ~datasource_cache_impl();
+    std::vector<std::string> plugin_names();
+    std::string plugin_directories();
+    void register_datasources(std::string const& path);
+    bool register_datasource(std::string const& path);
+    boost::shared_ptr<datasource> create(parameters const& params, bool bind=true);
+private:
+    std::map<std::string,boost::shared_ptr<PluginInfo> > plugins_;
+    bool registered_;
+    bool insert(std::string const&  name,const lt_dlhandle module);
+    std::vector<std::string> plugin_directories_;
+
 };
 }
 
+typedef singleton<detail::datasource_cache_impl, CreateStatic> datasource_cache;
+
+}
+
 #endif // MAPNIK_DATASOURCE_CACHE_HPP