From aefb49ba043c39949f7e03394bf288d1a8c6d400 Mon Sep 17 00:00:00 2001 From: Oliver Kurz Date: Mon, 20 Dec 2021 13:34:30 +0100 Subject: [PATCH] Move 'dsn' functionality into only place where used In b1da48eac I removed the "dsn" function and its usage as it looked like that would not be necessary anymore at all and we did not have any test coverage for that. This turned out to be a false assumption leading to a production openQA instance to not start anymore with the error message ``` openqa-webui-daemon: Can't connect to data source 'HASH(0x55d82c060598)' because I can't work out what driver to use (it doesn't seem to contain a 'dbi:driver:' prefix and the DBI_DRIVER env var is not set)> ``` so the change was quickly reverted. This commit now moves the 'dsn' functionality to the only place where it is needed, simplifies the call and adds complete test coverage for the according method with a separate, new test module "t/15-shared-plugin-gru.t". Tests did not cover the 'dsn' function anyway. Verified by calling ``` env OPENQA_BASEDIR=$PWD/t/data OPENQA_DATABASE=test script/openqa-gru ``` locally. --- lib/OpenQA/Schema.pm | 5 ---- lib/OpenQA/Shared/Plugin/Gru.pm | 30 +++++++++++++++--------- t/15-shared-plugin-gru.t | 41 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 t/15-shared-plugin-gru.t diff --git a/lib/OpenQA/Schema.pm b/lib/OpenQA/Schema.pm index 3cc3e8ad7e9..e5c6283dd85 100644 --- a/lib/OpenQA/Schema.pm +++ b/lib/OpenQA/Schema.pm @@ -51,11 +51,6 @@ sub disconnect_db () { $SINGLETON = undef; } -sub dsn { - my $self = shift; - return $self->storage->connect_info->[0]->{dsn}; -} - sub deploy ($self, $force_overwrite = 0) { # lock config file to ensure only one thing will deploy/upgrade DB at once # we use a file in prjdir/db as the lock file as the install process and diff --git a/lib/OpenQA/Shared/Plugin/Gru.pm b/lib/OpenQA/Shared/Plugin/Gru.pm index ae2625e41e4..05c9df14dba 100644 --- a/lib/OpenQA/Shared/Plugin/Gru.pm +++ b/lib/OpenQA/Shared/Plugin/Gru.pm @@ -40,18 +40,31 @@ sub register_tasks ($self) { ); } +# allow the continuously polled stats to be available on an +# unauthenticated route to prevent recurring broken requests to the login +# provider if not logged in +# +# hard/impossible to mock due to name collision of "remove" method on +# Test::MockObject, hence marking as +# uncoverable statement +sub _allow_unauthenticated_minion_stats ($app) { + my $route = $app->routes->find('minion_stats')->remove; # uncoverable statement + $app->routes->any('/minion')->add_child($route); # uncoverable statement +} + sub register ($self, $app, $config) { $self->app($app) unless $self->app; my $schema = $app->schema; my $conn = Mojo::Pg->new; - if (ref $schema->storage->connect_info->[0] eq 'HASH') { - $self->dsn($schema->dsn); - $conn->username($schema->storage->connect_info->[0]->{user}); - $conn->password($schema->storage->connect_info->[0]->{password}); + my $connect_info = $schema->storage->connect_info->[0]; + if (ref $connect_info eq 'HASH') { + $self->dsn($connect_info->{dsn}); + $conn->username($connect_info->{user}); + $conn->password($connect_info->{password}); } else { - $self->dsn($schema->storage->connect_info->[0]); + $self->dsn($connect_info); } $conn->dsn($self->dsn()); @@ -82,12 +95,7 @@ sub register ($self, $app, $config) { # Enable the Minion Admin interface under /minion my $auth = $app->routes->under('/minion')->to('session#ensure_admin'); $app->plugin('Minion::Admin' => {route => $auth}); - # allow the continuously polled stats to be available on an - # unauthenticated route to prevent recurring broken requests to the login - # provider if not logged in - my $route = $app->routes->find('minion_stats')->remove; - $app->routes->any('/minion')->add_child($route); - + _allow_unauthenticated_minion_stats($app); my $gru = OpenQA::Shared::Plugin::Gru->new($app); $app->helper(gru => sub ($c) { $gru }); } diff --git a/t/15-shared-plugin-gru.t b/t/15-shared-plugin-gru.t new file mode 100644 index 00000000000..ecd063d9da1 --- /dev/null +++ b/t/15-shared-plugin-gru.t @@ -0,0 +1,41 @@ +#!/usr/bin/env perl +# Copyright SUSE LLC +# SPDX-License-Identifier: GPL-2.0-or-later + +use Test::Most; + +use Test::Warnings ':report_warnings'; +use Test::MockModule; +use Test::MockObject; +use FindBin; +use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib"; +use OpenQA::Test::TimeLimit '5'; +use OpenQA::App; +use OpenQA::Shared::Plugin::Gru; # SUT + +ok my $gru = OpenQA::Shared::Plugin::Gru->new, 'can instantiate gru plugin'; + +my $connect_info = {dsn => 'my_dsn', user => 'user', password => 'foo'}; +my $storage = Test::MockObject->new->set_always(connect_info => [$connect_info]); +my $schema = Test::MockObject->new->set_always(storage => $storage)->set_true('search_path_for_tests'); +my %config = (misc_limits => {minion_job_max_age => 0}); +my $minion = Test::MockObject->new->set_true('on'); +my $log = Test::MockObject->new->set_true('level', 'info'); +my $under = Test::MockObject->new->set_true('to'); +my $routes = Test::MockObject->new->set_always(under => $under); +my $app + = Test::MockObject->new->set_always(schema => $schema)->set_always(config => \%config)->set_always(minion => $minion) + ->set_always(log => $log)->set_always(routes => $routes)->set_true('plugin', 'helper'); +OpenQA::App->set_singleton($app); +my $mock = Test::MockModule->new('OpenQA::Shared::Plugin::Gru'); +$mock->noop('_allow_unauthenticated_minion_stats'); +my $pg = Test::MockObject->new->set_true('dsn', 'username', 'password', 'search_path', 'find'); +my $pg_module = Test::MockModule->new('Mojo::Pg')->redefine(new => $pg); +ok $gru->register($app, undef), 'can register gru plugin'; +$pg->called_ok('username', 'pg connection initialized with username'); +is(($pg->call_args(0))[1], $connect_info->{user}, 'pg connection username is correct'); +is(($pg->call_args(2))[1], $connect_info->{password}, 'pg connection password is correct'); +is(($app->call_args(4))[1], 'Minion', 'minion initialized'); +is(($app->call_args(4))[2]->{Pg}, $pg, 'pg connection initialized on minion'); + +done_testing;