From 660b3936b63b46c2113b8824a415144ba7d0d462 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 23 Jul 2021 16:04:27 +0000 Subject: [PATCH] Bug 15067: (QA follow-up) Ensure update is idempotent These tables really shoul;d have some unique keys, we need a test so we can add those This ended up being a bigger work than expected, RM feel free to reject or move to another bug and let the risk of duplicated languages This is an old one, let's get it in :-) Signed-off-by: Nick Clemens Signed-off-by: Jonathan Druart --- C4/Installer.pm | 9 ++- .../bug_15067-add_additional_languages.perl | 81 +++++++++++++++++++ .../bug_15067-add_newlanguages.sql | 34 -------- .../data/mysql/mandatory/subtag_registry.sql | 2 +- t/db_dependent/Installer.t | 7 +- 5 files changed, 95 insertions(+), 38 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_15067-add_additional_languages.perl delete mode 100644 installer/data/mysql/atomicupdate/bug_15067-add_newlanguages.sql diff --git a/C4/Installer.pm b/C4/Installer.pm index 7aa85def4a..868a664554 100644 --- a/C4/Installer.pm +++ b/C4/Installer.pm @@ -30,7 +30,7 @@ use vars qw(@ISA @EXPORT); BEGIN { require Exporter; @ISA = qw( Exporter ); - push @EXPORT, qw( primary_key_exists foreign_key_exists index_exists column_exists TableExists marc_framework_sql_list); + push @EXPORT, qw( primary_key_exists unique_key_exists foreign_key_exists index_exists column_exists TableExists marc_framework_sql_list); }; =head1 NAME @@ -646,6 +646,13 @@ sub foreign_key_exists { return $infos =~ m|CONSTRAINT `$constraint_name` FOREIGN KEY|; } +sub unique_key_exists { + my ( $table_name, $constraint_name ) = @_; + my $dbh = C4::Context->dbh; + my (undef, $infos) = $dbh->selectrow_array(qq|SHOW CREATE TABLE $table_name|); + return $infos =~ m|UNIQUE KEY `$constraint_name`|; +} + sub index_exists { my ( $table_name, $key_name ) = @_; my $dbh = C4::Context->dbh; diff --git a/installer/data/mysql/atomicupdate/bug_15067-add_additional_languages.perl b/installer/data/mysql/atomicupdate/bug_15067-add_additional_languages.perl new file mode 100644 index 0000000000..b6eda77e39 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_15067-add_additional_languages.perl @@ -0,0 +1,81 @@ +$DBversion = 'XXX'; +if( CheckVersion( $DBversion ) ) { + if( !unique_key_exists( 'language_subtag_registry', 'uniq_lang' ) ) { + my $dupe_languages = $dbh->selectall_arrayref(q| + SELECT subtag, type FROM language_subtag_registry GROUP BY subtag, type HAVING COUNT(*) > 1 + |, { Slice => {} }); + if ( @$dupe_languages ) { + warn "You have duplicated languages in the language_subtag_registry table in your database, unique constraint cannot be added"; + } else { + $dbh->do(q{ + ALTER TABLE language_subtag_registry + ADD UNIQUE KEY uniq_lang (subtag, type) + }); + } + }; + + if( !unique_key_exists( 'language_descriptions', 'uniq_desc' ) ) { + my $dupe_language_descriptions = $dbh->selectall_arrayref(q| + SELECT subtag, lang, type FROM language_descriptions GROUP BY subtag, lang, type HAVING COUNT(*) > 1 + |, { Slice => {} }); + if ( @$dupe_language_descriptions ) { + warn "You have duplicated language descriptionss in the language_descriptions table in your database, unique constraint cannot be added"; + } else { + $dbh->do(q{ + ALTER TABLE language_descriptions + ADD UNIQUE KEY uniq_desc (subtag, type, lang) + }); + } + }; + + if( !unique_key_exists( 'language_rfc4646_to_iso639', 'uniq_code' ) ) { + my $dupe_language_rfc = $dbh->selectall_arrayref(q| + SELECT rfc4646_subtag, iso639_2_code FROM language_rfc4646_to_iso639 GROUP BY rfc4646_subtag, iso639_2_code HAVING COUNT(*) > 1 + |, { Slice => {} }); + if ( @$dupe_language_rfc ) { + warn "You have duplicated languages in the language_rfc4646_to_iso639 in your database, unique constraint cannot be added"; + } else { + $dbh->do(q{ + ALTER TABLE language_rfc4646_to_iso639 + ADD UNIQUE KEY uniq_code (rfc4646_subtag, iso639_2_code) + }); + } + }; + + $dbh->do(q{ + INSERT IGNORE INTO language_subtag_registry (subtag, type, description, added) + VALUES + ('et', 'language', 'Estonian', now()), + ('lv', 'language', 'Latvian', now()), + ('lt', 'language', 'Lithuanian', now()), + ('iu', 'language', 'Inuktitut', now()), + ('ik', 'language', 'Inupiaq', now()) + }); + + $dbh->do(q{ + INSERT IGNORE INTO language_descriptions (subtag, type, lang, description) + VALUES + ('et', 'language', 'en', 'Estonian'), + ('et', 'language', 'et', 'Eesti'), + ('lv', 'language', 'en', 'Latvian'), + ('lv', 'language', 'lv', 'Latvija'), + ('lt', 'language', 'en', 'Lithuanian'), + ('lt', 'language', 'lt', 'Lietuvių'), + ('iu', 'language', 'en', 'Inuktitut'), + ('iu', 'language', 'iu', 'ᐃᓄᒃᑎᑐᑦ'), + ('ik', 'language', 'en', 'Inupiaq'), + ('ik', 'language', 'ik', 'Iñupiaq') + }); + + $dbh->do(q{ + INSERT IGNORE INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) + VALUES + ('et', 'est'), + ('lv', 'lav'), + ('lt', 'lit'), + ('iu', 'iku'), + ('ik', 'ipk') + }); + + NewVersion( $DBversion, 15067, "Add missing languages"); +} diff --git a/installer/data/mysql/atomicupdate/bug_15067-add_newlanguages.sql b/installer/data/mysql/atomicupdate/bug_15067-add_newlanguages.sql deleted file mode 100644 index 0dd5d3da70..0000000000 --- a/installer/data/mysql/atomicupdate/bug_15067-add_newlanguages.sql +++ /dev/null @@ -1,34 +0,0 @@ --- Estonian - -INSERT INTO language_subtag_registry (subtag, type, description, added) VALUES ('et', 'language', 'Estonian', now()); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('et', 'language', 'en', 'Estonian'); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('et', 'language', 'et', 'Eesti'); -INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('et', 'est'); - --- Latvian - -INSERT INTO language_subtag_registry (subtag, type, description, added) VALUES ('lv', 'language', 'Latvian', now()); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('lv', 'language', 'en', 'Latvian'); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('lv', 'language', 'lv', 'Latvija'); -INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('lv', 'lav'); - --- Lithuanian - -INSERT INTO language_subtag_registry (subtag, type, description, added) VALUES ('lt', 'language', 'Lithuanian', now()); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('lt', 'language', 'en', 'Lithuanian'); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('lt', 'language', 'lt', 'Lietuvių'); -INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('lt', 'lit'); - --- Inuktitut - -INSERT INTO language_subtag_registry (subtag, type, description, added) VALUES ('iu', 'language', 'Inuktitut', now()); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('iu', 'language', 'en', 'Inuktitut'); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('iu', 'language', 'iu', 'ᐃᓄᒃᑎᑐᑦ'); -INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('iu', 'iku'); - --- Inupiaq - -INSERT INTO language_subtag_registry (subtag, type, description, added) VALUES ('ik', 'language', 'Inupiaq', now()); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('ik', 'language', 'en', 'Inupiaq'); -INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('ik', 'language', 'ik', 'Iñupiaq'); -INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('ik', 'ipk'); diff --git a/installer/data/mysql/mandatory/subtag_registry.sql b/installer/data/mysql/mandatory/subtag_registry.sql index 10aa5f216b..08761d9256 100644 --- a/installer/data/mysql/mandatory/subtag_registry.sql +++ b/installer/data/mysql/mandatory/subtag_registry.sql @@ -1861,7 +1861,7 @@ VALUES ('iu', 'language', 'Inuktitut', now()); INSERT INTO language_rfc4646_to_iso639 (rfc4646_subtag, iso639_2_code) VALUES ('iu', 'iku'); ---Inupiaq +-- Inupiaq INSERT INTO language_descriptions (subtag, type, lang, description) VALUES ('ik', 'language', 'en', 'Inupiaq'); diff --git a/t/db_dependent/Installer.t b/t/db_dependent/Installer.t index 8dfd916755..15e2985af8 100755 --- a/t/db_dependent/Installer.t +++ b/t/db_dependent/Installer.t @@ -22,14 +22,14 @@ # Add more tests here!!! use Modern::Perl; -use Test::More tests => 19; +use Test::More tests => 21; use File::Temp qw(tempfile); use utf8; use Koha::Database; BEGIN { - use_ok('C4::Installer', qw( column_exists index_exists foreign_key_exists primary_key_exists marc_framework_sql_list )); + use_ok('C4::Installer', qw( column_exists index_exists unique_key_exists foreign_key_exists primary_key_exists marc_framework_sql_list )); } ok( my $installer = C4::Installer->new(), 'Testing NewInstaller' ); @@ -70,6 +70,9 @@ ok( ! index_exists( 'borrowers', 'xxx'), 'Constraint xxx does not exist' ); ok( foreign_key_exists( 'borrowers', 'borrowers_ibfk_1' ), 'FK borrowers_ibfk_1 exists' ); ok( ! foreign_key_exists( 'borrowers', 'xxx' ), 'FK xxxx does not exist' ); +ok( unique_key_exists( 'items', 'itembarcodeidx' ), 'UNIQUE KEY itembarcodeidx exists' ); +ok( ! unique_key_exists( 'borrowers', 'xxx' ), 'UNIQUE KEY xxxx does not exist' ); + ok( primary_key_exists( 'borrowers', 'borrowernumber'), 'Borrowers has primary key on borrowernumber'); ok( ! primary_key_exists( 'borrowers', 'email'), 'Borrowers does not have a primary key on email'); -- 2.39.5