https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43663 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 206962 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=206962&action=edit Bug 43663: Add Koha::Objects::Mixin::SingleTableInheritance Three independent places in the Koha codebase have reinvented the same pattern - a Koha::Objects plural class that, given a row, decides which Koha::Object subclass should represent it, based on the value of one column: Koha::File::Transports (dispatching on 'transport'), Koha::Auth::Identity::Providers from bug 24880 (dispatching on 'protocol', not yet merged), and a variant discussed during the bug 42870 (pluggable email transports) review. Each reinvention is the same hand-rolled object_class method body and the same shape of _polymorphic_field/_polymorphic_map pair, with each subclass's own dispatch key typed out a second time by hand inside the subclass itself - nothing ties that string to the plural class's map entry it has to match, so the two can silently drift. This extracts the shared behaviour into a proper Koha::Objects::Mixin::* (a convention Koha already has and uses elsewhere, e.g. Koha::Objects::Mixin::AdditionalFields). A consuming plural class declares _sti_field and _sti_classes (an explicit, reviewed list - no namespace scanning or auto-discovery); each STI subclass self-declares its own dispatch key via _sti_key, so that key only ever needs to be written once. A consumer needing genuinely custom mapping logic can still override _sti_map directly - ordinary Perl method resolution makes that a free escape hatch. Named for Single Table Inheritance, the term most ORMs (Rails/ ActiveRecord, Doctrine, Hibernate) use for exactly this pattern - one table, one discriminator column, several subclasses. Deliberately not called "polymorphic": in Rails/ActiveRecord specifically, "polymorphic" already names a different pattern (a foreign key that can point at more than one parent table), and reusing that word here would mislead anyone coming from that background. A third method, _sti_base_class (the class to fall back to when no object is passed, or its value matches no known subclass), has a working default: it's inferred from the koha_object_class declared on the plural class's own DBIC result class - the same "what's the default row class for this table" fact Koha::Object's own row-to-class resolution already relies on elsewhere, so it rarely needs restating here. A consumer whose desired fallback genuinely differs can still override it directly. The mixin's dispatch logic is provided as _sti_object_class, not object_class itself: Koha::Objects already defines an empty object_class stub, and composed via `use parent qw( Koha::Objects Koha::Objects::Mixin::SingleTableInheritance )`, Perl's default depth-first MRO searches all of Koha::Objects' own ancestry before ever considering the second parent/mixin, so that empty stub always wins silently. Every consuming plural class must therefore define one required line of its own: `sub object_class { return shift->_sti_object_class(@_) }` - documented as mandatory in the mixin's own POD, not an optional convenience. Adds t/db_dependent/Koha/Objects/Mixin/SingleTableInheritance.t - the mixin's own contract tests. Rather than depend on a real production consumer (Koha::File::Transports isn't migrated onto the mixin until the next commit), the fixtures are self-contained: small test-only subclasses/plural classes, defined in the test file itself, that reuse the real file_transports table and the real, pre-existing Koha::File::Transport base class (both untouched by this commit). TestBuilder's build_object() can't be used for these, since it tries to Module::Load the class from a .pm on disk; build() plus a direct find() against the fixture plural class is used instead. Covers: the map built from _sti_classes/_sti_key, per-row dispatch, the inferred _sti_base_class default (both with no object and for an unrecognised value), and that overriding _sti_map directly in a consumer is still honoured. Also updates t/db_dependent/TestBuilder.t's generic "Test all classes" scan, which detects STI/polymorphic classes across the whole codebase by checking for a hand-rolled _polymorphic_field/_polymorphic_map pair. Broadened to also recognise this mixin's _sti_field/_sti_map, so Koha::File::Transports (migrated in the next commit) stays covered by this net instead of silently falling through to the "regular class" branch and failing (ref() would be a specific subclass, but object_class() with no argument correctly returns the base class). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.