Complete AbstractMapper abstraction, add style tests#10
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors AbstractMapper to remove SQL/PDO-specific internals by making fetch() and fetchAll() part of the mapper’s abstract contract, and adds a comprehensive PHPUnit test suite for naming “styles” plus an in-memory mapper test double to exercise the full mapper/styling behavior without a database.
Changes:
- Make
AbstractMapper::fetch()andAbstractMapper::fetchAll()abstract, and remove the previous statement/hydration-specific implementation. - Add
InMemoryMapper(test double) and multiple integration tests that validate typed fetching, FK resolution, and persist/flush round-trips across styles. - Add style-specific unit tests and update QA tooling configs (PHPCS + PHPStan) to accommodate test entity stubs.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AbstractMapper.php | Moves fetch()/fetchAll() into the abstract contract and removes SQL/hydration internals. |
| tests/AbstractMapperTest.php | Updates the test anonymous mapper to implement the new abstract fetch()/fetchAll() methods. |
| tests/InMemoryMapper.php | Adds an array-backed mapper used by tests to exercise the mapper contract end-to-end. |
| tests/Styles/AbstractStyleTest.php | Adds unit tests for AbstractStyle inflection and name conversion helpers. |
| tests/Styles/StandardTest.php | Adds coverage for Standard naming conventions (table/entity, columns/properties, FK, composition). |
| tests/Styles/PluralTest.php | Adds coverage for Plural naming conventions. |
| tests/Styles/CakePHPTest.php | Adds coverage for CakePHP naming conventions. |
| tests/Styles/NorthWindTest.php | Adds coverage for NorthWind naming conventions. |
| tests/Styles/SakilaTest.php | Adds coverage for Sakila naming conventions. |
| tests/Styles/CakePHP/CakePHPIntegrationTest.php | Integration tests for CakePHP style using InMemoryMapper. |
| tests/Styles/CakePHP/Author.php | CakePHP-style entity stub for integration tests. |
| tests/Styles/CakePHP/Category.php | CakePHP-style entity stub for integration tests. |
| tests/Styles/CakePHP/Comment.php | CakePHP-style entity stub for integration tests. |
| tests/Styles/CakePHP/Post.php | CakePHP-style entity stub for integration tests. |
| tests/Styles/CakePHP/PostCategory.php | CakePHP-style entity stub for integration tests. |
| tests/Styles/Plural/PluralIntegrationTest.php | Integration tests for Plural style using InMemoryMapper. |
| tests/Styles/Plural/Author.php | Plural-style entity stub for integration tests. |
| tests/Styles/Plural/Category.php | Plural-style entity stub for integration tests. |
| tests/Styles/Plural/Comment.php | Plural-style entity stub for integration tests. |
| tests/Styles/Plural/Post.php | Plural-style entity stub for integration tests. |
| tests/Styles/Plural/PostCategory.php | Plural-style entity stub for integration tests. |
| tests/Styles/NorthWind/NorthWindIntegrationTest.php | Integration tests for NorthWind style using InMemoryMapper. |
| tests/Styles/NorthWind/Authors.php | NorthWind-style entity stub for integration tests. |
| tests/Styles/NorthWind/Categories.php | NorthWind-style entity stub for integration tests. |
| tests/Styles/NorthWind/Comments.php | NorthWind-style entity stub for integration tests. |
| tests/Styles/NorthWind/PostCategories.php | NorthWind-style entity stub for integration tests. |
| tests/Styles/NorthWind/Posts.php | NorthWind-style entity stub for integration tests. |
| tests/Styles/Sakila/SakilaIntegrationTest.php | Integration tests for Sakila style using InMemoryMapper. |
| tests/Styles/Sakila/Author.php | Sakila-style entity stub for integration tests. |
| tests/Styles/Sakila/Category.php | Sakila-style entity stub for integration tests. |
| tests/Styles/Sakila/Comment.php | Sakila-style entity stub for integration tests. |
| tests/Styles/Sakila/Post.php | Sakila-style entity stub for integration tests. |
| tests/Styles/Sakila/PostCategory.php | Sakila-style entity stub for integration tests. |
| phpstan.neon.dist | Updates ignore patterns to account for InMemoryMapper dynamic property access in tests. |
| phpcs.xml.dist | Excludes a naming-convention sniff for test code/stubs that use snake_case properties. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Remove SQL-specific internals from AbstractMapper: make fetch() and fetchAll() abstract, remove createStatement(), fetchHydrated(), and parseHydrated() which encoded PDO/hydration patterns that don't belong in a generic data layer. The only abstract methods are now flush(), fetch(), and fetchAll(). Add comprehensive style tests: 6 pure naming-convention test files (AbstractStyle, Standard, CakePHP, NorthWind, Plural, Sakila) covering table/entity, column/property, foreign key, and composition naming for all styles. Add InMemoryMapper test double that exercises the full AbstractMapper contract and all 7 Stylable methods using arrays instead of SQL. Add 4 integration test files with entity stubs that verify typed fetch, nested FK resolution, and persist/flush round-trips through the in-memory backend.
alganet
force-pushed
the
abstract-mapper
branch
from
8cdd235 to
5296b7a
Compare
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.
Remove SQL-specific internals from AbstractMapper: make fetch() and fetchAll() abstract, remove createStatement(), fetchHydrated(), and parseHydrated() which encoded PDO/hydration patterns that don't belong in a generic data layer. The only abstract methods are now flush(), fetch(), and fetchAll().
Add comprehensive style tests: 6 pure naming-convention test files (AbstractStyle, Standard, CakePHP, NorthWind, Plural, Sakila) covering table/entity, column/property, foreign key, and composition naming for all styles.
Add InMemoryMapper test double that exercises the full AbstractMapper contract and all 7 Stylable methods using arrays instead of SQL. Add 4 integration test files with entity stubs that verify typed fetch, nested FK resolution, and persist/flush round-trips through the in-memory backend.