From 979ff589d9c617ceade865797e6cbc5005c6f0f5 Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop Date: Tue, 19 Sep 2023 12:07:55 +0000 Subject: [PATCH] Bug 34587: Remove counter files from API embedding to improve performance Rather than fetching the counter files and embedding the counter logs, we now add a foreign key to the data provider in the counter logs table and fetch them directly. Signed-off-by: Jessica Zairo Signed-off-by: Michaela Sieber Signed-off-by: Nick Clemens Signed-off-by: Tomas Cohen Arazi --- Koha/ERM/EUsage/CounterFile.pm | 5 +- Koha/REST/V1/ERM/EUsage/CounterLogs.pm | 49 +++++++++++++++++++ Koha/REST/V1/ERM/EUsage/UsageDataProviders.pm | 19 +++++-- Koha/Schema/Result/ErmCounterLog.pm | 34 ++++++++++++- Koha/Schema/Result/ErmUsageDataProvider.pm | 21 +++++++- .../swagger/definitions/erm_counter_log.yaml | 5 ++ .../definitions/erm_usage_data_provider.yaml | 5 ++ api/v1/swagger/paths/erm_counter_logs.yaml | 49 +++++++++++++++++++ api/v1/swagger/swagger.yaml | 2 + .../erm_usage_statistics_tables.pl | 4 +- installer/data/mysql/kohastructure.sql | 4 +- ...sageStatisticsDataProvidersCounterLogs.vue | 20 ++++---- .../ERM/UsageStatisticsDataProvidersList.vue | 39 ++++++--------- 13 files changed, 209 insertions(+), 47 deletions(-) create mode 100644 Koha/REST/V1/ERM/EUsage/CounterLogs.pm create mode 100644 api/v1/swagger/paths/erm_counter_logs.yaml diff --git a/Koha/ERM/EUsage/CounterFile.pm b/Koha/ERM/EUsage/CounterFile.pm index 6534b1749d..e9ed5b52dc 100644 --- a/Koha/ERM/EUsage/CounterFile.pm +++ b/Koha/ERM/EUsage/CounterFile.pm @@ -515,7 +515,10 @@ sub _add_counter_log_entry { filename => $self->filename, #TODO: add eventual exceptions coming from the COUNTER report to logdetails? - logdetails => undef + logdetails => undef, + + # TEST: retrieving counter logs directly rather than embedding them in counter files requires the provider id + usage_data_provider_id => $self->usage_data_provider_id } )->store; } diff --git a/Koha/REST/V1/ERM/EUsage/CounterLogs.pm b/Koha/REST/V1/ERM/EUsage/CounterLogs.pm new file mode 100644 index 0000000000..41e5348f22 --- /dev/null +++ b/Koha/REST/V1/ERM/EUsage/CounterLogs.pm @@ -0,0 +1,49 @@ +package Koha::REST::V1::ERM::EUsage::CounterLogs; + +# Copyright 2023 PTFS Europe + +# 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 Mojo::Base 'Mojolicious::Controller'; + +use Koha::ERM::EUsage::CounterLogs; + +use Scalar::Util qw( blessed ); +use Try::Tiny qw( catch try ); + +=head1 API + +=head2 Methods + +=head3 list + +=cut + +sub list { + my $c = shift->openapi->valid_input or return; + + return try { + my $counter_logs_set = Koha::ERM::EUsage::CounterLogs->new; + my $counter_logs = $c->objects->search($counter_logs_set); + return $c->render( status => 200, openapi => $counter_logs ); + } catch { + $c->unhandled_exception($_); + }; +} + +1; diff --git a/Koha/REST/V1/ERM/EUsage/UsageDataProviders.pm b/Koha/REST/V1/ERM/EUsage/UsageDataProviders.pm index e863389729..8f93299334 100644 --- a/Koha/REST/V1/ERM/EUsage/UsageDataProviders.pm +++ b/Koha/REST/V1/ERM/EUsage/UsageDataProviders.pm @@ -24,6 +24,7 @@ use Mojo::Base 'Mojolicious::Controller'; use Koha::ERM::EUsage::UsageDataProviders; use Koha::ERM::EUsage::MonthlyUsages; +use Koha::ERM::EUsage::CounterFiles; use Scalar::Util qw( blessed ); use Try::Tiny qw( catch try ); @@ -38,14 +39,13 @@ use Try::Tiny qw( catch try ); sub list { my $c = shift->openapi->valid_input or return; - use Data::Dumper; return try { my $usage_data_providers_set = Koha::ERM::EUsage::UsageDataProviders->new; my $usage_data_providers = $c->objects->search($usage_data_providers_set); - if ( $c->validation->output->{"x-koha-embed"}[0] - && $c->validation->output->{"x-koha-embed"}[0] eq 'counter_files' ) - { + # if ( $c->validation->output->{"x-koha-embed"}[0] + # && $c->validation->output->{"x-koha-embed"}[0] eq 'counter_files' ) + # { foreach my $provider (@$usage_data_providers) { my $title_dates = _get_earliest_and_latest_dates( 'TR', @@ -96,8 +96,17 @@ sub list { $database_dates->{latest_date} ? $database_dates->{latest_date} : ''; + + my @last_run = Koha::ERM::EUsage::CounterFiles->search( + { + usage_data_provider_id => $provider->{erm_usage_data_provider_id}, + }, + { columns => [ { date_uploaded => { max => "date_uploaded" } }, ] } + )->unblessed; + $provider->{last_run} = $last_run[0][0]->{date_uploaded} ? $last_run[0][0]->{date_uploaded} : ''; + } - } + # } return $c->render( status => 200, openapi => $usage_data_providers ); } catch { diff --git a/Koha/Schema/Result/ErmCounterLog.pm b/Koha/Schema/Result/ErmCounterLog.pm index 03bf162701..aa971fa037 100644 --- a/Koha/Schema/Result/ErmCounterLog.pm +++ b/Koha/Schema/Result/ErmCounterLog.pm @@ -47,6 +47,14 @@ foreign key to borrowers foreign key to erm_counter_files +=head2 usage_data_provider_id + + data_type: 'integer' + is_foreign_key: 1 + is_nullable: 1 + +foreign key to erm_usage_data_providers + =head2 importdate data_type: 'timestamp' @@ -80,6 +88,8 @@ __PACKAGE__->add_columns( { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, "counter_files_id", { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, + "usage_data_provider_id", + { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, "importdate", { data_type => "timestamp", @@ -147,9 +157,29 @@ __PACKAGE__->belongs_to( }, ); +=head2 usage_data_provider + +Type: belongs_to + +Related object: L + +=cut + +__PACKAGE__->belongs_to( + "usage_data_provider", + "Koha::Schema::Result::ErmUsageDataProvider", + { erm_usage_data_provider_id => "usage_data_provider_id" }, + { + is_deferrable => 1, + join_type => "LEFT", + on_delete => "CASCADE", + on_update => "CASCADE", + }, +); + -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-03-16 17:38:56 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:v22verlpwR3+7qLwsxJjtw +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-11 10:09:06 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:5tmoa4Our5qmolU0OWXjcQ sub koha_object_class { diff --git a/Koha/Schema/Result/ErmUsageDataProvider.pm b/Koha/Schema/Result/ErmUsageDataProvider.pm index c1360a4cff..a65fa28789 100644 --- a/Koha/Schema/Result/ErmUsageDataProvider.pm +++ b/Koha/Schema/Result/ErmUsageDataProvider.pm @@ -208,6 +208,23 @@ __PACKAGE__->has_many( { cascade_copy => 0, cascade_delete => 0 }, ); +=head2 erm_counter_logs + +Type: has_many + +Related object: L + +=cut + +__PACKAGE__->has_many( + "erm_counter_logs", + "Koha::Schema::Result::ErmCounterLog", + { + "foreign.usage_data_provider_id" => "self.erm_usage_data_provider_id", + }, + { cascade_copy => 0, cascade_delete => 0 }, +); + =head2 erm_usage_databases Type: has_many @@ -311,8 +328,8 @@ __PACKAGE__->has_many( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-08-09 11:00:27 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:B99vhhVjbyR/7VoGHcrnFA +# Created by DBIx::Class::Schema::Loader v0.07049 @ 2023-10-11 10:09:06 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:MSvyv/Abt3S3fqZF21v/7w # __PACKAGE__->add_columns( # '+active' => { is_boolean => 1 } diff --git a/api/v1/swagger/definitions/erm_counter_log.yaml b/api/v1/swagger/definitions/erm_counter_log.yaml index 897dcee683..2eb130181c 100644 --- a/api/v1/swagger/definitions/erm_counter_log.yaml +++ b/api/v1/swagger/definitions/erm_counter_log.yaml @@ -29,6 +29,11 @@ properties: type: - string - "null" + usage_data_provider_id: + description: logdetails of the counter_log + type: + - integer + - "null" additionalProperties: false required: diff --git a/api/v1/swagger/definitions/erm_usage_data_provider.yaml b/api/v1/swagger/definitions/erm_usage_data_provider.yaml index f1c11bd3b0..baa6c5d092 100644 --- a/api/v1/swagger/definitions/erm_usage_data_provider.yaml +++ b/api/v1/swagger/definitions/erm_usage_data_provider.yaml @@ -143,6 +143,11 @@ properties: type: - string - "null" + last_run: + description: last time the harvester was run + type: + - string + - "null" additionalProperties: false required: diff --git a/api/v1/swagger/paths/erm_counter_logs.yaml b/api/v1/swagger/paths/erm_counter_logs.yaml new file mode 100644 index 0000000000..8ef053165d --- /dev/null +++ b/api/v1/swagger/paths/erm_counter_logs.yaml @@ -0,0 +1,49 @@ +/erm/counter_logs: + get: + x-mojo-to: ERM::EUsage::CounterLogs#list + operationId: listErmCounterLogs + tags: + - counter_log + summary: List counter_logs + produces: + - application/json + parameters: + - description: Case insensitive search on counter_log usage_data_provider_id + in: query + name: usage_data_provider_id + required: false + type: integer + - $ref: "../swagger.yaml#/parameters/match" + - $ref: "../swagger.yaml#/parameters/order_by" + - $ref: "../swagger.yaml#/parameters/page" + - $ref: "../swagger.yaml#/parameters/per_page" + - $ref: "../swagger.yaml#/parameters/q_param" + - $ref: "../swagger.yaml#/parameters/q_body" + responses: + 200: + description: A list of counter_logs + schema: + items: + $ref: "../swagger.yaml#/definitions/erm_counter_log" + type: array + 400: + description: Bad request + schema: + $ref: "../swagger.yaml#/definitions/error" + 403: + description: Access forbidden + schema: + $ref: "../swagger.yaml#/definitions/error" + 500: + description: |- + Internal server error. Possible `error_code` attribute values: + * `internal_server_error` + schema: + $ref: "../swagger.yaml#/definitions/error" + 503: + description: Under maintenance + schema: + $ref: "../swagger.yaml#/definitions/error" + x-koha-authorization: + permissions: + erm: 1 \ No newline at end of file diff --git a/api/v1/swagger/swagger.yaml b/api/v1/swagger/swagger.yaml index 9f067d3f3c..7cebf43089 100644 --- a/api/v1/swagger/swagger.yaml +++ b/api/v1/swagger/swagger.yaml @@ -275,6 +275,8 @@ paths: $ref: "./paths/erm_counter_files.yaml#/~1erm~1counter_files~1{erm_counter_files_id}" "/erm/counter_files/{erm_counter_files_id}/file/content": $ref: "./paths/erm_counter_files.yaml#/~1erm~1counter_files~1{erm_counter_files_id}~1file~1content" + /erm/counter_logs: + $ref: ./paths/erm_counter_logs.yaml#/~1erm~1counter_logs /erm/counter_registry: $ref: ./paths/erm_counter_registries.yaml#/~1erm~1counter_registry /erm/default_usage_reports: diff --git a/installer/data/mysql/atomicupdate/erm_usage_statistics_tables.pl b/installer/data/mysql/atomicupdate/erm_usage_statistics_tables.pl index f513c48429..a4014b99bb 100644 --- a/installer/data/mysql/atomicupdate/erm_usage_statistics_tables.pl +++ b/installer/data/mysql/atomicupdate/erm_usage_statistics_tables.pl @@ -64,12 +64,14 @@ return { `erm_counter_log_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'primary key', `borrowernumber` int(11) DEFAULT NULL COMMENT 'foreign key to borrowers', `counter_files_id` int(11) DEFAULT NULL COMMENT 'foreign key to erm_counter_files', + `usage_data_provider_id` int(11) DEFAULT NULL COMMENT 'foreign key to erm_usage_data_providers', `importdate` timestamp DEFAULT NULL DEFAULT current_timestamp() COMMENT 'counter file import date', `filename` varchar(80) DEFAULT NULL COMMENT 'name of the counter file', `logdetails` longtext DEFAULT NULL COMMENT 'details from the counter log', PRIMARY KEY (`erm_counter_log_id`), CONSTRAINT `erm_counter_log_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `erm_counter_log_ibfk_2` FOREIGN KEY (`counter_files_id`) REFERENCES `erm_counter_files` (`erm_counter_files_id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `erm_counter_log_ibfk_2` FOREIGN KEY (`counter_files_id`) REFERENCES `erm_counter_files` (`erm_counter_files_id`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `erm_counter_log_ibfk_3` FOREIGN KEY (`usage_data_provider_id`) REFERENCES `erm_usage_data_providers` (`erm_usage_data_provider_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; } ); diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 96bfdeb9fd..b4ce0c21a3 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2959,12 +2959,14 @@ CREATE TABLE `erm_counter_logs` ( `erm_counter_log_id` int(11) NOT NULL AUTO_INCREMENT COMMENT 'primary key', `borrowernumber` int(11) DEFAULT NULL COMMENT 'foreign key to borrowers', `counter_files_id` int(11) DEFAULT NULL COMMENT 'foreign key to erm_counter_files', + `usage_data_provider_id` int(11) DEFAULT NULL COMMENT 'foreign key to erm_usage_data_providers', `importdate` timestamp DEFAULT NULL DEFAULT current_timestamp() COMMENT 'counter file import date', `filename` varchar(80) DEFAULT NULL COMMENT 'name of the counter file', `logdetails` longtext DEFAULT NULL COMMENT 'details from the counter log', PRIMARY KEY (`erm_counter_log_id`), CONSTRAINT `erm_counter_log_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE ON UPDATE CASCADE, - CONSTRAINT `erm_counter_log_ibfk_2` FOREIGN KEY (`counter_files_id`) REFERENCES `erm_counter_files` (`erm_counter_files_id`) ON DELETE CASCADE ON UPDATE CASCADE + CONSTRAINT `erm_counter_log_ibfk_2` FOREIGN KEY (`counter_files_id`) REFERENCES `erm_counter_files` (`erm_counter_files_id`) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT `erm_counter_log_ibfk_3` FOREIGN KEY (`usage_data_provider_id`) REFERENCES `erm_usage_data_providers` (`erm_usage_data_provider_id`) ON DELETE CASCADE ON UPDATE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersCounterLogs.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersCounterLogs.vue index 3ae5372f12..9545d6d462 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersCounterLogs.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersCounterLogs.vue @@ -40,7 +40,7 @@ export default { building_table: false, tableOptions: { columns: this.getTableColumns(), - options: { embed: "counter_logs" }, + options: {}, url: () => this.table_url(), table_settings: this.counter_log_table_settings, add_filters: true, @@ -77,7 +77,7 @@ export default { ) }, table_url() { - let url = `/api/v1/erm/counter_files?usage_data_provider_id=${this.$route.params.usage_data_provider_id}` + let url = `/api/v1/erm/counter_logs?usage_data_provider_id=${this.$route.params.usage_data_provider_id}` return url }, download_counter_file(counter_log, dt, event) { @@ -86,25 +86,25 @@ export default { counter_log.counter_files_id + "/file/content" }, - delete_counter_file(counter_file, dt, event) { + delete_counter_file(counter_log, dt, event) { this.setConfirmationDialog( { title: this.$__( "Are you sure you want to remove this file?" ), - message: counter_file.filename, + message: counter_log.filename, accept_label: this.$__("Yes, delete"), cancel_label: this.$__("No, do not delete"), }, () => { const client = APIClient.erm client.counter_files - .delete(counter_file.erm_counter_files_id) + .delete(counter_log.counter_files_id) .then( success => { this.setMessage( this.$__("File %s deleted").format( - counter_file.filename + counter_log.filename ), true ) @@ -126,8 +126,8 @@ export default { { title: __("Import date"), render: function (data, type, row, meta) { - const date = row.date_uploaded.substr(0, 10) - const time = row.date_uploaded.substr(11, 8) + const date = row.importdate.substr(0, 10) + const time = row.importdate.substr(11, 8) return `${date} ${time}` }, searchable: true, @@ -136,8 +136,8 @@ export default { { title: __("Imported by"), render: function (data, type, row, meta) { - const importer = row.counter_logs[0].borrowernumber - ? `Borrowernumber ${row.counter_logs[0].borrowernumber}` + const importer = row.borrowernumber + ? `Borrowernumber ${row.borrowernumber}` : "Cronjob" return importer }, diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue index 115b0131c3..7ebc4bcecb 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/UsageStatisticsDataProvidersList.vue @@ -66,7 +66,7 @@ export default { building_table: false, tableOptions: { columns: this.getTableColumns(), - options: { embed: "counter_files" }, + options: {}, url: () => this.table_url(), table_settings: this.usage_data_provider_table_settings, add_filters: true, @@ -172,8 +172,16 @@ export default { () => { usage_data_provider.active = 1 delete usage_data_provider.erm_usage_data_provider_id - const counter_files = usage_data_provider.counter_files - delete usage_data_provider.counter_files + // const counter_files = usage_data_provider.counter_files + // delete usage_data_provider.counter_files + delete usage_data_provider.earliest_platform + delete usage_data_provider.latest_platform + delete usage_data_provider.earliest_title + delete usage_data_provider.latest_title + delete usage_data_provider.earliest_database + delete usage_data_provider.latest_database + delete usage_data_provider.earliest_item + delete usage_data_provider.latest_item const client = APIClient.erm client.usage_data_providers .update(usage_data_provider, id) @@ -185,8 +193,8 @@ export default { ).format(name), true ) - usage_data_provider.counter_files = - counter_files + // usage_data_provider.counter_files = + // counter_files dt.draw() }, error => {} @@ -319,26 +327,7 @@ export default { }, { title: __("Last run"), - render: function (data, type, row, meta) { - const counter_files = row.counter_files - if (counter_files.length === 0) { - return "No run history found" - } - const findMostRecent = counter_files.sort( - (a, b) => - new Date(b.date_uploaded) - - new Date(a.date_uploaded) - ) - const date = findMostRecent[0].date_uploaded.substr( - 0, - 10 - ) - const time = findMostRecent[0].date_uploaded.substr( - 11, - 8 - ) - return `${date} ${time}` - }, + data: "last_run", searchable: true, orderable: true, }, -- 2.39.5