Dagfinn Ilmari Mannsåker [Mon, 23 Jan 2017 11:58:18 +0000]
Fix typo in search_literal POD
Matt S Trout [Thu, 8 Dec 2016 18:38:46 +0000]
Introduce GOVERNANCE document and empty RESOLUTIONS file.
To understand the process that lead to this commit, you'll probably need
to read all of:
http://lists.scsys.co.uk/pipermail/dbix-class/2016-October/date.html
http://lists.scsys.co.uk/pipermail/dbix-class/2016-November/date.html
http://lists.scsys.co.uk/pipermail/dbix-class/2016-December/date.html
Any attempt on my part to summarise it would likely seem insufficiently
accurate to me and biased to at least some readers, so I'm not even going
to pretend to try. If you're trying to achieve a tl;dr, I suggest checking
the December archive in the hopes that somebody posts a summary there some
time after I push this commit.
Peter Rabbitson [Fri, 30 Sep 2016 16:50:58 +0000]
Merge the last bits of indirect callchain optimization
This set of commits (again - merge for easier bisect) is exclusively dealing
with various wantarray()-aware methods, most notably ::ResultSet::search()
Wide smoke of downstream adds only 3 extra dists to the list of "passes tests
but warns about indirect-sugar overrides" as shown in
12e7015a. In the cases
below all overrides are that of search() - a rather legitimate problem to be
warning about
Catalyst::Controller::DBIC::API
DBIx::Class::Helpers
DBIx::Class::ResultSet::AccessorsEverywhere
No other known breakage as of this commit
Peter Rabbitson [Tue, 27 Sep 2016 18:20:13 +0000]
Simplify internal implementation of as_subselect_rs
Zero functional changes
Peter Rabbitson [Thu, 22 Sep 2016 09:46:18 +0000]
Audit and minimize use of last major indirect method: search()
I am not entirely sure how I missed it during
1b822bd3, but oh well. This
should be the last highly volatile part ( as far as downstream is concerned ).
As previously - zero functional changes apart from no longer calling search()
at several spots (the SanityChecker ensures none of this results in silent
breakage)
All spots that *do* require wantarray()-specific behavior remained explicit
wrappers for search(), instead of doing the wantarray() check themselves: this
is a deliberate choice to allow DBIC::Helpers::ResultSet::IgnoreWantarray or
similar libraries to continue operating by simply hooking the search() method
Peter Rabbitson [Tue, 27 Sep 2016 17:49:19 +0000]
Simplify guarded pass-through added to CDBI in
ee333775
Peter Rabbitson [Fri, 30 Sep 2016 07:08:55 +0000]
Retire the ASSERT_NO_INTERNAL_WANTARRAY macro
It was a good idea for its time, and helped clean up the codebase a lot, but
ASSERT_NO_INTERNAL_INDIRECT_CALLS currently covers all its functionality and
does so in a way less fragile (stateless) manner
Mark several more methods as indirect_sugar, leaving only one forgotten spot
for last (see next commit)
No functional changes
Read under -w
Peter Rabbitson [Fri, 30 Sep 2016 13:15:36 +0000]
Audit and annotate all context-sensitive spots in ::Ordered
Ensure an upcoming commit will not disturb the established (silly but still)
API of the resultset-returning methods. Review, annotate and tighten up spots
that have to do with wantarray-like behavior
Not using the ASSERT_NO_INTERNAL_WANTARRAY macro as it is about to be retired
in a subsequent commit. Instead adjust the INDIRECT guard to correctly interpret
eval frames
Zero functional changes
Peter Rabbitson [Wed, 28 Sep 2016 09:25:47 +0000]
Restore the context sensitive m2m helper calling of ->search
Subtly modified in
11e469d9, this prevents things like the ::IgnoreWantarray
helper from taking effect in this case.
An audit of all other wantarray() invoking sites did not reveal other issues
Peter Rabbitson [Thu, 29 Sep 2016 10:59:27 +0000]
Mark forgotten ::Row::id() method as indirect_sugar
Discouraged legacy sugar, which does not even work properly with multicolumn
keys in scalar context. Mark properly as INDIRECT to ensure DBIC does not rely
on it anywhere
Also adjust the SanityChecker to not complain about shadowing of sugar methods
with generated ones (i.e. column accessors) - while unfortunate, this kind of
thing happens quite often (especially with such a generic name as 'id') and
warning about it would make no sense (left alone that methods which are
..._generated_from_resultsource_metadata generally do not invoke next::method
anyway)
Peter Rabbitson [Fri, 30 Sep 2016 06:36:35 +0000]
Tighten up code in ResultSetColumns, add INDIRECT annotations
No functional changes (nothing else in lib/ and t/ had to change)
Peter Rabbitson [Tue, 27 Sep 2016 17:35:15 +0000]
Fix func_rs() and as_subselect_rs() to start behaving as advertised
No idea how it never got noticed, but both have been broken since the very
first commits that introduced the methods (
4fa7bc22 / e4bb6727 ). While
changing them 7 years later is a rather serious modification of behavior,
the old way never worked without users having to force-scalar each call site.
If someone has been relying on e.g. [ func_rs(...) ] to return actual result
objects instead of the resultset instance - things will blow up rather quickly
and loudly (aside from the carp()-ed warning encouraging users to switch to
scalar ctx explicitly)
[ func( ... ) ] of course continues to behave like before (directly returning
raw values off the cursor... sigh)
Peter Rabbitson [Fri, 30 Sep 2016 10:25:15 +0000]
Tighten up select list processing in ::SQLMaker
Optimize some of the codepaths (do not recurse in spots where it makes no
practical difference).
Deprecate searches with no explicit select-list ( can't remove it outright due
to downstream breakage :/ )
Dagfinn Ilmari Mannsåker [Wed, 28 Sep 2016 11:39:12 +0000]
Fix building on perls with no . in @INC
Perl 5.26 will be able to be built with no . in @INC,
and Debian are already building their 5.24 without it.
To cope with this, do -It/lib -MFoo instead of -Mt::lib::Foo.
Peter Rabbitson [Mon, 26 Sep 2016 15:36:54 +0000]
(travis) Work around RT#117959
A real fix for this ticket is pending, but had to be bumped a bit
Peter Rabbitson [Tue, 27 Sep 2016 13:17:00 +0000]
Remove the only use of the CAG 'inherited_ro_instance' group
Introduced for reasons unknown back in
93405cf0, it is currently nothing but
baggage - especially given the lack of name synchronization as described in
one of the comments in
28ef9468
grep.cpan.me indicates no use in the wild, so just kill it with fire
Henry Van Styn [Tue, 27 Sep 2016 12:46:14 +0000]
Improve exception text during write operations on uninserted objects
Peter Rabbitson [Tue, 27 Sep 2016 12:19:39 +0000]
Merge the relationship resolution rework
Done as a merge to aid bisecting IFF it ever becomes necessary
Wide testing indicates zero need for downstream changes (aside from several
warnings)
Peter Rabbitson [Sat, 30 Jul 2016 14:03:12 +0000]
Promote resolve_relationship_condition to a 1st-class API method
The encapsulated logic is just too complex to try to replicate externally,
especially now that everything within DBIC itself uses this method underneath.
Patches to the only widely known user (::Resultset::RecursiveUpdate) will
follow shortly
Peter Rabbitson [Thu, 1 Sep 2016 18:22:55 +0000]
Protect several resolve_relationship_condition() callsites
Some external use of DBIx::Class::ParameterizedJoinHack revealed a couple
sites where the relationship resolution may unexpectedly, yet non-fatally
fail. This protects all the reasonable places (partially reverting
b47fb9c0),
downgrading the exceptions to once-per-callsite warnings.
I did not have time to dig to find the underlying problem, there may very well
be a real bug lurking around :/
For reproduction of the (now) warnings: see https://github.com/ctrlo/lenio
Peter Rabbitson [Sat, 24 Sep 2016 13:08:53 +0000]
Switch infer_values_based_on to require_join_free_values in cond resolver
This further simplifies the cognitive surface of the condition resolver API
just like
786c1cdd and a3ae79ed. During the sprint to add at least *some*
sanity to the codepath infer_values_based_on was introduced as a stopgap to
allow
83a6b244 to somehow proceed forward. Since then the amount of spots
where this logic is necessary steadily went down, bringing us to the current
place: there is just a single spot in the entire codebase that passes a
non-empty inferrence structure.
Given the entire codepath is rather baroque, the entire idea of inferrence is
pushed to new_related instead, leaving the API of the resolver itself even
simpler.
There are no known issues as a result, verified by re-running the entire test
plan for downstreams as described in
12e7015a.
Peter Rabbitson [Thu, 11 Aug 2016 09:06:59 +0000]
Switch reverse_relationship_info() to the relcond resolver
Prompted by a PR from @mzealey, a code audit showed the entire implementation
to be severely lacking. Switched to proper relationship resolution, with the
added benefit of support for custom conds whenever possible.
As of this commit every single relationship introspection now goes through a
central point: _resolve_relationship_condition(). No more random ... eq 'HASH'
checks all over the place.
There should be zero functional changes as a result (aside from better custom
cond introspection)
Peter Rabbitson [Wed, 10 Aug 2016 14:19:54 +0000]
Add an extra RV to the relationship resolver
A certain spot in the codebase check whether a relationship is "simple".
This additional flag allows to consider coderef conditions as well, instead
of simply punting with "not a HASH? - no can do"
See next commit for the actual switchover
While at it fix a subtle bug introduced in
b5ce6748 - originally the helper
is_literal_value recognized -ident as a valid literal. Later on during the
migration into SQLA this logic was lost (I do not exactly recall the details),
yet the DBIC side was never adjusted. All callsites were audited to make sure
nothing else was missed.
Peter Rabbitson [Sun, 18 Sep 2016 10:28:18 +0000]
Standardize indication of lack of join_free_condition after resolution
There should be zero functional changes as a result
Peter Rabbitson [Thu, 11 Aug 2016 09:06:59 +0000]
Some cleanup of the resolve_relationship_condition callsites
Zero functional changes
Read under -w
Peter Rabbitson [Wed, 10 Aug 2016 14:16:33 +0000]
Stop accepting foreign_values => undef/rowobj in the resolver
There are just a few spots that need this, things are complex enough as it is
Introduces a subtle change in behavior - now results of $foreign->get_columns
are scrutinized just as a plain hashref, and as a result the sanity checks are
somewhat relaxed.
There should not be any fallout due to this - tested on a wide range of
downstreams
Adjust some tested-for exceptions added in
7e5a0e7c as a result of the above
Read under -w
Peter Rabbitson [Wed, 10 Aug 2016 16:07:50 +0000]
Explicitly normalize results of relationship resolution
Done as a separate commit to aid bisecting (in case it turns out problematic)
Peter Rabbitson [Tue, 7 Jul 2015 20:14:42 +0000]
Remove last internal use of the legacy _resolve_condition (find)
Also fixes the overly coarse 'is it a HASH' check added in
49ca473e/096f4212
Peter Rabbitson [Wed, 10 Aug 2016 08:12:22 +0000]
Tighten up value inferrence in relationship resolution
Read under -w
Peter Rabbitson [Sat, 20 Aug 2016 09:31:36 +0000]
Extract couple more stateless functions from DBIHacks (like
497d0451)
Zero functional changes
Peter Rabbitson [Sat, 27 Aug 2016 08:31:01 +0000]
Fix unpredictable use of reverse_relationship_info() in the SQLT parser
When reverse_relationship_info got introduced in
de60a93d, it was inexplicably
mis-applied at the very spot it was needed in the first place. A result class
pair can (and sometimes do) have more than one relationship between them,
possibly with differing cascade_* settings. Grabbing the first set of values
from the multi-member hash is inconsistent at best.
Fix so that if at least one "hard-dependency" is encountered we go ahead with
marking the reverse part as a CASCADE
Peter Rabbitson [Sun, 21 Aug 2016 08:07:17 +0000]
With time couple DBIHacks methods became single-callsite only
Remove _inner_join_to_node and _resolve_ident_sources from the callchain
entirely
Peter Rabbitson [Mon, 19 Sep 2016 12:14:27 +0000]
Centralize specification of expected Result class base in the codebase
Some parts of the stack need to be able to disambiguate Result instances from
other objects. In odrder to avoid tight coupling introduce a single override
point $DBIx::Class::ResultSource::__expected_result_class_isa for esoteric
use cases
No functional changes
Peter Rabbitson [Wed, 14 Sep 2016 16:07:00 +0000]
(travis) Make sure everything works even when we are SAD
http://www.martin-evans.me.uk/node/119
http://www.dagolden.com/index.php/1771/why-perl_unicode-makes-me-sad/
Peter Rabbitson [Tue, 16 Aug 2016 09:56:08 +0000]
(travis) Revert
81cf62797 and b4532c43f (both fixed since)
Peter Rabbitson [Tue, 13 Sep 2016 10:57:00 +0000]
(travis) Remove execution bits from the travis scripts
No functional changes
Peter Rabbitson [Mon, 5 Sep 2016 12:27:24 +0000]
Remove use of Try::Tiny entirely (the missing part of
ddcc02d1)
While at the time it seemed expedient to keep relying on Try::Tiny::catch and
only replace Try::Tiny::try internally, it turns out that the current naming
behavior of T::T [1] means we can not get DBIC::Carp to report a friendly
callsite, as finding which catch{} frames are skippable becomes problematic.
Additionally this drops a flurry of runtime Sub::Name calls which in turn
is likely to take less time ( note - this has not been explicitly timed, but
seems to pop up often in profiles: https://youtu.be/PYCbumw0Fis?t=1919 )
In any case - one less dep that we do not really use is always a win
Despite the large changeset there should be zero functional changes
This essentially reverts the entirety of
9b58d129
Read under -w
[1] https://metacpan.org/diff/file?source=DOY/Try-Tiny-0.14&target=DOY/Try-Tiny-0.15#lib/Try/Tiny.pm
Peter Rabbitson [Tue, 13 Sep 2016 05:03:52 +0000]
Remove non-functional part of ::Storage::DBI::Sybase::_ping
This code has existed in one form or another since about
322b7a6b, and failed
multiple levels of review and later was left as-is due to a lack of a testing
rig :/
It seems that the simple change is sufficient, but more testing with more
obscure Sybase driver combinations is certainly needed
Peter Rabbitson [Tue, 13 Sep 2016 04:41:16 +0000]
Fix silent failures on autoinc PK without an is_auto_increment attribute
Back in
fabbd5cc the refactor of the subsystem left out this corner case,
resulting in drivers without a functioning last_insert_id() to either fail to
retrieve the autoinc value, or in case of the suboptimal ->_identity()-based
implementations ( MSSQL / Sybase ) to *reuse* the last successfully retrieved
value (sigh)
Since the entire codepath is a hot mess, not changing much aside from an extra
pass to enable implicit inferrence of an is_auto_increment setting.
Several different codepaths in the test suite were used to ensure everything
meshes correctly with retrieve_on_insert and similar
Peter Rabbitson [Mon, 12 Sep 2016 12:12:35 +0000]
Fixup several tests silently broken by
12e7015a
A number of tests that we do not run during CI (lack of RDBMS) have run-time
tests concerning backcompat along the lines of:
$schema->class('Artist')->load_components('PK::Auto...')
The above causes the class' mro to be switched after $schema instantiation
and subsequently triggers the sanitychecker. Instead of ripping these lines
out (without a way to test them) - simply augment the ::Artist based set of
classes to preempt the check failures in the first place.
Peter Rabbitson [Tue, 13 Sep 2016 15:15:48 +0000]
Fix for upcoming (not yet available via DBD::SQLite) libsqlite version
Peter Rabbitson [Mon, 22 Aug 2016 11:19:41 +0000]
Centralize loading of DBIx::Class::Exception
No functional, nor even load-order changes
Peter Rabbitson [Mon, 22 Aug 2016 10:27:22 +0000]
Remove hostile test added by stupid-me 7 years ago in
3bb4eb8f
Peter Rabbitson [Tue, 9 Aug 2016 12:55:06 +0000]
Remove bizarre $_[$#_] idiom from the codebase
Zero functional changes
Peter Rabbitson [Fri, 2 Jan 2015 14:10:13 +0000]
Only normalize conditions during resolution time, instead on every ->search
The normalization operation isn't cheap. Should result in no changes.
Peter Rabbitson [Sat, 7 Nov 2015 10:49:37 +0000]
Add an explicit deduplication of identical condition in cond normalizer
In order to make everything work consistently add a "lax serializer" based
on Data::Dumper, as Storable is sensitive to IV vs PVIV differences.
While at it tighten up the serialize/dump env in DBIC::_Util
Peter Rabbitson [Sat, 20 Aug 2016 09:09:39 +0000]
Fix SQLA condition normalizer sometimes stripping -value ops
Peter Rabbitson [Wed, 10 Aug 2016 15:16:32 +0000]
Extract two stateless DBIHacks routines into a utility package
Further commits will need them in places where $storage isn't yet available.
There are zero functional changes
Best read under -C -C -M --color-words
Peter Rabbitson [Tue, 9 Aug 2016 08:53:30 +0000]
Some cosmetic fixes in ANFANG
Peter Rabbitson [Wed, 10 Aug 2016 13:16:56 +0000]
(travis) Temporarily allow-fail all trusty-based builds
Until the fallout from https://www.traviscistatus.com/incidents/2p40l49r3yxd
( https://github.com/travis-ci/travis-ci/issues/6439 ) is fixed...
Peter Rabbitson [Wed, 10 Aug 2016 13:14:41 +0000]
(travis) Fix unixodbc-dev overwrite on newer CI images
Peter Rabbitson [Tue, 9 Aug 2016 16:45:06 +0000]
(travis) Remove makefile fixup, now hardcoded in the subrepo
Peter Rabbitson [Mon, 8 Aug 2016 18:49:39 +0000]
(travis) Work around RT#116788
Peter Rabbitson [Tue, 9 Aug 2016 06:29:04 +0000]
(travis) Build cperl 5.25.x properly
Peter Rabbitson [Mon, 8 Aug 2016 18:45:11 +0000]
(travis) Better diagnostics when perl compilation bails out
Peter Rabbitson [Mon, 1 Aug 2016 16:43:44 +0000]
Bump version to accomodate pre-beta testers
Peter Rabbitson [Sat, 30 Jul 2016 12:54:01 +0000]
Add missing contribs ( GH#6 / GH#28 )
Also mailmap fixups to go with
93f1cd05
Peter Rabbitson [Mon, 1 Aug 2016 16:42:53 +0000]
Some (very minor) fixups of emit_dups calls in
e570488a
Dagfinn Ilmari Mannsåker [Fri, 29 Jul 2016 14:43:11 +0000]
Fix "overriden" typo
Peter Rabbitson [Tue, 26 Jul 2016 07:00:50 +0000]
(travis) cPerl 5.24.0 has shipped
Peter Rabbitson [Tue, 26 Jul 2016 17:41:27 +0000]
Enable the schema SanChecks on 5.8 as well
See how DarkPAN will react to this - it may be just fine. Backing it out in
a standalone commit so it can be reinstated easily before stable
Peter Rabbitson [Wed, 13 Jul 2016 15:06:30 +0000]
Fix inexplicable 5.8.x C3 errors - roll back
e6efde04
The optimization ends up breaking the entire C3 subsystem at a crucial time
when add_columns has to see the correct register_column in order for IC::DT
to populate the right column_info.
Most importantly it seems that Class::C3::XS makes this optimization entirely
moot (in fact minimal testing indicates things getting *faster* instead)
Peter Rabbitson [Sun, 3 Apr 2016 13:43:28 +0000]
Recognize more authorship based on the historic repo (if available)
Peter Rabbitson [Thu, 28 Jul 2016 12:59:15 +0000]
Merge the ResultSource diagnostics rework
...And this is what the products that we make do:
these are the consequences. They either empower
people, or they steal bits of their lives.
Because experiences are all we have in life:
if you think about them as grains of sand in an
hour glass, once those grains are gone – they are
gone. And experiences with people and experiences
with things: they use up the same grains.
That's why we have a profound responsibility to
respect the experiences of the people that we
build for...
-- Aral Balkan: Free is a Lie TNW 2014
https://youtu.be/upu0gwGi4FE?t=1548
This set of commits is unusual - the 2+kloc of changes (in lib/ alone) do not
add any new runtime functionality, nor do these changes alter significantly
any aspect of DBIC's runtime operation. Instead this is a culmination of a
nearly 4 months long death-march [1] ensuring the increasingly complex and
more frequent ( courtesy of rising use of Moo(se) ) failure modes can be
reasoned about and acted upon by ordinary users, without the need to reach
out to a support channel.
The changeset has been extensively tested against 247 downstream CPAN dists
(as described at the end of commit
12e7015a) and against several darkpan
test suites. As of this merge there are no known issues except RT#114440 [2]
and a number of dists (enumerated in
12e7015a) now emitting *REALLY LOUD*
though warranted and actionable, diagnostic messages.
The diagnostic is emitted directly on STDERR - this was a deliberate choice
designed to:
1) prevent various test suites from failing due to unexpected warnings
2) make the warnings *harder* to silence by a well meaning but often too
eager-yet-not-sufficiently-dilligent staffer, before the warnings had
a chance to reach a senior developer
What follows is a little bit of gory technical details on the commit series,
as the work is both generic/interesting enough to be applied to other large
scale systems, and is "clever" enough to not be easily reasoned about without
a summary. Think of this as a blog post within an unusual medium ;)
=== BACKGROUND
==============
Some necessary history: DBIC as a project is rather old [3][4]. When it got
started Moose wasn't a thing. Neither (for perspective) was jQuery or even
Tw(i)tt(e)r. The software it was modeled on (Class::DBI) has "single-level"
metadata: you have one class per table, and columns/accessor were defined on
that class and that was it. At the time mst made the brilliant decision to
keep the original class-based API (so that the CDBI test suite can be reused
almost verbatim, see
ea2e61bf) while at the same time moving the metadata to
a "metaclass instance" of sorts. The way this worked was for each level of:
- Individual Result Class (class itself, not instance)
- Result Class attached to a Schema class
- Result Class attached to a Schema instance
to have a separate copy-on-the-spot created metadata instance object of
DBIx::Class::ResultSource. One can easily see this by executing:
~/dbic_checkout$ perl -Ilib -It/lib -MDBICTest -MData::Dumper -e '
my $s = DBICTest->init_schema;
$Data::Dumper::Maxdepth = 1;
warn Dumper [
DBICTest::Schema::Artist->result_source_instance,
DBICTest::Schema->source("Artist"),
$s->source("Artist"),
]
'
The technique (and ingenious design) worked great. The downside was that
nobody ever really audited the entire stack past the original implementation.
The codebase grew, and mistakes started to seep in: sometimes modifications
(add_columns, etc) would happen on a derivative metadata instance, while the
getters would still be invoked on the "parent" (which at this point was
oblivious of its "child" existence, and vice versa). In addition there was a
weird accessor split: given a result instance one could reach *different*
metadata instances via either result_source() or result_source_instance(). To
add insult to the injury the latter method is never defined anywhere, and was
always dynamically brought to life at runtime via an accessor maker call on
each individual class [5].
If that wasn't bad enough, some (but crucially *not* all) routines used to
manipulate resultsource metadata were proxied [6] to the main Result classes,
also aiming at allowing the reuse of the existing Class::DBI test suite, and
to provide a more familiar environment to Class::DBI converts. The complete
map of current metadata manipulation methods and their visibility from a
typical ResultClass can be seen at the end of commit message
28ef9468.
The downside was that to an outsider it would seem only natural that if in
order to make something metadata-related happen, one normally calls:
SomeResultClass->set_primary_key
then it makes sense that one should be able to override it via:
sub SomeResultClass::set_primary_key {
my $ret = shift->next::method(@_);
{ do extra stuff }
}
That thinking has been applied to pretty much all straight-pass-through
getters in the wild, with the expectation that DBIC will respect them
throughout, like e.g. [7]. In reality this never happened - half of DBIC
would never even look at the Result class and instead simply called the
needed method on the result source instance directly. As noted in
28ef9468:
the overwhelmingly common practice is to hook a method in a Result class and
to "hope for the best". A rare example of "doing it right" would be
DBIx::Class::ResultSource::MultipleTableInheritance [8], but as can be seen
from its SYNOPSIS the API is rather counterintuitive ( what is table_class()
anyway?! ) and more importantly - the earlier example seems "just right".
Another innovation (remember: pre-Moose) was the use of the just-in-time
implemented [9] alternative C3 method resolution order (MRO)[10] right on top
of the default perl DFS MRO. While DBIC used multiple inheritance (MI) from
the start, with all the corresponding problems [11][12][13] and non-scalable
"solutions" [14], it wasn't until C3 MRO became available that the true
potential of the resulting plugin system became clear. To this day (mid-2016)
MI, as used within the DBIC ecosystem, remains the single most flexible (and
thus superior given the problem domain) plugin-system on CPAN, easily
surpassing rigid delegation, and having an upper hand on role-based solutions
as promoted by the Moo(se) ecosystem. It must be noted that delegation and/or
roles are not without uses - they are an excellent (and frankly should be a
default) choice for many application-level systems. It is the mid-level to
low-level libraries like DBIC, where the stateless nature of a predictable
yet non-coordinated call-order resolution truly begins to shine.
=== PROBLEM(S)
==============
Things stayed undisturbed for a while, until around 2012~2013 folks started
showing up with more and more complaints which all traced to Moo(se)-based
subclassing. Originally the C3 MRO composition worked just fine, because
almost invariably a ->load_components() call (which explicitly switches the
callER MRO) would have happened early enough in the life of any end-user
Result/ResultSource class. But when extends()/with() got more prominent this
was lost. The more complex the inheritance chain - the more likely that the
topmost leaf class is in fact stuck under DFS mro with everything going
sideways from there. Sometimes with truly mindbending failure cases as [15].
There was no clear solution at the time, and aside from some toothless
documentation warnings [16] nothing was done to address this (in fact even
the doc-patch itself is incomplete as noted in [17]).
The inconsistencies, and the resulting mistakes, however, were all localized,
and even though the problems were often major, each instance was sufficiently
different (and bizarre) that each individual deployment could neither report
them properly, nor find the time to reason through the layers of history in
order to arrive at a solution they fully understand. Yet the original design
which solidified towards the end of 2007 was *just* good enough to keep being
kicked down the road.
But people kept writing more and more MOP-inspired stuff. Given the general
tendency of perl code to get "all over the place", the desire was only
natural to standardize on "one true way" of doing OO throughout an entire
end-user project/app. And there were more and more ways in the wild to
combine/abstract individual Result classes and ResultSet components. The
comprehensive DBIx::Class::Helpers [18] are just the tip of the heap of
all possible permutations DBIC is exposed to. Towards mid-2015 it became
utterly untenable to brush off problems with "meh, just don't do that and all
will be be fine".
On the personal front I first ran into the baroque jenga tower head-on when
I tried to make sense of the ResultSource subsystem in an airport lounge
pre-YAPC::EU 2011 (Riga). I honestly do not remember *why* I started digging
in this direction but the result of that attempt (and the later effort to
revive it) got immortalized in my local tree [19]. Enough said.
Next was the dash to implement sane relationship resolution semantics in
03f6d1f7, and then in 350e8d57 (which was actually needed to allow for
d0cefd99 to take place... sigh). During that journey 4006691d made a subtle
but fatal in the long run change - it upset the balance of which source
instance object we looked at during *some* (but not all) codepaths. The
really sad part is that I had the feeling that something is not right, and
even made a record of it as the last paragraph of
350e8d57. But light
testing did not reveal anything, and I irresponsibly shipped everything
as-is a bit later. It wasn't until Oct 2015 that someone noticed this being
an actual problem [20]. Early attempts to fix it quickly demonstrated just
how deep the rabbit hole goes, and were the main reason the entirety of
this work was undertaken: the accumulated debt simply did not leave any room
for a half-way solution :/
=== SOLUTION(S)
===============
The writeup below describes only the final set of commits: it does not cover
driving into and backing out of at least 3 dead-ends, nor does it cover the
5 distinct rewrites and re-shuffles of the entire stack as more and more
involved testing revealed more and more involved failure modes. I must stress
that if you plan to undertake a similar crusade against another projects
architectural debt you are in for a rough (but *not* impossible!) ride. The
height of the "tenacity-bar" necessary to pull off such work is not reflected
in any way within the seemingly effortless walkthrough that follows. It is
also worth acknowledging that the code at times is incredibly terse and hard
to follow: this was a deliberate choice as the extra diagnostic sites that
are enabled during runtime had to be implemented as "close to the VM", so to
speak, as possible. In isolation none of the contortions are warranted, but
because I ended up with so many of them the result does pay off. See comments
within individual commit messages for various performance impacts for more
info.
As first order of business some mechanism was needed to track the logical
relationship between the 3 levels of ResultSource instances as shown earlier
in this writeup. Luckily, the user-unfriendly nature of the metadata stack
meant there are very few spots on CPAN (and to the best of my knowledge
on DarkPAN) that do anything exotic with the subsystem. This means the
simplest thing would in fact work and was implemented as
534aff61: corral
all instantiations of ResultSource objects (and Schema objects while we are
at it) [21]. This code ensured that nothing in the stack will create an
instance of either class-type without our knowledge. With that in place, we
also provide an explicit clone method [22] encouraging folks to use that
whenever possible. The switch of all relevant callsites within DBIC itself
was verified through another check within new [23], guarded by the same
compile-time assertion constant (which in turn was provided by both the CI
and the local smoke-script from
5b87fc0f)
With the above in place, ensuring 99.99% of the ResultSource "derivative"
instances were obtained via $rsrc->clone, it was time for
0ff33686. A simple
private registry hash with object addresses as keys and this hash as values:
{
derivatives => {
addr_derived_rsrc_1 => $reference_to_infohash_of_derived_rsrc_1,
addr_derived_rsrc_2 => $reference_to_infohash_of_derived_rsrc_2,
...
},
weakref => $weak_reference_of_self,
}
As necessary for any structure holding addresses of object references, a
CLONE "renumbering" routine takes care of keeping everything in sync on
iThread spawns (if you believe that iThreads are evil and one shouldn't go
through the trouble: be reminded that any call of fork() within a Win32 perl
is effectively an iThread, and fork() can and *is* being called by some CPAN
modules [24] implicitly).
Now that we had a good handle on "what came from where", the first major
diagnostic milestone
73f54e27 could be covered. As can be seen in the table
of methods in commit
28ef9468 there are only a handful of attributes on an
actual ResultSource class. A couple new Class::Accessor::Grouped method types
were added, which would behave just like the 'simple' and 'component_class'
they were replacing, but with a twist:
- any setter-based change would record its callsite in any derivative that
was being tracked by
0ff33686, effectively marking that derivative stale
- any getter call would consult its own entry in the metadata instance
"stale log", and complain that things have moved on based on the callsite
the setter left earlier
The result is the exact warning as described in commit message
73f54e27. Of
course there are some extra considerations - some high-level setters (e.g.
remove_columns) do call a getter underneath to do their job. These cases had
to be short-circuited by using a local()-based "setter callstack" mark. But
in general the changeset has been surprisingly non-invasive: once the proper
hook points were identified the rest was a breeze. There was also a brief
scratching of heads when the last stages of DarkPAN tests emitted errors
which I myself could not explain for a while, until the reason (and trivial
solution) were identified in
d56e05c7 and [25].
As a brief detour, I considered switching ResultSource to a proper Moo class,
but quickly abandoned this idea as there are no provision for clean get-time
triggers. Nevertheless the attempt was a useful demonstration what does it
take to switch a low-level class (which means many somewhat questionable uses
by consumers in the wild) to Moo(se) with zero loss of functionality. The
result is preserved for posterity as
8ae83f0e [26].
While working on the above and
f064a2ab (the solution to RT#107462 [20]), it
occurred to me that the confusion of having both result_source_instance()
and result_source() can be reduced further by forcing all "getter" calls to
go through result_source() which is defined in Row.pm and is thus always
available. The result was the improved diagnostic as described in the commit
message of
e570488a, but also a useful set of assertions that were used to
weed out many of the wrinkles [27].
The next major step was to resolve once and for all the fallout from
incorrect inheritance composition. The highly dynamic nature of all Perl's
programs, an "eternal compile/execute/compile/execute... cycle", meant that
just "fixing things" as DBIC sees them would not work - calling set_mro()
could do little when called late enough. This led to the revert of the
originally-promising "forced c3-fication" of the stack
7648acb5. Instead
the practical design turned out to be "let the user know and carry on".
The first part of getting there was to devise a way to precisely and very
quickly tell "what does a class look like right now?" I have been brooding
over how to do this since mid-February, but it wasn't until I noticed
the excellent App::Isa::Splain [28] by @kentfredric, that the final interface
came into focus:
296248c3 (with several minor fixups later on). Here I want
to take a moment to apologize to @kentfredric, as he was led on a several
week long wild-goose chase due to a misguided comment of mine [29] :(
Amusingly while implementing this I hit a wall related to perl 5.8 (for the
first time in 6+ years): As stated in the timings at the end of commit
message
296248c3 and as elaborated in [30] - the non-core MRO is just too
expensive to work with. This resulted in a 1.5 week long detour to try to
squeeze every last ounce of performance. Amusingly I ran into a lot of
"interesting" stuff along the way [31][32][33] The result was not only a
semi-usable 5.8 implementation, but even running on 5.10+ was sped up about
2 times in the end, which translated into tangible gains in the end: the
number cited as 16% in
12e7015a was originally 28%(!). The moral of this
story? - gerontoperlia [34] makes your modern foundation code better.
With a reliable way to tell what each methods "variant stack" looks like, it
was trivial to implement the 'valid_c3_composition' part of ::SanityChecker -
one would simply check a class' MRO, and in case of 'dfs' compare all stacks
to what they would look like if the MRO were 'c3' [35].
In parallel but unrelated to the above the ever increasing tightening of
various DBIC internal callpaths (
e5053694, d99f2db7, 3b020224 ) had to be
addressed in some way. The urgency truly "hit home" when testing revealed
RT#114440 [2] - it was nothing short of a miracle this code survived that
long without being utterly broken by other components. The solution came out
of crossing the work on describe_class_methods (
296248c3) with the concept
of the fail_on_internal_call guard (
77c3a5dc). We already have a list of
method "shadowing stacks" (to borrow @kentfredric's terminology) - if we find
a way to annotate methods in a way that we can tell when a "non-overrideable"
method was in fact overridden - we will be able to report this to the user.
The somewhat fallen out of favor subsystem of function attributes was chosen
to carry out the "annotation" task. It must be noted that this is one of the
few uses of attributes on CPAN that is architecturally consistent with how
attributes were originally implemented. An attribute is meant to attach to
a specific reference ( in our case a code reference ), instead of a name.
This is also why the FETCH/MODIFY_type_ATTRIBUTE API operates strictly with
references. As an illustration why tracking attributes by name is fraught
with peril consider the following:
perl -e '
use Data::Dumper;
use Moose;
use MooseX::MethodAttributes;
sub somemethod :Method_expected_to_always_returns_true { return 1 };
around somemethod => sub { return 0 };
warn Dumper {
attributes => __PACKAGE__->meta->get_method("somemethod")->attributes,
result => __PACKAGE__->somemethod
};
'
It should also be noted that as of this merge describe_class_methods lacks
a mechanism to "see" code references captured by around-type modifiers, and
by extension the "around-ed" function's attributes will not appear in the
"shadowed stack". A future modification of Class::Method::Modifiers, allowing
minimal introspection of what was done to which coderef should alleviate most
of this problem.
Once all relevant methods were tagged with a 'DBIC_method_is_indirect_sugar'
attribute in
1b822bd3, it was trivial to implement the schema sanity check
no_indirect_method_overrides which simply ensures no user-provided method
"shadows" a superclass method with the 'sugar' attribute set [36].
The success of the attribute-based approach prompted a pass of annotating
all the methods DBIC generates for one reason or another:
09d8fb4a. Aside
from enabling the last improvement, it also allowed to replicate a part of
the DBIx::Class::IntrospectableM2M functionality in core, without elevating
the status of the m2m sugar methods in any way (the historic labeling of
these helpers as relationships is a long standing source of confusion). See
the commit message of
09d8fb4a for a couple use-cases.
The last piece of the puzzle
28ef9468 addressed the "override and hope for
the best" duality of ResultSource proxied methods as described at the start
of this writeup and at [37]. What we essentially do is add an around() [38]
for every method in ResultSource, which then checks whether it was called via
ResultSourceProxy (inherited from DBIx::Class::Core), or directly via the
ResultSource instance: i.e. MySchema::Result::Foo->proxied vs $rsrc->proxied
IFF we are called directly and there *is* an override of the same method on
the currently-used $rsrc->result_class we either follow one of the options
as given by an attribute annotation [37], or we emit a diag message so that
the user can do something about it.
That was easy wasn't it?
=== FINAL THOUGHTS
==================
This work took about 50 person-days to carry out, and for obvious reasons
expanded to cover a much larger period of actual wall-time. While I am by
far not the most efficient developer that I have met, I am pretty sure that
the process of planning, designing, implementing and testing all of this
could not have been significantly accelerated. Even at the (laughable) rate
of $50/h The Perl Foundation is willing to pay for unique talent [39] this
endeavor would cost at least $20,000 USD - way beyond the scope (and aim?)
of a TPF grant. On the other hand it would be surprising if this work can
be qualified as unnecessary. I personally estimate that the savings due to
the proper diagnostics alone will "make up" for the effort within the first
month of wide deployment of these improvements. Time will tell of course, as
the stream of questions is only about to start come the first days of August.
In any case - this project is by far not the only one in dire need of such
"humane" overhaul. Moo, Catalyst, various pieces of the toolchain, and other
staples of what is known as "modern perl5" are in similar or worse shape:
a situation which can *not* be rectified simply by "writing patches" without
a concerted effort directed by a single [40] dedicated individual.
I yet again strongly urge the "powers of Perl" to reconsider their hands-off
approach to funding the consistently shrinking pool of maintainers. *PLEASE*
consider stealing (in the spirit of tradition) the proven successful model of
RubyTogether [41] before you end up losing even more maintainers like myself.
Peter "ribasushi" Rabbitson
Outgoing maintainer of a cornerstone Perl5 ecosystem project
( in the future you may be needing https://archive.org/web/ to see some of these )
[1] https://gist.github.com/ribasushi/
6ea33c921927c7571f02e5c8b09688ef
[2] https://rt.cpan.org/Ticket/Display.html?id=114440#txn-1627249
[3] http://static.spanner.org/lists/cdbi/2005/07/25/
90c9f5f1.html
[4] http://lists.digitalcraftsmen.net/pipermail/classdbi/2005-August/000039.html
[5] https://metacpan.org/source/RIBASUSHI/DBIx-Class-0.082840/lib/DBIx/Class/ResultSourceProxy/Table.pm#L17-21
[6] https://metacpan.org/source/RIBASUSHI/DBIx-Class-0.082840/lib/DBIx/Class/ResultSourceProxy.pm#L53-87
[7] https://metacpan.org/source/VANSTYN/RapidApp-1.2000/lib/RapidApp/DBIC/Component/VirtualColumnsExt.pm#L52-67
[8] https://metacpan.org/pod/DBIx::Class::ResultSource::MultipleTableInheritance#SYNOPSIS
[9] https://twitter.com/hashtag/dammitstevan
[10] https://en.wikipedia.org/wiki/C3_linearization
[11] https://en.wikipedia.org/wiki/Multiple_inheritance#The_diamond_problem
[12] http://static.spanner.org/lists/cdbi/2005/07/25/
caf44f84.html
[13] http://static.spanner.org/lists/cdbi/2005/07/26/
e593c147.html
[14] http://static.spanner.org/lists/cdbi/2005/07/26/
ea509a6a.html (... tell people "Be Careful!" )
[15] https://blog.afoolishmanifesto.com/posts/mros-and-you/
[16] https://metacpan.org/pod/DBIx::Class::ResultSet#ResultSet-subclassing-with-Moose-and-similar-constructor-providers
[17] https://github.com/dbsrgits/dbix-class/pull/49#issuecomment-
47637403
[18] https://metacpan.org/release/DBIx-Class-Helpers
[19] http://i.imgur.com/A3acsCD.png
[20] https://rt.cpan.org/Ticket/Display.html?id=107462
[21] https://github.com/dbsrgits/dbix-class/blob/
534aff61/lib/DBIx/Class/_Util.pm#L1082-L1135
[22] https://github.com/dbsrgits/dbix-class/blob/
534aff61/lib/DBIx/Class/ResultSource.pm#L160-L184
[23] https://github.com/dbsrgits/dbix-class/blob/
534aff61/lib/DBIx/Class/ResultSource.pm#L126-L143
[24] http://grep.cpan.me/?q=my+%5C%24pid%3D+fork+dist%3DXML-Twig
[25] https://github.com/ctrlo/GADS/pull/9/files
[26] http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class-Historic.git;a=commitdiff;h=
8ae83f0e
[27] https://github.com/dbsrgits/dbix-class/blob/
e570488a/t/lib/DBICTest/BaseSchema.pm#L379-L528
[28] https://metacpan.org/pod/App::Isa::Splain#SYNOPSIS
[29] https://github.com/kentnl/Devel-Isa-Explainer/issues/1#issuecomment-
212248379
[30] https://github.com/dbsrgits/dbix-class/blob/
12e7015a/lib/DBIx/Class/Schema/SanityChecker.pm#L92-L102
[31] https://twitter.com/ribasushi/status/
753678208076242944
[32] https://github.com/dbsrgits/dbix-class/commit/
296248c3#diff-c13797cc2e5864c4a1d6a92ba65871b6R801
[33] https://github.com/dbsrgits/dbix-class/blob/
1cf2ad8b/lib/DBIx/Class/_Util.pm#L663-L664
[34] https://youtu.be/2Ln0YHtKgaI?t=3731
[35] https://github.com/dbsrgits/dbix-class/blob/
12e7015a/lib/DBIx/Class/Schema/SanityChecker.pm#L484-L505
[36] https://github.com/dbsrgits/dbix-class/blob/
12e7015a/lib/DBIx/Class/Schema/SanityChecker.pm#L359-L394
[37] https://github.com/dbsrgits/dbix-class/blob/
28ef9468/lib/DBIx/Class/MethodAttributes.pm#L242-L298
[38] https://github.com/dbsrgits/dbix-class/blob/
28ef9468/lib/DBIx/Class/ResultSourceProxy.pm#L137-L333
[39] http://news.perlfoundation.org/2016/02/grant-proposal-perl-6-performa.html#comment-
38362169
[40] http://queue.acm.org/detail.cfm?id=2349257 ( ... quality happens only if somebody has the responsibility for it, and that "somebody" can be no more than one single person )
[41] https://rubytogether.org/roadmap
Peter Rabbitson [Mon, 6 Jun 2016 11:58:31 +0000]
Prevent invisible skipping of ResultSource proxy overrides
*** NOTE ***
This does not add any new default functionality, nor does it alter DBIC's
behavior from how it solidified back in 2006: all this does is alert a user
when things are 99% not DWIM-ing (10 years overdue but better late than...)
*** NOTE ***
During the original design of DBIC the "ResultSourceProxy" system was
established in order to allow easy transition from Class::DBI. Sadly
it was not well abstracted away: it is rather difficult to use a custom
ResultSource subclass. The expansion of the DBIC project never addressed
this properly in the years since.
As a result when one wishes to override a part of the ResultSource
functionality, the overwhelmingly common practice is to hook a method in a
Result class and "hope for the best".
The subtle changes of various internal call-chains (mainly
4006691d) make
this silent uncertainty untenable. As a solution any such override will now
issue a descriptive warning that it has been bypassed during a direct
$rsrc->overriden_function invocation. A user now *must* determine how each
individual override must behave in this situation, and tag it with one of
the provided attributes.
For completeness the blueprint off which this solution was devised is
provided below:
I = indirect (helper) method, never invoked by DBIC itself
* Rsrc method types
. = rsrc_instance_specific_attribute type accessor (getter+setter)
s = setter calling a . internally
g = getter calling a . internally
c = custom accessor
* Result method types
P = proxied directly into ::Core via ::ResultSourceProxy (overridable)
X = a ::Core proxy to ::ResultSource with extra logic (overridable)
m = misc... stuff
___ Indirect methods ( the sanity checker warns when one "covers" these )
/
| __ Rsrc methods somehow tied into the metadata state
| /
|| _ Available to .../Result/... via ResultSourceProxy
|| /
|||
|||
DBIx::Class::ResultSource::View:
. is_virtual,
. deploy_depends_on,
. view_definition
DBIx::Class::ResultSource:
c schema
. source_name # no proxy, but see FIXME at top of ::ResultSourceProxy
. _columns
. _ordered_columns
. _primaries
. _relationships
. _unique_constraints
.P column_info_from_storage
. name
.P result_class
.P resultset_attributes
.P resultset_class
.P source_info
. sqlt_deploy_callback
IsX add_column
sX add_columns
sX add_relationship,
IsP remove_column
sP remove_columns
sP add_unique_constraint
IsP add_unique_constraints
sP sequence
sP set_primary_key
IgP column_info
gP columns_info
gP columns
gP has_column
gP has_relationship
gP primary_columns
gP relationship_info
gP relationships
gP unique_constraint_columns
gP unique_constraint_names
gP unique_constraints
DBIx::Class::ResultSourceProxy::Table:
m table
m _init_result_source_instance
Peter Rabbitson [Sat, 4 Jun 2016 15:02:00 +0000]
Expand annotations to cover all generated methods
This is needed for the next commit, as we need a reliable way to tell gened
methods apart from everything else. Given we will be taking the hit of adding
the attributes, just go ahead and annotate *everything*, to be done with all
auto-generated subs once and for all.
This also solves @vanstyn's long-time gripe of not being able to tell where
in a random schema one has declared m2m "relationships" (a typical customer is
*very* unlikely to be using DBIC::IntrospectableM2M)
As of this commit a typical Result can be introspected for m2m as follows:
~$ perl -Ilib -It/lib -MDBICTest -MPackage::Stash -e '
my $meths = Package::Stash->new("DBICTest::Schema::Artwork")
->get_all_symbols("CODE");
for my $m (sort keys %$meths ) {
print "$m\n" if grep {
$_ =~ /^DBIC_method_is_m2m_sugar/
} attributes::get($meths->{$m});
}
'
While the more involved "complete method map" looks as follows:
~$ perl -Ilib -It/lib -MDBICTest -MPackage::Stash -e '
my $meths = Package::Stash->new("DBICTest::Schema::CD")
->get_all_symbols("CODE");
for my $m (sort keys %$meths ) {
if ( my @attrs = attributes::get($meths->{$m}) ) {
print "\n$m\n";
print " $_\n" for @attrs;
}
}
'
Peter Rabbitson [Mon, 23 May 2016 09:08:17 +0000]
Introducing DBIx::Class::Schema::SanityChecker
This gives us comprehensive diagnostic on incorrect component composition
and other hard to track... stuff.
Given the huge amount of changes to call chains (specifically the changes
in
77c3a5dc and e5053694), and the fallout seen on CPAN and darkpan due to
these modifications, the status quo became clearly untennable. To mitigate
the (often silent) breakage a brand new "sanity check" framework was
introduced as part of the ::Schema setup-cycle (and is enabled by default)
Same DBIx::Class::Helper v2.032002 test time shoots from 65.5s all the way
to 76.0s, a 16% slowdown. However the moment the framework is disabled by
flipping $schema->schema_sanity_checker to a defined-but-false value - the
startup impact is entirely gone.
The changset was extensively tested against the following set of downstream
dists (a superset of
c8b1011e), with each warning hand-confirmed to be a
valid description of a real problem:
--- actual bash script passing on a *heavily* massaged PERL5LIB
set -o pipefail
export PERL_CPANM_OPT=
export PERL5LIB="/home/rabbit/devel/dbic/dbgit/lib:$PERL5LIB"
export DBICTEST_SQLT_DEPLOY=0
export DBIC_ASSERT_NO_ERRONEOUS_METAINSTANCE_USE=1
export DBIC_ASSERT_NO_FAILING_SANITY_CHECKS=1
# these fail with ERRONEOUS_METAINSTANCE_USE alone
# (S::L fails due to PG_DSN but I think is ok besides that)
for d in \
DBICx::Shortcuts \
DBIx::Class::Bootstrap::Simple \
DBIx::Class::Preview \
DBIx::Class::Schema::Loader \
Pinto \
; do \
DBIC_ASSERT_NO_ERRONEOUS_METAINSTANCE_USE=0 \
DBIC_ASSERT_NO_FAILING_SANITY_CHECKS=0 \
DBICTEST_PG_DSN= \
cpanm -v --reinstall $d 2>&1 \
| tee -a /dev/shm/umpfh \
| grep -P -B1 'sanity check|emit_|^(Building and testing|Result:)' || exit 1 \
; done
# these emit various san-check related problems
for d in \
RapidApp \
Data::OFAC \
DBIx::Class::VirtualColumns \
"DBD::SQLite@1.35 Handel" \
DBIx::Class::RDBOHelpers \
CatalystX::CRUD::ModelAdapter::DBIC \
DBICx::Indexing \
DBICx::TestDatabase \
DBIx::Class::BitField \
DBIx::Class::I18NColumns \
DBIx::Class::PhoneticSearch \
DBIx::Class::RandomColumns \
DBIx::Class::ResultSource::MultipleTableInheritance \
DBIx::Class::Result::ProxyField \
DBIx::Class::Schema::PopulateMore \
DBIx::Class::Tree \
Foorum \
Interchange6::Schema \
Test::DBIx::Class \
TreePath \
; do \
DBIC_ASSERT_NO_FAILING_SANITY_CHECKS=0 \
cpanm -v --reinstall $d 2>&1 \
| tee -a /dev/shm/umpfh \
| grep -P -B1 'sanity check|emit_|^(Building and testing|Result:)' || exit 1 \
; done
# these are entirely unaffected \o/
for d in \
Dancer2::Plugin::DBIC \
App::DBCritic \
App::DH \
AproJo \
Articulate \
Authorization::RBAC \
BackPAN::Index \
Bio::Chado::Schema \
Bot::BasicBot::Pluggable::Module::Notes \
Bracket \
Business::Cart::Generic \
Business::DPD \
Catalyst::Authentication::Credential::Facebook \
Catalyst::Authentication::Store::DBIx::Class \
Catalyst::Controller::DBIC::API \
Catalyst::Model::DBIC::Plain \
Catalyst::Model::DBIC::Schema \
Catalyst::Model::DBIC::Schema::PerRequest \
Catalyst::Model::FormFu \
Catalyst::Plugin::Authentication::Store::DBIC \
Catalyst::Plugin::Authorization::Abilities \
Catalyst::Plugin::AutoCRUD \
Catalyst::Plugin::DBIC::Schema::Profiler \
Catalyst::Plugin::Session::Store::DBIC \
Catalyst::TraitFor::Controller::DBIC::DoesPaging \
Catalyst::TraitFor::Model::DBIC::Schema::RequestConnectionPool \
Catalyst::TraitFor::Model::DBIC::Schema::Result \
Catalyst::TraitFor::Model::DBIC::Schema::WithCurrentUser \
Catalyst::View::CSV \
CatalystX::Controller::ExtJS::REST::SimpleExcel \
CatalystX::Crudite \
CatalystX::Eta \
CatalystX::OAuth2 \
CatalystX::Resource \
CGI::Application::Plugin::Authentication::Driver::DBIC \
CGI::Application::Plugin::DBIC::Schema \
CGI::Application::Plugin::DBIx::Class \
CGI::Application::Plugin::ExtJS \
CGI::Session::Driver::dbic \
Cookieville \
Dancer2::Plugin::Auth::Extensible::Provider::DBIC \
Dancer2::Session::DBIC \
Dancer::Plugin::Auth::Extensible::Provider::DBIC \
Dancer::Plugin::Auth::RBAC::Credentials::DBIC \
Dancer::Plugin::Auth::RBAC::Permissions::DBIC \
Dancer::Plugin::DBIC \
Dancer::Session::DBIC \
Data::Morph \
DBICx::AutoDoc \
DBICx::Backend::Move \
DBICx::DataDictionary \
DBICx::Deploy \
DBICx::Hooks \
DBICx::MapMaker \
DBICx::MaterializedPath \
DBICx::Modeler \
DBICx::Sugar \
DBICx::TxnInsert \
DBIx::Class::AlwaysUpdate \
DBIx::Class::AuditAny \
DBIx::Class::AuditLog \
DBIx::Class::BatchUpdate \
DBIx::Class::Candy \
DBIx::Class::ColumnDefault \
DBIx::Class::CompressColumns \
DBIx::Class::Cursor::Cached \
DBIx::Class::CustomPrefetch \
DBIx::Class::DateTime::Epoch \
DBIx::Class::DeleteAction \
DBIx::Class::DeploymentHandler \
DBIx::Class::DigestColumns \
DBIx::Class::DynamicDefault \
DBIx::Class::DynamicSubclass \
DBIx::Class::EasyFixture \
DBIx::Class::ElasticSync \
DBIx::Class::EncodeColumns \
DBIx::Class::EncodedColumn \
DBIx::Class::Factory \
DBIx::Class::Fixtures \
DBIx::Class::ForceUTF8 \
DBIx::Class::FormatColumns \
DBIx::Class::FormTools \
DBIx::Class::FromSledge \
DBIx::Class::FrozenColumns \
DBIx::Class::GeomColumns \
DBIx::Class::Graph \
DBIx::Class::Helpers \
DBIx::Class::HTML::FormFu \
DBIx::Class::HTMLWidget \
DBIx::Class::Indexed \
DBIx::Class::InflateColumn::Authen::Passphrase \
DBIx::Class::InflateColumn::BigFloat \
DBIx::Class::InflateColumn::Boolean \
DBIx::Class::InflateColumn::Currency \
DBIx::Class::InflateColumn::DateTime::Duration \
DBIx::Class::InflateColumn::DateTime::WithTimeZone \
DBIx::Class::InflateColumn::DateTimeX::Immutable \
DBIx::Class::InflateColumn::FS \
DBIx::Class::InflateColumn::IP \
DBIx::Class::InflateColumn::Markup::Unified \
DBIx::Class::InflateColumn::Math::Currency \
DBIx::Class::InflateColumn::Object::Enum \
DBIx::Class::InflateColumn::Path::Class \
DBIx::Class::InflateColumn::Serializer \
DBIx::Class::InflateColumn::Serializer::JSYNC \
DBIx::Class::InflateColumn::Serializer::Role::HashContentAccessor \
DBIx::Class::InflateColumn::Serializer::Sereal \
DBIx::Class::InflateColumn::Time \
DBIx::Class::InflateColumn::TimeMoment \
DBIx::Class::InflateColumn::URI \
DBIx::Class::IntrospectableM2M \
DBIx::Class::Journal \
DBIx::Class::LibXMLdoc \
DBIx::Class::LookupColumn \
DBIx::Class::MaterializedPath \
DBIx::Class::Migration \
DBIx::Class::Numeric \
DBIx::Class::Objects \
DBIx::Class::OptimisticLocking \
DBIx::Class::ParameterizedJoinHack \
DBIx::Class::PassphraseColumn \
DBIx::Class::QueriesTime \
DBIx::Class::QueryLog \
DBIx::Class::QueryLog::WithStackTrace \
DBIx::Class::QueryProfiler \
DBIx::Class::RandomStringColumns \
DBIx::Class::Relationship::Predicate \
DBIx::Class::Report \
DBIx::Class::Result::ColumnData \
DBIx::Class::ResultSet::AccessorsEverywhere \
DBIx::Class::ResultSet::Data::Pageset \
DBIx::Class::ResultSet::Excel \
DBIx::Class::ResultSet::Faceter \
DBIx::Class::ResultSet::HashRef \
DBIx::Class::ResultSet::RecursiveUpdate \
DBIx::Class::Result::Validation \
DBIx::Class::SaltedPasswords \
DBIx::Class::Schema::Config \
DBIx::Class::Schema::Diff \
DBIx::Class::Schema::RestrictWithObject \
DBIx::Class::Schema::ResultSetAccessors \
DBIx::Class::Schema::Versioned::Inline \
DBIx::Class::Service \
DBIx::Class::SingletonRows \
DBIx::Class::Storage::DBI::mysql::backup \
DBIx::Class::Storage::DBI::ODBC::OPENEDGE \
DBIx::Class::Storage::DBI::OpenEdge \
DBIx::Class::StorageReadOnly \
DBIx::Class::Storage::TxnEndHook \
DBIx::Class::TimeStamp \
DBIx::Class::Tokenize \
DBIx::Class::TopoSort \
DBIx::Class::Tree::CalculateSets \
DBIx::Class::Tree::Mobius \
DBIx::Class::UnicornLogger \
DBIx::Class::UserStamp \
DBIx::Class::UUIDColumns \
DBIx::Class::Validation \
DBIx::Class::Validation::Structure \
DBIx::Class::WebForm \
DBIx::Class::Wrapper \
DBIx::Table::TestDataGenerator \
Data::Importer \
Dwimmer \
ETLp \
ExtJS::Generator::DBIC \
Finance::QuoteDB \
Form::Processor::Model::DBIC \
Form::Sensible::Reflector::DBIC \
FormValidator::Simple::Plugin::DBIC::Unique \
Galileo \
GenOO \
HTML::FormFu::ExtJS \
HTML::FormFu::Model::DBIC \
HTML::FormHandler::Model::DBIC \
Hyle \
IronMan::Schema \
KiokuDB::Backend::DBI \
Log::Log4perl::Appender::DBIx::Class \
Mixin::ExtraFields::Driver::DBIC \
Module::CPANTS::ProcessCPAN \
Mojolicious::Plugin::DBICAdmin \
MooseX::Types::DBIx::Class \
OpusVL::AppKit \
OpusVL::AppKit::Schema::AppKitAuthDB \
OpusVL::Preferences \
OpusVL::SysParams \
Prosody \
Pulp \
RackMan \
Reaction \
Schema::RackTables \
Tapper::MCP \
Tapper::Schema \
Template::Provider::CustomDBIC \
Template::Provider::DBIC \
Template::Provider::PerContextDBIC \
Template::Provider::PrefixDBIC \
Test::DBIC::ExpectedQueries \
Test::DBIC::Schema::Connector \
Test::DBIx::Class::Schema \
Test::Fixture::DBIC::Schema \
Tie::DBIx::Class \
Types::DBIx::Class \
WebAPI::DBIC \
WebNano::Controller::CRUD \
Web::Util::DBIC::Paging \
Web::Util::ExtPaging \
WWW::Hashbang::Pastebin \
WWW::RobotRules::DBIC \
YAWF \
YATT::Lite \
Yeb::Plugin::DBIC \
"DBD::SQLite@1.35 Catalyst::ActionRole::BuildDBICResult DBIx::NoSQL Jedi::Plugin::Session Jedi::Plugin::Auth" \
"Test::More@1.001014 Test::DBIx::Class::Stats" \
"Mojolicious@3.91 ExpenseTracker" \
"Dancer2@0.166001 Strehler Strehler::Element::Extra Strehler::RSS" \
; do \
cpanm -v --reinstall $d 2>&1 \
| tee -a /dev/shm/umpfh \
| grep -P -B1 '^(Building and testing|Result:)' || exit 1 \
; done
echo
echo 'YAY!'
exit 0
Peter Rabbitson [Fri, 27 May 2016 14:14:28 +0000]
Annotate every indirect sugar-method
Now that the churn is over we can add annotations to each method a user ought
to never override. See next commit for the actual use case and diagnostics
emitter.
Unfortunately this adds yet another small compile-time hit, similar to
73f54e27 (a hit incurred regardless whether the upcoming validation framework
is used or not). Complete test of DBIx::Class::Helpers v2.032002 goes from
about ~64.6 seconds CPU time up to ~65.5, adding another ~1% of startup speed
loss. The savings in debugging sessions should make this all worth it... or
so one hopes.
Peter Rabbitson [Mon, 23 May 2016 09:08:17 +0000]
Some test suite corrections ahead of next commits
Splitting this off for easier reading
Peter Rabbitson [Fri, 22 Jul 2016 10:59:40 +0000]
An extra bit of diag on incomplete rsrc re-register
Due to the counterintuitive nature of the metadata subsystem, a user wishing
to modify the metadata for a result class at runtime (post $schema instance
initialization), may end up in a situation where *everything* appears to work
but falls apart on the next call to My::Schema->connect. In fact I myself made
this very mistake in https://github.com/ctrlo/GADS/pull/1/files, even though
I was pretty well aware of the dangers at the time.
In order to make this go away for good reuse the meta-metadata kept around to
track rsrc ancestry and modifications, and emit a warning alerting folks to
the potential problem (the *actual* problematic desync will also be warned
about at a later step by the stale-metadata diag).
Peter Rabbitson [Fri, 22 Apr 2016 10:39:00 +0000]
Comprehensive diagnostic on incorrect ResultSource metadata use
This commit is the second part of the permanent RT#107462 solution
f064a2ab.
Given the amount of changes to the resultsource metadata subsystem, I can
not be certain that everything has been accounted for, even despite the
comprehensive assertion harness added in the previous commits passing with
flying colors on the entire reverse dep list detailed in
c8b1011e.
As Dave Howorth correctly pointed out in [1], the diagnostic of why something
stopped working within the metadata subsystem is pretty daunting, especially
given the ass-backward nature of DBIC's implementation of it. The (minimal
but present) performance hit is deemed worth it in order to be able to
present this information to downstream. One unexpected bit of good news is
that none of the downstreams tested emitted the warning, which is an extra
point of confidence that the main change of
f064a2ab, and the even more
dangerous change in
9e36e3ec are both solid.
The gist here is that this:
~/devel/dbic$ perl -Ilib -It/lib -MDBICTest -e '
my $art = DBICTest->init_schema->resultset("Artist")->find(1);
DBICTest::Schema::Artist->add_column("foo");
DBICTest::Schema->source("Artist")->add_columns("foo");
$art->has_column("foo");
'
now emits a comprehensive non-trappable warning along the lines of:
DBIx::Class::ResultSource::Table=HASH(0x2a32660) (the metadata instance
of source 'Artist') is *OUTDATED*, and does not reflect the modifications
of its *ancestors* as follows:
* DBIx::Class::ResultSource::Table=HASH(0x24ed770)->add_column(...) at -e line 4
* DBIx::Class::ResultSource::Table=HASH(0x2955da8)->add_columns(...) at -e line 6
Stale metadata accessed by 'getter' DBIx::Class::ResultSource::Table=HASH(0x2a32660)->has_column(...)
within the callstack beginning at lib/DBIx/Class/ResultSource.pm line 231.
DBIx::Class::ResultSource::get_rsrc_instance_specific_attribute(DBIx::Class::ResultSource::Table=HASH(0x2a32660), "_columns") called at (eval 95) line 2
DBIx::Class::ResultSource::_columns(DBIx::Class::ResultSource::Table=HASH(0x2a32660)) called at lib/DBIx/Class/ResultSource.pm line 732
DBIx::Class::ResultSource::has_column(DBIx::Class::ResultSource::Table=HASH(0x2a32660), "foo") called at (eval 70) line 19
DBIx::Class::ResultSourceProxy::has_column(DBICTest::Artist=HASH(0x311e338), "foo") called at -e line 8
The performance hit consistently measures in the ~1.5% range: the test suite
of @frioux's DBIx::Class::Helpers v2.032002 consistently completes within
roughly ~63.7 CPU seconds at the base of this branch, yet climbs to ~64.6 as
of this commit (on an idle low-clocked Xeon L3426)
The warning can not be disabled for the time being (aside from monkeypatching
DBIC::ResultSource) - the wide-range testing indicates it only fires on real
legitimate problems. Hopefully I am making the right call...
[1] http://lists.scsys.co.uk/pipermail/dbix-class/2016-January/012127.html
Peter Rabbitson [Thu, 14 Apr 2016 22:33:17 +0000]
Centralize all user-side rsrc calls to go through result_source()
This ensures the user will always get a sensible exception when the rsrc
metadata object has not yet been initialized (as introduced in
5298bbb5):
Before:
~$ perl -e 'use base "DBIx::Class::Core"; __PACKAGE__->add_column("foo")'
Can't locate object method "result_source_instance" via package "main" at .../ResultSourceProxy.pm line 29.
After:
~$ perl -e 'use base "DBIx::Class::Core"; __PACKAGE__->add_column("foo")'
DBIx::Class::Row::result_source(): No ResultSource instance registered for 'main', did you forget to call main->table(...) ? at -e line 1
Add a shitload of assertions to track we are doing the right thing in all
cases. This more or less concludes the rsrc changeset necessary to resolve
all ambiguities. The next commit adds user-visible warnings when things go
off the rails
The changeset was successfully tested against the list of distributions
in
c8b1011e with no ill effects being observed. Thus I am pretty damn
confident I rather nailed it >.>
Peter Rabbitson [Mon, 6 Jun 2016 12:34:55 +0000]
Fold column_info() into columns_info()
Not sure how I never noticed the utter code duplication.
Peter Rabbitson [Mon, 25 Apr 2016 09:53:54 +0000]
Resolve $rsrc instance duality on metadata traversal
Make result_source a straight wrapper around result_source_instance. This
should fix all the fallout introduced in 0.082800 (
4006691d), which sadly
went undetected all the way until ~7 months after its release. Ultimately
this is my fault, as I had an early warning, and even later made a conjecture
which spot exactly may blow up in my face (read towards end of
350e8d57)
Exploit the fact that result_source_instance until very recently was a toss-up
between a CAG 'inherited' and a Class::Data::Inheritable (removed in
5e0eea35)
with CAG not really involved in result-instance level calls, and the latter
making it downright impractical due to the closure-based approach. Combined
with the fact that result_source was a 'simple'-type accessor pointing at
the '_result_source' hash-slot allows us (at least seemingly) to switch to a
setup where result_source is nothing but a wrapper around a CAG inherited
accessor result_source_instance which point to the named slot '_result_source'
The changeset is deceptively small, and is kept this way for easier auditing.
See next commit for the armada of additional testing to verify the entire
stack is in fact still solid.
Peter Rabbitson [Thu, 14 Apr 2016 07:27:33 +0000]
Keep track of result source instance ancestry
The oddball external registry (instead of directly-linked objects) is due
to shit like
31399b48
For now this doesn't realy do anything: See several commits higher why this
is needed in the first place.
Peter Rabbitson [Fri, 13 May 2016 17:15:18 +0000]
Fully separate parent and child resultsource metadata
Ensure that all attributes are shallow-copied on "clone". Currently this
means that the following are *NO LONGER* shared between rsrc clones:
_unique_constraints
_primaries
source_info
This seems just as cocky and reckless as the clusterfuck in
4006691d/RT#107462
and it most likely will have the same invisible-yet-dire consequences for
various downstreams. However there is a plan in place several commits ahead
allowing sidestepping the impossibility to debug a potential fallout.
Peter Rabbitson [Thu, 7 Apr 2016 11:20:30 +0000]
Add a clone method to ResultSource, switch obvious spots to it
Not messing with the ::ResultSourceProxy::Table clusterfuck for now, too many
things can go wrong. Instead will explicitly instrument the callsites in
subsequent commits.
Also add assertions this does not get routed around: such use will throw from
here on out as long as one enables the necessary assert:
~$ DBIC_ASSERT_NO_ERRONEOUS_METAINSTANCE_USE=1 perl -Ilib -MDBIx::Class -e '
bless ({}, "DBIx::Class::ResultSource")
'
Peter Rabbitson [Wed, 13 Jul 2016 13:45:37 +0000]
Revert C3-fication
d009cb7d and fixups 7f068248 and 983f766d
While on its surface this was a good idea, it actually hides problems even
more: by the time we arrive at a useful hook-point to check the current MRO,
something likely already changed it from under us, and the old effects are
all masked away for good.
So instead scale back as much as possible, and set 'c3' where needed as
lazily as practical. In order to satisfy the mro requirements imposed by
5e0eea35 we do the "flip" during the ->source() stage.
Additionally we record the original setting any time we switch the mro on
foreign classes (two such spots in the codebase). A later commit will use
this information to add the final bit of sanity to this clusterfuck.
Peter Rabbitson [Mon, 25 Apr 2016 09:53:54 +0000]
Rename variables/shuffle some code, preparing for next commits
Zero functional changes
Read under -w
Peter Rabbitson [Fri, 22 Jul 2016 12:15:53 +0000]
Abstract our internal capture_stderr test routine
Will need it for even more tests later on, but not sufficiently often to
warrant depending on Capture::Tiny - just go with what we need
Peter Rabbitson [Wed, 20 Jul 2016 12:13:49 +0000]
Fix misleading error on deployment_statements in void ctx
Due to how Context::Preserve operates the following would result in a
non-sensical error:
perl -MDBIx::Class::Schema -e '
DBIx::Class::Schema->connect("dbi:SQLite::memory:")->deployment_statements;
1
'
Peter Rabbitson [Wed, 20 Jul 2016 10:17:49 +0000]
Fix false negatives in lean_startup.t
Before this commit a 'use Devel::Dwarn' somewhere deep in e.g. ::SQLMaker
would *not* have been detected.
Also fix improperly applied C-ism: the following decidedly does not DTRT
do 1 while { ... }
I never noticed this being a problem until a fatfingered `>&1` turned into
a `>1`, which in turn created a file named 1 in the current directory with
garbage in it, which *in turn* the `do 1 ...` tried to execute. Sigh...
Peter Rabbitson [Sat, 16 Jul 2016 11:59:30 +0000]
Fix error-eating thinko from
6c7ca962
$@ is not visible in $SIG{__DIE__}
Peter Rabbitson [Sat, 16 Jul 2016 11:52:00 +0000]
Restore TODO checking for Taint + pkg_gen inconsitencies
Now that https://github.com/Test-More/test-more/issues/683 we can bring the
TODO back - will get an alert for an eventual fix...
Peter Rabbitson [Sat, 16 Jul 2016 11:48:52 +0000]
Add an explicit Sub::Quote dep in ::_Util
Failures too confusing otherwise
Peter Rabbitson [Sat, 16 Jul 2016 11:29:40 +0000]
Work around the FIXME in the previous commit
Based on @haarg's excellent detective work: https://is.gd/perl_mro_taint_wtf
Peter Rabbitson [Thu, 14 Jul 2016 13:20:03 +0000]
Get rid of Package::Stash \o/
Internal tooling advanced sufficiently without planning for any of that:
a good indicator things are on the right track!
Read under -w
Peter Rabbitson [Thu, 14 Jul 2016 12:50:02 +0000]
Add a get_subname to _Util
Peter Rabbitson [Wed, 13 Jul 2016 16:28:23 +0000]
Insulate DBIC::Carp from rogue can() overrides
Peter Rabbitson [Thu, 14 Jul 2016 11:03:26 +0000]
Add more forceful (STDERR-direct) warning emitter
Switch some of the most critical announements to it
Peter Rabbitson [Thu, 30 Jun 2016 19:55:55 +0000]
Fix POISON_ENV warning missed in both
5c33c8be and 44c1a75d
Peter Rabbitson [Wed, 13 Jul 2016 15:51:00 +0000]
Silence inactionable warning (mainly on travis)
Peter Rabbitson [Wed, 13 Jul 2016 15:57:18 +0000]
(travis) Switch to our own copy of the Firebird ODBC driver
Sourceforge is just way too unstable
Peter Rabbitson [Sun, 3 Jul 2016 08:43:20 +0000]
(travis) Properly diagnose potential OOM in 50_after_success.bash
Idea originally introduced in
ac4e80df
Peter Rabbitson [Wed, 29 Jun 2016 17:29:27 +0000]
(travis) Add poisoning to the base non-clean build run
Peter Rabbitson [Fri, 24 Jun 2016 16:31:01 +0000]
Fill in missing documentation in ::Schema / ::ResultSource
Peter Rabbitson [Fri, 1 Jul 2016 10:22:48 +0000]
Audit all local() calls within lib/ and t/lib
Correct some of them to fire less frequently (local is *expensive*)
Peter Rabbitson [Mon, 27 Jun 2016 08:29:27 +0000]
Expand describe_class_methods testing yet again
This should be the end of adjustments, so many corner cases...
Peter Rabbitson [Wed, 29 Jun 2016 22:49:39 +0000]
Use a single cache struct for entirety of describe_class_methods
This will allow influencing the cache from outside like shown below, (but
please, DO NOT DO SO), and in turn will make sanity checks on 5.8 somewhat
acceptable *by default* \o/
$...::__describe_class_query_cache->{"!internal!"} = {};
Peter Rabbitson [Wed, 29 Jun 2016 22:40:15 +0000]
Add hash-based ISA lookup to RV of describe_class_methods