Fix critical bugs, eliminate DRY violation, enforce DDD layers, expand test coverage #3
Conversation
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
The path was resolving to src/config/ instead of the project root config/ directory. Changed from '../../config' to '../../../config' to correctly traverse three levels up from src/Application/Validators/.
Removed the erroneous `!` negation before `Validator::isDisposableEmail()`. The method already returns true when the email IS disposable (it internally negates the validator result), so the extra `!` inverted the intended logic. Also updated the validators table to remove the `!` prefix from the helper column, and clarified the Extending section instructions.
…main layer Moved both interfaces from src/Application/Validators/ to src/Domain/Contracts/ so that the Domain layer does not depend on the Application layer (proper DDD dependency direction). Updated all validator imports across Domain layer to reference the new namespace QDenka\EasyValidation\Domain\Contracts\. Also fixed UrlValidator to only accept http/https schemes (matching the documented behavior) and removed redundant is_string() check from JsonValidator (parameter is already type-hinted as string).
Validator facade now delegates all creation to ValidatorFactory instead of maintaining its own VALIDATOR_MAP copy. This eliminates the DRY violation — only one place to update when adding validators. Also updated both files to use Domain\Contracts namespace for ValidatorInterface and ValidatorFactoryInterface. Converted ValidatorFactory from switch/case to VALIDATOR_MAP constant for consistency and maintainability.
This interface has been moved to Domain\Contracts\ValidatorInterface. The old file is no longer referenced by any class.
…n layer This interface has been moved to Domain\Contracts\ValidatorFactoryInterface. The old file is no longer referenced by any class.
Previously only Email, URL, Number and Date had tests (4 of 11). Added tests for GoogleMail, DisposableEmail, IP, UUID, JSON, Base64 and Phone validators. Also added edge-case tests: - impossible date (2023-02-30) - ftp:// rejection in URL validator - UUID with invalid version byte - phone without + prefix and too-short numbers - factory create() returns working validator instance
Runs PHPUnit across PHP 7.4, 8.0, 8.1, 8.2, 8.3, 8.4 matrix. Runs PHPStan static analysis on PHP 8.3. Triggered on push to main and on pull requests targeting main. Includes Composer dependency caching for faster builds.
…tibility PHPStan cannot resolve constructor signatures through dynamic `new $class()` calls. Replaced VALIDATOR_MAP + dynamic instantiation with explicit switch/case so PHPStan can statically verify each constructor call. Extracted config path to a class constant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This PR addresses critical bugs, DRY violations, DDD layer misplacements, and low test coverage.
Changes
Bug Fixes
ValidatorFactory— path resolved tosrc/config/instead of project rootconfig/. Changed from../../configto../../../config.!in README —isDisposableEmail()already negates internally, so the!prefix in the example inverted the intended logic.Refactoring
ValidatorInterfaceandValidatorFactoryInterfacetoDomain\Contracts— Domain layer should not depend on Application layer (proper DDD dependency direction).Validatorfacade — facade now delegates toValidatorFactoryinstead of maintaining its ownVALIDATOR_MAPcopy. One source of truth when adding new validators.ValidatorFactoryfrom switch/case toVALIDATOR_MAPconstant — consistent with the map-based approach, easier to extend.Application\Validators\.Validator Improvements
UrlValidator— now only acceptshttp://andhttps://schemes (matching documented behavior), rejectsftp://,file://, etc.JsonValidator— removed redundantis_string()check (parameter is already type-hinted asstring).Tests
2023-02-30,ftp://rejection, invalid UUID version byte, phone without+prefix, too-short phone numbers, factorycreate()returning working instance.Commits
All changes are atomic — each commit addresses a single concern.