Add test for kv_validate() and fix some bugs

Fixed bugs:
- The maxcount option didn't work, was more of an alias for mincount
  instead.
- rmwhitespace wasn't applied when the field gave an _err.
- The func option didn't accept a non-array as argument.

2 files changed, 244 insertions, 23 deletions

diff --git a/lib/TUWF/Misc.pm b/lib/TUWF/Misc.pm
index d9ff3c0..75f2839 100644
--- a/lib/TUWF/Misc.pm
+++ b/lib/TUWF/Misc.pm
@@ -36,19 +36,16 @@ sub kv_validate {
 
     # check each value and add it to %ret
     for (@values) {
-      my $valid = _validate($_, $templates, $f);
-      if(!ref $valid) {
-        $_ = $valid;
-        next;
-      }
-      push @err, [ $f->{$src}, $$valid, $f->{$$valid} ];
+      my $errfield = _validate($_, $templates, $f);
+      next if !$errfield;
+      push @err, [ $f->{$src}, $errfield, $f->{$errfield} ];
       last;
     }
     $ret{$f->{$src}} = $f->{multi} ? \@values : $values[0];
 
     # check mincount/maxcount
     push @err, [ $f->{$src}, 'mincount', $f->{mincount} ] if $f->{mincount} && @values < $f->{mincount};
-    push @err, [ $f->{$src}, 'maxcount', $f->{maxcount} ] if $f->{maxcount} && @values < $f->{maxcount};
+    push @err, [ $f->{$src}, 'maxcount', $f->{maxcount} ] if $f->{maxcount} && @values > $f->{maxcount};
   }
 
   $ret{_err} = \@err if @err;
@@ -57,8 +54,7 @@ sub kv_validate {
 
 
 # Internal function used by kv_validate, checks one value on the validation
-# rules, returns scalarref containing the failed rule on error, new value
-# otherwise
+# rules, the name of the failed rule on error, undef otherwise
 sub _validate { # value, \%templates, \%rules
   my($v, $t, $r) = @_;
 
@@ -68,33 +64,35 @@ sub _validate { # value, \%templates, \%rules
 
   # remove whitespace
   if($v && $r->{rmwhitespace}) {
-    $v =~ s/\r//g;
-    $v =~ s/^[\s\n]+//;
-    $v =~ s/[\s\n]+$//;
+    $_[0] =~ s/\r//g;
+    $_[0] =~ s/^[\s\n]+//;
+    $_[0] =~ s/[\s\n]+$//;
+    $v = $_[0]
   }
 
   # empty
   if(!defined($v) || length($v) < 1) {
-    return \'required' if $r->{required};
-    return exists $r->{default} ? $r->{default} : $v;
+    return 'required' if $r->{required};
+    $_[0] = $r->{default} if exists $r->{default};
+    return undef;
   }
 
   # length
-  return \'minlength' if $r->{minlength} && length $v < $r->{minlength};
-  return \'maxlength' if $r->{maxlength} && length $v > $r->{maxlength};
+  return 'minlength' if $r->{minlength} && length $v < $r->{minlength};
+  return 'maxlength' if $r->{maxlength} && length $v > $r->{maxlength};
   # min/max
-  return \'min'       if defined($r->{min}) && (!looks_like_number($v) || $v < $r->{min});
-  return \'max'       if defined($r->{max}) && (!looks_like_number($v) || $v > $r->{max});
+  return 'min'       if defined($r->{min}) && (!looks_like_number($v) || $v < $r->{min});
+  return 'max'       if defined($r->{max}) && (!looks_like_number($v) || $v > $r->{max});
   # enum
-  return \'enum'      if $r->{enum} && !grep $_ eq $v, @{$r->{enum}};
+  return 'enum'      if $r->{enum} && !grep $_ eq $v, @{$r->{enum}};
   # regex
-  return \'regex'     if $r->{regex} && (ref($r->{regex}) eq 'ARRAY' ? ($v !~ m/$r->{regex}[0]/) : ($v !~  m/$r->{regex}/));
+  return 'regex'     if $r->{regex} && (ref($r->{regex}) eq 'ARRAY' ? ($v !~ m/$r->{regex}[0]/) : ($v !~  m/$r->{regex}/));
   # template
-  return \'template'  if $r->{template} && ref($v = _validate($v, $t, $t->{$r->{template}}));
+  return 'template'  if $r->{template} && _validate($_[0], $t, $t->{$r->{template}});
   # function
-  return \'func'      if $r->{func} && !$r->{func}[0]->($v);
+  return 'func'      if $r->{func} && (ref($r->{func}) eq 'ARRAY' ? !$r->{func}[0]->($_[0]) : !$r->{func}->($_[0]));
   # passed validation
-  return $v;
+  return undef;
 }
 
 
diff --git a/t/kv_validate.t b/t/kv_validate.t
new file mode 100644
index 0000000..1e97839
--- /dev/null
+++ b/t/kv_validate.t
@@ -0,0 +1,223 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+my @tests;
+my %templates;
+BEGIN{@tests=(
+  # definition of a single field
+  # input parameters
+  # expected output
+
+  # required / default
+  { param => 'name' },
+  [],
+  { name => undef, _err => [[ 'name', 'required', 1 ]] },
+
+  { param => 'name' },
+  [ name => '' ],
+  { name => '', _err => [[ 'name', 'required', 1 ]] },
+
+  { param => 'name', required => 'x' },
+  [ name => '' ],
+  { name => '', _err => [[ 'name', 'required', 'x' ]] },
+
+  { param => 'name', required => 0 },
+  [ name => '' ],
+  { name => '' },
+
+  { param => 'name', required => 0, default => undef },
+  [ name => '' ],
+  { name => undef },
+
+  { param => 'name' },
+  [ name => '0' ],
+  { name => '0' },
+
+  # rmwhitespace
+  { param => 'name' },
+  [ name => " Va\rlid \n " ],
+  { name => 'Valid' },
+
+  { param => 'name', rmwhitespace => 0 },
+  [ name => " Va\rlid \n " ],
+  { name => " Va\rlid \n " },
+
+  { param => 'name' },
+  [ name => '  ' ],
+  { name => '', _err => [[ 'name', 'required', 1 ]] },
+
+  # min / max
+  { param => 'age', min => 0, max => 0 },
+  [ age => 0 ],
+  { age => 0 },
+
+  { param => 'age', min => 0, max => 0 },
+  [ age => 1 ],
+  { age => 1, _err => [[ 'age', 'max', 0 ]] },
+
+  { param => 'age', min => 0, max => 0 },
+  [ age => 0.5 ],
+  { age => 0.5, _err => [[ 'age', 'max', 0 ]] },
+
+  { param => 'age', min => 0, max => 0 },
+  [ age => -1 ],
+  { age => -1, _err => [[ 'age', 'min', 0 ]] },
+
+  { param => 'age', min => 0, max => 1000 },
+  [ age => '1e3' ],
+  { age => '1e3' },
+
+  { param => 'age', min => 0, max => 1000 },
+  [ age => '1e4' ],
+  { age => '1e4', _err => [[ 'age', 'max', 1000 ]] },
+
+  { param => 'age', min => 0, max => 0 },
+  [ age => 'x' ],
+  { age => 'x', _err => [[ 'age', 'min', 0 ]] },
+
+  # minlength / maxlength
+  { param => 'msg', minlength => 2 },
+  [ msg => 'ab' ],
+  { msg => 'ab' },
+
+  { param => 'msg', minlength => 2 },
+  [ msg => 'a' ],
+  { msg => 'a', _err => [[ 'msg', 'minlength', 2 ]] },
+
+  { param => 'msg', maxlength => 2 },
+  [ msg => 'ab' ],
+  { msg => 'ab' },
+
+  { param => 'msg', maxlength => 2 },
+  [ msg => 'abc' ],
+  { msg => 'abc', _err => [[ 'msg', 'maxlength', 2 ]] },
+
+  { param => 'msg', minlength => 2 },
+  [ msg => '  a  ' ],
+  { msg => 'a', _err => [[ 'msg', 'minlength', 2 ]] },
+
+  { param => 'msg', maxlength => 2 },
+  [ msg => '     ab     ' ],
+  { msg => 'ab' },
+
+  # enum
+  { param => 'type', enum => ['a'..'z'] },
+  [ type => 'a' ],
+  { type => 'a' },
+
+  { param => 'type', enum => ['a'..'z'] },
+  [ type => 'y' ],
+  { type => 'y' },
+
+  { param => 'type', enum => ['a'..'z'] },
+  [ type => 'Y' ],
+  { type => 'Y', _err => [[ 'type', 'enum', ['a'..'z'] ]] },
+
+  # multi / maxcount / mincount
+  { param => 'board' },
+  [ board => 1, board => 2 ],
+  { board => 1 }, # Not sure I like this behaviour.
+
+  { param => 'board', multi => 1 },
+  [ board => 1, board => 2 ],
+  { board => [1,2] },
+
+  { param => 'board', multi => 1 },
+  [ board => 1 ],
+  { board => [1] },
+
+  { param => 'board', multi => 1 },
+  [ board => '' ],
+  { board => [''], _err => [[ 'board', 'required', 1 ]] },
+
+  { param => 'board', multi => 1, min => 1 },
+  [ board => 0 ],
+  { board => [0], _err => [[ 'board', 'min', 1 ]] },
+
+  { param => 'board', multi => 1, maxcount => 1 },
+  [ board => 1 ],
+  { board => [1] },
+
+  { param => 'board', multi => 1, maxcount => 1 },
+  [ board => 1, board => 2 ],
+  { board => [1,2], _err => [[ 'board', 'maxcount', 1 ]] },
+
+  { param => 'board', multi => 1, mincount => 1 },
+  [ board => 1 ],
+  { board => [1] },
+
+  { param => 'board', multi => 1, mincount => 2 },
+  [ board => 1 ],
+  { board => [1], _err => [[ 'board', 'mincount', 2 ]] },
+
+  # regex
+  do { my $r = qr/^[0-9a-f]{3}$/i; (
+  { param => 'hex', regex => $r },
+  [ hex => '0F3' ],
+  { hex => '0F3' },
+
+  { param => 'hex', regex => $r },
+  [ hex => '0134' ],
+  { hex => '0134', _err => [[ 'hex', 'regex', $r ]] },
+
+  { param => 'hex', regex => $r },
+  [ hex => '03X' ],
+  { hex => '03X', _err => [[ 'hex', 'regex', $r ]] },
+
+  { param => 'hex', regex => [$r, 1,2,3] },
+  [ hex => '03X' ],
+  { hex => '03X', _err => [[ 'hex', 'regex', [$r, 1,2,3] ]] },
+  )},
+
+  # func
+  do { my $f = sub { $_[0] =~ y/a-z/A-Z/; $_[0] =~ /^X/ }; (
+  { param => 't', func => $f },
+  [ t => 'xyz' ],
+  { t => 'XYZ' },
+
+  { param => 't', func => $f },
+  [ t => 'zyx' ],
+  { t => 'ZYX', _err => [[ 't', 'func', $f ]] },
+
+  { param => 't', func => [$f,1,2,3] },
+  [ t => 'zyx' ],
+  { t => 'ZYX', _err => [[ 't', 'func', [$f,1,2,3] ]] },
+  )},
+
+  # template
+  do {
+    $templates{hex} = { regex => qr/^[0-9a-f]+$/i };
+    $templates{crc32} = { template => 'hex', minlength => 8, maxlength => 8 };
+  ()},
+  { param => 'crc', template => 'hex' },
+  [ crc => '12345678' ],
+  { crc => '12345678' },
+  
+  { param => 'crc', template => 'crc32' },
+  [ crc => '12345678' ],
+  { crc => '12345678' },
+
+  { param => 'crc', template => 'hex' },
+  [ crc => '12x45678' ],
+  { crc => '12x45678', _err => [[ 'crc', 'template', 'hex' ]] },
+
+  { param => 'crc', template => 'crc32' },
+  [ crc => '123456789' ],
+  { crc => '123456789', _err => [[ 'crc', 'template', 'crc32' ]] },
+)}
+
+use Test::More tests => 1+@tests/3;
+
+BEGIN { use_ok('TUWF::Misc', 'kv_validate') };
+
+sub getfield {
+  my($n, $f) = @_;
+  map +($f->[$_*2] eq $n ? $f->[$_*2+1] : ()), @$f ? 0..$#$f/2 : ();
+}
+
+for my $i (0..$#tests/3) {
+  my($fields, $params, $exp) = ($tests[$i*3], $tests[$i*3+1], $tests[$i*3+2]);
+  is_deeply(kv_validate({ param => sub { getfield($_[0], $params) } }, \%templates, [$fields]), $exp, 'Test '.($i+1));
+}