gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 19871: Move FK constraint deletion handling to centralized exception translation 

Move the FK constraint deletion exception handling from Koha::Object::delete
to the centralized ExceptionTranslator utility. This consolidates all
DBIx::Class exception translation logic in one place and simplifies the
Object delete method.

Changes:
- Add FK constraint deletion pattern matching to ExceptionTranslator
- Remove manual exception handling from Koha::Object::delete
- Add test coverage for FK constraint deletion translation
- Simplify Object delete method to use centralized translation

The centralized system now handles both FK constraint scenarios:
- Child row constraint failures (add/update operations)
- Parent row constraint failures (delete operations)

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 2025-09-21 15:34:34 +01:00 committed by Lucas Gass
parent 17b117ca36
commit 41d39199b2
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
3 changed files with 88 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
Koha/Object.pm
View File

@ -173,6 +173,7 @@ sub store {
try {
return $self->_result()->update_or_insert() ? $self : undef;
} catch {
# Use centralized exception translation
warn $_->{msg} if ref($_) eq 'DBIx::Class::Exception';
@ -192,7 +193,7 @@ sub store {
}
# Delegate to centralized exception translation
$self->_result->result_source->schema->translate_exception($_, $columns_info);
$self->_result->result_source->schema->translate_exception( $_, $columns_info );
}
}
@ -238,20 +239,9 @@ sub delete {
try {
$deleted = $self->_result()->delete;
} catch {
if ( ref($_) eq 'DBIx::Class::Exception' ) {
if ( $_->{msg} =~
/Cannot delete or update a parent row\: a foreign key constraint fails \(\`(?<database>.*?)\`\.\`(?<table>.*?)\`, CONSTRAINT \`(?<constraint>.*?)\` FOREIGN KEY \(\`(?<fk>.*?)\`\) REFERENCES \`.*\` \(\`(?<column>.*?)\`\)/
)
{
Koha::Exceptions::Object::FKConstraintDeletion->throw(
column => $+{column},
constraint => $+{constraint},
fk => $+{fk},
table => $+{table},
);
}
$_->rethrow();
}
# Delegate to centralized exception translation
$self->_result->result_source->schema->translate_exception($_);
};
if ( ref $deleted ) {

31
Koha/Schema/Util/ExceptionTranslator.pm
View File

@ -89,6 +89,7 @@ sub translate_exception {
# 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(
@ -97,6 +98,20 @@ sub translate_exception {
);
}
}
# Foreign key constraint deletion failures (parent row deletion blocked)
elsif ( $msg =~
/Cannot delete or update a parent row\: a foreign key constraint fails \(\`(?<database>.*?)\`\.\`(?<table>.*?)\`, CONSTRAINT \`(?<constraint>.*?)\` FOREIGN KEY \(\`(?<fk>.*?)\`\) REFERENCES \`.*\` \(\`(?<column>.*?)\`\)/
)
{
Koha::Exceptions::Object::FKConstraintDeletion->throw(
column => $+{column},
constraint => $+{constraint},
fk => $+{fk},
table => $+{table},
);
}
# Duplicate key violations
elsif ( $msg =~ /Duplicate entry '(.*?)' for key '(?<key>.*?)'/ ) {
Koha::Exceptions::Object::DuplicateID->throw(
@ -104,8 +119,10 @@ sub translate_exception {
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};
@ -116,12 +133,14 @@ sub translate_exception {
type => $type,
value => $value,
property => $property =~ /(\w+\.\w+)$/
? $1
: $property, # results in table.column without quotes or backticks
? $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};
@ -132,9 +151,9 @@ sub translate_exception {
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
? $1
: $property, # results in table.column without quotes or backticks
value => 'Invalid enum value', # We don't have access to the object here
);
}
}
@ -164,4 +183,4 @@ Koha Development Team
=cut
1;
1;

95
t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t
View File

@ -19,7 +19,7 @@
use Modern::Perl;
use Test::NoWarnings;
use Test::More tests => 7;
use Test::More tests => 8;
use Test::Exception;
use Koha::Database;
@ -36,13 +36,15 @@ subtest 'foreign_key_constraint_translation' => sub {
$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';
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';
}
'Koha::Exceptions::Object::FKConstraint', 'FK constraint exception is properly translated';
$schema->storage->txn_rollback;
};
@ -53,13 +55,13 @@ subtest 'duplicate_key_translation' => sub {
$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';
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';
}
'Koha::Exceptions::Object::DuplicateID', 'Duplicate key exception is properly translated';
$schema->storage->txn_rollback;
};
@ -70,13 +72,13 @@ subtest 'bad_value_translation' => sub {
$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';
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';
}
'Koha::Exceptions::Object::BadValue', 'Bad value exception is properly translated';
$schema->storage->txn_rollback;
};
@ -87,17 +89,14 @@ subtest 'enum_truncation_translation' => sub {
$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 $exception = bless { msg => "Data truncated for column 'status' at row 1" }, 'DBIx::Class::Exception';
my $columns_info = {
status => { data_type => 'enum' }
};
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';
Koha::Schema::Util::ExceptionTranslator->translate_exception( $exception, $columns_info );
}
'Koha::Exceptions::Object::BadValue', 'Enum truncation exception is properly translated';
$schema->storage->txn_rollback;
};
@ -108,20 +107,39 @@ subtest 'non_dbix_exception_passthrough' => sub {
$schema->storage->txn_begin;
# Create a regular exception (not DBIx::Class::Exception)
my $exception = bless {
msg => "Some other error"
}, 'Some::Other::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';
}
qr/Some::Other::Exception/, 'Non-DBIx::Class exceptions are rethrown unchanged';
$schema->storage->txn_rollback;
};
subtest 'fk_constraint_deletion_translation' => sub {
plan tests => 1;
$schema->storage->txn_begin;
# Create a mock DBIx::Class::Exception for FK constraint deletion
my $exception =
bless { msg =>
"Cannot delete or update a parent 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::FKConstraintDeletion', 'FK constraint deletion exception is properly translated';
$schema->storage->txn_rollback;
};
@ -132,23 +150,26 @@ subtest 'schema_safe_do_method' => sub {
$schema->storage->txn_begin;
# Test successful operation
my $result = $schema->safe_do(sub {
return "success";
});
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->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;
1;