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

os-autoinst · Oct 9, 2024 · 279401f · 279401f
1 parent 00e5ba5
commit 279401f
Show file tree

Hide file tree

Showing 9 changed files with 76 additions and 179 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 HashedParams Gru YAML);
+    $server->plugin($_) for qw(Helpers MIMETypes CSRF REST 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,10 +354,7 @@ sub _prepare_settings {
     my ($self, $table, $entry) = @_;
     my $validation = $self->validation;
     my $hp;
-    # 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
+    # accept modern application/json encoded hashes
     my $error;
     if ($self->req->headers->content_type =~ /^application\/json/) {
         try {
@@ -377,7 +374,7 @@ sub _prepare_settings {
         $validation->input($hp);
     }
     else {
-        $hp = $self->hparams();
+        return 'Invalid request Content-Type ' . $self->req->headers->content_type . '. Expecting application/json.';
     }
 
     for my $par (@{$TABLES{$table}->{required}}) {
@@ -397,8 +394,8 @@ sub _prepare_settings {
     my @keys;
     if ($hp->{settings}) {
         for my $k (keys %{$hp->{settings}}) {
-            $k = trim $k;
             my $value = trim $hp->{settings}->{$k};
+            $k = trim $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
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', form => {name => '64bit-ipmi', backend => 'ipmi', 'settings[TEST]' => 'ipmi'})
-          ->status_is(200);
+        $t->post_ok('/api/v1/machines',
+            json => {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', form => {name => 'powerpc', backend => 'qemu', 'settings[TEST]' => 'power'})
+    $t->post_ok('/api/v1/machines', json => {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', form => {name => $m, backend => 'qemu', 'settings[TEST]' => 'test'})
+        $t->post_ok('/api/v1/machines', json => {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', form => {name => "testmachine"})->status_is(400)
+$t->post_ok('/api/v1/machines', json => {name => "testmachine"})->status_is(400)
   ->json_is('/error', 'Missing parameter: backend');
-$t->post_ok('/api/v1/machines', form => {backend => "kde/usb"})->status_is(400)
+$t->post_ok('/api/v1/machines', json => {backend => "kde/usb"})->status_is(400)
   ->json_is('/error', 'Missing parameter: 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 => {})->status_is(400)->json_is('/error', 'Missing parameter: backend, name');
 
 $t->post_ok('/api/v1/machines',
-    form => {name => "testmachine", backend => "qemu", "settings[TEST]" => "val1", "settings[TEST2]" => "val1"})
+    json => {name => "testmachine", backend => "qemu", "settings" => {"TEST" => "val1", "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',
-    form => {name => "testmachineQ", backend => "qemu", "settings[TEST]" => "'v'al1", "settings[TEST2]" => "va'l\'1"})
+    json => {name => "testmachineQ", backend => "qemu", "settings" => {"TEST" => "'v'al1", "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', form => {name => "testmachineZ", backend => "qemu", "settings[TE'S\'T]" => "'v'al1"})
-  ->status_is(200);
+$t->post_ok('/api/v1/machines',
+    json => {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', form => {name => "testmachine", backend => "qemu"})->status_is(400);    #already exists
+$t->post_ok('/api/v1/machines', json => {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",
-    form => {name => "testmachine", backend => "qemu", "settings[TEST2]" => "val1"})->status_is(200);
+    json => {name => "testmachine", backend => "qemu", settings => {"TEST2" => "val1"}})->status_is(200);
 
 $t->get_ok("/api/v1/machines/$machine_id")->status_is(200);
 is_deeply(
@@ -143,6 +143,9 @@ $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);
 
@@ -170,12 +173,13 @@ $t->delete_ok("/api/v1/machines/$machine_id")->status_is(404);    #not found
 subtest 'trim whitespace characters' => sub {
     $t->post_ok(
         '/api/v1/machines',
-        form => {
+        json => {
             name => " create_with_space ",
             backend => " qemu ",
-            "settings[ TEST ]" => " test value  ",
-            "settings[TEST2  ]" => " test value2  ",
-        })->status_is(200);
+            settings => {
+                " TEST " => " test value  ",
+                "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(
@@ -200,12 +204,13 @@ subtest 'trim whitespace characters' => sub {
 
     $t->put_ok(
         "/api/v1/machines/$id",
-        form => {
+        json => {
             name => "  update_with_space ",
             backend => "qemu ",
-            "settings[ TEST ]" => " new test value  ",
-            "settings[ TEST3]" => "  new test value3 ",
-        })->status_is(200);
+            settings => {
+                " TEST " => " new test value  ",
+                " TEST3" => "  new test value3 "
+            }})->status_is(200);
     $t->get_ok("/api/v1/machines/$id")->status_is(200);
     $t->json_is(
         '' => {
@@ -231,10 +236,10 @@ subtest 'trim whitespace characters' => sub {
 # switch to operator (default client) and try some modifications
 client($t);
 $t->post_ok('/api/v1/machines',
-    form => {name => "testmachine", backend => "qemu", "settings[TEST]" => "val1", "settings[TEST2]" => "val1"})
+    json => {name => "testmachine", backend => "qemu", "settings" => {"TEST" => "val1", "TEST2" => "val1"}})
   ->status_is(403);
 $t->put_ok("/api/v1/machines/$machine_id",
-    form => {name => "testmachine", backend => "qemu", "settings[TEST2]" => "val1"})->status_is(403);
+    json => {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 {

diff --git a/t/api/06-products.t b/t/api/06-products.t
@@ -60,27 +60,28 @@ is_deeply(
 
 
 # no arch
-$t->post_ok('/api/v1/products', form => {distri => "opensuse", flavor => "DVD", version => 13.2})->status_is(400);
+$t->post_ok('/api/v1/products', json => {distri => "opensuse", flavor => "DVD", version => 13.2})->status_is(400);
 
 # no distri
-$t->post_ok('/api/v1/products', form => {arch => "x86_64", flavor => "DVD", version => 13.2})->status_is(400);
+$t->post_ok('/api/v1/products', json => {arch => "x86_64", flavor => "DVD", version => 13.2})->status_is(400);
 
 # no flavor
-$t->post_ok('/api/v1/products', form => {arch => "x86_64", distri => "opensuse", version => 13.2})->status_is(400);
+$t->post_ok('/api/v1/products', json => {arch => "x86_64", distri => "opensuse", version => 13.2})->status_is(400);
 
 # no version
-$t->post_ok('/api/v1/products', form => {arch => "x86_64", distri => "opensuse", flavor => "DVD"})->status_is(400);
+$t->post_ok('/api/v1/products', json => {arch => "x86_64", distri => "opensuse", flavor => "DVD"})->status_is(400);
 
 $t->post_ok(
     '/api/v1/products',
-    form => {
+    json => {
         arch => "x86_64",
         distri => "opensuse",
         flavor => "DVD",
         version => 13.2,
-        "settings[TEST]" => "val1",
-        "settings[TEST2]" => "val1"
-    })->status_is(200);
+        "settings" => {
+            "TEST" => "val1",
+            "TEST2" => "val1"
+        }})->status_is(200);
 my $product_id = $t->tx->res->json->{id};
 my $event = OpenQA::Test::Case::find_most_recent_event($t->app->schema, 'table_create');
 is_deeply(
@@ -89,7 +90,7 @@ is_deeply(
     'product event was logged correctly'
 );
 
-$t->post_ok('/api/v1/products', form => {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2})
+$t->post_ok('/api/v1/products', json => {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2})
   ->status_is(400);    #already exists
 
 $t->get_ok("/api/v1/products/$product_id")->status_is(200);
@@ -119,7 +120,8 @@ is_deeply(
 ) || diag explain $t->tx->res->json;
 
 $t->put_ok("/api/v1/products/$product_id",
-    form => {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2, "settings[TEST2]" => "val1"})
+    json =>
+      {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2, "settings" => {"TEST2" => "val1"}})
   ->status_is(200);
 
 $t->get_ok("/api/v1/products/$product_id")->status_is(200);
@@ -152,7 +154,7 @@ subtest 'server-side limit has precedence over user-specified limit' => sub {
     #create test-products
     for my $i (2 .. 4) {
         $t->post_ok('/api/v1/products',
-            form => {arch => "x86_64", distri => "opensuse-$i", flavor => "DVD", version => 13.2})->status_is(200);
+            json => {arch => "x86_64", distri => "opensuse-$i", flavor => "DVD", version => 13.2})->status_is(200);
     }
 
     $t->get_ok('/api/v1/products?limit=10', 'query with exceeding user-specified limit for products')->status_is(200);
@@ -270,16 +272,18 @@ $t->delete_ok("/api/v1/products/$product_id")->status_is(404);    #not found
 client($t);
 $t->post_ok(
     '/api/v1/products',
-    form => {
+    json => {
         arch => "x86_64",
         distri => "opensuse",
         flavor => "DVD",
         version => 13.2,
-        "settings[TEST]" => "val1",
-        "settings[TEST2]" => "val1"
-    })->status_is(403);
+        "settings" => {
+            "TEST" => "val1",
+            "TEST2" => "val1"
+        }})->status_is(403);
 $t->put_ok("/api/v1/products/$product_id",
-    form => {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2, "settings[TEST2]" => "val1"})
+    json =>
+      {arch => "x86_64", distri => "opensuse", flavor => "DVD", version => 13.2, "settings" => {"TEST2" => "val1"}})
   ->status_is(403);
 $t->delete_ok("/api/v1/products/$product_id")->status_is(403);