fixed a few merge bugs in Mojo::Parameters

mojolicious · Jan 1, 2015 · bcc7c48 · bcc7c48
1 parent e131ce2
commit bcc7c48
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 38 deletions.
diff --git a/Changes b/Changes
@@ -5,6 +5,7 @@
   - Improved Mojo::URL performance.
   - Fixed fragment and userinfo normalization bugs in Mojo::URL.
   - Fixed query charset bug in Mojo::URL.
+  - Fixed a few merge bugs in Mojo::Parameters.
 
 5.70  2014-12-18
   - Improved Mojo::Template performance.

diff --git a/lib/Mojo/Message/Request.pm b/lib/Mojo/Message/Request.pm
@@ -133,7 +133,7 @@ sub param { shift->params->param(@_) }
 sub params {
   my $self = shift;
   return $self->{params}
-    ||= $self->body_params->clone->merge($self->query_params);
+    ||= $self->body_params->clone->append($self->query_params);
 }
 
 sub parse {

diff --git a/lib/Mojo/Parameters.pm b/lib/Mojo/Parameters.pm
@@ -14,13 +14,14 @@ sub append {
   my $self = shift;
 
   my $params = $self->params;
-  while (my ($name, $value) = splice @_, 0, 2) {
-
-    # Single value
-    if (ref $value ne 'ARRAY') { push @$params, $name => $value }
+  my @pairs = @_ == 1 ? @{shift->params} : @_;
+  while (my ($name, $value) = splice @pairs, 0, 2) {
 
     # Multiple values
-    else { push @$params, $name => (defined $_ ? "$_" : '') for @$value }
+    if (ref $value eq 'ARRAY') { push @$params, $name => $_ // '' for @$value }
+
+    # Single value
+    else { push @$params, $name => $value }
   }
 
   return $self;
@@ -41,7 +42,12 @@ sub every_param { shift->_param(@_) }
 
 sub merge {
   my $self = shift;
-  push @{$self->params}, @{$_->params} for @_;
+
+  my @pairs = @_ == 1 ? @{shift->params} : @_;
+  while (my ($name, $value) = splice @pairs, 0, 2) {
+    defined $value ? $self->param($name => $value) : $self->remove($name);
+  }
+
   return $self;
 }
 
@@ -234,9 +240,13 @@ following new ones.
   $params = $params->append(foo => 'ba&r');
   $params = $params->append(foo => ['ba&r', 'baz']);
   $params = $params->append(foo => ['bar', 'baz'], bar => 23);
+  $params = $params->append(Mojo::Parameters->new);
 
 Append parameters. Note that this method will normalize the parameters.
 
+  # "foo=bar&foo=baz"
+  Mojo::Parameters->new('foo=bar')->append(Mojo::Parameters->new('foo=baz'));
+
   # "foo=bar&foo=baz"
   Mojo::Parameters->new('foo=bar')->append(foo => 'baz');
 
@@ -264,14 +274,22 @@ array reference. Note that this method will normalize the parameters.
 
 =head2 merge
 
-  $params = $params->merge(Mojo::Parameters->new(foo => 'b&ar', baz => 23));
+  $params = $params->merge(foo => 'ba&r');
+  $params = $params->merge(foo => ['ba&r', 'baz']);
+  $params = $params->merge(foo => ['bar', 'baz'], bar => 23);
+  $params = $params->merge(Mojo::Parameters->new);
 
-Merge L<Mojo::Parameters> objects. Note that this method will normalize the
-parameters.
+Merge parameters. Note that this method will normalize the parameters.
 
-  # "foo=bar&foo=baz"
+  # "foo=baz"
   Mojo::Parameters->new('foo=bar')->merge(Mojo::Parameters->new('foo=baz'));
 
+  # "yada=yada&foo=baz"
+  Mojo::Parameters->new('foo=bar&yada=yada')->merge(foo => 'baz');
+
+  # "yada=yada"
+  Mojo::Parameters->new('foo=bar&yada=yada')->merge(foo => undef);
+
 =head2 new
 
   my $params = Mojo::Parameters->new;

diff --git a/lib/Mojo/Path.pm b/lib/Mojo/Path.pm
@@ -217,7 +217,7 @@ that C<%2F> will be treated as C</> for security reasons.
 
   $path = $path->merge('/foo/bar');
   $path = $path->merge('foo/bar');
-  $path = $path->merge(Mojo::Path->new('foo/bar'));
+  $path = $path->merge(Mojo::Path->new);
 
 Merge paths. Note that this method will normalize both paths if necessary and
 that C<%2F> will be treated as C</> for security reasons.

diff --git a/lib/Mojo/URL.pm b/lib/Mojo/URL.pm
@@ -117,12 +117,7 @@ sub query {
   if (@_ > 1) { $q->params([])->parse(@_) }
 
   # Merge with array
-  elsif (ref $_[0] eq 'ARRAY') {
-    while (my $name = shift @{$_[0]}) {
-      my $value = shift @{$_[0]};
-      defined $value ? $q->param($name => $value) : $q->remove($name);
-    }
-  }
+  elsif (ref $_[0] eq 'ARRAY') { $q->merge(@{$_[0]}) }
 
   # Append hash
   elsif (ref $_[0] eq 'HASH') { $q->append(%{$_[0]}) }
@@ -378,6 +373,9 @@ defaults to a L<Mojo::Path> object.
   # "perldoc"
   Mojo::URL->new('http://example.com/perldoc/Mojo')->path->parts->[0];
 
+  # "/perldoc/DOM/HTML"
+  Mojo::URL->new('http://example.com/perldoc/Mojo')->path->merge('DOM/HTML');
+
   # "http://example.com/DOM/HTML"
   Mojo::URL->new('http://example.com/perldoc/Mojo')->path('/DOM/HTML');
 
@@ -417,12 +415,15 @@ Normalized version of L</"scheme">.
   $url      = $url->query('a=1&b=2');
   $url      = $url->query(Mojo::Parameters->new);
 
-Query part of this URL, pairs in an array will be merged and pairs in a hash
-appended, defaults to a L<Mojo::Parameters> object.
+Query part of this URL, pairs in an array reference will be merged and pairs
+in a hash reference appended, defaults to a L<Mojo::Parameters> object.
 
   # "2"
   Mojo::URL->new('http://example.com?a=1&b=2')->query->param('b');
 
+  # "a=2&b=2&c=3"
+  Mojo::URL->new('http://example.com?a=1&b=2')->query->merge(a => 2, c => 3);
+
   # "http://example.com?a=2&c=3"
   Mojo::URL->new('http://example.com?a=1&b=2')->query(a => 2, c => 3);
 

diff --git a/t/mojo/parameters.t b/t/mojo/parameters.t
@@ -10,14 +10,13 @@ is $params->to_string,  'foo=b%3Bar&baz=23', 'right format';
 is $params2->to_string, 'x=1&y=2',           'right format';
 is $params->to_string,  'foo=b%3Bar&baz=23', 'right format';
 is_deeply $params->params, ['foo', 'b;ar', 'baz', 23], 'right structure';
-
-# Append
-is_deeply $params->params, ['foo', 'b;ar', 'baz', 23], 'right structure';
 $params->append(a => 4, a => 5, b => 6, b => 7);
 is $params->to_string, 'foo=b%3Bar&baz=23&a=4&a=5&b=6&b=7', 'right format';
 push @$params, c => 'f;oo';
 is $params->to_string, 'foo=b%3Bar&baz=23&a=4&a=5&b=6&b=7&c=f%3Boo',
   'right format';
+is $params->remove('a')->to_string, 'foo=b%3Bar&baz=23&b=6&b=7&c=f%3Boo',
+  'right format';
 
 # Clone
 my $clone = $params->clone;
@@ -26,12 +25,21 @@ push @$clone, c => 9;
 isnt "$params", "$clone", 'unequal parameters';
 
 # Merge
+$params = Mojo::Parameters->new('foo=b%3Bar&baz=23&a=4&a=5&b=6&b=7&c=f%3Boo');
 $params->merge($params2);
 is $params->to_string, 'foo=b%3Bar&baz=23&a=4&a=5&b=6&b=7&c=f%3Boo&x=1&y=2',
   'right format';
 is $params2->to_string, 'x=1&y=2', 'right format';
+is $params->merge(baz => undef)->to_string,
+  'foo=b%3Bar&a=4&a=5&b=6&b=7&c=f%3Boo&x=1&y=2', 'right format';
+is $params->merge(y => 3, z => [4, 5])->to_string,
+  'foo=b%3Bar&a=4&a=5&b=6&b=7&c=f%3Boo&x=1&y=3&z=4&z=5', 'right format';
+is $params->merge(Mojo::Parameters->new(z => 6))->to_string,
+  'foo=b%3Bar&a=4&a=5&b=6&b=7&c=f%3Boo&x=1&y=3&z=6', 'right format';
 
 # Param
+$params
+  = Mojo::Parameters->new('foo=b%3Bar&a=4&a=5&b=6&b=7&c=f%3Boo&x=1&y=3&z=6');
 is_deeply $params->param('foo'), 'b;ar', 'right structure';
 is_deeply $params->every_param('foo'), ['b;ar'], 'right structure';
 is_deeply $params->every_param('a'), [4, 5], 'right structure';
@@ -41,27 +49,21 @@ $params->param(foo => 'bar');
 is_deeply [$params->param('foo')], ['bar'], 'right structure';
 is_deeply $params->param(foo => qw(baz yada))->every_param('foo'),
   [qw(baz yada)], 'right structure';
-
-# Remove
-$params->parse('q=1&w=2&e=3&e=4&r=6&t=7');
-is $params->remove('r')->to_string, 'q=1&w=2&e=3&e=4&t=7', 'right format';
-$params->remove('e');
-is $params->to_string, 'q=1&w=2&t=7', 'right format';
-
-# Hash
-is_deeply $params->to_hash, {q => 1, w => 2, t => 7}, 'right structure';
-
-# List names
-is_deeply [$params->param], [qw(q t w)], 'right structure';
+is_deeply [$params->param], [qw(a b c foo x y z)], 'right structure';
 
 # Append
-$params->append('a', 4, 'a', 5, 'b', 6, 'b', 7);
-is_deeply $params->to_hash,
-  {a => [4, 5], b => [6, 7], q => 1, w => 2, t => 7}, 'right structure';
+$params = Mojo::Parameters->new('q=1');
+$params->append(a => 4, a => 5, b => 6, b => 7);
+is_deeply $params->to_hash, {a => [4, 5], b => [6, 7], q => 1},
+  'right structure';
+is_deeply [$params->param], [qw(a b q)], 'right structure';
 $params = Mojo::Parameters->new(foo => '', bar => 'bar');
 is $params->to_string, 'foo=&bar=bar', 'right format';
 $params = Mojo::Parameters->new(bar => 'bar', foo => '');
 is $params->to_string, 'bar=bar&foo=', 'right format';
+is $params->append($params2)->to_string, 'bar=bar&foo=&x=1&y=2',
+  'right format';
+is $params2->to_string, 'x=1&y=2', 'right format';
 
 # "0"
 $params = Mojo::Parameters->new(foo => 0);