⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Containerize sokil databases#1638

Merged
alganet merged 1 commit intoRespect:mainfrom
alganet:sokil-singletons
Jan 29, 2026
Merged

Containerize sokil databases#1638
alganet merged 1 commit intoRespect:mainfrom
alganet:sokil-singletons

Conversation

@alganet
Copy link
Member

@alganet alganet commented Jan 28, 2026

The main focus of this change is to make those optional dependencies more testable.

Unfortunately, some phpstan-ignores had to be included, since ::set is not a PsrContainer method. We're only using it on tests though, so it's fine. It targets our php-di container for testing purposes only. The real implementation only relies on ::get.

This change also has the side effect of improving the performance of those validators by not instantiating their databases each time a iso validator is built, achieving massive improvements in those scenarios. A small benchmark with no assertions was added to track that improvement.


This brings the total coverage to almost 99%, addressing one of the key uncoverable paths that existed before.

@codecov
Copy link

codecov bot commented Jan 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.00%. Comparing base (0381718) to head (d0fdab5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1638      +/-   ##
============================================
+ Coverage     98.11%   99.00%   +0.88%     
- Complexity      960      962       +2     
============================================
  Files           197      197              
  Lines          2231     2214      -17     
============================================
+ Hits           2189     2192       +3     
+ Misses           42       22      -20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the ISO-code-related validators to resolve Sokil ISO Codes database instances via ContainerRegistry, making the optional dependency behavior testable and enabling reuse of those database instances. It also adds targeted unit tests for missing optional components and introduces a PhpBench benchmark to track performance.

Changes:

  • Add container-managed, optional bindings for Sokil ISO Codes database classes via ContainerRegistry.
  • Update ISO-code-related validators (country/currency/language/subdivision/phone) to fetch databases from the container and throw MissingComposerDependencyException when unavailable.
  • Expand unit tests for “missing optional dependency” behavior and add an ISO-codes benchmark suite.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/ContainerRegistry.php Registers optional container entries for Sokil ISO Codes databases and adds optional() helper.
src/Exceptions/MissingClassException.php Adds a PSR-11 NotFoundExceptionInterface implementation for missing optional classes.
src/Validators/CountryCode.php Resolves Countries from the container instead of instantiating directly.
src/Validators/CurrencyCode.php Resolves Currencies from the container instead of instantiating directly.
src/Validators/LanguageCode.php Resolves Languages from the container instead of instantiating directly.
src/Validators/SubdivisionCode.php Resolves Countries/Subdivisions from the container instead of instantiating directly.
src/Validators/Phone.php Resolves Countries from the container when a country code is provided.
tests/unit/ContainerRegistryTest.php Adds coverage for ContainerRegistry::optional() throwing on missing classes.
tests/unit/Validators/CountryCodeTest.php Adds test for missing ISO Codes component via container override.
tests/unit/Validators/CurrencyCodeTest.php Adds test for missing ISO Codes component via container override.
tests/unit/Validators/LanguageCodeTest.php Adds test for missing ISO Codes component via container override.
tests/unit/Validators/SubdivisionCodeTest.php Adds test for missing ISO Codes component via container override.
tests/unit/Validators/PhoneTest.php Adds test for missing ISO Codes component via container override.
tests/benchmark/IsoCodesBench.php Adds benchmark coverage for ISO-code validator construction/evaluation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@henriquemoody henriquemoody left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change! You're on a row!

Let's just agree on the approach with the container registry, but other than that, no reason not to merge it.

@alganet alganet force-pushed the sokil-singletons branch 2 times, most recently from db1ff30 to f457713 Compare January 29, 2026 17:58
@alganet alganet requested a review from henriquemoody January 29, 2026 17:58
@alganet alganet marked this pull request as ready for review January 29, 2026 17:58
The main focus of this change is to make those optional dependencies
more testable.

Unfortunately, some phpstan-ignores had to be included, since ::set
is not a PsrContainer method. We're only using it on tests though,
so it's fine. It targets our php-di container for testing purposes
only. The real implementation only relies on ::get.

This change also has the side effect of improving the performance
of those validators by not instantiating their databases each time
a iso validator is built, achieving massive improvements in those
scenarios. A small benchmark with no assertions was added to track
that improvement.
@alganet alganet merged commit d8e31db into Respect:main Jan 29, 2026
7 of 8 checks passed
@alganet alganet deleted the sokil-singletons branch January 29, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants