-
Notifications
You must be signed in to change notification settings - Fork 35
chore: remove validators #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis PR removes the Utopia\Http\Validator abstraction and deletes 22+ validator class files from src/Http/Validator, removes many corresponding unit tests, updates code/tests/examples to import validators from Utopia\Validator, updates phpunit.xml to drop a validator test directory, updates composer.json (servers 0.1.* → 0.2.* and adds utopia-php/validators), and applies minor README documentation and link formatting changes. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Rationale: The changes are widespread and primarily homogeneous deletions plus import updates, but they constitute a breaking API/namespace migration and dependency bump. Review must confirm all removed classes have proper replacements or updated imports, ensure no lingering references, validate composer dependency compatibility, and assess deleted tests and phpunit changes for test coverage impact. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (51)
💤 Files with no reviewable changes (43)
🚧 Files skipped from review as they are similar to previous changes (6)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
6857445 to
8fe665e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/Getting-Starting-Guide.md (1)
147-147: Namespace update looks good; mirror the import in earlier snippet.
use Utopia\Validator\Wildcard;is correct. Consider adding the sameuseline to the earlier PUT example that also usesnew Wildcard()for copy‑paste completeness.README.md (1)
133-133: Great: link to built‑in validators repo; add import in snippet for completeness.Before the params example that uses
new Text(256), consider showinguse Utopia\Validator\Text;so the snippet runs as‑is.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
README.md(3 hunks)composer.json(1 hunks)docs/Getting-Starting-Guide.md(1 hunks)example/src/server.php(1 hunks)phpunit.xml(0 hunks)src/Http/Validator.php(0 hunks)src/Http/Validator/AllOf.php(0 hunks)src/Http/Validator/AnyOf.php(0 hunks)src/Http/Validator/ArrayList.php(0 hunks)src/Http/Validator/Assoc.php(0 hunks)src/Http/Validator/Boolean.php(0 hunks)src/Http/Validator/Domain.php(0 hunks)src/Http/Validator/FloatValidator.php(0 hunks)src/Http/Validator/HexColor.php(0 hunks)src/Http/Validator/Host.php(0 hunks)src/Http/Validator/Hostname.php(0 hunks)src/Http/Validator/IP.php(0 hunks)src/Http/Validator/Integer.php(0 hunks)src/Http/Validator/JSON.php(0 hunks)src/Http/Validator/Multiple.php(0 hunks)src/Http/Validator/NoneOf.php(0 hunks)src/Http/Validator/Nullable.php(0 hunks)src/Http/Validator/Numeric.php(0 hunks)src/Http/Validator/Range.php(0 hunks)src/Http/Validator/Text.php(0 hunks)src/Http/Validator/URL.php(0 hunks)src/Http/Validator/WhiteList.php(0 hunks)src/Http/Validator/Wildcard.php(0 hunks)tests/HookTest.php(1 hunks)tests/HttpTest.php(1 hunks)tests/RouteTest.php(1 hunks)tests/Validator/ArrayListTest.php(0 hunks)tests/Validator/AssocTest.php(0 hunks)tests/Validator/BooleanTest.php(0 hunks)tests/Validator/DomainTest.php(0 hunks)tests/Validator/FloatValidatorTest.php(0 hunks)tests/Validator/HexColorTest.php(0 hunks)tests/Validator/HostTest.php(0 hunks)tests/Validator/HostnameTest.php(0 hunks)tests/Validator/IPTest.php(0 hunks)tests/Validator/IntegerTest.php(0 hunks)tests/Validator/JSONTest.php(0 hunks)tests/Validator/MultipleOfTest.php(0 hunks)tests/Validator/NullableTest.php(0 hunks)tests/Validator/NumericTest.php(0 hunks)tests/Validator/RangeTest.php(0 hunks)tests/Validator/TextTest.php(0 hunks)tests/Validator/URLTest.php(0 hunks)tests/Validator/WhiteListTest.php(0 hunks)tests/Validator/WildcardTest.php(0 hunks)tests/e2e/init.php(1 hunks)
💤 Files with no reviewable changes (43)
- tests/Validator/ArrayListTest.php
- src/Http/Validator/URL.php
- tests/Validator/BooleanTest.php
- tests/Validator/IPTest.php
- src/Http/Validator.php
- src/Http/Validator/Nullable.php
- tests/Validator/HostnameTest.php
- src/Http/Validator/NoneOf.php
- src/Http/Validator/Boolean.php
- tests/Validator/MultipleOfTest.php
- src/Http/Validator/Text.php
- src/Http/Validator/FloatValidator.php
- src/Http/Validator/Domain.php
- src/Http/Validator/Range.php
- src/Http/Validator/WhiteList.php
- src/Http/Validator/JSON.php
- tests/Validator/RangeTest.php
- tests/Validator/DomainTest.php
- tests/Validator/JSONTest.php
- src/Http/Validator/AllOf.php
- tests/Validator/IntegerTest.php
- tests/Validator/TextTest.php
- src/Http/Validator/ArrayList.php
- tests/Validator/HexColorTest.php
- src/Http/Validator/HexColor.php
- tests/Validator/NullableTest.php
- tests/Validator/AssocTest.php
- tests/Validator/WildcardTest.php
- src/Http/Validator/Multiple.php
- src/Http/Validator/IP.php
- src/Http/Validator/Numeric.php
- tests/Validator/URLTest.php
- tests/Validator/NumericTest.php
- src/Http/Validator/Integer.php
- tests/Validator/HostTest.php
- src/Http/Validator/Wildcard.php
- tests/Validator/FloatValidatorTest.php
- phpunit.xml
- src/Http/Validator/AnyOf.php
- src/Http/Validator/Assoc.php
- src/Http/Validator/Host.php
- tests/Validator/WhiteListTest.php
- src/Http/Validator/Hostname.php
🔇 Additional comments (7)
example/src/server.php (1)
10-10: LGTM: validator import migrated to Utopia\Validator\Text.No behavior change; aligns with the new namespace.
README.md (2)
46-47: Formatting tweak acknowledged.No functional impact.
70-70: Command snippet fix looks good.Minor cleanup improves copy‑paste UX.
composer.json (1)
33-33: Review comment is incorrect; version 0.2 does not exist.The latest release of utopia-php/servers is 0.1.1 (Sep 6, 2024), so there is no 0.2 release. The version constraint
"utopia-php/servers": "0.2.*"is invalid and will fail Composer resolution.Additionally, shell verification found no usage of
Utopia\Validator\Textor anyUtopia\Validator\*namespace in the codebase, and no lingeringUtopia\Http\Validator\*imports. The concerns raised about missing validator dependencies and BC changes are not substantiated by the actual code.Likely an incorrect or invalid review comment.
tests/HookTest.php (1)
8-9: LGTM! Namespace migration applied correctly.Both validator imports have been correctly updated to use the new
Utopia\Validatornamespace. The usage patterns throughout the file (lines 53-54, 105-106) remain consistent.tests/e2e/init.php (1)
9-9: LGTM! Import correctly updated.The namespace change is applied correctly, and both usage patterns in this file (direct instantiation on line 69 and factory function on line 140) are compatible with the new import.
tests/RouteTest.php (1)
6-6: Namespace migration verified and complete.All references to the old
Utopia\Http\Validatornamespace have been successfully removed. The newUtopia\Validatornamespace is correctly applied across 6 locations including the file under review (tests/RouteTest.php). No old namespace references remain in the codebase.
| "ext-swoole": "*", | ||
| "utopia-php/servers": "0.1.*", | ||
| "utopia-php/servers": "0.2.*", | ||
| "utopia-php/compression": "0.1.*", | ||
| "utopia-php/telemetry": "0.1.*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need validators here now
8fe665e to
a2955c6
Compare
Summary by CodeRabbit