From e306fa1091e512d3aff21f22653e8371cf3c3364 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 14 Dec 2023 07:54:22 +0000 Subject: [PATCH] Bug 35536: Remove Method->delete and reorder rollbacks Content-Type: text/plain; charset=utf-8 We removed the need for Method->delete, so we can remove that. Also we should now move all RemovePlugins calls BEFORE the last rollback. Nice and clean :) Test plan: Search for wrong order with: grep -Pzo "txn_rollback;\n.*RemovePlugins" $(git grep -l RemovePlugins) No occurrences left? Think of another grep :) Check number of records in plugin_data/methods. Repeat: prove $(git grep -l Koha::Plugin | grep -P "^t\/db") And check number of records again. Same? Bonus: Install a plugin before following test plan. You could use the attached 'DONT PUSH' example patch. Having 3 recs in data and 45 in methods with that plugin only (before/after tests). Signed-off-by: Marcel de Rooy --- Koha/Plugins/Method.pm | 24 ------ t/db_dependent/Koha/Plugins/BackgroundJob.t | 2 +- .../Koha/Plugins/Barcode_transform_hooks.t | 2 +- .../Plugins/Biblio_and_Items_plugin_hooks.t | 2 +- .../Koha/Plugins/Circulation_hooks.t | 2 +- t/db_dependent/Koha/Plugins/Method.t | 73 ------------------- t/db_dependent/Koha/Plugins/Patron.t | 4 +- t/db_dependent/Koha/Plugins/authority_hooks.t | 2 +- 8 files changed, 6 insertions(+), 105 deletions(-) delete mode 100755 t/db_dependent/Koha/Plugins/Method.t diff --git a/Koha/Plugins/Method.pm b/Koha/Plugins/Method.pm index 0d66712708..ceda593f39 100644 --- a/Koha/Plugins/Method.pm +++ b/Koha/Plugins/Method.pm @@ -17,10 +17,6 @@ package Koha::Plugins::Method; use Modern::Perl; -use Koha::Database; -use Koha::Cache::Memory::Lite; -use Koha::Plugins; - use base qw(Koha::Object); =head1 NAME @@ -31,26 +27,6 @@ Koha::Plugin::Method - Koha Plugin Method Object class =head2 Class Methods -=head3 delete - - Delete method record. - But also delete associated plugin_data records. - Clear cache key. - -=cut - -sub delete { - my ($self) = @_; - - # Remove corresponding records from plugin_data. Clear cache. - my $cond = { plugin_class => $self->plugin_class }; - Koha::Database->new->schema->resultset('PluginData')->search($cond)->delete; - Koha::Cache::Memory::Lite->clear_from_cache( Koha::Plugins->ENABLED_PLUGINS_CACHE_KEY ); - - # Now do the actual delete - return $self->SUPER::delete; -} - =head3 type =cut diff --git a/t/db_dependent/Koha/Plugins/BackgroundJob.t b/t/db_dependent/Koha/Plugins/BackgroundJob.t index a4c8e58e29..c5ccd3763f 100755 --- a/t/db_dependent/Koha/Plugins/BackgroundJob.t +++ b/t/db_dependent/Koha/Plugins/BackgroundJob.t @@ -84,6 +84,6 @@ subtest 'background_tasks() hooks tests' => sub { is_deeply( $tasks, $bj->core_types_to_classes, 'Only core mapping returned when plugins disabled' ); - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t index 202f1bfdd9..234f7d572f 100755 --- a/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t +++ b/t/db_dependent/Koha/Plugins/Barcode_transform_hooks.t @@ -99,6 +99,6 @@ subtest 'patron_barcode_transform() and item_barcode_transform() hook tests' => qr/Plugin error \(Test Plugin\): Exception 'Koha::Exception' thrown 'item_barcode_transform called with parameter: THISISATEST'/, 'Koha::Item->store calls the item_barcode_transform hook'; - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t index 1ca1d5116e..7f818dfd48 100755 --- a/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t +++ b/t/db_dependent/Koha/Plugins/Biblio_and_Items_plugin_hooks.t @@ -83,6 +83,6 @@ subtest 'after_biblio_action() and after_item_action() hooks tests' => sub { qr/after_biblio_action called with action: delete, id: $biblio_id/, 'DelBiblio calls the hook with action=delete biblio_id passed'; - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Plugins/Circulation_hooks.t b/t/db_dependent/Koha/Plugins/Circulation_hooks.t index 02911a01c5..47ac5d58af 100755 --- a/t/db_dependent/Koha/Plugins/Circulation_hooks.t +++ b/t/db_dependent/Koha/Plugins/Circulation_hooks.t @@ -120,6 +120,6 @@ subtest 'after_circ_action() hook tests' => sub { 'AddReturn calls the after_circ_action hook'; }; - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; diff --git a/t/db_dependent/Koha/Plugins/Method.t b/t/db_dependent/Koha/Plugins/Method.t deleted file mode 100755 index 6559926518..0000000000 --- a/t/db_dependent/Koha/Plugins/Method.t +++ /dev/null @@ -1,73 +0,0 @@ -#!/usr/bin/perl - -# Copyright 2023 Rijksmuseum, Koha development team -# -# This file is part of Koha -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use Modern::Perl; -use Test::MockModule; -use Test::More tests => 1; - -use t::lib::TestBuilder; - -use Koha::Database; -use Koha::Cache::Memory::Lite; -use Koha::Plugins; -use Koha::Plugins::Methods; - -my $builder = t::lib::TestBuilder->new; -my $schema = Koha::Database->new->schema; - -$schema->storage->txn_begin; - -# We need to mock can_load from Module::Load::Conditional in Koha/Plugins -my $mock = Test::MockModule->new('Koha::Plugins')->mock( can_load => 1 ); - -subtest 'delete' => sub { - plan tests => 7; - - my $count_m = Koha::Plugins::Methods->count; - my $count_d = $schema->resultset('PluginData')->count; - Koha::Cache::Memory::Lite->get_instance->clear_from_cache( Koha::Plugins->ENABLED_PLUGINS_CACHE_KEY ); - my @enabled_plugins = Koha::Plugins->get_enabled_plugins; - - # Mock creation of new plugin - my $method = $builder->build_object( { class => 'Koha::Plugins::Methods' } ); - my $values = { plugin_class => $method->plugin_class, plugin_key => '__ENABLED__', plugin_value => 1 }; - my $data = $builder->build( { source => 'PluginData', value => $values } ); - - # Note: no_auto => 1 in next call prevent loading of a not-existing module - my $plugin_mock = Test::MockModule->new( $method->plugin_class, no_auto => 1 ) - ->mock( new => bless( { mocked => 1 }, $method->plugin_class ) ); - - # Check for new method and if get_enabled_plugins sees it too - is( Koha::Plugins::Methods->count, $count_m + 1, 'Expect a new method' ); - is( $schema->resultset('PluginData')->count, $count_d + 1, 'Expect entry in plugin_data' ); - Koha::Cache::Memory::Lite->get_instance->clear_from_cache( Koha::Plugins->ENABLED_PLUGINS_CACHE_KEY ); - is( scalar Koha::Plugins->get_enabled_plugins, 1 + @enabled_plugins, 'Recheck get_enabled_plugins' ); - - # Delete the method from plugin_methods, check table and cache - Koha::Plugins::Methods->search( { plugin_class => $method->plugin_class } )->delete; - is( - Koha::Cache::Memory::Lite->get_instance->get_from_cache( Koha::Plugins->ENABLED_PLUGINS_CACHE_KEY ), - undef, 'Cache has been cleared by delete method' - ); - is( Koha::Plugins::Methods->count, $count_m, 'Original number of methods' ); - is( $schema->resultset('PluginData')->count, $count_d, 'Original count in plugin_data' ); - is( scalar Koha::Plugins->get_enabled_plugins, scalar @enabled_plugins, 'Original number of enabled plugins' ); -}; - -$schema->storage->txn_rollback; diff --git a/t/db_dependent/Koha/Plugins/Patron.t b/t/db_dependent/Koha/Plugins/Patron.t index 363a2fa0e3..eb001c8881 100755 --- a/t/db_dependent/Koha/Plugins/Patron.t +++ b/t/db_dependent/Koha/Plugins/Patron.t @@ -87,8 +87,6 @@ subtest 'check_password hook tests' => sub { ok( $patron->set_password({ password => '4321' }), 'Patron password updated with good string' ); ok( $patron->set_password({ password => 'explosion', skip_validation => 1}), 'Patron password updated via skip validation'); - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; - -1; diff --git a/t/db_dependent/Koha/Plugins/authority_hooks.t b/t/db_dependent/Koha/Plugins/authority_hooks.t index 464fb16262..959795d2f8 100755 --- a/t/db_dependent/Koha/Plugins/authority_hooks.t +++ b/t/db_dependent/Koha/Plugins/authority_hooks.t @@ -65,6 +65,6 @@ subtest 'after_authority_action hook' => sub { qr/after_authority_action called with action: delete, id: $id/, 'DelAuthority calls the hook with action=delete, id passed'; - $schema->storage->txn_rollback; Koha::Plugins->RemovePlugins; + $schema->storage->txn_rollback; }; -- 2.30.2