Repository navigation
Bring TableGateway and RowGateway to Mago compliance - #233
Open
simon-mundy wants to merge 13 commits into
Open
simon-mundy wants to merge 13 commits into
simon-mundy wants to merge 13 commits into
Conversation
The gateway dependencies stay nullable so subclasses can still fill them
in before initialize(). Each read now goes through a private guard that
throws a catalogued exception when the value is unset:
RuntimeException::forUninitializedProperty() for featureSet, sql and
resultSetPrototype, and the existing forMissingAdapter(),
forMissingTable(), forMissingSqlObject() and forMissingPrimaryKeyColumn()
elsewhere. A statement that returns no result now throws
forMissingStatementResult() instead of a TypeError, and RowGateway::save()
throws forMissingSavedRow() when the saved row cannot be read back.
Saving a row whose composite primary key changed in only one column
reselected with null for the unchanged columns. It now reselects by the
new key.
getRawState('table') on Delete, Insert and Update, and
getRawState(Select::TABLE), get conditional return types so the
gateways read a typed table. ResultInterface declares its Iterator
template, which also clears four Adapter and ResultSet findings.
RowGatewayFeature checks that each primary key column from metadata is a
string.
Every gateway class and interface is tagged @api. AbstractTableGateway,
AbstractRowGateway, both FeatureSets and EventFeature join the
too-many-methods exclusions.
Twelve gateway findings stay baselined:
- class-level cyclomatic-complexity and kan-defect on the two abstract
gateways and TableGateway\Feature\FeatureSet, and cyclomatic-complexity
on RowGatewayFeature;
- no-boolean-flag-parameter on populate(), a public signature;
- five less-specific-nested-argument-type findings where row data reaches
the Sql builders: populate() accepts any array per RowPrototypeInterface,
so the values cannot be narrowed to ColumnValue without validating them.
simon-mundy
force-pushed
the
qa/mago-gateways
branch
from
October 11, 2026 23:49
cf06e37 to
ed02b5a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
The protected property is restored to match its TableGateway twin, so subclasses that read it keep working. The class was the only one in src tagged @Final, which made the analyzer treat the protected property as private and report it unused. It is tagged @api instead, like the TableGateway FeatureSet and every other gateway class.
executeDelete(), executeInsert() and executeUpdate() replaced an aliased table on the statement with the bare table before preparing it, then put the alias back afterwards. Delete, Insert and Update now drop the alias themselves when they render, through AbstractPreparableSql::unaliasTable(), so the swap no longer changes the SQL. It also left the caller's statement holding the bare table if preparing or executing threw, because the restore was not in a finally block. The swap and the private unaliasTable() helper are removed, so the gateway no longer modifies the statement it is given. The three *ShouldResetTableToUnaliasedTable tests, which asserted the swapped state, now assert that the statement keeps its alias. dataChangingCallOnAliasedTableRendersTheBareTable still covers the rendered SQL.
GatewayTable and GatewayWhere describe the TableGatewayInterface contract, so the interface defines them. AbstractTableGateway and TableGateway import them from the interface, and GatewayJoin, used only by AbstractTableGateway, stays there.
…sTable() PHPUnit reports a CoversMethod target that no longer exists as a warning, which fails the coverage run.
PHPUnit on PHP 8.4 rejects the private static unaliasTable() as a CoversMethod target, which failed the coverage run. CoversClass also attributes the private helpers without naming each one.
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.
Takes
src/TableGatewayandsrc/RowGatewayfrom 225 baselined findings to 12. It also fixes a composite-key bug inRowGateway::save().Design: nullable dependencies with guards
$adapter,$table,$sql,$featureSetand$resultSetPrototypeonAbstractTableGateway, and$table,$primaryKeyColumnand$sqlonAbstractRowGateway, stay nullable. Subclasses can still fill them in beforeinitialize()runs, so no subclass breaks. Every read now goes through a private guard that throws a catalogued exception when the value is unset:adapter(getAdapter(),initialize())TableGateway\Exception\RuntimeException::forMissingAdapter()(existing)table(getTable(),initialize())TableGateway\Exception\RuntimeException::forMissingTable()(existing)featureSet,sql,resultSetPrototypeafter initialisationTableGateway\Exception\RuntimeException::forUninitializedProperty()sql,primaryKeyColumnRowGateway\Exception\RuntimeException::forMissingSqlObject(),forMissingPrimaryKeyColumn()(existing)Each of these used to end in a
TypeErroror a call onnull.Bug fixed
['one', 'two']after changing onlytworeselected withone = NULL, and wrote an undefined-index warningone = 1, two = 3saveUpdateChangingOneColumnOfACompositeKeyReselectsByTheNewKeyfails on #232.Other behaviour changes
$statement->execute()returningnullnow throwsforMissingStatementResult()from both gateways. It used to throw aTypeError.RowGateway::save()throwsforMissingSavedRow()when the saved row cannot be read back. It used to pass a non-array topopulate().AbstractRowGateway::offsetGet()returnsnullfor an unknown column instead of raising an undefined-index warning.__get()still throws.AbstractRowGateway::populate()'s first parameter is renamed from$rowDatato$data, matchingRowPrototypeInterface. A caller passing it by name asrowData:must update.__clone()on a gateway that has not been initialised no longer callsclone null.RowGatewayFeaturechecks that every primary key column it reads fromMetadataFeatureis a string, and throws the existingforUnusablePrimaryKey()otherwise. It also re-indexes the columns as a list, soRowGatewayreads column[0]correctly for a keyed array.RowGateway\Feature\FeatureSetis tagged@apiinstead of@final, matchingTableGateway\Feature\FeatureSet.executeDelete(),executeInsert()andexecuteUpdate()leave the statement's table as given.Delete,InsertandUpdatedrop an alias when they render, so the SQL is unchanged, and a failed query no longer leaves the statement without its alias.Types
GatewayTableandGatewayWhereonTableGatewayInterface, andGatewayJoinonAbstractTableGateway. They type the interface, both gateway classes, and the$where,$setand$joinsarguments.$setuses the SqlColumnValuealias.Delete,InsertandUpdate::getRawState('table'), andSelect::getRawState(Select::TABLE), get conditional return types. The gateways now read the table through these calls rather than indexing the whole raw state.ResultInterfacedeclares@extends Iterator<array-key, mixed>, which also clears four findings inPdo\Result,ResultInterfaceandAbstractResultSet.@throwstags carry the SqlRuntimeExceptionfromprepareStatementForSqlObject()through every gateway method it reaches.getAdapter()rather than the magicadapterproperty.@apiandmago.toml@api, as in Bring Sql\Predicate to full Mago compliance #230 to Bring the Sql DML classes to Mago compliance #232.AbstractTableGateway,AbstractRowGateway, bothFeatureSets andEventFeaturejoin thetoo-many-methodsexclusions, with a justification comment. Their method count is the size of their public API: one method per gateway operation, hook or event.offsetSet()andoffsetUnset()returnstaticfor chaining whereArrayAccessdeclaresvoid. Both carry a scoped@mago-expect analysis:incompatible-return-type.Still baselined (12)
cyclomatic-complexityandkan-defectonAbstractTableGatewayandAbstractRowGateway, andkan-defectonTableGateway\Feature\FeatureSet. Clearing these needs the classes split up.cyclomatic-complexityonRowGatewayFeature. This one is new: the per-column string check pushes the class over the threshold, and every shorter form I tried still crossed it.no-boolean-flag-parameteronpopulate(array $data, bool $rowExistsInDatabase), a public signature.less-specific-nested-argument-typefindings where row data reachesDelete::where(),Select::where(),Insert::values(),Update::set()andUpdate::where().populate()accepts any array underRowPrototypeInterface, so the values can't be narrowed toColumnValuewithout validating each one at runtime.Result
src/TableGateway+src/RowGateway, both baselines* Before this PR, 67 of the gateway mutants were "killed" only because they raised analyzer findings the baseline didn't cover. With the files clean, 39 of those survived at first. The new tests kill all of them: feature-event arguments, initialise-before-use for each operation, overridable execute hooks, closure paths and explicit columns.
mago format --check,lint,analyzeandguardare clean.