Bug 19871: Add centralized DBIx::Class exception translation system
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 Sponsored-by: OpenFifth <https://openfifth.co.uk> Signed-off-by: Jackie Usher <jackie.usher@westsussex.gov.uk> Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Martin Renvoize
Lucas Gass
parent
5f442507f2
commit
eb4a781da3
|
|
@ -173,54 +173,26 @@ sub store {
|
|||
try {
|
||||
return $self->_result()->update_or_insert() ? $self : undef;
|
||||
} catch {
|
||||
# Use centralized exception translation
|
||||
warn $_->{msg} if ref($_) eq 'DBIx::Class::Exception';
|
||||
|
||||
# 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};
|
||||
# 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);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,167 @@
|
|||
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;
|
||||
|
||||
=encoding utf8
|
||||
|
||||
=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;
|
||||
|
|
@ -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;
|
||||
Loading…
Reference in New Issue