Bugzilla – Attachment 149266 Details for
Bug 32334
Sync comments in database with schema
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 32334: (QA follow-up) Remove fh and verbose parameter in favor of messages
Bug-32334-QA-follow-up-Remove-fh-and-verbose-param.patch (text/plain), 10.90 KB, created by
Jonathan Druart
on 2023-04-07 11:49:19 UTC
(
hide
)
Description:
Bug 32334: (QA follow-up) Remove fh and verbose parameter in favor of messages
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2023-04-07 11:49:19 UTC
Size:
10.90 KB
patch
obsolete
>From 0434a0e541162be2801fafef58020c579904604f Mon Sep 17 00:00:00 2001 >From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Date: Wed, 15 Feb 2023 14:47:45 +0000 >Subject: [PATCH] Bug 32334: (QA follow-up) Remove fh and verbose parameter in > favor of messages > >The module returns messages. The script can print them in verbose >mode. Test script adjusted accordingly. > >Test plan: >Run t/db_dependent/Koha/Database/Commenter.t > >Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> >Signed-off-by: David Nind <david@davidnind.com> > >Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >--- > Koha/Database/Commenter.pm | 47 ++++++++++++------------ > misc/maintenance/sync_db_comments.pl | 12 +++--- > t/db_dependent/Koha/Database/Commenter.t | 24 ++++++------ > 3 files changed, 41 insertions(+), 42 deletions(-) > >diff --git a/Koha/Database/Commenter.pm b/Koha/Database/Commenter.pm >index 66d8d05ce95..9d4959e0ea6 100644 >--- a/Koha/Database/Commenter.pm >+++ b/Koha/Database/Commenter.pm >@@ -54,22 +54,21 @@ Koha::Database::Commenter - Manage column comments in database > =head2 new > > $mgr = Koha::Database::Commenter->new({ >- dbh => $dbh, database => $d, fh => $fh, schema_file => $s >+ dbh => $dbh, database => $d, schema_file => $s > }); > > Object constructor. >- Param dbh is mandatory. Params database, fh and schema_file are >+ Param dbh is mandatory. Params database and schema_file are > optional. > Param database can be used to move away from current database of > db handle. >- Param fh can be used to redirect output. > Param schema_file is needed for resetting to schema. Falls back to > the constant for Koha structure file. > > =cut > > sub new { >- my ( $class, $params ) = @_; # params: database, dbh, fh, schema_file >+ my ( $class, $params ) = @_; # params: database, dbh, schema_file > my $self = bless $params // {}, $class; > > Koha::Exceptions::MissingParameter->throw( parameter => 'dbh' ) unless $self->{dbh}; >@@ -77,7 +76,6 @@ sub new { > unless ref($self->{dbh}) eq DBI_HANDLE_CLASS; > > $self->{database} //= ( $self->{dbh}->selectrow_array('SELECT DATABASE()') )[0]; >- $self->{fh} //= *STDOUT; > $self->{schema_file} //= KOHA_STRUCTURE; > $self->{schema_info} = {}; > >@@ -86,68 +84,68 @@ sub new { > > =head2 clear > >- $object->clear({ dry_run => 0, table => $table, verbose => 0 }); >+ $object->clear({ dry_run => 0, table => $table }, $messages ); > > Clears all current column comments in storage. > If table is passed, only that table is changed. >- Dry run only prints sql statements. >+ Dry run only returns sql statements in $messages (arrayref). > > =cut > > sub clear { >- my ( $self, $params ) = @_; # dry_run, table, verbose >+ my ( $self, $params, $messages ) = @_; # dry_run, table > my $cols = $self->_fetch_stored_comments($params); > foreach my $col ( @$cols ) { > next if !$col->{column_comment}; > next if $params->{table} && $col->{table_name} ne $params->{table}; >- $self->_change_column( $col->{table_name}, $col->{column_name}, undef, $params ); # undef clears >+ $self->_change_column( $col->{table_name}, $col->{column_name}, undef, $params, $messages ); # undef clears > } > } > > =head2 reset_to_schema > >- $object->reset_to_schema({ dry_run => 0, table => $table, verbose => 0 }); >+ $object->reset_to_schema({ dry_run => 0, table => $table }, $messages ); > > Resets column comments in storage to schema definition. > Other column comments are cleared. > When you pass table, only that table is changed. >- Dry run only prints sql statements. >+ Dry run only returns sql statements in $messages (arrayref). > > =cut > > sub reset_to_schema { >- my ( $self, $params ) = @_; # dry_run, table, verbose >- $self->clear($params); >+ my ( $self, $params, $messages ) = @_; # dry_run, table >+ $self->clear( $params, $messages ); > my $schema_comments = $self->_fetch_schema_comments; > foreach my $table ( sort keys %$schema_comments ) { > next if $params->{table} && $table ne $params->{table}; > foreach my $col ( sort keys %{$schema_comments->{$table}} ) { >- $self->_change_column( $table, $col, $schema_comments->{$table}->{$col}, $params ); >+ $self->_change_column( $table, $col, $schema_comments->{$table}->{$col}, $params, $messages ); > } > } > } > > =head2 renumber > >- $object->renumber({ dry_run => 0, table => $table, verbose => 0 }); >+ $object->renumber({ dry_run => 0, table => $table }, $messages ); > > This is primarily meant for testing purposes (verifying results across > whole database). > It adds comments like Comment_1, Comment_2 etc. > When you pass table, only that table is changed. Otherwise all tables > are affected; note that the column counter does not reset by table. >- Dry run only prints sql statements. >+ Dry run only returns sql statements in $messages (arrayref). > > =cut > > sub renumber { >- my ( $self, $params ) = @_; # dry_run, table, verbose >+ my ( $self, $params, $messages ) = @_; # dry_run, table > my $cols = $self->_fetch_stored_comments($params); > my $i = 0; > foreach my $col ( @$cols ) { > next if $params->{table} && $col->{table_name} ne $params->{table}; > $i++; >- $self->_change_column( $col->{table_name}, $col->{column_name}, "Column_$i", $params ); >+ $self->_change_column( $col->{table_name}, $col->{column_name}, "Column_$i", $params, $messages ); > } > } > >@@ -203,7 +201,7 @@ ORDER BY table_name, column_name|; > sub _change_column { > # NOTE: We do not want to use DBIx schema here, but we use stored structure, > # since we only want to change comment not actual table structure. >- my ( $self, $table_name, $column_name, $comment, $params ) = @_; # params: dry_run, verbose >+ my ( $self, $table_name, $column_name, $comment, $params, $messages ) = @_; # params: dry_run > $params //= {}; > > my $dbh = $self->{dbh}; >@@ -239,18 +237,19 @@ sub _change_column { > } > $rv =~ s/\s+$//; # remove trailing spaces > >- # Print >+ # Dry run > if( $params->{dry_run} ) { >- print { $self->{fh} } "$rv;\n"; >+ push @$messages, "$rv;" if $messages; > return; > } >+ > # Deploy > eval { $dbh->do($rv) }; > if( $@ ) { > warn "Failure for $table_name:$column_name"; >- print { $self->{fh} } "-- FAILED: $rv;\n"; >- } elsif( $params->{verbose} ) { >- print { $self->{fh} } "$rv;\n"; >+ push @$messages, "-- FAILED: $rv;" if $messages; >+ } else { >+ push @$messages, "$rv;" if $messages; > } > } > >diff --git a/misc/maintenance/sync_db_comments.pl b/misc/maintenance/sync_db_comments.pl >index 3382467e5b2..5014c4ac680 100755 >--- a/misc/maintenance/sync_db_comments.pl >+++ b/misc/maintenance/sync_db_comments.pl >@@ -26,6 +26,8 @@ use Pod::Usage qw( pod2usage ); > use C4::Context; > use Koha::Database::Commenter; > >+sub alert_dry_run { print "-- DRY RUN\n" if $_[0]->{dry_run}; } >+ > my $cmd_args = {}; > GetOptions( > 'clear' => \$cmd_args->{clear}, >@@ -40,24 +42,24 @@ GetOptions( > $cmd_args->{dry_run} = !$cmd_args->{commit}; > > my $commenter = Koha::Database::Commenter->new({ database => delete $cmd_args->{database}, dbh => C4::Context->dbh }); >+my $messages = $cmd_args->{verbose} || $cmd_args->{dry_run} ? [] : undef; > if( $cmd_args->{help} ) { > pod2usage( -verbose => 2 ); > } elsif( ($cmd_args->{clear}||0) + ($cmd_args->{renumber}||0) + ($cmd_args->{reset}||0) > 1 ) { > print "You cannot pass the clear, renumber and reset flags together\n"; > } elsif( delete $cmd_args->{clear} ) { > alert_dry_run( $cmd_args ); >- $commenter->clear( $cmd_args ); >+ $commenter->clear( $cmd_args, $messages ); > } elsif( delete $cmd_args->{reset} ) { > alert_dry_run( $cmd_args ); >- $commenter->reset_to_schema( $cmd_args ); >+ $commenter->reset_to_schema( $cmd_args, $messages ); > } elsif( delete $cmd_args->{renumber} ) { > alert_dry_run( $cmd_args ); >- $commenter->renumber( $cmd_args ); >+ $commenter->renumber( $cmd_args, $messages ); > } else { > pod2usage( -verbose => 1 ); > } >- >-sub alert_dry_run { print "-- DRY RUN\n" if $_[0]->{dry_run}; } >+print join("\n", @$messages), "\n" if $messages && @$messages; > > __END__ > >diff --git a/t/db_dependent/Koha/Database/Commenter.t b/t/db_dependent/Koha/Database/Commenter.t >index cf1375311eb..4ca9180ec3c 100755 >--- a/t/db_dependent/Koha/Database/Commenter.t >+++ b/t/db_dependent/Koha/Database/Commenter.t >@@ -24,31 +24,29 @@ subtest '->new, dry_run' => sub { > > # Another exception: delete schema file and reset should raise exception > my $filename = create_schema_file(); >- my $stdout; >- open my $fh, '>', \$stdout; >- $mgr = Koha::Database::Commenter->new({ dbh => $dbh, schema_file => $filename, fh => $fh }); >+ $mgr = Koha::Database::Commenter->new({ dbh => $dbh, schema_file => $filename }); > unlink $filename; > throws_ok { $mgr->reset_to_schema({ dry_run => 1, table => 'biblio' }) } 'Koha::Exceptions::FileNotFound', 'Schema deleted'; > > # Clear comments for article_requests in dry run mode >- $stdout = q{}; >- $mgr->clear({ table => 'article_requests', dry_run => 1 }); >- like( $stdout, qr/COLUMN `toc_request`.*DEFAULT 0;$/m, 'No comment for toc_request' ); >+ my $messages = []; >+ $mgr->clear( { table => 'article_requests', dry_run => 1 }, $messages ); >+ is( grep( { /COLUMN `toc_request`.*DEFAULT 0;$/m } @$messages ), 1, 'No comment for toc_request' ); > > # Renumber this field in dry run mode >- $stdout = q{}; >- $mgr->renumber({ table => 'article_requests', dry_run => 1 }); >- like( $stdout, qr/COLUMN `toc_request`.*COMMENT 'Column_\d+';$/m, 'Numbered comment for toc_request' ); >+ $messages = []; >+ $mgr->renumber( { table => 'article_requests', dry_run => 1 }, $messages ); >+ is( grep( { /COLUMN `toc_request`.*COMMENT 'Column_\d+';$/m } @$messages ), 1, 'Numbered comment for toc_request' ); > > # Reset in dry run mode, first fix schema file again > # Our fake schema contains only one column for article_requests now. > $filename = create_schema_file(); >- $mgr = Koha::Database::Commenter->new({ dbh => $dbh, schema_file => $filename, fh => $fh }); >- $stdout = q{}; >- $mgr->reset_to_schema({ table => 'article_requests', dry_run => 1 }); >+ $mgr = Koha::Database::Commenter->new({ dbh => $dbh, schema_file => $filename }); >+ $messages = []; >+ $mgr->reset_to_schema( { table => 'article_requests', dry_run => 1 }, $messages ); > # We expect an ALTER clearing toc_request first, followed by an ALTER adding comment. > # Note: This is based on the fair assumption that toc_request had a comment! This test could fail if it had not. >- like( $stdout, qr/ALTER.*toc_request.*DEFAULT 0;(.*\n)+ALTER.*toc_request.*COMMENT.*$/m, 'Reset for one-columned article_requests' ); >+ like( join("\n", @$messages), qr/ALTER.*toc_request.*DEFAULT 0;(.*\n)+ALTER.*toc_request.*COMMENT.*$/m, 'Reset for one-columned article_requests' ); > > $schema->storage->txn_rollback; > }; >-- >2.25.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 32334
:
144391
|
144392
|
144393
|
144394
|
146712
|
146713
|
146714
|
146715
|
146716
|
146717
|
146718
|
146719
|
146720
|
148266
|
148299
|
148335
|
148338
|
148339
|
148340
|
148341
|
148342
|
148343
|
148344
|
148345
|
148346
|
148347
|
148348
|
149259
|
149260
|
149261
|
149262
|
149263
|
149264
|
149265
| 149266 |
149267
|
149268
|
149269
|
149270