Repository navigation
Add the Metadata exception namespace - #191
Merged
simon-mundy merged 2 commits intoSep 29, 2026
Merged
Conversation
This was referenced Sep 28, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
9 tasks done
simon-mundy
force-pushed
the
exception/pr-2-metadata-exception
branch
from
September 28, 2026 06:38
86526bb to
bf8f4a8
Compare
This was referenced Sep 28, 2026
simon-mundy
force-pushed
the
exception/pr-2-metadata-exception
branch
from
September 28, 2026 22:52
bf8f4a8 to
1496903
Compare
simon-mundy
added a commit
to simon-mundy/phpdb
that referenced
this pull request
Sep 28, 2026
… constructors The last 5 throw sites outside the components covered by php-db#191 to php-db#194. Message text is byte-identical, verified template by template against the originals. - Add ResultSet\Exception\RuntimeException::forUnbufferedIteration() - Add forNonAdapterAwareDelegate() to Exception\ContainerException, and give the three reasons that AdapterInterfaceFactory and AbstractAdapterInterfaceFactory passed as inline strings their own factories: forMissingConfigService(), forMissingConfiguration() and forMissingDriver(), each composing the reason into the SERVICE template - Retype the delegator's non-AdapterAware guard from the root Exception\RuntimeException to ContainerException, which extends it, so the failure carries PSR-11's ContainerExceptionInterface as a container failure should - Own the "not found in container" wording as ContainerException::MISSING_SERVICE while still throwing the ServiceManager's own ServiceNotFoundException. Laminas annotates that class @Final, so this package cannot own it; the throw site renders the template itself, which docs/book/exceptions.md now records as the single documented exception to the convention - Derive the message assertions in the six test files the earlier PRs do not touch from the templates rather than restating them One literal assertion is deliberately left in test/unit/Container/TableIdentifierFactoryFactoryTest.php: it asserts Sql\Exception\InvalidArgumentException::EMPTY_SEPARATOR, a constant php-db#192 introduces, so it can only be migrated once that has landed. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
Metadata was the only component with no exception namespace, so its failures were invisible to catch (PhpDb\Exception\ExceptionInterface). - Add Metadata\Exception\InvalidArgumentException and RuntimeException, matching the other components. The namespace declares no ExceptionInterface of its own: both classes extend their PhpDb\Exception counterparts, where the single marker is declared, so they implement it by inheritance - Register both concrete classes in ExceptionHierarchyTest's EXCEPTION_CLASSES map, so they come under the structural guards - Replace the seven bare \Exception throws in AbstractSource with named constructors and repoint the eleven @throws annotations at them - Converge the two "table does not exist" messages on 'Table "%s" does not exist', matching the Trigger and View wording already in the class - Derive the AbstractSource message assertions from the templates rather than restating them, and call the factories in the test body so they record coverage, which a data provider does not Refs php-db#189 Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
simon-mundy
force-pushed
the
exception/pr-2-metadata-exception
branch
from
September 29, 2026 03:33
1496903 to
88b5bf5
Compare
simon-mundy
added a commit
to simon-mundy/phpdb
that referenced
this pull request
Sep 29, 2026
… constructors The last 5 throw sites outside the components covered by php-db#191 to php-db#194. Message text is byte-identical, verified template by template against the originals. - Add ResultSet\Exception\RuntimeException::forUnbufferedIteration() - Add forNonAdapterAwareDelegate() to Exception\ContainerException, and give the three reasons that AdapterInterfaceFactory and AbstractAdapterInterfaceFactory passed as inline strings their own factories: forMissingConfigService(), forMissingConfiguration() and forMissingDriver(), each composing the reason into the SERVICE template - Retype the delegator's non-AdapterAware guard from the root Exception\RuntimeException to ContainerException, which extends it, so the failure carries PSR-11's ContainerExceptionInterface as a container failure should - Own the "not found in container" wording as ContainerException::MISSING_SERVICE while still throwing the ServiceManager's own ServiceNotFoundException. Laminas annotates that class @Final, so this package cannot own it; the throw site renders the template itself, which docs/book/exceptions.md now records as the single documented exception to the convention - Derive the message assertions in the six test files the earlier PRs do not touch from the templates rather than restating them One literal assertion is deliberately left in test/unit/Container/TableIdentifierFactoryFactoryTest.php: it asserts Sql\Exception\InvalidArgumentException::EMPTY_SEPARATOR, a constant php-db#192 introduces, so it can only be migrated once that has landed. Signed-off-by: Simon Mundy <simon.mundy@peptolab.com>
simon-mundy
added
documentation
tyrsson
previously approved these changes
Sep 29, 2026
simon-mundy
dismissed
tyrsson’s stale review
September 29, 2026 03:48
The merge-base changed after approval.
tyrsson
approved these changes
Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Types of changes
Checklist:
Description
PR 2 of 6 from #189, and the first worked example of the convention PR 1 documents.
PhpDb\Metadatawas the only component with no exception namespace at all.Metadata\Source\AbstractSourcethrew bare\Exceptionat seven sites, so every Metadata failure was invisible tocatch (PhpDb\Exception\ExceptionInterface).Metadata\Exception\ExceptionInterface,InvalidArgumentExceptionandRuntimeException, matching the other components.\Exceptionthrows with named constructors:forUnknownColumn(),forUnknownConstraint(),forUnknownTable(),forUnknownTrigger(),forUnknownView()andforUnsupportedTableType().@throws Exceptionannotations, which resolved to the new namespace rather than a class once theuse Exception;import went.AbstractSourcereported the same missing table two ways:'"%s" does not exist'ingetColumnNames()and'Table "%s" does not exist'ingetTable(). They now share'Table "%s" does not exist', which matches theTrigger "%s"andView "%s"wording already in the class and names what is actually missing. Verified that no test inphpdb-mysql,phpdb-pgsqlorphpdb-sqliteasserts on the old form.Six named constructors rather than the seven listed on #189, because that convergence removed the need for a separate
forUnknownTableName().Motivation and Context
Refs #189. Depends on #190 for the convention and for the naming guard that this PR's constants have to satisfy.
Message text is otherwise byte-identical, so this is a mechanical change apart from the one converged message. The retype from bare
\Exceptionis not a practical BC break: the new classes extend\RuntimeException, which extends\Exception, so existingcatch (\Exception)blocks still match.How has this been tested?
mago format --check,mago lintandmago analyzeall exit 0. Full unit suite passes: 1583 tests, 3459 assertions.Metadata\Exception\RuntimeExceptionis at 6/6 methods and 7/7 lines covered. Getting there caught a defect worth noting: the first version of the test built its cases by calling the factories inside the data provider, which runs outside the coverage-collected region, so all six recorded zero coverage while every assertion passed. The provider now supplies the factory name, its arguments and its template, and the factory is called in the test body.Expected messages are rendered from the public templates rather than restated, in the new test and in the eight
AbstractSourceTestmessage assertions. Verified these still fail on a real defect by swapping the two arguments inforUnsupportedTableType(), which produced 2 failures.Baseline changes are limited to three
src/Metadata/Exception/entries inanalyzer-baseline.toml, matching how the sibling components' exception classes are already baselined forclass-must-be-finalandmissing-api-or-internal.lint-baseline.tomlis untouched by this commit.