From 5479dfbe0ebde1a2158b2c619a29586a32a82865 Mon Sep 17 00:00:00 2001
From: Dane Springmeyer <dane@dbsgeo.com>
Date: Thu, 20 Dec 2012 14:24:56 -0800
Subject: [PATCH] change mapnik::value::to_bool to return false if a string is
 empty - closes #1665

---
 CHANGELOG.md                          |  2 ++
 bindings/python/mapnik_expression.cpp |  6 +++++
 include/mapnik/value.hpp              |  3 +--
 tests/python_tests/filter_test.py     | 38 ++++++++++++++++++++++++++-
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31c4ad432..3a3d55750 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,8 @@ For a complete change history, see the git log.
 
 ## Future
 
+- `<Filter>[attr]</Filter>` now returns false if attr is an empty string (#1665)
+
 - Added 64 bit integer support in expressions and feature ids (#1661,#1662)
 
 - Added support for DBF `Logical` type: #1614
diff --git a/bindings/python/mapnik_expression.cpp b/bindings/python/mapnik_expression.cpp
index 109b21e15..d3bd8b1dd 100644
--- a/bindings/python/mapnik_expression.cpp
+++ b/bindings/python/mapnik_expression.cpp
@@ -59,6 +59,11 @@ mapnik::value expression_evaluate_(mapnik::expr_node const& expr, mapnik::Featur
     return boost::apply_visitor(mapnik::evaluate<mapnik::Feature,mapnik::value>(f),expr);
 }
 
+bool expression_evaluate_to_bool_(mapnik::expr_node const& expr, mapnik::Feature const& f)
+{
+    return boost::apply_visitor(mapnik::evaluate<mapnik::Feature,mapnik::value>(f),expr).to_bool();
+}
+
 // path expression
 path_expression_ptr parse_path_(std::string const& path)
 {
@@ -82,6 +87,7 @@ void export_expression()
                                                   "TODO"
                                                   "",no_init)
         .def("evaluate", &expression_evaluate_)
+        .def("to_bool", &expression_evaluate_to_bool_)
         .def("__str__",&expression_to_string_);
     ;
 
diff --git a/include/mapnik/value.hpp b/include/mapnik/value.hpp
index cb4fa7d33..6cd0944a3 100644
--- a/include/mapnik/value.hpp
+++ b/include/mapnik/value.hpp
@@ -624,8 +624,7 @@ struct to_bool : public boost::static_visitor<value_bool>
 
     value_bool operator() (value_unicode_string const& ustr) const
     {
-        boost::ignore_unused_variable_warning(ustr);
-        return true;
+        return !ustr.isEmpty();
     }
 
     value_bool operator() (value_null const& val) const
diff --git a/tests/python_tests/filter_test.py b/tests/python_tests/filter_test.py
index 7fcbdccf0..cf3575d78 100644
--- a/tests/python_tests/filter_test.py
+++ b/tests/python_tests/filter_test.py
@@ -257,7 +257,7 @@ def test_expressions_with_null_equality():
         expr = mapnik.Expression("[prop] is null")
         eq_(expr.evaluate(f),eq[1])
 
-def test_expressions_with_null_equality():
+def test_expressions_with_null_equality2():
     for eq in null_equality:
         context = mapnik.Context()
         f = mapnik.Feature(context,0)
@@ -275,5 +275,41 @@ def test_expressions_with_null_equality():
         expr = mapnik.Expression("[prop] != null")
         eq_(expr.evaluate(f),not eq[1])
 
+truthyness = [
+  [u'hello',True,unicode],
+  [u'',False,unicode],
+  [0,False,int],
+  [123,True,int],
+  [0.0,False,float],
+  [123.123,True,float],
+  [.1,True,float],
+  [False,False,int], # TODO - should become bool
+  [True,True,int], # TODO - should become bool
+  [None,False,None]
+]
+
+def test_expressions_for_thruthyness():
+    context = mapnik.Context()
+    for eq in truthyness:
+        f = mapnik.Feature(context,0)
+        f["prop"] = eq[0]
+        eq_(f["prop"],eq[0])
+        if eq[0] is None:
+            eq_(f["prop"] is None, True)
+        else:
+            eq_(isinstance(f['prop'],eq[2]),True,'%s is not an instance of %s' % (f['prop'],eq[2]))
+        expr = mapnik.Expression("[prop]")
+        eq_(expr.to_bool(f),eq[1])
+        expr = mapnik.Expression("not [prop]")
+        eq_(expr.to_bool(f),not eq[1])
+    # also test if feature does not have property at all
+    f2 = mapnik.Feature(context,1)
+    # no property existing will return value_null since
+    # https://github.com/mapnik/mapnik/commit/562fada9d0f680f59b2d9f396c95320a0d753479#include/mapnik/feature.hpp
+    eq_(f2["prop"] is None,True)
+    expr = mapnik.Expression("[prop]")
+    eq_(expr.evaluate(f2),None)
+    eq_(expr.to_bool(f2),False)
+
 if __name__ == "__main__":
     [eval(run)() for run in dir() if 'test_' in run]