From af3b17305d31694de7cdeacdab9e715886cd87b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 14:48:37 +0200 Subject: [PATCH 01/16] add errors module --- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 31 ++++-- lib/Zonemaster/Backend/Errors.pm | 141 ++++++++++++++++++++++++ lib/Zonemaster/Backend/RPCAPI.pm | 22 ++-- script/zonemaster_db_exporter.psgi | 35 ++++++ 4 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 lib/Zonemaster/Backend/Errors.pm create mode 100644 script/zonemaster_db_exporter.psgi diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 908e2aa3c..6cda68f3a 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -10,6 +10,7 @@ use Encode; use JSON::PP; use Zonemaster::Backend::DB; +use Zonemaster::Backend::Errors; with 'Zonemaster::Backend::DB'; @@ -207,8 +208,12 @@ sub get_test_params { my $dbh = $self->dbh; my ( $params_json ) = $dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + eval { $result = decode_json( encode_utf8( $params_json ) ); }; - die "$@ \n" if $@; + + die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } @@ -224,25 +229,35 @@ sub test_results { my $result; eval { my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); - $result = $hrefs->{$test_id}; + $result = $hrefs->{$test_id}; + }; + + die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + eval { # This workaround is needed to properly handle all versions of perl and the DBD::Pg module # More details in the zonemaster backend issue #570 if (utf8::is_utf8($result->{params}) ) { - $result->{params} = decode_json( encode_utf8($result->{params}) ); + $result->{params} = decode_json( encode_utf8($result->{params}) ); } else { - $result->{params} = decode_json( $result->{params} ); + $result->{params} = decode_json( $result->{params} ); } - if (utf8::is_utf8($result->{results} ) ) { + if (defined $result->{results} ) { + if (utf8::is_utf8($result->{results} ) ) { $result->{results} = decode_json( encode_utf8($result->{results}) ); - } - else { + } + else { $result->{results} = decode_json( $result->{results} ); + } + } else { + $result->{results} = []; } }; - die "$@ \n" if $@; + + die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id }) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm new file mode 100644 index 000000000..068e67046 --- /dev/null +++ b/lib/Zonemaster/Backend/Errors.pm @@ -0,0 +1,141 @@ +package Zonemaster::Backend::Error; +use Moose; +use Data::Dumper; + + +has 'message' => ( + is => 'rw', + isa => 'Str', + required => 1, +); + +has 'code' => ( + is => 'rw', + isa => 'Int', + required => 1, +); + +has 'data' => ( + is => 'rw', + isa => 'Any', + default => undef, +); + +sub as_hash { + my $self = shift; + my $error = { + code => $self->code, + message => $self->message + }; + $error->{data} = $self->data if defined $self->data; + return $error; +} + +sub as_string { + my $self = shift; + my $str = sprintf "%s (code %d)", $self->message, $self->code; + if (defined $self->data) { + $str .= sprintf "; Context: %s", $self->_data_dump; + } + return $str; +} + +sub _data_dump { + my $self = shift; + local $Data::Dumper::Indent = 0; + local $Data::Dumper::Terse = 1; + my $data = Dumper($self->data); + $data =~ s/[\n\r]/ /g; + return $data ; +} + +package Zonemaster::Backend::Error::Internal; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Internal server error' +); + +has '+code' => ( + default => -32603 +); + +has 'reason' => ( + isa => 'Str', + is => 'rw', + initializer => 'reason', +); + +has 'method' => ( + is => 'rw', + isa => 'Str', + builder => '_build_method' +); + +has 'id' => ( + is => 'rw', + isa => 'Int', + default => 0, +); + +sub _build_method { + my $s = 0; + while (my @c = caller($s)) { + $s ++; + last if $c[3] eq 'Moose::Object::new'; + } + my @c = caller($s); + if ($c[3] =~ /^(.*)::handle_exception$/ ) { + @c = caller(++$s); + } + + return $c[3]; +} + +around 'reason' => sub { + my $orig = shift; + my $self = shift; + + my ( $value, $setter, $attr ) = @_; + + # reader + return $self->$orig if not $value; + + # trim new lines + $value =~ s/\n/ /g; + $value =~ s/^\s+|\s+$//g; + + # initializer + return $setter->($value) if $setter; + + # writer + $self->$orig($value); +}; + + +sub as_string { + my $self = shift; + my $str = sprintf "Internal error %0.3d: Unexpected error in the `%s` method: [%s]", $self->id, $self->method, $self->reason; + if (defined $self->data) { + $str .= sprintf "; Context: %s", $self->_data_dump; + } + return $str; +} + + +package Zonemaster::Backend::Error::ResourceNotFound; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Resource not found' +); + +has '+code' => ( + default => -32000 +); + +1; diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index efbc70e0a..8245c3783 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -26,7 +26,7 @@ use Zonemaster::Backend; use Zonemaster::Backend::Config; use Zonemaster::Backend::Translator; use Zonemaster::Backend::Validator; -use Log::Any qw( $log ); +use Zonemaster::Backend::Errors; my $zm_validator = Zonemaster::Backend::Validator->new; my %json_schemas; @@ -68,18 +68,26 @@ sub _init_db { my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); $self->{db} = $dbclass->from_config( $self->{config} ); }; - if ($@) { handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); + if ($@) { } } sub handle_exception { - my ( $method, $exception, $exception_id ) = @_; + my ( $_method, $exception, $exception_id ) = @_; + + if ( !$exception->isa('Zonemaster::Backend::Error') ) { + my $reason = $exception; + $exception = Zonemaster::Backend::Error::Internal->new(reason => $reason, id => $exception_id); + } + + if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { + $log->error($exception->as_string); + } else { + $log->notice($exception->as_string); + } - $exception =~ s/\n/ /g; - $exception =~ s/^\s+|\s+$//g; - $log->error("Internal error $exception_id: Unexpected error in the $method API call: [$exception]"); - die "Internal error $exception_id \n"; + die $exception->as_hash; } $json_schemas{version_info} = joi->object->strict; diff --git a/script/zonemaster_db_exporter.psgi b/script/zonemaster_db_exporter.psgi new file mode 100644 index 000000000..bb7144a2d --- /dev/null +++ b/script/zonemaster_db_exporter.psgi @@ -0,0 +1,35 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use Zonemaster::Backend::Config; +use Zonemaster::Backend::DB; +use Net::Prometheus; + +my $client = Net::Prometheus->new; + +my $tests_gauge = $client->new_gauge( + name => 'zonemaster_tests_total', + help => 'Total number of tests', + labels => [ 'state' ], +); + +my $config = Zonemaster::Backend::Config->load_config(); +my $dbtype = $config->DB_engine; +my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); +my $db = $dbclass->from_config( $config ); + +my $prom_app = $client->psgi_app; + +sub { + my $queued = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 0'); + $tests_gauge->labels('queued')->set($queued->{count}); + + my $finished = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 100'); + $tests_gauge->labels('finished')->set($finished->{count}); + + my $running = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress > 0 and progress < 100'); + $tests_gauge->labels('running')->set($running->{count}); + + $prom_app->(@_); +}; From 14625325a94ba71c937ca531d6d651b08bae0f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 15:20:28 +0200 Subject: [PATCH 02/16] improve errors --- lib/Zonemaster/Backend/DB/MySQL.pm | 21 ++++++++++++++++++--- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 11 ++++------- lib/Zonemaster/Backend/DB/SQLite.pm | 22 +++++++++++++++++++--- lib/Zonemaster/Backend/Errors.pm | 23 +++++++++++++++++++++-- lib/Zonemaster/Backend/RPCAPI.pm | 3 ++- 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index ef607b51f..473ce396d 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -10,6 +10,7 @@ use DBI qw(:utils); use JSON::PP; use Zonemaster::Backend::Validator qw( untaint_ipv6_address ); +use Zonemaster::Backend::Errors; with 'Zonemaster::Backend::DB'; @@ -228,12 +229,14 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + my $result; eval { $result = decode_json( $params_json ); }; - warn "decoding of params_json failed (testi_id: [$test_id]):".Dumper($params_json) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return decode_json( $params_json ); } @@ -249,8 +252,20 @@ sub test_results { my $result; my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, CONVERT_TZ(`creation_time`, \@\@session.time_zone, '+00:00') AS creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - $result->{params} = decode_json( $result->{params} ); - $result->{results} = decode_json( $result->{results} ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + + eval { + $result->{params} = decode_json( $result->{params} ); + + if (defined $result->{results}) { + $result->{results} = decode_json( $result->{results} ); + } else { + $result->{results} = []; + } + }; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 6cda68f3a..b249b26a8 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -213,7 +213,7 @@ sub get_test_params { eval { $result = decode_json( encode_utf8( $params_json ) ); }; - die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } @@ -227,12 +227,9 @@ sub test_results { if ( $results ); my $result; - eval { - my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); - $result = $hrefs->{$test_id}; - }; + my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); + $result = $hrefs->{$test_id}; - die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; eval { @@ -257,7 +254,7 @@ sub test_results { } }; - die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id }) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 7dc784e3c..ecea51f89 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -11,6 +11,8 @@ use Digest::MD5 qw(md5_hex); use JSON::PP; use Log::Any qw( $log ); +use Zonemaster::Backend::Errors; + with 'Zonemaster::Backend::DB'; has 'dbh' => ( @@ -244,12 +246,14 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + my $result; eval { $result = decode_json( $params_json ); }; - $log->warn( "decoding of params_json failed (test_id: [$test_id]):".Dumper($params_json) ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } @@ -265,8 +269,20 @@ sub test_results { my $result; my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - $result->{params} = decode_json( $result->{params} ); - $result->{results} = decode_json( $result->{results} ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + + eval { + $result->{params} = decode_json( $result->{params} ); + + if (defined $result->{results}) { + $result->{results} = decode_json( $result->{results} ); + } else { + $result->{results} = []; + } + }; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 068e67046..903579d2c 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -25,7 +25,8 @@ sub as_hash { my $self = shift; my $error = { code => $self->code, - message => $self->message + message => $self->message, + error => ref($self), }; $error->{data} = $self->data if defined $self->data; return $error; @@ -114,10 +115,23 @@ around 'reason' => sub { $self->$orig($value); }; +around 'as_hash' => sub { + my $orig = shift; + my $self = shift; + + my $href = $self->$orig; + + $href->{exception_id} = $self->id; + $href->{reason} = $self->reason; + $href->{method} = $self->method; + + return $href; +}; + sub as_string { my $self = shift; - my $str = sprintf "Internal error %0.3d: Unexpected error in the `%s` method: [%s]", $self->id, $self->method, $self->reason; + my $str = sprintf "Internal error %0.3d (%s): Unexpected error in the `%s` method: [%s]", $self->id, ref($self), $self->method, $self->reason; if (defined $self->data) { $str .= sprintf "; Context: %s", $self->_data_dump; } @@ -138,4 +152,9 @@ has '+code' => ( default => -32000 ); +package Zonemaster::Backend::Error::JsonError; +use Moose; + +extends 'Zonemaster::Backend::Error::Internal'; + 1; diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 8245c3783..9f1a32bf0 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -68,8 +68,9 @@ sub _init_db { my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); $self->{db} = $dbclass->from_config( $self->{config} ); }; - handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); + if ($@) { + handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); } } From 6484d04bca52ca1b215c55e113c1000911f04929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 16:03:28 +0200 Subject: [PATCH 03/16] change log level --- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- lib/Zonemaster/Backend/RPCAPI.pm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index ecea51f89..3568080ab 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -270,7 +270,7 @@ sub test_results { my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + #die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 9f1a32bf0..1be68d685 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -85,7 +85,7 @@ sub handle_exception { if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { $log->error($exception->as_string); } else { - $log->notice($exception->as_string); + $log->info($exception->as_string); } die $exception->as_hash; From f9d0d8d2ef43f669e07f7e4f9d3ec1c8d39591f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 16:07:55 +0200 Subject: [PATCH 04/16] fix comment --- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 3568080ab..ecea51f89 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -270,7 +270,7 @@ sub test_results { my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - #die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); From 5f551c66cbc2d559185c4de143deea662de06626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Mon, 16 Aug 2021 13:50:47 +0200 Subject: [PATCH 05/16] remove unrelated file --- script/zonemaster_db_exporter.psgi | 35 ------------------------------ 1 file changed, 35 deletions(-) delete mode 100644 script/zonemaster_db_exporter.psgi diff --git a/script/zonemaster_db_exporter.psgi b/script/zonemaster_db_exporter.psgi deleted file mode 100644 index bb7144a2d..000000000 --- a/script/zonemaster_db_exporter.psgi +++ /dev/null @@ -1,35 +0,0 @@ -#!/usr/bin/env perl -use strict; -use warnings; - -use Zonemaster::Backend::Config; -use Zonemaster::Backend::DB; -use Net::Prometheus; - -my $client = Net::Prometheus->new; - -my $tests_gauge = $client->new_gauge( - name => 'zonemaster_tests_total', - help => 'Total number of tests', - labels => [ 'state' ], -); - -my $config = Zonemaster::Backend::Config->load_config(); -my $dbtype = $config->DB_engine; -my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); -my $db = $dbclass->from_config( $config ); - -my $prom_app = $client->psgi_app; - -sub { - my $queued = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 0'); - $tests_gauge->labels('queued')->set($queued->{count}); - - my $finished = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 100'); - $tests_gauge->labels('finished')->set($finished->{count}); - - my $running = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress > 0 and progress < 100'); - $tests_gauge->labels('running')->set($running->{count}); - - $prom_app->(@_); -}; From febf070303c40ef04adbd374cca1fb868a3a0aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Mon, 16 Aug 2021 17:16:37 +0200 Subject: [PATCH 06/16] improve code --- lib/Zonemaster/Backend/DB/MySQL.pm | 12 ++++++++---- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 12 ++++++++---- lib/Zonemaster/Backend/DB/SQLite.pm | 12 ++++++++---- lib/Zonemaster/Backend/Errors.pm | 8 ++++---- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 473ce396d..0dde54a81 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -229,14 +229,16 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; my $result; eval { $result = decode_json( $params_json ); }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return decode_json( $params_json ); } @@ -253,7 +255,8 @@ sub test_results { my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, CONVERT_TZ(`creation_time`, \@\@session.time_zone, '+00:00') AS creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); @@ -265,7 +268,8 @@ sub test_results { } }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index b249b26a8..2c601473b 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -209,11 +209,13 @@ sub get_test_params { my $dbh = $self->dbh; my ( $params_json ) = $dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; eval { $result = decode_json( encode_utf8( $params_json ) ); }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -230,7 +232,8 @@ sub test_results { my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; eval { # This workaround is needed to properly handle all versions of perl and the DBD::Pg module @@ -254,7 +257,8 @@ sub test_results { } }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) + if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index ecea51f89..f02eee3dd 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -246,14 +246,16 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $params_json; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; my $result; eval { $result = decode_json( $params_json ); }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -270,7 +272,8 @@ sub test_results { my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); @@ -282,7 +285,8 @@ sub test_results { } }; - die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 903579d2c..221d60f2b 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -4,19 +4,19 @@ use Data::Dumper; has 'message' => ( - is => 'rw', + is => 'ro', isa => 'Str', required => 1, ); has 'code' => ( - is => 'rw', + is => 'ro', isa => 'Int', required => 1, ); has 'data' => ( - is => 'rw', + is => 'ro', isa => 'Any', default => undef, ); @@ -70,7 +70,7 @@ has 'reason' => ( ); has 'method' => ( - is => 'rw', + is => 'ro', isa => 'Str', builder => '_build_method' ); From c5378a618b32049aa6d8892ed0de747f39f54350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Tue, 17 Aug 2021 16:05:25 +0200 Subject: [PATCH 07/16] add more error types --- lib/Zonemaster/Backend/DB.pm | 3 ++- lib/Zonemaster/Backend/DB/MySQL.pm | 7 ++++--- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 6 +++--- lib/Zonemaster/Backend/DB/SQLite.pm | 7 ++++--- lib/Zonemaster/Backend/Errors.pm | 26 +++++++++++++++++++++++++ 5 files changed, 39 insertions(+), 10 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index 36f5e5984..a49a95a06 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -62,7 +62,8 @@ sub add_api_user { die "username or api_key not provided to the method add_api_user\n" unless ( $username && $api_key ); - die "User already exists\n" if ( $self->user_exists( $username ) ); + die Zonemaster::Backend::Error::Conflict->new( message => 'User already exists', data => { username => $username } ) + if ( $self->user_exists( $username ) ); my $result = $self->add_api_user_to_db( $username, $api_key ); diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 0dde54a81..7151cc790 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -129,7 +129,7 @@ sub user_authorized { sub create_new_batch_job { my ( $self, $username ) = @_; - my ( $batch_id, $creaton_time ) = $self->dbh->selectrow_array( " + my ( $batch_id, $creation_time ) = $self->dbh->selectrow_array( " SELECT batch_id, batch_jobs.creation_time AS batch_creation_time @@ -143,7 +143,8 @@ sub create_new_batch_job { LIMIT 1 ", undef, $username ); - die "You can't create a new batch job, job:[$batch_id] started on:[$creaton_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); $self->dbh->do( "INSERT INTO batch_jobs (username) VALUES(?)", undef, $username ); my ( $new_batch_id ) = $self->dbh->{mysql_insertid}; @@ -389,7 +390,7 @@ sub add_batch_job { $dbh->{AutoCommit} = 1; } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 2c601473b..61b6d6597 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -154,8 +154,8 @@ sub create_new_batch_job { test_results.progress<>100 LIMIT 1 ", undef, $username ); - - die "You can't create a new batch job, job:[$batch_id] started on:[$creation_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); my ( $new_batch_id ) = $dbh->selectrow_array( "INSERT INTO batch_jobs (username) VALUES (?) RETURNING id", undef, $username ); @@ -358,7 +358,7 @@ sub add_batch_job { $dbh->commit(); } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index f02eee3dd..7ee765d70 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -147,7 +147,7 @@ sub user_authorized { sub create_new_batch_job { my ( $self, $username ) = @_; - my ( $batch_id, $creaton_time ) = $self->dbh->selectrow_array( " + my ( $batch_id, $creation_time ) = $self->dbh->selectrow_array( " SELECT batch_id, batch_jobs.creation_time AS batch_creation_time @@ -160,7 +160,8 @@ sub create_new_batch_job { LIMIT 1 " ); - die "You can't create a new batch job, job:[$batch_id] started on:[$creaton_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); $self->dbh->do("INSERT INTO batch_jobs (username) VALUES(" . $self->dbh->quote( $username ) . ")" ); my ( $new_batch_id ) = $self->dbh->sqlite_last_insert_rowid; @@ -383,7 +384,7 @@ sub add_batch_job { $dbh->{AutoCommit} = 1; } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 221d60f2b..54c7f78d9 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -152,6 +152,32 @@ has '+code' => ( default => -32000 ); +package Zonemaster::Backend::Error::PermissionDenied; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Permission denied' +); + +has '+code' => ( + default => -32001 +); + +package Zonemaster::Backend::Error::Conflict; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Conflicting resource' +); + +has '+code' => ( + default => -32002 +); + package Zonemaster::Backend::Error::JsonError; use Moose; From 2cc1bf8029007da6d7cc924b29892801414501e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Tue, 17 Aug 2021 16:29:06 +0200 Subject: [PATCH 08/16] remove error id --- lib/Zonemaster/Backend/Errors.pm | 15 ++++--------- lib/Zonemaster/Backend/RPCAPI.pm | 36 ++++++++++++++++---------------- 2 files changed, 22 insertions(+), 29 deletions(-) diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 54c7f78d9..fe6deec5d 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -34,9 +34,9 @@ sub as_hash { sub as_string { my $self = shift; - my $str = sprintf "%s (code %d)", $self->message, $self->code; + my $str = sprintf "%s (code %d).", $self->message, $self->code; if (defined $self->data) { - $str .= sprintf "; Context: %s", $self->_data_dump; + $str .= sprintf " Context: %s", $self->_data_dump; } return $str; } @@ -75,12 +75,6 @@ has 'method' => ( builder => '_build_method' ); -has 'id' => ( - is => 'rw', - isa => 'Int', - default => 0, -); - sub _build_method { my $s = 0; while (my @c = caller($s)) { @@ -121,7 +115,6 @@ around 'as_hash' => sub { my $href = $self->$orig; - $href->{exception_id} = $self->id; $href->{reason} = $self->reason; $href->{method} = $self->method; @@ -131,9 +124,9 @@ around 'as_hash' => sub { sub as_string { my $self = shift; - my $str = sprintf "Internal error %0.3d (%s): Unexpected error in the `%s` method: [%s]", $self->id, ref($self), $self->method, $self->reason; + my $str = sprintf "Caught %s in the `%s` method: %s", ref($self), $self->method, $self->reason; if (defined $self->data) { - $str .= sprintf "; Context: %s", $self->_data_dump; + $str .= sprintf " Context: %s", $self->_data_dump; } return $str; } diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 1be68d685..f4bc3ad41 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -44,7 +44,7 @@ sub new { bless( $self, $type ); if ( ! $params || ! $params->{config} ) { - handle_exception('new', "Missing 'config' parameter", '001'); + handle_exception('new', "Missing 'config' parameter"); } $self->{config} = $params->{config}; @@ -70,16 +70,16 @@ sub _init_db { }; if ($@) { - handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); + handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]"); } } sub handle_exception { - my ( $_method, $exception, $exception_id ) = @_; + my ( $_method, $exception ) = @_; if ( !$exception->isa('Zonemaster::Backend::Error') ) { my $reason = $exception; - $exception = Zonemaster::Backend::Error::Internal->new(reason => $reason, id => $exception_id); + $exception = Zonemaster::Backend::Error::Internal->new( reason => $reason ); } if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { @@ -102,7 +102,7 @@ sub version_info { }; if ($@) { - handle_exception('version_info', $@, '003'); + handle_exception( 'version_info', $@ ); } return \%ver; @@ -115,7 +115,7 @@ sub profile_names { my %profiles; eval { %profiles = $self->{config}->PUBLIC_PROFILES }; if ( $@ ) { - handle_exception( 'profile_names', $@, '004' ); + handle_exception( 'profile_names', $@ ); } return [ keys %profiles ]; @@ -156,7 +156,7 @@ sub get_host_by_name { }; if ($@) { - handle_exception('get_host_by_name', $@, '005'); + handle_exception( 'get_host_by_name', $@ ); } return \@adresses; @@ -206,7 +206,7 @@ sub get_data_from_parent_zone { return \%result; }; if ($@) { - handle_exception('get_data_from_parent_zone', $@, '006'); + handle_exception( 'get_data_from_parent_zone', $@ ); } elsif ($result) { return $result; @@ -300,7 +300,7 @@ $extra_validators{start_domain_test} = sub { return @errors; }; if ($@) { - handle_exception('start_domain_test_validate_syntax', $@, '008'); + handle_exception( 'start_domain_test_validate_syntax', $@ ); } else { return @errors; @@ -340,7 +340,7 @@ sub start_domain_test { $result = $self->{db}->create_new_test( $params->{domain}, $params, $self->{config}->ZONEMASTER_age_reuse_previous_test ); }; if ($@) { - handle_exception('start_domain_test', $@, '009'); + handle_exception( 'start_domain_test', $@ ); } return $result; @@ -358,7 +358,7 @@ sub test_progress { $result = $self->{db}->test_progress( $test_id ); }; if ($@) { - handle_exception('test_progress', $@, '010'); + handle_exception( 'test_progress', $@ ); } return $result; @@ -378,7 +378,7 @@ sub get_test_params { $result = $self->{db}->get_test_params( $test_id ); }; if ($@) { - handle_exception('get_test_params', $@, '011'); + handle_exception( 'get_test_params', $@ ); } return $result; @@ -419,7 +419,7 @@ sub get_test_results { my $previous_locale = $translator->locale; if ( !$translator->locale( $locale ) ) { - handle_exception( 'get_test_results', "Failed to set locale: $locale", '017' ); + handle_exception( 'get_test_results', "Failed to set locale: $locale" ); } eval { $translator->data } if $translator; # Provoke lazy loading of translation data @@ -478,7 +478,7 @@ sub get_test_results { $result->{results} = \@zm_results; }; if ($@) { - handle_exception('get_test_results', $@, '012'); + handle_exception( 'get_test_results', $@ ); } $translator->locale( $previous_locale ); @@ -510,7 +510,7 @@ sub get_test_history { $results = $self->{db}->get_test_history( $params ); }; if ($@) { - handle_exception('get_test_history', $@, '013'); + handle_exception( 'get_test_history', $@ ); } return $results; @@ -539,7 +539,7 @@ sub add_api_user { } }; if ($@) { - handle_exception('add_api_user', $@, '014'); + handle_exception( 'add_api_user', $@ ); } return $result; @@ -579,7 +579,7 @@ sub add_batch_job { $results = $self->{db}->add_batch_job( $params ); }; if ($@) { - handle_exception('add_batch_job', $@, '015'); + handle_exception( 'add_batch_job', $@ ); } return $results; @@ -599,7 +599,7 @@ sub get_batch_job_result { $result = $self->{db}->get_batch_job_result($batch_id); }; if ($@) { - handle_exception('get_batch_job_result', $@, '016'); + handle_exception( 'get_batch_job_result', $@ ); } return $result; From b7716d53e3aacec0bdb113a58d8de6bd41660d53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Tue, 17 Aug 2021 16:29:47 +0200 Subject: [PATCH 09/16] use error classes instead of strings --- lib/Zonemaster/Backend/DB.pm | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index a49a95a06..aaadd70b3 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -51,7 +51,8 @@ sub get_db_class { sub user_exists { my ( $self, $user ) = @_; - die "username not provided to the method user_exists\n" unless ( $user ); + die Zonemaster::Backend::Error::Internal->new( reason => "username not provided to the method user_exists") + unless ( $user ); return $self->user_exists_in_db( $user ); } @@ -59,15 +60,16 @@ sub user_exists { sub add_api_user { my ( $self, $username, $api_key ) = @_; - die "username or api_key not provided to the method add_api_user\n" - unless ( $username && $api_key ); + die Zonemaster::Backend::Error::Internal->new( reason => "username or api_key not provided to the method add_api_user") + unless ( $username && $api_key ); die Zonemaster::Backend::Error::Conflict->new( message => 'User already exists', data => { username => $username } ) if ( $self->user_exists( $username ) ); my $result = $self->add_api_user_to_db( $username, $api_key ); - die "add_api_user_to_db not successful\n" unless ( $result ); + die Zonemaster::Backend::Error::Internal->new( reason => "add_api_user_to_db not successful") + unless ( $result ); return $result; } From 5f9ee3ef68dc3da61356c7ae057bc94463651751 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Mon, 23 Aug 2021 09:52:17 +0200 Subject: [PATCH 10/16] remove specialized as_hash method for internal error --- lib/Zonemaster/Backend/Errors.pm | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index fe6deec5d..9d310e99d 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -109,19 +109,6 @@ around 'reason' => sub { $self->$orig($value); }; -around 'as_hash' => sub { - my $orig = shift; - my $self = shift; - - my $href = $self->$orig; - - $href->{reason} = $self->reason; - $href->{method} = $self->method; - - return $href; -}; - - sub as_string { my $self = shift; my $str = sprintf "Caught %s in the `%s` method: %s", ref($self), $self->method, $self->reason; From 9d22befef93e939889549aa0d717a96a19d0af5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Mon, 23 Aug 2021 09:56:25 +0200 Subject: [PATCH 11/16] remove unused variable --- lib/Zonemaster/Backend/RPCAPI.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index bc2b6669b..b4c936979 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -75,7 +75,7 @@ sub _init_db { } sub handle_exception { - my ( $_method, $exception ) = @_; + my ( undef, $exception ) = @_; if ( !$exception->isa('Zonemaster::Backend::Error') ) { my $reason = $exception; From 9dc0b3dc433c94f387e265609843116fd0b319b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Mon, 23 Aug 2021 09:59:53 +0200 Subject: [PATCH 12/16] improve code formatting --- lib/Zonemaster/Backend/DB.pm | 8 ++++---- lib/Zonemaster/Backend/DB/MySQL.pm | 8 ++++---- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 8 ++++---- lib/Zonemaster/Backend/DB/SQLite.pm | 8 ++++---- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index aaadd70b3..09875a82c 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -52,7 +52,7 @@ sub user_exists { my ( $self, $user ) = @_; die Zonemaster::Backend::Error::Internal->new( reason => "username not provided to the method user_exists") - unless ( $user ); + unless ( $user ); return $self->user_exists_in_db( $user ); } @@ -61,15 +61,15 @@ sub add_api_user { my ( $self, $username, $api_key ) = @_; die Zonemaster::Backend::Error::Internal->new( reason => "username or api_key not provided to the method add_api_user") - unless ( $username && $api_key ); + unless ( $username && $api_key ); die Zonemaster::Backend::Error::Conflict->new( message => 'User already exists', data => { username => $username } ) - if ( $self->user_exists( $username ) ); + if ( $self->user_exists( $username ) ); my $result = $self->add_api_user_to_db( $username, $api_key ); die Zonemaster::Backend::Error::Internal->new( reason => "add_api_user_to_db not successful") - unless ( $result ); + unless ( $result ); return $result; } diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 7151cc790..932fcbd53 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -231,7 +231,7 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $params_json; + unless defined $params_json; my $result; eval { @@ -239,7 +239,7 @@ sub get_test_params { }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) - if $@; + if $@; return decode_json( $params_json ); } @@ -257,7 +257,7 @@ sub test_results { $result = $hrefs->{$test_id}; die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $result; + unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); @@ -270,7 +270,7 @@ sub test_results { }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) - if $@; + if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 61b6d6597..bcd5dfcdd 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -210,12 +210,12 @@ sub get_test_params { my ( $params_json ) = $dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $params_json; + unless defined $params_json; eval { $result = decode_json( encode_utf8( $params_json ) ); }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) - if $@; + if $@; return $result; } @@ -233,7 +233,7 @@ sub test_results { $result = $hrefs->{$test_id}; die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $result; + unless defined $result; eval { # This workaround is needed to properly handle all versions of perl and the DBD::Pg module @@ -258,7 +258,7 @@ sub test_results { }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) - if $@; + if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 7ee765d70..fb0603122 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -248,7 +248,7 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $params_json; + unless defined $params_json; my $result; eval { @@ -256,7 +256,7 @@ sub get_test_params { }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) - if $@; + if $@; return $result; } @@ -274,7 +274,7 @@ sub test_results { $result = $hrefs->{$test_id}; die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) - unless defined $result; + unless defined $result; eval { $result->{params} = decode_json( $result->{params} ); @@ -287,7 +287,7 @@ sub test_results { }; die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) - if $@; + if $@; return $result; } From 169019f88c5abe92b805c9751ccedb18752e0d8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Wed, 25 Aug 2021 15:39:08 +0200 Subject: [PATCH 13/16] improve code formatting --- lib/Zonemaster/Backend/DB/MySQL.pm | 2 +- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 2 +- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index 932fcbd53..d95070993 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -144,7 +144,7 @@ sub create_new_batch_job { ", undef, $username ); die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) - if ( $batch_id ); + if ( $batch_id ); $self->dbh->do( "INSERT INTO batch_jobs (username) VALUES(?)", undef, $username ); my ( $new_batch_id ) = $self->dbh->{mysql_insertid}; diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index bcd5dfcdd..1177066a7 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -155,7 +155,7 @@ sub create_new_batch_job { LIMIT 1 ", undef, $username ); die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) - if ( $batch_id ); + if ( $batch_id ); my ( $new_batch_id ) = $dbh->selectrow_array( "INSERT INTO batch_jobs (username) VALUES (?) RETURNING id", undef, $username ); diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index fb0603122..72919151e 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -161,7 +161,7 @@ sub create_new_batch_job { " ); die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) - if ( $batch_id ); + if ( $batch_id ); $self->dbh->do("INSERT INTO batch_jobs (username) VALUES(" . $self->dbh->quote( $username ) . ")" ); my ( $new_batch_id ) = $self->dbh->sqlite_last_insert_rowid; From b32c1b99f22120de6a4464f3564bfd0ae1c8b9b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Wed, 25 Aug 2021 15:49:49 +0200 Subject: [PATCH 14/16] make reason read only --- lib/Zonemaster/Backend/Errors.pm | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 9d310e99d..cc25cf877 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -65,8 +65,7 @@ has '+code' => ( has 'reason' => ( isa => 'Str', - is => 'rw', - initializer => 'reason', + is => 'ro' ); has 'method' => ( @@ -89,24 +88,16 @@ sub _build_method { return $c[3]; } -around 'reason' => sub { - my $orig = shift; - my $self = shift; - - my ( $value, $setter, $attr ) = @_; - - # reader - return $self->$orig if not $value; +around 'BUILDARGS', sub { + my ($orig, $class, %args) = @_; - # trim new lines - $value =~ s/\n/ /g; - $value =~ s/^\s+|\s+$//g; - - # initializer - return $setter->($value) if $setter; + if(exists $args{reason}) { + # trim new lines + $args{reason} =~ s/\n/ /g; + $args{reason} =~ s/^\s+|\s+$//g; + } - # writer - $self->$orig($value); + $class->$orig(%args); }; sub as_string { From 1c803dfa9ce6f269d0941c154409d217290de9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 26 Aug 2021 10:03:10 +0200 Subject: [PATCH 15/16] remove method name from handle_exception calls --- lib/Zonemaster/Backend/RPCAPI.pm | 34 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index b4c936979..b36a05813 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -44,7 +44,7 @@ sub new { bless( $self, $type ); if ( ! $params || ! $params->{config} ) { - handle_exception('new', "Missing 'config' parameter"); + handle_exception("Missing 'config' parameter"); } $self->{config} = $params->{config}; @@ -70,12 +70,12 @@ sub _init_db { }; if ($@) { - handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]"); + handle_exception("Failed to initialize the [$dbtype] database backend module: [$@]"); } } sub handle_exception { - my ( undef, $exception ) = @_; + my ( $exception ) = @_; if ( !$exception->isa('Zonemaster::Backend::Error') ) { my $reason = $exception; @@ -102,7 +102,7 @@ sub version_info { }; if ($@) { - handle_exception( 'version_info', $@ ); + handle_exception( $@ ); } return \%ver; @@ -115,7 +115,7 @@ sub profile_names { my %profiles; eval { %profiles = $self->{config}->PUBLIC_PROFILES }; if ( $@ ) { - handle_exception( 'profile_names', $@ ); + handle_exception( $@ ); } return [ keys %profiles ]; @@ -156,7 +156,7 @@ sub get_host_by_name { }; if ($@) { - handle_exception( 'get_host_by_name', $@ ); + handle_exception( $@ ); } return \@adresses; @@ -206,7 +206,7 @@ sub get_data_from_parent_zone { return \%result; }; if ($@) { - handle_exception( 'get_data_from_parent_zone', $@ ); + handle_exception( $@ ); } elsif ($result) { return $result; @@ -300,7 +300,7 @@ $extra_validators{start_domain_test} = sub { return @errors; }; if ($@) { - handle_exception( 'start_domain_test_validate_syntax', $@ ); + handle_exception( $@ ); } else { return @errors; @@ -340,7 +340,7 @@ sub start_domain_test { $result = $self->{db}->create_new_test( $params->{domain}, $params, $self->{config}->ZONEMASTER_age_reuse_previous_test ); }; if ($@) { - handle_exception( 'start_domain_test', $@ ); + handle_exception( $@ ); } return $result; @@ -358,7 +358,7 @@ sub test_progress { $result = $self->{db}->test_progress( $test_id ); }; if ($@) { - handle_exception( 'test_progress', $@ ); + handle_exception( $@ ); } return $result; @@ -378,7 +378,7 @@ sub get_test_params { $result = $self->{db}->get_test_params( $test_id ); }; if ($@) { - handle_exception( 'get_test_params', $@ ); + handle_exception( $@ ); } return $result; @@ -419,7 +419,7 @@ sub get_test_results { my $previous_locale = $translator->locale; if ( !$translator->locale( $locale ) ) { - handle_exception( 'get_test_results', "Failed to set locale: $locale" ); + handle_exception( "Failed to set locale: $locale" ); } eval { $translator->data } if $translator; # Provoke lazy loading of translation data @@ -478,7 +478,7 @@ sub get_test_results { $result->{results} = \@zm_results; }; if ($@) { - handle_exception( 'get_test_results', $@ ); + handle_exception( $@ ); } $translator->locale( $previous_locale ); @@ -510,7 +510,7 @@ sub get_test_history { $results = $self->{db}->get_test_history( $params ); }; if ($@) { - handle_exception( 'get_test_history', $@ ); + handle_exception( $@ ); } return $results; @@ -539,7 +539,7 @@ sub add_api_user { } }; if ($@) { - handle_exception( 'add_api_user', $@ ); + handle_exception( $@ ); } return $result; @@ -579,7 +579,7 @@ sub add_batch_job { $results = $self->{db}->add_batch_job( $params ); }; if ($@) { - handle_exception( 'add_batch_job', $@ ); + handle_exception( $@ ); } return $results; @@ -599,7 +599,7 @@ sub get_batch_job_result { $result = $self->{db}->get_batch_job_result($batch_id); }; if ($@) { - handle_exception( 'get_batch_job_result', $@ ); + handle_exception( $@ ); } return $result; From c50e8b46ace5c19288115fde945fd52451cb002c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Tue, 31 Aug 2021 15:15:26 +0200 Subject: [PATCH 16/16] update manifest --- MANIFEST | 1 + 1 file changed, 1 insertion(+) diff --git a/MANIFEST b/MANIFEST index 28b38eb1a..a62312754 100644 --- a/MANIFEST +++ b/MANIFEST @@ -29,6 +29,7 @@ lib/Zonemaster/Backend/DB.pm lib/Zonemaster/Backend/DB/MySQL.pm lib/Zonemaster/Backend/DB/PostgreSQL.pm lib/Zonemaster/Backend/DB/SQLite.pm +lib/Zonemaster/Backend/Errors.pm lib/Zonemaster/Backend/RPCAPI.pm lib/Zonemaster/Backend/TestAgent.pm lib/Zonemaster/Backend/Translator.pm