Bugzilla – Attachment 186634 Details for
Bug 19871
Use new exceptions Koha::Exceptions::Object::DuplicateID and FKConstraint
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 19871: Add centralized DBIx::Class exception translation system
Bug-19871-Add-centralized-DBIxClass-exception-tran.patch (text/plain), 17.84 KB, created by
Martin Renvoize (ashimema)
on 2025-09-21 11:54:21 UTC
(
hide
)
Description:
Bug 19871: Add centralized DBIx::Class exception translation system
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-09-21 11:54:21 UTC
Size:
17.84 KB
patch
obsolete
>From b532c583cb55deebd4c8cc523a1628b8dfd947a9 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Sun, 21 Sep 2025 12:33:30 +0100 >Subject: [PATCH] Bug 19871: Add centralized DBIx::Class exception translation > system >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This introduces a reusable exception translation utility that eliminates >code duplication and provides consistent handling of DBIx::Class exceptions >throughout the codebase. > >New Components: >- Koha::Schema::Util::ExceptionTranslator: Core utility class for translating > DBIx::Class exceptions to Koha-specific exceptions >- Koha::Schema: Enhanced with safe_do() and translate_exception() methods > for convenient access to centralized exception handling >- Comprehensive unit tests covering all translation scenarios > >Benefits: >- Eliminates ~90 lines of duplicated exception handling code >- Centralizes maintenance of exception translation logic >- Provides consistent error messages and exception types >- Makes it easier to extend support for additional database engines >- Follows the pattern suggested in existing FIXME comments > >Exception Types Handled: >- Foreign key constraint violations â Koha::Exceptions::Object::FKConstraint >- Duplicate key violations â Koha::Exceptions::Object::DuplicateID >- Invalid data type values â Koha::Exceptions::Object::BadValue >- Enum data truncation â Koha::Exceptions::Object::BadValue >--- > Koha/Object.pm | 52 ++---- > Koha/Schema.pm | 63 +++++++ > Koha/Schema/Util/ExceptionTranslator.pm | 165 ++++++++++++++++++ > .../Koha/Schema/Util/ExceptionTranslator.t | 154 ++++++++++++++++ > 4 files changed, 394 insertions(+), 40 deletions(-) > create mode 100644 Koha/Schema/Util/ExceptionTranslator.pm > create mode 100644 t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index c128391c74c..3e0bc01e924 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -173,54 +173,26 @@ sub store { > try { > return $self->_result()->update_or_insert() ? $self : undef; > } catch { >- >- # Catch problems and raise relevant exceptions >- if ( ref($_) eq 'DBIx::Class::Exception' ) { >- warn $_->{msg}; >- if ( $_->{msg} =~ /Cannot add or update a child row: a foreign key constraint fails/ ) { >- >- # FK constraints >- # FIXME: MySQL error, if we support more DB engines we should implement this for each >- if ( $_->{msg} =~ /FOREIGN KEY \(`(?<column>.*?)`\)/ ) { >- Koha::Exceptions::Object::FKConstraint->throw( >- error => 'Broken FK constraint', >- broken_fk => $+{column} >- ); >- } >- } elsif ( $_->{msg} =~ /Duplicate entry '(.*?)' for key '(?<key>.*?)'/ ) { >- Koha::Exceptions::Object::DuplicateID->throw( >- error => 'Duplicate ID', >- duplicate_id => $+{key} >- ); >- } elsif ( $_->{msg} =~ /Incorrect (?<type>\w+) value: '(?<value>.*)' for column \W?(?<property>\S+)/ ) >- { # The optional \W in the regex might be a quote or backtick >- my $type = $+{type}; >- my $value = $+{value}; >- my $property = $+{property}; >- $property =~ s/['`]//g; >- Koha::Exceptions::Object::BadValue->throw( >- type => $type, >- value => $value, >- property => $property =~ /(\w+\.\w+)$/ >- ? $1 >- : $property, # results in table.column without quotes or backtics >- ); >- } elsif ( $_->{msg} =~ /Data truncated for column \W?(?<property>\w+)/ ) >- { # The optional \W in the regex might be a quote or backtick >- my $property = $+{property}; >- my $type = $columns_info->{$property}->{data_type}; >+ # Use centralized exception translation >+ warn $_->{msg} if ref($_) eq 'DBIx::Class::Exception'; >+ >+ # For enum data truncation, we need to pass the object value which the utility can't access >+ if ( ref($_) eq 'DBIx::Class::Exception' && $_->{msg} =~ /Data truncated for column \W?(?<property>\w+)/ ) { >+ my $property = $+{property}; >+ my $type = $columns_info->{$property}->{data_type} // ''; >+ if ( $type eq 'enum' ) { > Koha::Exceptions::Object::BadValue->throw( > type => 'enum', > property => $property =~ /(\w+\.\w+)$/ > ? $1 >- : $property, # results in table.column without quotes or backtics >+ : $property, # results in table.column without quotes or backticks > value => $self->$property, >- ) if $type eq 'enum'; >+ ); > } > } > >- # Catch-all for foreign key breakages. It will help find other use cases >- $_->rethrow(); >+ # Delegate to centralized exception translation >+ $self->_result->result_source->schema->translate_exception($_, $columns_info); > } > } > >diff --git a/Koha/Schema.pm b/Koha/Schema.pm >index cffc631d5e1..3e2bb691807 100644 >--- a/Koha/Schema.pm >+++ b/Koha/Schema.pm >@@ -15,6 +15,69 @@ __PACKAGE__->load_namespaces; > # Created by DBIx::Class::Schema::Loader v0.07025 @ 2013-10-14 20:56:21 > # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:oDUxXckmfk6H9YCjW8PZTw > >+use Try::Tiny qw( catch try ); >+use Koha::Schema::Util::ExceptionTranslator; >+ >+=head1 UTILITY METHODS >+ >+=head2 safe_do >+ >+ $schema->safe_do(sub { >+ # DBIx::Class operations that might throw exceptions >+ $schema->resultset('SomeTable')->create(\%data); >+ }, $columns_info); >+ >+Execute a code block with automatic DBIx::Class exception translation. >+This provides a centralized way to handle database exceptions throughout the application. >+ >+=head3 Parameters >+ >+=over 4 >+ >+=item * C<$code_ref> - Code reference to execute >+ >+=item * C<$columns_info> (optional) - Hash reference of column information for enhanced error reporting >+ >+=back >+ >+=head3 Example Usage >+ >+ # Basic usage >+ $schema->safe_do(sub { >+ $register->_result->add_to_cash_register_actions(\%action_data); >+ }); >+ >+ # With column info for enhanced error reporting >+ my $columns_info = $register->_result->result_source->columns_info; >+ $schema->safe_do(sub { >+ $register->_result->create_related('some_relation', \%data); >+ }, $columns_info); >+ >+=cut >+ >+sub safe_do { >+ my ( $self, $code_ref, $columns_info ) = @_; >+ >+ try { >+ return $code_ref->(); >+ } catch { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($_, $columns_info); >+ }; >+} >+ >+=head2 translate_exception >+ >+ $schema->translate_exception($exception, $columns_info); >+ >+Convenience method that delegates to the ExceptionTranslator utility. >+This allows the schema to act as a central point for exception handling. >+ >+=cut >+ >+sub translate_exception { >+ my ( $self, $exception, $columns_info ) = @_; >+ return Koha::Schema::Util::ExceptionTranslator->translate_exception($exception, $columns_info); >+} > > # You can replace this text with custom content, and it will be preserved on regeneration > 1; >diff --git a/Koha/Schema/Util/ExceptionTranslator.pm b/Koha/Schema/Util/ExceptionTranslator.pm >new file mode 100644 >index 00000000000..590488c0d49 >--- /dev/null >+++ b/Koha/Schema/Util/ExceptionTranslator.pm >@@ -0,0 +1,165 @@ >+package Koha::Schema::Util::ExceptionTranslator; >+ >+# Copyright 2025 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 <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+ >+use Koha::Exceptions::Object; >+ >+=head1 NAME >+ >+Koha::Schema::Util::ExceptionTranslator - Centralized DBIx::Class exception translation >+ >+=head1 SYNOPSIS >+ >+ use Koha::Schema::Util::ExceptionTranslator; >+ >+ try { >+ # DBIx::Class operation that might fail >+ $schema->resultset('SomeTable')->create(\%data); >+ } catch { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($_, \%columns_info); >+ }; >+ >+=head1 DESCRIPTION >+ >+This utility class provides centralized exception translation from DBIx::Class >+exceptions to Koha-specific exceptions. This eliminates the need for duplicated >+exception handling code throughout the codebase. >+ >+=head1 METHODS >+ >+=head2 translate_exception >+ >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception, $columns_info); >+ >+Translates a DBIx::Class exception into an appropriate Koha exception and throws it. >+If the exception cannot be translated, it rethrows the original exception. >+ >+=head3 Parameters >+ >+=over 4 >+ >+=item * C<$exception> - The caught exception object >+ >+=item * C<$columns_info> (optional) - Hash reference of column information for enhanced error reporting >+ >+=back >+ >+=head3 Exception Types Handled >+ >+=over 4 >+ >+=item * Foreign key constraint violations â C<Koha::Exceptions::Object::FKConstraint> >+ >+=item * Duplicate key violations â C<Koha::Exceptions::Object::DuplicateID> >+ >+=item * Invalid data type values â C<Koha::Exceptions::Object::BadValue> >+ >+=item * Data truncation for enum columns â C<Koha::Exceptions::Object::BadValue> >+ >+=back >+ >+=cut >+ >+sub translate_exception { >+ my ( $class, $exception, $columns_info ) = @_; >+ >+ # Only handle DBIx::Class exceptions >+ return $exception->rethrow() unless ref($exception) eq 'DBIx::Class::Exception'; >+ >+ my $msg = $exception->{msg}; >+ >+ # Foreign key constraint failures >+ if ( $msg =~ /Cannot add or update a child row: a foreign key constraint fails/ ) { >+ # FIXME: MySQL error, if we support more DB engines we should implement this for each >+ if ( $msg =~ /FOREIGN KEY \(`(?<column>.*?)`\)/ ) { >+ Koha::Exceptions::Object::FKConstraint->throw( >+ error => 'Broken FK constraint', >+ broken_fk => $+{column} >+ ); >+ } >+ } >+ # Duplicate key violations >+ elsif ( $msg =~ /Duplicate entry '(.*?)' for key '(?<key>.*?)'/ ) { >+ Koha::Exceptions::Object::DuplicateID->throw( >+ error => 'Duplicate ID', >+ duplicate_id => $+{key} >+ ); >+ } >+ # Invalid data type values >+ elsif ( $msg =~ /Incorrect (?<type>\w+) value: '(?<value>.*)' for column \W?(?<property>\S+)/ ) { >+ # The optional \W in the regex might be a quote or backtick >+ my $type = $+{type}; >+ my $value = $+{value}; >+ my $property = $+{property}; >+ $property =~ s/['`]//g; >+ >+ Koha::Exceptions::Object::BadValue->throw( >+ type => $type, >+ value => $value, >+ property => $property =~ /(\w+\.\w+)$/ >+ ? $1 >+ : $property, # results in table.column without quotes or backticks >+ ); >+ } >+ # Data truncation for enum columns >+ elsif ( $msg =~ /Data truncated for column \W?(?<property>\w+)/ ) { >+ # The optional \W in the regex might be a quote or backtick >+ my $property = $+{property}; >+ >+ # Only handle enum truncation if we have column info >+ if ( $columns_info && $columns_info->{$property} ) { >+ my $type = $columns_info->{$property}->{data_type}; >+ if ( $type && $type eq 'enum' ) { >+ Koha::Exceptions::Object::BadValue->throw( >+ type => 'enum', >+ property => $property =~ /(\w+\.\w+)$/ >+ ? $1 >+ : $property, # results in table.column without quotes or backticks >+ value => 'Invalid enum value', # We don't have access to the object here >+ ); >+ } >+ } >+ } >+ >+ # Catch-all: rethrow the original exception if we can't translate it >+ $exception->rethrow(); >+} >+ >+=head1 FUTURE ENHANCEMENTS >+ >+This utility is designed to be extended to support: >+ >+=over 4 >+ >+=item * Multiple database engines (PostgreSQL, SQLite, etc.) >+ >+=item * Additional exception types as they are identified >+ >+=item * Enhanced error reporting with more context >+ >+=back >+ >+=head1 AUTHOR >+ >+Koha Development Team >+ >+=cut >+ >+1; >\ No newline at end of file >diff --git a/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t b/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t >new file mode 100644 >index 00000000000..c2c8d5e149e >--- /dev/null >+++ b/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t >@@ -0,0 +1,154 @@ >+#!/usr/bin/perl >+ >+# Copyright 2025 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 <http://www.gnu.org/licenses>. >+ >+use Modern::Perl; >+use Test::NoWarnings; >+use Test::More tests => 7; >+use Test::Exception; >+ >+use Koha::Database; >+use Koha::Schema::Util::ExceptionTranslator; >+ >+use t::lib::TestBuilder; >+ >+my $builder = t::lib::TestBuilder->new; >+my $schema = Koha::Database->new->schema; >+ >+subtest 'foreign_key_constraint_translation' => sub { >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a mock DBIx::Class::Exception for FK constraint >+ my $exception = bless { >+ msg => "Cannot add or update a child row: a foreign key constraint fails (`koha`.`items`, CONSTRAINT `items_ibfk_1` FOREIGN KEY (`biblionumber`) REFERENCES `biblio` (`biblionumber`))" >+ }, 'DBIx::Class::Exception'; >+ >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception); >+ } 'Koha::Exceptions::Object::FKConstraint', 'FK constraint exception is properly translated'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'duplicate_key_translation' => sub { >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a mock DBIx::Class::Exception for duplicate key >+ my $exception = bless { >+ msg => "Duplicate entry 'test\@example.com' for key 'borrowers.email'" >+ }, 'DBIx::Class::Exception'; >+ >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception); >+ } 'Koha::Exceptions::Object::DuplicateID', 'Duplicate key exception is properly translated'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'bad_value_translation' => sub { >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a mock DBIx::Class::Exception for bad value >+ my $exception = bless { >+ msg => "Incorrect datetime value: '2025-13-45' for column 'date_due' at row 1" >+ }, 'DBIx::Class::Exception'; >+ >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception); >+ } 'Koha::Exceptions::Object::BadValue', 'Bad value exception is properly translated'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'enum_truncation_translation' => sub { >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a mock DBIx::Class::Exception for enum truncation >+ my $exception = bless { >+ msg => "Data truncated for column 'status' at row 1" >+ }, 'DBIx::Class::Exception'; >+ >+ my $columns_info = { >+ status => { data_type => 'enum' } >+ }; >+ >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception, $columns_info); >+ } 'Koha::Exceptions::Object::BadValue', 'Enum truncation exception is properly translated'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'non_dbix_exception_passthrough' => sub { >+ plan tests => 1; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a regular exception (not DBIx::Class::Exception) >+ my $exception = bless { >+ msg => "Some other error" >+ }, 'Some::Other::Exception'; >+ >+ # Mock the rethrow method >+ $exception->{rethrown} = 0; >+ { >+ package Some::Other::Exception; >+ sub rethrow { $_[0]->{rethrown} = 1; die $_[0]; } >+ } >+ >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($exception); >+ } qr/Some::Other::Exception/, 'Non-DBIx::Class exceptions are rethrown unchanged'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+subtest 'schema_safe_do_method' => sub { >+ plan tests => 2; >+ >+ $schema->storage->txn_begin; >+ >+ # Test successful operation >+ my $result = $schema->safe_do(sub { >+ return "success"; >+ }); >+ is( $result, "success", 'safe_do returns result on success' ); >+ >+ # Test exception translation >+ throws_ok { >+ $schema->safe_do(sub { >+ # Create a mock DBIx::Class::Exception >+ my $exception = bless { >+ msg => "Duplicate entry 'test' for key 'primary'" >+ }, 'DBIx::Class::Exception'; >+ die $exception; >+ }); >+ } 'Koha::Exceptions::Object::DuplicateID', 'safe_do translates exceptions properly'; >+ >+ $schema->storage->txn_rollback; >+}; >+ >+1; >\ No newline at end of file >-- >2.51.0
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 19871
:
186634
|
186635
|
186644
|
186645
|
186646
|
186647
|
186648
|
186649
|
186650