Bugzilla – Attachment 186649 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: Move enum data truncation handling to centralized exception translation
Bug-19871-Move-enum-data-truncation-handling-to-ce.patch (text/plain), 7.93 KB, created by
Martin Renvoize (ashimema)
on 2025-09-21 15:15:13 UTC
(
hide
)
Description:
Bug 19871: Move enum data truncation handling to centralized exception translation
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2025-09-21 15:15:13 UTC
Size:
7.93 KB
patch
obsolete
>From 6a2f1464d2674782fcac22d33fdc409ffcb47496 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Sun, 21 Sep 2025 16:07:34 +0100 >Subject: [PATCH] Bug 19871: Move enum data truncation handling to centralized > exception translation > >Complete the migration of enum data truncation exception handling from >Koha::Object to the centralized ExceptionTranslator utility. This eliminates >the last remaining manual DBIx::Class exception handling in Object code. > >Changes: >- Add optional object parameter to ExceptionTranslator->translate_exception() >- Update Schema wrapper methods to accept and pass object parameter >- Enhance enum truncation handling to extract actual property values when object is available >- Remove manual enum exception handling from Koha::Object::store() >- Add comprehensive test coverage for enum handling with and without object context >- Maintain contextual warning behavior for store() operations > >The centralized system now handles ALL DBIx::Class exception scenarios: >- FK constraint failures (add/update and delete operations) >- Duplicate key violations >- Invalid data type values >- Enum data truncation with actual property values > >This completes the centralization effort while providing better error reporting >than the previous manual approach. >--- > Koha/Object.pm | 21 ++-------- > Koha/Schema.pm | 8 ++-- > Koha/Schema/Util/ExceptionTranslator.pm | 13 ++++-- > .../Koha/Schema/Util/ExceptionTranslator.t | 41 ++++++++++++++++++- > 4 files changed, 57 insertions(+), 26 deletions(-) > >diff --git a/Koha/Object.pm b/Koha/Object.pm >index b35658411f2..be6834bef71 100644 >--- a/Koha/Object.pm >+++ b/Koha/Object.pm >@@ -174,26 +174,11 @@ sub store { > return $self->_result()->update_or_insert() ? $self : undef; > } catch { > >- # Use centralized exception translation >+ # Log DBIx::Class exceptions for debugging (expected by tests) > 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 backticks >- value => $self->$property, >- ); >- } >- } >- >- # Delegate to centralized exception translation >- $self->_result->result_source->schema->translate_exception( $_, $columns_info ); >+ # Delegate to centralized exception translation with object context for enum handling >+ $self->_result->result_source->schema->translate_exception( $_, $columns_info, $self ); > } > } > >diff --git a/Koha/Schema.pm b/Koha/Schema.pm >index 3e2bb691807..f3036efd0fa 100644 >--- a/Koha/Schema.pm >+++ b/Koha/Schema.pm >@@ -56,12 +56,12 @@ This provides a centralized way to handle database exceptions throughout the app > =cut > > sub safe_do { >- my ( $self, $code_ref, $columns_info ) = @_; >+ my ( $self, $code_ref, $columns_info, $object ) = @_; > > try { > return $code_ref->(); > } catch { >- Koha::Schema::Util::ExceptionTranslator->translate_exception($_, $columns_info); >+ Koha::Schema::Util::ExceptionTranslator->translate_exception($_, $columns_info, $object); > }; > } > >@@ -75,8 +75,8 @@ 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); >+ my ( $self, $exception, $columns_info, $object ) = @_; >+ return Koha::Schema::Util::ExceptionTranslator->translate_exception($exception, $columns_info, $object); > } > > # You can replace this text with custom content, and it will be preserved on regeneration >diff --git a/Koha/Schema/Util/ExceptionTranslator.pm b/Koha/Schema/Util/ExceptionTranslator.pm >index 22586f7c76a..6740bbcb92b 100644 >--- a/Koha/Schema/Util/ExceptionTranslator.pm >+++ b/Koha/Schema/Util/ExceptionTranslator.pm >@@ -80,7 +80,7 @@ If the exception cannot be translated, it rethrows the original exception. > =cut > > sub translate_exception { >- my ( $class, $exception, $columns_info ) = @_; >+ my ( $class, $exception, $columns_info, $object ) = @_; > > # Only handle DBIx::Class exceptions > return $exception->rethrow() unless ref($exception) eq 'DBIx::Class::Exception'; >@@ -148,12 +148,19 @@ sub translate_exception { > if ( $columns_info && $columns_info->{$property} ) { > my $type = $columns_info->{$property}->{data_type}; > if ( $type && $type eq 'enum' ) { >+ my $value = 'Invalid enum value'; # Default value >+ >+ # If we have an object, try to get the actual property value >+ if ( $object && $object->can($property) ) { >+ eval { $value = $object->$property; }; >+ } >+ > 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 >+ : $property, # results in table.column without quotes or backticks >+ value => $value, > ); > } > } >diff --git a/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t b/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t >index 967808ffbc9..0a978a4124d 100755 >--- a/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t >+++ b/t/db_dependent/Koha/Schema/Util/ExceptionTranslator.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > use Test::NoWarnings; >-use Test::More tests => 8; >+use Test::More tests => 9; > use Test::Exception; > > use Koha::Database; >@@ -144,6 +144,45 @@ subtest 'fk_constraint_deletion_translation' => sub { > $schema->storage->txn_rollback; > }; > >+subtest 'enum_truncation_with_object_value' => sub { >+ plan tests => 2; >+ >+ $schema->storage->txn_begin; >+ >+ # Create a mock object with a property accessor >+ my $mock_object = bless { test_enum => 'invalid_value' }, 'TestObject'; >+ >+ # Add the can method to simulate a real object >+ { >+ no strict 'refs'; >+ *{"TestObject::can"} = sub { >+ my ( $self, $method ) = @_; >+ return $method eq 'test_enum' ? sub { return $self->{test_enum} } : undef; >+ }; >+ *{"TestObject::test_enum"} = sub { return $_[0]->{test_enum} }; >+ } >+ >+ # Create a mock DBIx::Class::Exception for enum data truncation >+ my $exception = bless { msg => "Data truncated for column 'test_enum'" }, 'DBIx::Class::Exception'; >+ >+ # Mock column info with enum type >+ my $columns_info = { test_enum => { data_type => 'enum' } }; >+ >+ # Test with object - should include the actual value >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception( $exception, $columns_info, $mock_object ); >+ } >+ 'Koha::Exceptions::Object::BadValue', 'Enum truncation with object throws BadValue exception'; >+ >+ # Test without object - should use default value >+ throws_ok { >+ Koha::Schema::Util::ExceptionTranslator->translate_exception( $exception, $columns_info ); >+ } >+ 'Koha::Exceptions::Object::BadValue', 'Enum truncation without object throws BadValue exception'; >+ >+ $schema->storage->txn_rollback; >+}; >+ > subtest 'schema_safe_do_method' => sub { > plan tests => 2; > >-- >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