Revert "Drop OpenQA::WebAPI::Plugin::HashedParams"

os-autoinst · Oct 9, 2024 · 754c5cc · 754c5cc
1 parent 617fbab
commit 754c5cc
Show file tree

Hide file tree

Showing 11 changed files with 182 additions and 109 deletions.
diff --git a/lib/OpenQA/Setup.pm b/lib/OpenQA/Setup.pm
@@ -363,7 +363,7 @@ sub setup_mojo_tmpdir () {
 
 sub load_plugins ($server, $monitoring_root_route = undef, %options) {
     push @{$server->plugins->namespaces}, 'OpenQA::WebAPI::Plugin';
-    $server->plugin($_) for qw(Helpers MIMETypes CSRF REST Gru YAML);
+    $server->plugin($_) for qw(Helpers MIMETypes CSRF REST HashedParams Gru YAML);
     $server->plugin('AuditLog') if $server->config->{global}{audit_enabled};
     # Load arbitrary plugins defined in config: 'plugins' in section
     # '[global]' can be a space-separated list of plugins to load, by

diff --git a/lib/OpenQA/WebAPI/Controller/API/V1/Table.pm b/lib/OpenQA/WebAPI/Controller/API/V1/Table.pm
@@ -354,7 +354,10 @@ sub _prepare_settings {
     my ($self, $table, $entry) = @_;
     my $validation = $self->validation;
     my $hp;
-    # accept modern application/json encoded hashes
+    # accept both traditional application/x-www-form-urlencoded parameters
+    # with hash entries having key names encoded like settings[value1]
+    # (see doc at the end of HashedParams.pm)
+    # as well as modern application/json encoded hashes
     my $error;
     if ($self->req->headers->content_type =~ /^application\/json/) {
         try {
@@ -374,7 +377,7 @@ sub _prepare_settings {
         $validation->input($hp);
     }
     else {
-        return 'Invalid request Content-Type ' . $self->req->headers->content_type . '. Expecting application/json.';
+        $hp = $self->hparams();
     }
 
     for my $par (@{$TABLES{$table}->{required}}) {
@@ -394,8 +397,8 @@ sub _prepare_settings {
     my @keys;
     if ($hp->{settings}) {
         for my $k (keys %{$hp->{settings}}) {
-            my $value = trim $hp->{settings}->{$k};
             $k = trim $k;
+            my $value = trim $hp->{settings}->{$k};
             $k =~ s/[^\]\[0-9a-zA-Z_\+]//g;
             push @settings, {key => $k, value => $value};
             push @keys, $k;

diff --git a/lib/OpenQA/WebAPI/Plugin/HashedParams.pm b/lib/OpenQA/WebAPI/Plugin/HashedParams.pm
@@ -0,0 +1,111 @@
+package OpenQA::WebAPI::Plugin::HashedParams;
+use Mojo::Base 'Mojolicious::Plugin';
+
+our $VERSION = '0.04';
+
+sub register {
+    my ($plugin, $app) = @_;
+
+    $app->helper(
+        hparams => sub {
+            my ($self, @permit) = @_;
+
+            if (!$self->stash('hparams')) {
+                my $hprms = $self->req->params->to_hash;
+                my $index = 0;
+                my @array;
+
+                foreach my $p (keys %$hprms) {
+                    my $key = $p;
+                    my $val = $hprms->{$p};
+                    $val =~ s/\\/\\\\/g;
+                    $val =~ s/\'/\\\'/g;
+
+                    $key =~ s/[^\]\[0-9a-zA-Z_\+]//g;
+                    $key =~ s/\[{2,}/\[/g;
+                    $key =~ s/\]{2,}/\]/g;
+                    $key =~ s/\\//g;
+                    $key =~ s/\'//g;
+
+                    my @list;
+                    foreach my $n (split /[\[\]]/, $key) {
+                        push @list, $n if length($n) > 0;
+                    }
+
+                    map $array[$index] .= "{'$list[$_]'}", 0 .. $#list;
+
+                    $array[$index] .= " = '$val';";
+                    $index++;
+                }
+
+                my $code = 'my $h = {};';
+                map { $code .= "\$h->$_" } @array;
+                $code .= '$h;';
+
+                my $ret = eval $code;
+
+                if ($@) {
+                    $self->stash(hparams => {});
+                    $self->stash(hparams_error => $@);
+                    return $self->stash('hparams');
+                }
+
+                if (keys %$ret) {
+                    if (@permit) {
+                        foreach my $k (keys %$ret) {
+                            delete $ret->{$k} if grep(/\Q$k/, @permit);
+                        }
+                    }
+
+                    $self->stash(hparams => $ret);
+                }
+            }
+            else {
+                $self->stash(hparams => {});
+            }
+            return $self->stash('hparams');
+        });
+}
+
+1;
+
+__END__
+
+=encoding utf8
+
+=head1 NAME
+
+Mojolicious::Plugin::HashedParams - Transformation request parameters into a hash and multi-hash
+
+=head1 SYNOPSIS
+
+  plugin 'HashedParams';
+
+  # Transmit params:
+  /route?message[body]=PerlOrDie&message[task][id]=32
+    or
+  <input type="text" name="message[task][id]" value="32"> 
+
+  get '/route' => sub {
+    my $self = shift;
+    # you can also use permit parameters
+    $self->hparams( qw(message) );
+    # return all parameters in the hash
+    $self->hparams();
+  };
+
+=head1 AUTHOR
+
+Grishkovelli L<[email protected]>
+
+=head1 Git
+
+L<https://github.com/grishkovelli/Mojolicious-Plugin-HashedParams>
+
+=head1 COPYRIGHT
+
+Copyright 2013, Grishkovelli.
+
+This library is free software; you can redistribute it and/or modify it under the same terms as Perl itself.
+
+=cut
diff --git a/script/openqa-load-templates b/script/openqa-load-templates
@@ -166,9 +166,8 @@ sub post_entry ($table, $entry) {
             $param{version} = $entry->{product}{version};
         }
         elsif ($key eq 'settings' && ref($entry->{settings}) eq "ARRAY") {
-            $param{settings} = {};
             for my $var (@{$entry->{settings}}) {
-                $param{settings}{$var->{key}} = $var->{value};
+                $param{'settings[' . $var->{key} . ']'} = $var->{value};
             }
         }
         else {
@@ -184,7 +183,7 @@ sub post_entry ($table, $entry) {
             {    # there is nothing to update in JobTemplates, the entry just exists or not
                 my $id = $res->json->{$table}[0]{id};
                 my $table_url_id = $url->clone->path($options{apibase} . '/' . decamelize($table) . "/$id");
-                my $res = $client->put($table_url_id, json => \%param)->res;
+                my $res = $client->put($table_url_id, form => \%param)->res;
                 print_error $res unless $res->is_success;
                 return 1;
             }
@@ -194,7 +193,7 @@ sub post_entry ($table, $entry) {
         }
     }
 
-    my $res = $client->post($table_url, json => \%param)->res;
+    my $res = $client->post($table_url, form => \%param)->res;
     print_error $res unless $res->is_success;
     return 1;
 }

diff --git a/t/40-script_load_dump_templates.t b/t/40-script_load_dump_templates.t
@@ -11,8 +11,6 @@ require OpenQA::Test::Database;
 use OpenQA::Test::Utils;
 use Test::Output;
 use Test::Warnings ':report_warnings';
-use Test::Mojo;
-use OpenQA::Test::Client 'client';
 use OpenQA::Test::TimeLimit '30';
 use OpenQA::Test::Utils qw(run_cmd test_cmd stop_service);
 use Mojo::JSON;    # booleans
@@ -62,33 +60,6 @@ my $expected = qr/JobGroups.+=> \{ added => 1, of => 1 \}/;
 test_once $args, $expected, 'Admin may load templates', 0, 'successfully loaded templates';
 test_once $args, qr/group with existing name/, 'Duplicate job group', 255, 'failed on duplicate job group';
 
-subtest 'test changing existing entries' => sub {
-    # delete job group so that we can load the template again without running into duplicate job group error
-    my $t = client(Test::Mojo->new(), apikey => $apikey, apisecret => $apisecret);
-    $t->get_ok("http://$host/api/v1/job_groups")->status_is(200);
-    my $jobgroup_id = (grep { $_->{name} eq 'openSUSE Leap 42.3 Updates' } @{$t->tx->res->json})[0]->{id};
-    $t->delete_ok("http://$host/api/v1/job_groups/$jobgroup_id")->status_is(200);
-    $t->get_ok("http://$host/api/v1/test_suites?name=uefi")->status_is(200);
-    my $test_suite_id = $t->tx->res->json->{TestSuites}->[0]->{id};
-
-    # overwrite testsuite settings
-    $t->put_ok("http://$host/api/v1/test_suites/$test_suite_id", json => {name => "uefi", settings => {UEFI => '42'}})
-      ->status_is(200);
-
-    # check overwriting testsuite settings
-    $t->get_ok("http://$host/api/v1/test_suites/$test_suite_id")->status_is(200)
-      ->json_is('/TestSuites/0/settings/0/value', '42');
-
-    # change testsuite settings back by reimporting template
-    $args = "$base_args --update $filename";
-    test_once $args, $expected, 'Admin may load templates', 0, 'successfully loaded templates with update flag';
-
-    # check overwriting testsuite settings
-    $t->get_ok("http://$host/api/v1/test_suites/$test_suite_id")->status_is(200);
-    is((grep { $_->{key} eq 'UEFI' } @{$t->tx->res->json->{TestSuites}->[0]->{settings}})[0]->{value},
-        '1', 'value changed back during update');
-};
-
 my $fh;
 my $tempfilename;
 ($fh, $tempfilename) = tempfile(UNLINK => 1, SUFFIX => '.json');

diff --git a/t/api/02-iso.t b/t/api/02-iso.t
@@ -563,8 +563,8 @@ for my $machine_separator (qw(@ :)) {
     $schema->txn_begin;
     subtest "Create dependency for jobs on different machines"
       . " - dependency setting are correct (using machine separator '$machine_separator')" => sub {
-        $t->post_ok('/api/v1/machines',
-            json => {name => '64bit-ipmi', backend => 'ipmi', settings => {'TEST' => 'ipmi'}})->status_is(200);
+        $t->post_ok('/api/v1/machines', form => {name => '64bit-ipmi', backend => 'ipmi', 'settings[TEST]' => 'ipmi'})
+          ->status_is(200);
         add_opensuse_test('supportserver1');
         add_opensuse_test('supportserver2', MACHINE => ['64bit-ipmi']);
         add_opensuse_test(
@@ -615,7 +615,7 @@ for my $machine_separator (qw(@ :)) {
 
 subtest 'Create dependency for jobs on different machines - best match and log error dependency' => sub {
     $schema->txn_begin;
-    $t->post_ok('/api/v1/machines', json => {name => 'powerpc', backend => 'qemu', settings => {'TEST' => 'power'}})
+    $t->post_ok('/api/v1/machines', form => {name => 'powerpc', backend => 'qemu', 'settings[TEST]' => 'power'})
       ->status_is(200);
 
     add_opensuse_test('install_ltp', MACHINE => ['powerpc']);
@@ -669,7 +669,7 @@ subtest 'Create dependency for jobs on different machines - log error parents' =
     $schema->txn_begin;
     my @machines = qw(ppc ppc-6G ppc-1G ppc-2G s390x);
     for my $m (@machines) {
-        $t->post_ok('/api/v1/machines', json => {name => $m, backend => 'qemu', settings => {'TEST' => 'test'}})
+        $t->post_ok('/api/v1/machines', form => {name => $m, backend => 'qemu', 'settings[TEST]' => 'test'})
           ->status_is(200);
     }
     add_opensuse_test('supportserver', MACHINE => ['ppc', '64bit', 's390x']);

diff --git a/t/api/05-machines.t b/t/api/05-machines.t
@@ -59,14 +59,14 @@ is_deeply(
 ) || diag explain $t->tx->res->json;
 
 
-$t->post_ok('/api/v1/machines', json => {name => "testmachine"})->status_is(400)
+$t->post_ok('/api/v1/machines', form => {name => "testmachine"})->status_is(400)
   ->json_is('/error', 'Missing parameter: backend');
-$t->post_ok('/api/v1/machines', json => {backend => "kde/usb"})->status_is(400)
+$t->post_ok('/api/v1/machines', form => {backend => "kde/usb"})->status_is(400)
   ->json_is('/error', 'Missing parameter: name');
-$t->post_ok('/api/v1/machines', json => {})->status_is(400)->json_is('/error', 'Missing parameter: backend, name');
+$t->post_ok('/api/v1/machines', form => {})->status_is(400)->json_is('/error', 'Missing parameter: backend, name');
 
 $t->post_ok('/api/v1/machines',
-    json => {name => "testmachine", backend => "qemu", "settings" => {"TEST" => "val1", "TEST2" => "val1"}})
+    form => {name => "testmachine", backend => "qemu", "settings[TEST]" => "val1", "settings[TEST2]" => "val1"})
   ->status_is(200);
 my $machine_id = $t->tx->res->json->{id};
 my $event = OpenQA::Test::Case::find_most_recent_event($t->app->schema, 'table_create');
@@ -80,19 +80,19 @@ $t->get_ok('/api/v1/machines', form => {name => "testmachine"})->status_is(200);
 is($t->tx->res->json->{Machines}->[0]->{id}, $machine_id);
 
 $t->post_ok('/api/v1/machines',
-    json => {name => "testmachineQ", backend => "qemu", "settings" => {"TEST" => "'v'al1", "TEST2" => "va'l\'1"}})
+    form => {name => "testmachineQ", backend => "qemu", "settings[TEST]" => "'v'al1", "settings[TEST2]" => "va'l\'1"})
   ->status_is(200);
 $t->get_ok('/api/v1/machines', form => {name => "testmachineQ"})->status_is(200);
 is($t->tx->res->json->{Machines}->[0]->{settings}->[0]->{value}, "'v'al1");
 is($t->tx->res->json->{Machines}->[0]->{settings}->[1]->{value}, "va'l\'1");
 
-$t->post_ok('/api/v1/machines',
-    json => {name => "testmachineZ", backend => "qemu", "settings" => {"TE'S\'T" => "'v'al1"}})->status_is(200);
+$t->post_ok('/api/v1/machines', form => {name => "testmachineZ", backend => "qemu", "settings[TE'S\'T]" => "'v'al1"})
+  ->status_is(200);
 $t->get_ok('/api/v1/machines', form => {name => "testmachineQ"})->status_is(200);
 is($t->tx->res->json->{Machines}->[0]->{settings}->[0]->{key}, "TEST");
 is($t->tx->res->json->{Machines}->[0]->{settings}->[0]->{value}, "'v'al1");
 
-$t->post_ok('/api/v1/machines', json => {name => "testmachine", backend => "qemu"})->status_is(400);    #already exists
+$t->post_ok('/api/v1/machines', form => {name => "testmachine", backend => "qemu"})->status_is(400);    #already exists
 
 $t->get_ok("/api/v1/machines/$machine_id")->status_is(200);
 is_deeply(
@@ -117,7 +117,7 @@ is_deeply(
 ) || diag explain $t->tx->res->json;
 
 $t->put_ok("/api/v1/machines/$machine_id",
-    json => {name => "testmachine", backend => "qemu", settings => {"TEST2" => "val1"}})->status_is(200);
+    form => {name => "testmachine", backend => "qemu", "settings[TEST2]" => "val1"})->status_is(200);
 
 $t->get_ok("/api/v1/machines/$machine_id")->status_is(200);
 is_deeply(
@@ -143,9 +143,6 @@ $t->put_ok("/api/v1/machines/$machine_id", json => {name => "testmachine", "sett
 $t->put_ok("/api/v1/machines/$machine_id", => {'Content-Type' => 'application/json'} => '{BROKEN JSON')->status_is(400)
   ->json_like('/error', qr/expected, at character offset/);
 
-$t->put_ok("/api/v1/machines/$machine_id", => {'Content-Type' => 'text/html'})->status_is(400)
-  ->json_like('/error', qr/Invalid request Content-Type/);
-
 $t->put_ok("/api/v1/machines/$machine_id",
     json => {name => "testmachine", backend => "qemu", "settings" => {"TEST2" => "val2"}})->status_is(200);
 
@@ -173,13 +170,12 @@ $t->delete_ok("/api/v1/machines/$machine_id")->status_is(404);    #not found
 subtest 'trim whitespace characters' => sub {
     $t->post_ok(
         '/api/v1/machines',
-        json => {
+        form => {
             name => " create_with_space ",
             backend => " qemu ",
-            settings => {
-                " TEST " => " test value  ",
-                "TEST2  " => " test value2  "
-            }})->status_is(200);
+            "settings[ TEST ]" => " test value  ",
+            "settings[TEST2  ]" => " test value2  ",
+        })->status_is(200);
     my $id = $t->tx->res->json->{id};
     $t->get_ok("/api/v1/machines/$id")->status_is(200);
     $t->json_is(
@@ -204,13 +200,12 @@ subtest 'trim whitespace characters' => sub {
 
     $t->put_ok(
         "/api/v1/machines/$id",
-        json => {
+        form => {
             name => "  update_with_space ",
             backend => "qemu ",
-            settings => {
-                " TEST " => " new test value  ",
-                " TEST3" => "  new test value3 "
-            }})->status_is(200);
+            "settings[ TEST ]" => " new test value  ",
+            "settings[ TEST3]" => "  new test value3 ",
+        })->status_is(200);
     $t->get_ok("/api/v1/machines/$id")->status_is(200);
     $t->json_is(
         '' => {
@@ -236,10 +231,10 @@ subtest 'trim whitespace characters' => sub {
 # switch to operator (default client) and try some modifications
 client($t);
 $t->post_ok('/api/v1/machines',
-    json => {name => "testmachine", backend => "qemu", "settings" => {"TEST" => "val1", "TEST2" => "val1"}})
+    form => {name => "testmachine", backend => "qemu", "settings[TEST]" => "val1", "settings[TEST2]" => "val1"})
   ->status_is(403);
 $t->put_ok("/api/v1/machines/$machine_id",
-    json => {name => "testmachine", backend => "qemu", "settings" => {"TEST2" => "val1"}})->status_is(403);
+    form => {name => "testmachine", backend => "qemu", "settings[TEST2]" => "val1"})->status_is(403);
 $t->delete_ok("/api/v1/machines/$machine_id")->status_is(403);
 
 subtest 'server-side limit has precedence over user-specified limit' => sub {