Item769: d2n and other IF / query functions should throw exceptions on error
Priority: Normal
Current State: Confirmed
Released In:
Target Release:
Applies To: Engine
Component:
Branches:
from
Item753 it became pretty obvious that the user can be quite mistified at the output of IF and
SEARCH when it returns 'true' due to an unexpected parse error
such as %IF{"d2n('12/Dec/2009') ...
which looks obvious (and actually is), but is outside the functioning of the current parseTime (ok, this may have changed, but the example is still valid)
so, I propose
- throw an error if there is a date Parse problem - so the user sees the RED i failed to parse your IF
- this isn't quite so simple, as the parse is done in IF_eval, and the RED errors are for IF_parse
- make sure there is a way for app writers to hide that - ie consider adding an
error="$format"
and additionally, if at all possible we should return
false for conditionals in which there is an eval error - false negatives are better than false positives?
see
Item753 for a bunch of examples like:
- IF{"d2n('1%20Dec%202008') < d2n('1%20Nov%202008')" then="true" else="false"}
should be false?
- IF{"d2n('1%20Dec%202008') > d2n('1%20Nov%202008')" then="true" else="false"}
- IF{"d2n('banana') = d2n('apple')" then="true" else="false"}
- IF{"d2n('1/Dec/2008') <= d2n('1/Nov/2008')" then="true" else="false"}
- IF{"d2n('1-Dec-2008') <= d2n('1-Nov-2008')" then="true" else="false"}
- IF{"d2n('Dec 2008') <= d2n('Nov 2008')" then="true" else="false"}
- IF{"d2n('2009') <= d2n('2008')" then="true" else="false"}
--
SvenDowideit - 15 Jan 2009
some part of what i was doing before i stopped
macpro:foswiki svend$ svn diff core/lib/Foswiki/Query/OP_d2n.pm UnitTestContrib/test/unit/Fn_IF.pm UnitTestContrib/test/unit/TimeTests.pm
Index: core/lib/Foswiki/Query/OP_d2n.pm
===================================================================
--- core/lib/Foswiki/Query/OP_d2n.pm (revision 1995)
+++ core/lib/Foswiki/Query/OP_d2n.pm (working copy)
@@ -23,13 +23,17 @@
$node,
sub {
my $date = shift;
+ my $gmtime = 0;
eval {
require Foswiki::Time;
- $date = Foswiki::Time::parseTime( $date, 1 );
+ $gmtime = Foswiki::Time::parseTime( $date, 1 );
};
+die "Failed to parseTime($date)" unless ($gmtime > 0);
+ throw Error::Simple("Failed to parseTime($date)") unless ($gmtime > 0);
+
# ignore $@
- return $date;
+ return $gmtime;
},
@_
);
Index: UnitTestContrib/test/unit/Fn_IF.pm
===================================================================
--- UnitTestContrib/test/unit/Fn_IF.pm (revision 1995)
+++ UnitTestContrib/test/unit/Fn_IF.pm (working copy)
@@ -1430,5 +1430,14 @@
}
+sub test_d2n_parseFailures {
+ my $this = shift;
+ #this should fail
+ $this->simpleTest(test => "d2n('apple') = d2n('orange')", then => 0, else => 1);
+
+ #not quite sure about
+ #$this->simpleTest(test => "d2n('apple') = '0'", then => 1, else => 0);
+}
+
1;
Index: UnitTestContrib/test/unit/TimeTests.pm
===================================================================
--- UnitTestContrib/test/unit/TimeTests.pm (revision 1995)
+++ UnitTestContrib/test/unit/TimeTests.pm (working copy)
@@ -178,6 +178,15 @@
$this->checkTime(0, 0, 0, 4, 2, 1995, " 1995-02-04");
$this->checkTime(0, 0, 0, 1, 1, 1995, " 1995 ");
+ $this->checkTime(0, 0, 0, 4, 2, 1995, "1995 02 04");
+ $this->checkTime(0, 0, 0, 1, 2, 1995, "1995 02");
+ $this->checkTime(0, 0, 0, 1, 1, 1995, "1995");
+
+#wonder what to do with URL-Encoded
+ $this->checkTime(0, 0, 0, 4, 2, 1995, "1995%2002%2004");
+ $this->checkTime(0, 0, 0, 1, 2, 1995, "1995%2002");
+ $this->checkTime(0, 0, 0, 1, 1, 1995, "1995");
+
}
sub test_parseErrors {
Confirmed, the error handling is poor and should be refactored.
--
CrawfordCurrie - 05 Feb 2009