From 751837de6804cd54418576d01e34f9e61ce4a870 Mon Sep 17 00:00:00 2001 From: Gustavo Freze Date: Tue, 21 Apr 2026 18:06:46 -0300 Subject: [PATCH 1/2] fix: Prevent shell injection and RCE risk in CI pipelines by passing commands as argument arrays to Symfony Process instead of shell strings. --- .claude/CLAUDE.md | 51 +++--- .claude/rules/code-style.md | 81 --------- .claude/rules/domain.md | 96 ---------- .claude/rules/github-workflows.md | 78 ++++++++ .claude/rules/php-library-code-style.md | 154 ++++++++++++++++ ...tation.md => php-library-documentation.md} | 14 +- .claude/rules/php-library-modeling.md | 163 +++++++++++++++++ .../{testing.md => php-library-testing.md} | 42 +++-- .editorconfig | 18 ++ .gitattributes | 43 +++-- Makefile | 25 +-- README.md | 8 +- composer.json | 59 +++---- phpunit.xml | 2 +- src/Internal/Client/DockerClient.php | 2 +- src/Internal/Commands/Command.php | 12 +- src/Internal/Commands/DockerCopy.php | 4 +- src/Internal/Commands/DockerExecute.php | 4 +- src/Internal/Commands/DockerInspect.php | 4 +- src/Internal/Commands/DockerList.php | 4 +- .../Commands/DockerNetworkConnect.php | 4 +- src/Internal/Commands/DockerNetworkCreate.php | 8 +- src/Internal/Commands/DockerNetworkPrune.php | 4 +- src/Internal/Commands/DockerPull.php | 4 +- src/Internal/Commands/DockerReaper.php | 35 ++-- src/Internal/Commands/DockerRemove.php | 4 +- src/Internal/Commands/DockerRun.php | 65 ++++--- src/Internal/Commands/DockerStop.php | 4 +- src/Internal/Containers/Address/Ports.php | 19 +- .../Containers/ContainerInspection.php | 8 +- src/Internal/Containers/ContainerLookup.php | 4 +- .../Definitions/CopyInstruction.php | 4 +- .../Definitions/EnvironmentVariable.php | 4 +- .../Containers/Definitions/PortMapping.php | 4 +- .../Containers/Definitions/VolumeMapping.php | 4 +- .../Environment/EnvironmentVariables.php | 6 +- src/Internal/Containers/HostEnvironment.php | 2 +- src/Internal/Containers/Models/Name.php | 2 +- src/Internal/Containers/ShutdownHook.php | 2 +- .../DockerCommandExecutionFailed.php | 4 +- src/Waits/Conditions/MySQL/MySQLReady.php | 9 +- src/Waits/ContainerWaitForDependency.php | 16 +- tests/Unit/FlywayDockerContainerTest.php | 22 +-- tests/Unit/GenericDockerContainerTest.php | 166 ++++++++++++++++-- .../Internal/Commands/DockerCommandsTest.php | 149 +++++++++++++--- .../Internal/Containers/Address/PortsTest.php | 17 ++ .../Containers/ContainerReaperTest.php | 44 +++++ .../Overrides/file_exists_inside_docker.php | 2 +- tests/Unit/Mocks/ClientMock.php | 8 +- tests/Unit/Mocks/CommandMock.php | 4 +- tests/Unit/Mocks/CommandWithTimeoutMock.php | 4 +- tests/Unit/Mocks/ShutdownHookMock.php | 23 +++ tests/Unit/MySQLDockerContainerTest.php | 26 ++- .../Waits/ContainerWaitForDependencyTest.php | 62 ++++++- tests/Unit/Waits/ContainerWaitForTimeTest.php | 4 +- tests/bootstrap.php | 9 - 56 files changed, 1133 insertions(+), 487 deletions(-) delete mode 100644 .claude/rules/code-style.md delete mode 100644 .claude/rules/domain.md create mode 100644 .claude/rules/github-workflows.md create mode 100644 .claude/rules/php-library-code-style.md rename .claude/rules/{documentation.md => php-library-documentation.md} (64%) create mode 100644 .claude/rules/php-library-modeling.md rename .claude/rules/{testing.md => php-library-testing.md} (60%) create mode 100644 .editorconfig create mode 100644 tests/Unit/Mocks/ShutdownHookMock.php delete mode 100644 tests/bootstrap.php diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 2b951ef..11885b0 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -1,43 +1,32 @@ # Project -PHP library (tiny-blocks). Immutable domain models, zero infrastructure dependencies in core. +PHP library (tiny-blocks ecosystem). Self-contained package: immutable models, zero infrastructure +dependencies in core, small public surface area. Public API at `src/` root; implementation details +under `src/Internal/`. -## Stack +## Rules -Refer to `composer.json` for the full dependency list, version constraints, and PHP version. - -## Project layout - -``` -src/ -├── .php # Primary contract for consumers -├── .php # Main implementation or extension point -├── Contracts/ # Interfaces for data returned to consumers -├── Internal/ # Implementation details (not part of public API) -│ └── Exceptions/ # Internal exception classes -└── Exceptions/ # Public exception classes (when part of the API) -tests/ -├── Models/ # Domain-specific fixtures reused across tests -├── Mocks/ # Test doubles for system boundaries -├── Unit/ # Unit tests for public API -└── Integration/ # Tests requiring real external resources (when applicable) -``` - -See `rules/domain.md` for folder conventions and naming rules. +All coding standards, architecture, naming, testing, and documentation conventions +are defined in `rules/`. Read the applicable rule files before generating any code or documentation. ## Commands -- Run tests: `make test`. -- Run lint: `make review`. -- Run `make help` to list all available commands. +- `make test` — run tests with coverage. +- `make mutation-test` — run mutation testing (Infection). +- `make review` — run lint. +- `make help` — list all available commands. ## Post-change validation -After any code change, run `make review` and `make test`. -If either fails, iterate on the fix while respecting all project rules until both pass. -Never deliver code that breaks lint or tests. +After any code change, run `make review`, `make test`, and `make mutation-test`. +If any fails, iterate on the fix while respecting all project rules until all pass. +Never deliver code that breaks lint, tests, or leaves surviving mutants. + +## File formatting -## Reference-first approach +Every file produced or modified must: -Always read all rule files and reference sources before generating any code or documentation. -Never generate from memory. Read the source and match the pattern exactly. +- Use **LF** line endings. Never CRLF. +- Have no trailing whitespace on any line. +- End with a single trailing newline. +- Have no consecutive blank lines (max one blank line between blocks). diff --git a/.claude/rules/code-style.md b/.claude/rules/code-style.md deleted file mode 100644 index 40a90e9..0000000 --- a/.claude/rules/code-style.md +++ /dev/null @@ -1,81 +0,0 @@ ---- -description: Pre-output checklist, naming, typing, comparisons, and PHPDoc rules for all PHP files in libraries. -paths: - - "src/**/*.php" - - "tests/**/*.php" ---- - -# Code style - -Semantic code rules for all PHP files. Formatting rules (PSR-1, PSR-4, PSR-12, line length) are enforced by `phpcs.xml` -and are not repeated here. Refer to `rules/domain.md` for domain modeling rules. - -## Pre-output checklist - -Verify every item before producing any PHP code. If any item fails, revise before outputting. - -1. `declare(strict_types=1)` is present. -2. All classes are `final readonly` by default. Use `class` (without `final` or `readonly`) only when the class is - designed as an extension point for consumers (e.g., `Collection`, `ValueObject`). Use `final class` without - `readonly` only when the parent class is not readonly (e.g., extending a third-party abstract class). -3. All parameters, return types, and properties have explicit types. -4. Constructor property promotion is used. -5. Named arguments are used at call sites for own code, tests, and third-party library methods (e.g., tiny-blocks). - Never use named arguments on native PHP functions (`array_map`, `in_array`, `preg_match`, `is_null`, - `iterator_to_array`, `sprintf`, `implode`, etc.). -6. No `else` or `else if` exists anywhere. Use early returns, polymorphism, or map dispatch instead. -7. No abbreviations appear in identifiers. Use `$index` instead of `$i`, `$account` instead of `$acc`. -8. No generic identifiers exist. Use domain-specific names instead: - `$data` → `$payload`, `$value` → `$totalAmount`, `$item` → `$element`, - `$info` → `$details`, `$result` → `$outcome`. -9. No raw arrays exist where a typed collection or value object is available. Use `tiny-blocks/collection` - (`Collection`, `Collectible`) instead of raw `array` for any list of domain objects. Raw arrays are acceptable - only for primitive configuration data, variadic pass-through, or interop at system boundaries. -10. No private methods exist except private constructors for factory patterns. Inline trivial logic at the call site - or extract it to a collaborator or value object. -11. Members are ordered: constants first, then constructor, then static methods, then instance methods. Within each - group, order by body size ascending (number of lines between `{` and `}`). Constants and enum cases, which have - no body, are ordered by name length ascending. -12. Constructor parameters are ordered by parameter name length ascending (count the name only, without `$` or type), - except when parameters have an implicit semantic order (e.g., `$start/$end`, `$from/$to`, `$startAt/$endAt`), - which takes precedence. The same rule applies to named arguments at call sites. - Example: `$id` (2) → `$value` (5) → `$status` (6) → `$precision` (9). -13. No O(N²) or worse complexity exists. -14. No logic is duplicated across two or more places (DRY). -15. No abstraction exists without real duplication or isolation need (KISS). -16. All identifiers, comments, and documentation are written in American English. -17. No justification comments exist (`// NOTE:`, `// REASON:`, etc.). Code speaks for itself. -18. `// TODO: ` is used when implementation is unknown, uncertain, or intentionally deferred. - Never leave silent gaps. -19. All class references use `use` imports at the top of the file. Fully qualified names inline are prohibited. -20. No dead or unused code exists. Remove unreferenced classes, methods, constants, and imports. -21. Never create public methods, constants, or classes in `src/` solely to serve tests. If production code does not - need it, it does not exist. - -## Naming - -- Names describe **what** in domain terms, not **how** technically: `$monthlyRevenue` instead of `$calculatedValue`. -- Generic technical verbs (`process`, `handle`, `execute`, `mark`, `enforce`, `manage`, `ensure`, `validate`, - `check`, `verify`, `assert`, `transform`, `parse`, `compute`, `sanitize`, `normalize`) **should be avoided**. - Prefer names that describe the domain operation. -- Booleans use predicate form: `isActive`, `hasPermission`, `wasProcessed`. -- Collections are always plural: `$orders`, `$lines`. -- Methods returning bool use prefixes: `is`, `has`, `can`, `was`, `should`. - -## Comparisons - -1. Null checks: use `is_null($variable)`, never `$variable === null`. -2. Empty string checks on typed `string` parameters: use `empty($variable)`, never `$variable === ''`. -3. Mixed or untyped checks (value may be `null`, empty string, `0`, or `false`): use `empty($variable)`. - -## American English - -All identifiers, enum values, comments, and error codes use American English spelling: -`canceled` (not `cancelled`), `organization` (not `organisation`), `initialize` (not `initialise`), -`behavior` (not `behaviour`), `modeling` (not `modelling`), `labeled` (not `labelled`), -`fulfill` (not `fulfil`), `color` (not `colour`). - -## PHPDoc - -- PHPDoc is restricted to interfaces only, documenting obligations and `@throws`. -- Never add PHPDoc to concrete classes. diff --git a/.claude/rules/domain.md b/.claude/rules/domain.md deleted file mode 100644 index f3b0eea..0000000 --- a/.claude/rules/domain.md +++ /dev/null @@ -1,96 +0,0 @@ ---- -description: Domain modeling rules for PHP libraries — folder structure, naming, value objects, exceptions, enums, and SOLID. -paths: - - "src/**/*.php" ---- - -# Domain modeling - -Libraries are self-contained packages. The core has no dependency on frameworks, databases, or I/O. -Refer to `rules/code-style.md` for the pre-output checklist applied to all PHP code. - -## Folder structure - -``` -src/ -├── .php # Primary contract for consumers -├── .php # Main implementation or extension point -├── .php # Public enum -├── Contracts/ # Interfaces for data returned to consumers -├── Internal/ # Implementation details (not part of public API) -│ ├── .php -│ └── Exceptions/ # Internal exception classes -├── / # Feature-specific subdirectory when needed -└── Exceptions/ # Public exception classes (when part of the API) -``` - -**Public API boundary:** Only interfaces, extension points, enums, and thin orchestration classes live at the -`src/` root. These classes define the contract consumers interact with and delegate all real work to collaborators -inside `src/Internal/`. If a class contains substantial logic (algorithms, state machines, I/O), it belongs in -`Internal/`, not at the root. - -The `Internal/` namespace signals classes that are implementation details. Consumers must not depend on them. -Never use `Entities/`, `ValueObjects/`, `Enums/`, or `Domain/` as folder names. - -## Nomenclature - -1. Every class, property, method, and exception name reflects the **domain concept** the library represents. - A math library uses `Precision`, `RoundingMode`; a money library uses `Currency`, `Amount`; a collection - library uses `Collectible`, `Order`. -2. Never use generic technical names: `Manager`, `Helper`, `Processor`, `Data`, `Info`, `Utils`, - `Item`, `Record`, `Entity`, `Exception`, `Ensure`, `Validate`, `Check`, `Verify`, - `Assert`, `Transform`, `Parse`, `Compute`, `Sanitize`, or `Normalize` as class suffixes or prefixes. -3. Name classes after what they represent: `Money`, `Color`, `Pipeline` — not after what they do technically. -4. Name methods after the operation in domain terms: `add()`, `convertTo()`, `splitAt()` — not `process()`, - `handle()`, `execute()`, `manage()`, `ensure()`, `validate()`, `check()`, `verify()`, `assert()`, - `transform()`, `parse()`, `compute()`, `sanitize()`, or `normalize()`. - -## Value objects - -1. Are immutable: no setters, no mutation after construction. Operations return new instances. -2. Compare by value, not by reference. -3. Validate invariants in the constructor and throw on invalid input. -4. Have no identity field. -5. Use static factory methods (e.g., `from`, `of`, `zero`) with a private constructor when multiple creation - paths exist. - -## Exceptions - -1. Extend native PHP exceptions (`DomainException`, `InvalidArgumentException`, `OverflowException`, etc.). -2. Are pure: no formatted `code`/`message` for HTTP responses. -3. Signal invariant violations only. -4. Name after the invariant violated, never after the technical type: - `PrecisionOutOfRange` — not `InvalidPrecisionException`. - `CurrencyMismatch` — not `BadCurrencyException`. - `ContainerWaitTimeout` — not `TimeoutException`. -5. Create the exception class directly with the invariant name and the appropriate native parent. The exception - is dedicated by definition when its name describes the specific invariant it guards. - -## Enums - -1. Are PHP backed enums. -2. Include domain-meaningful methods when needed (e.g., `Order::ASCENDING_KEY`). - -## Extension points - -1. When a class is designed to be extended by consumers (e.g., `Collection`, `ValueObject`), it uses `class` - instead of `final readonly class`. All other classes use `final readonly class`. -2. Extension point classes use a private constructor with static factory methods (`createFrom`, `createFromEmpty`) - as the only creation path. -3. Internal state is injected via the constructor and stored in a `private readonly` property. - -## Principles - -- **Immutability**: all models and value objects adopt immutability. Operations return new instances. -- **Zero dependencies**: the library's core has no dependency on frameworks, databases, or I/O. -- **Small surface area**: expose only what consumers need. Hide implementation in `Internal/`. - -## SOLID reference - -| Principle | Failure signal | -|---------------------------|---------------------------------------------| -| S — Single responsibility | Class does two unrelated things | -| O — Open/closed | Adding a feature requires editing internals | -| L — Liskov substitution | Subclass throws on parent method | -| I — Interface segregation | Interface has unused methods | -| D — Dependency inversion | Constructor uses `new ConcreteClass()` | diff --git a/.claude/rules/github-workflows.md b/.claude/rules/github-workflows.md new file mode 100644 index 0000000..a369ba4 --- /dev/null +++ b/.claude/rules/github-workflows.md @@ -0,0 +1,78 @@ +--- +description: Naming, ordering, inputs, security, and structural rules for all GitHub Actions workflow files. +paths: + - ".github/workflows/**/*.yml" + - ".github/workflows/**/*.yaml" +--- + +# Workflows + +Structural and stylistic rules for GitHub Actions workflow files. Refer to `shell-scripts.md` for Bash conventions used +inside `run:` steps, and to `terraforms.md` for Terraform conventions used in `terraform/`. + +## Pre-output checklist + +Verify every item before producing any workflow YAML. If any item fails, revise before outputting. + +1. File name follows the convention: `ci-.yml` for reusable CI, `cd-.yml` for dispatch CD. +2. `name` field follows the pattern `CI — ` or `CD — `, using sentence case after the dash + (e.g., `CD — Run migration`, not `CD — Run Migration`). +3. Reusable workflows use `workflow_call` trigger. CD workflows use `workflow_dispatch` trigger. +4. Each workflow has a single responsibility. CI tests code. CD deploys it. Never combine both. +5. Every input has a `description` field. Descriptions use American English and end with a period. +6. Input names use `kebab-case`: `service-name`, `dry-run`, `skip-build`. +7. Inputs are ordered: required first, then optional. Each group by **name length ascending**. +8. Choice input options are in **alphabetical order**. +9. `env`, `outputs`, and `with` entries are ordered by **key length ascending**. +10. `permissions` keys are ordered by **key length ascending** (`contents` before `id-token`). +11. Top-level workflow keys follow canonical order: `name`, `on`, `concurrency`, `permissions`, `env`, `jobs`. +12. Job-level properties follow canonical order: `if`, `name`, `needs`, `uses`, `with`, `runs-on`, + `environment`, `timeout-minutes`, `strategy`, `outputs`, `permissions`, `env`, `steps`. +13. All other YAML property names within a block are ordered by **name length ascending**. +14. Jobs follow execution order: `load-config` → `lint` → `test` → `build` → `deploy`. +15. Step names start with a verb and use sentence case: `Setup PHP`, `Run lint`, `Resolve image tag`. +16. Runtime versions are resolved from the service repo's native dependency file (`composer.json`, `go.mod`, + `package.json`). No version is hardcoded in any workflow. +17. Service-specific overrides live in a pipeline config file (e.g., `.pipeline.yml`) in the service repo, + not in the workflows repository. +18. The `load-config` job reads the pipeline config file at runtime with safe fallback to defaults when absent. +19. Top-level `permissions` defaults to read-only (`contents: read`). Jobs escalate only the permissions they + need. +20. AWS authentication uses OIDC federation exclusively. Static access keys are forbidden. +21. Secrets are passed via `secrets: inherit` from callers. No secret is hardcoded. +22. Sensitive values fetched from SSM are masked with `::add-mask::` before assignment. +23. Third-party actions are pinned to the latest available full commit SHA with a version comment: + `uses: aws-actions/configure-aws-credentials@ # v4.0.2`. Always verify the latest + version before generating a workflow. +24. First-party actions (`actions/*`) are pinned to the latest major version tag available: + `actions/checkout@v4`. Always check for the most recent major version before generating a workflow. +25. Production deployments require GitHub Environments protection rules (manual approval). +26. Every job sets `timeout-minutes` to prevent indefinite hangs. CI jobs: 10–15 minutes. CD jobs: 20–30 + minutes. Adjust only with justification in a comment. +27. CI workflows set `concurrency` with `group` scoped to the PR and `cancel-in-progress: true` to avoid + redundant runs. +28. CD workflows set `concurrency` with `group` scoped to the environment and `cancel-in-progress: false` to + prevent interrupted deployments. +29. CD workflows use `if: ${{ !cancelled() }}` to allow to deploy after optional build steps. +30. Inline logic longer than 3 lines is extracted to a script in `scripts/ci/` or `scripts/cd/`. + +## Style + +- All text (workflow names, step names, input descriptions, comments) uses American English with correct + spelling and punctuation. Sentences and descriptions end with a period. + +## Callers + +- Callers trigger on `pull_request` targeting `main` only. No `push` trigger. +- Callers in service repos are static (~10 lines) and pass only `service-name` or `app-name`. +- Callers reference workflows with `@main` during development. Pin to a tag or SHA for production. + +## Image tagging + +- CD deploy builds: `-sha-` + `latest`. + +## Migrations + +- Migrations run **before** service deployment (schema first, code second). +- `cd-migrate.yml` supports `dry-run` mode (`flyway validate`) for pre-flight checks. +- Database credentials are fetched from SSM at runtime, never stored in workflow files. diff --git a/.claude/rules/php-library-code-style.md b/.claude/rules/php-library-code-style.md new file mode 100644 index 0000000..7ec196e --- /dev/null +++ b/.claude/rules/php-library-code-style.md @@ -0,0 +1,154 @@ +--- +description: Pre-output checklist, naming, typing, complexity, and PHPDoc rules for all PHP files in libraries. +paths: + - "src/**/*.php" + - "tests/**/*.php" +--- + +# Code style + +Semantic code rules for all PHP files. Formatting rules (PSR-1, PSR-4, PSR-12, line length) are enforced by `phpcs.xml` +and are not repeated here. Refer to `php-library-modeling.md` for library modeling rules. + +## Pre-output checklist + +Verify every item before producing any PHP code. If any item fails, revise before outputting. + +1. `declare(strict_types=1)` is present. +2. All classes are `final readonly` by default. Use `class` (without `final` or `readonly`) only when the class is + designed as an extension point for consumers (e.g., `Collection`, `ValueObject`). Use `final class` without + `readonly` only when the parent class is not readonly (e.g., extending a third-party abstract class). +3. All parameters, return types, and properties have explicit types. +4. Constructor property promotion is used. +5. Named arguments are used at call sites for own code, tests, and third-party library methods (e.g., tiny-blocks). + Never use named arguments on native PHP functions (`array_map`, `in_array`, `preg_match`, `is_null`, + `iterator_to_array`, `sprintf`, `implode`, etc.) or PHPUnit assertions (`assertEquals`, `assertSame`, + `assertTrue`, `expectException`, etc.). +6. No `else` or `else if` exists anywhere. Use early returns, polymorphism, or map dispatch instead. +7. No abbreviations appear in identifiers. Use `$index` instead of `$i`, `$account` instead of `$acc`. +8. No generic identifiers exist. Use domain-specific names instead: + `$data` → `$payload`, `$value` → `$totalAmount`, `$item` → `$element`, + `$info` → `$currencyDetails`, `$result` → `$conversionOutcome`. +9. No raw arrays exist where a typed collection or value object is available. Use the `tiny-blocks/collection` + fluent API (`Collection`, `Collectible`) when data is `Collectible`. Use `createLazyFrom` when elements are + consumed once. Raw arrays are acceptable only for primitive configuration data, variadic pass-through, and + interop at system boundaries. See "Collection usage" below for the full rule and example. +10. No private methods exist except private constructors for factory patterns. Inline trivial logic at the call site + or extract it to a collaborator or value object. +11. Members are ordered: constants first, then constructor, then static methods, then instance methods. Within each + group, order by body size ascending (number of lines between `{` and `}`). Constants and enum cases, which have + no body, are ordered by name length ascending. +12. Constructor parameters are ordered by parameter name length ascending (count the name only, without `$` or type), + except when parameters have an implicit semantic order (e.g., `$start/$end`, `$from/$to`, `$startAt/$endAt`), + which takes precedence. Parameters with default values go last, regardless of name length. The same rule + applies to named arguments at call sites. + Example: `$id` (2) → `$value` (5) → `$status` (6) → `$precision` (9). +13. Time and space complexity are first-class design concerns. + - No `O(N²)` or worse time complexity exists unless the problem inherently requires it and the cost is + documented in PHPDoc on the interface method. + - Space complexity is kept minimal: prefer lazy/streaming pipelines (`createLazyFrom`) over materializing + intermediate collections. + - Never re-iterate the same source; fuse stages when possible. + - Public interface methods document time and space complexity in Big O form (see "PHPDoc" section). +14. No logic is duplicated across two or more places (DRY). +15. No abstraction exists without real duplication or isolation need (KISS). +16. All identifiers, comments, and documentation are written in American English. +17. No justification comments exist (`// NOTE:`, `// REASON:`, etc.). Code speaks for itself. +18. `// TODO: ` is used when implementation is unknown, uncertain, or intentionally deferred. + Never leave silent gaps. +19. All class references use `use` imports at the top of the file. Fully qualified names inline are prohibited. +20. No dead or unused code exists. Remove unreferenced classes, methods, constants, and imports. +21. Never create public methods, constants, or classes in `src/` solely to serve tests. If production code does not + need it, it does not exist. +22. Always use the most current and clean syntax available in the target PHP version. Prefer match to switch, + first-class callables over `Closure::fromCallable()`, readonly promotion over manual assignment, enum methods + over external switch/if chains, named arguments over positional ambiguity (except where excluded by rule 5), + and `Collection::map` over foreach accumulation. +23. No vertical alignment of types in parameter lists or property declarations. Use a single space between + type and variable name. Never pad with extra spaces to align columns: + `public OrderId $id` — not `public OrderId $id`. +24. Opening brace `{` follows PSR-12: on a **new line** for classes, interfaces, traits, enums, and methods + (including constructors); on the **same line** for closures and control structures (`if`, `for`, `foreach`, + `while`, `switch`, `match`, `try`). +25. Never pass an argument whose value equals the parameter's default. Omit the argument entirely. + Example — `toArray(KeyPreservation $keyPreservation = KeyPreservation::PRESERVE)`: + `$collection->toArray(keyPreservation: KeyPreservation::PRESERVE)` → `$collection->toArray()`. + Only pass the argument when the value differs from the default. +26. No trailing comma in any multi-line list. This applies to parameter lists (constructors, methods, + closures), argument lists at call sites, array literals, match arms, and any other comma-separated + multi-line structure. The last element never has a comma after it. PHP accepts trailing commas in + parameter lists, but this project prohibits them for visual consistency. + Example — correct: + ``` + new Precision( + value: 2, + rounding: RoundingMode::HALF_UP + ); + ``` + Example — prohibited: + ``` + new Precision( + value: 2, + rounding: RoundingMode::HALF_UP, + ); + ``` + +## Casing conventions + +- Internal code (variables, methods, classes): **`camelCase`**. +- Constants and enum-backed values when representing codes: **`SCREAMING_SNAKE_CASE`**. + +## Naming + +- Names describe **what** in domain terms, not **how** technically: `$monthlyRevenue` instead of `$calculatedValue`. +- Generic technical verbs are avoided. See `php-library-modeling.md` — Nomenclature. +- Booleans use predicate form: `isActive`, `hasPermission`, `wasProcessed`. +- Collections are always plural: `$orders`, `$lines`. +- Methods returning bool use prefixes: `is`, `has`, `can`, `was`, `should`. + +## Comparisons + +1. Null checks: use `is_null($variable)`, never `$variable === null`. +2. Empty string checks on typed `string` parameters: use `$variable === ''`. Avoid `empty()` on typed strings + because `empty('0')` returns `true`. +3. Mixed or untyped checks (value may be `null`, empty string, `0`, or `false`): use `empty($variable)`. + +## American English + +All identifiers, enum values, comments, and error codes use American English spelling: +`canceled` (not `cancelled`), `organization` (not `organisation`), `initialize` (not `initialise`), +`behavior` (not `behaviour`), `modeling` (not `modelling`), `labeled` (not `labelled`), +`fulfill` (not `fulfil`), `color` (not `colour`). + +## PHPDoc + +- PHPDoc is restricted to interfaces only, documenting obligations, `@throws`, and complexity. +- Never add PHPDoc to concrete classes. +- Document `@throws` for every exception the method may raise. +- Document time and space complexity in Big O form. When a method participates in a fused pipeline (e.g., collection + pipelines), express cost as a two-part form: call-site cost + fused-pass contribution. Include a legend defining + variables (e.g., `N` for input size, `K` for number of stages). + +## Collection usage + +When a property or parameter is `Collectible`, use its fluent API. Never break out to raw array functions such as +`array_map`, `array_filter`, `iterator_to_array`, or `foreach` + accumulation. The same applies to `filter()`, +`reduce()`, `each()`, and all other `Collectible` operations. Chain them fluently. Never materialize with +`iterator_to_array` to then pass into a raw `array_*` function. + +**Prohibited — `array_map` + `iterator_to_array` on a Collectible:** + +```php +$names = array_map( + static fn(Element $element): string => $element->name(), + iterator_to_array($collection) +); +``` + +**Correct — fluent chain with `map()` + `toArray()`:** + +```php +$names = $collection + ->map(transformations: static fn(Element $element): string => $element->name()) + ->toArray(keyPreservation: KeyPreservation::DISCARD); +``` diff --git a/.claude/rules/documentation.md b/.claude/rules/php-library-documentation.md similarity index 64% rename from .claude/rules/documentation.md rename to .claude/rules/php-library-documentation.md index 353899c..d7ac6da 100644 --- a/.claude/rules/documentation.md +++ b/.claude/rules/php-library-documentation.md @@ -1,7 +1,7 @@ --- description: Standards for README files and all project documentation in PHP libraries. paths: - - "**/*.md" + - "**/*.md" --- # Documentation @@ -17,8 +17,12 @@ paths: includes a brief heading describing what it demonstrates. 7. If the library exposes multiple entry points, strategies, or container types, document each with its own subsection and example. -8. **License** and **Contributing** sections at the end. -9. Write strictly in American English. See `rules/code-style.md` American English section for spelling conventions. +8. **FAQ** section: include entries for common pitfalls, non-obvious behaviors, or design decisions that users + frequently ask about. Each entry is a numbered question as heading (e.g., `### 01. Why does X happen?`) + followed by a concise explanation. Only include entries that address real confusion points. +9. **License** and **Contributing** sections at the end. +10. Write strictly in American English. See `php-library-code-style.md` American English section for spelling + conventions. ## Structured data @@ -31,4 +35,6 @@ paths: 1. Keep language concise and scannable. 2. Never include placeholder content (`TODO`, `TBD`). 3. Code examples must be syntactically correct and self-contained. -4. Do not document `Internal/` classes or private API. Only document what consumers interact with. +4. Code examples include every `use` statement needed to compile. Each example stands alone — copyable into + a fresh file without modification. +5. Do not document `Internal/` classes or private API. Only document what consumers interact with. diff --git a/.claude/rules/php-library-modeling.md b/.claude/rules/php-library-modeling.md new file mode 100644 index 0000000..bedb733 --- /dev/null +++ b/.claude/rules/php-library-modeling.md @@ -0,0 +1,163 @@ +--- +description: Library modeling rules — folder structure, public API boundary, naming, value objects, exceptions, enums, extension points, and complexity. +paths: + - "src/**/*.php" +--- + +# Library modeling + +Libraries are self-contained packages. The core has no dependency on frameworks, databases, or I/O. Refer to +`php-library-code-style.md` for the pre-output checklist applied to all PHP code. + +## Folder structure + +``` +src/ +├── .php # Primary contract for consumers +├── .php # Main implementation or extension point +├── .php # Public enum +├── Contracts/ # Interfaces for data returned to consumers +├── Internal/ # Implementation details (not part of public API) +│ ├── .php +│ └── Exceptions/ # Internal exception classes +├── / # Feature-specific subdirectory when needed +└── Exceptions/ # Public exception classes (when part of the API) +``` + +Never use `Models/`, `Entities/`, `ValueObjects/`, `Enums/`, or `Domain/` as folder names. + +## Public API boundary + +Only interfaces, extension points, enums, and thin orchestration classes live at the `src/` root. These classes +define the contract consumers interact with and delegate all real work to collaborators inside `src/Internal/`. +If a class contains substantial logic (algorithms, state machines, I/O), it belongs in `Internal/`, not at the root. + +The `Internal/` namespace signals classes that are implementation details. Consumers must not depend on them. +Breaking changes inside `Internal/` are not semver-breaking for the library. + +## Nomenclature + +1. Every class, property, method, and exception name reflects the **concept** the library represents. A math library + uses `Precision`, `RoundingMode`; a money library uses `Currency`, `Amount`; a collection library uses + `Collectible`, `Order`. +2. Name classes after what they represent: `Money`, `Color`, `Pipeline` — not after what they do technically. +3. Name methods after the operation in the library's vocabulary: `add()`, `convertTo()`, `splitAt()`. + +### Always banned + +These names carry zero semantic content. Never use them anywhere, as class suffixes, prefixes, or method names: + +- `Data`, `Info`, `Utils`, `Item`, `Record`, `Entity`. +- `Exception` as a class suffix (e.g., `FooException` — use `Foo` when it already extends a native exception). + +### Anemic verbs (banned by default) + +These verbs hide what is actually happening behind a generic action. Banned unless the verb **is** the operation +that constitutes the library's reason to exist (e.g., a JSON parser may have `parse()`; a hashing library may +have `compute()`): + +- `ensure`, `validate`, `check`, `verify`, `assert`, `mark`, `enforce`, `sanitize`, `normalize`, `compute`, + `transform`, `parse`. + +When in doubt, prefer the domain operation name. `Password::hash()` beats `Password::compute()`; `Email::parse()` +is fine in a parser library but suspicious elsewhere (use `Email::from()` instead). + +### Architectural roles (allowed with justification) + +These names describe a role the library offers as a building block. Acceptable when the class **is** that role +(e.g., `EventHandler` in an events library, `CacheManager` in a cache library, `Upcaster` in an event-sourcing +library). Not acceptable on domain objects inside the library (value objects, enums, contract interfaces): + +- `Manager`, `Handler`, `Processor`, `Service`, and their verb forms `process`, `handle`, `execute`. + +The test: if the consumer instantiates or extends this class to integrate with the library, the role name is +legitimate. If the class models a concept the consumer manipulates (a money amount, a country code, a color), +the role name is wrong. + +## Value objects + +1. Are immutable: no setters, no mutation after construction. Operations return new instances. +2. Compare by value, not by reference. +3. Validate invariants in the constructor and throw on invalid input. +4. Have no identity field. +5. Use static factory methods (e.g., `from`, `of`, `zero`) with a private constructor when multiple creation paths + exist. The factory name communicates the semantic intent. + +## Exceptions + +1. Every failure throws a **dedicated exception class** named after the invariant it guards — never + `throw new DomainException('...')`, `throw new InvalidArgumentException('...')`, + `throw new RuntimeException('...')`, or any other generic native exception thrown directly. If the invariant + is worth throwing for, it is worth a named class. +2. Dedicated exception classes **extend** the appropriate native PHP exception (`DomainException`, + `InvalidArgumentException`, `OverflowException`, etc.) — the native class is the parent, never the thing that + is thrown. Consumers that catch the broad standard types continue to work; consumers that need precise handling + can catch the specific classes. +3. Exceptions are pure: no transport-specific fields (`code` populated with HTTP status, formatted `message` meant + for end-user display). Formatting to any transport happens at the consumer's boundary, not inside the library. +4. Exceptions signal invariant violations only, not control flow. +5. Name the class after the invariant violated, never after the technical type: + - `PrecisionOutOfRange` — not `InvalidPrecisionException`. + - `CurrencyMismatch` — not `BadCurrencyException`. + - `ContainerWaitTimeout` — not `TimeoutException`. +6. A descriptive `message` argument is allowed and encouraged when it carries **debugging context** — the violating + value, the boundary that was crossed, the state the library was in. The class name identifies the invariant; + the message describes the specific violation for stack traces and test assertions. Do not build messages meant + for end-user display or transport rendering. Keep them short, factual, and in American English. +7. Public exceptions live in `src/Exceptions/`. Internal exceptions live in `src/Internal/Exceptions/`. + +**Prohibited** — throwing a native exception directly: + +```php +if ($value < 0) { + throw new InvalidArgumentException('Precision cannot be negative.'); +} +``` + +**Correct** — dedicated class, no message (class name is sufficient): + +```php +// src/Exceptions/PrecisionOutOfRange.php +final class PrecisionOutOfRange extends InvalidArgumentException +{ +} + +// at the callsite +if ($value < 0) { + throw new PrecisionOutOfRange(); +} +``` + +**Correct** — dedicated class with debugging context: + +```php +if ($value < 0 || $value > 16) { + throw new PrecisionOutOfRange(sprintf('Precision must be between 0 and 16, got %d.', $value)); +} +``` + +## Enums + +1. Are PHP backed enums. +2. Include methods when they carry vocabulary meaning (e.g., `Order::ASCENDING_KEY`, `RoundingMode::apply()`). +3. Live at the `src/` root when public. Enums used only by internals live in `src/Internal/`. + +## Extension points + +1. When a class is designed to be extended by consumers (e.g., `Collection`, `ValueObject`), it uses `class` instead + of `final readonly class`. All other classes use `final readonly class`. +2. Extension point classes use a private constructor with static factory methods (`createFrom`, `createFromEmpty`) + as the only creation path. +3. Internal state is injected via the constructor and stored in a `private readonly` property. + +## Time and space complexity + +1. Every public method has predictable, documented complexity. Document Big O in PHPDoc on the interface + (see `php-library-code-style.md`, "PHPDoc" section). +2. Algorithms run in `O(N)` or `O(N log N)` unless the problem inherently requires worse. `O(N²)` or worse must + be justified and documented. +3. Prefer lazy/streaming evaluation over materializing intermediate results. In pipeline-style libraries, fuse + stages so a single pass suffices. +4. Memory usage is bounded and proportional to the output, not to the sum of intermediate stages. +5. Validate complexity claims with benchmarks against a reference implementation when optimizing critical paths. + Parity testing against the reference library is the validation standard for optimization work. diff --git a/.claude/rules/testing.md b/.claude/rules/php-library-testing.md similarity index 60% rename from .claude/rules/testing.md rename to .claude/rules/php-library-testing.md index af8edff..610b928 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/php-library-testing.md @@ -1,12 +1,13 @@ --- description: BDD Given/When/Then structure, PHPUnit conventions, test organization, and fixture rules for PHP libraries. paths: - - "tests/**/*.php" + - "tests/**/*.php" --- # Testing conventions -Framework: **PHPUnit**. Refer to `rules/code-style.md` for the code style checklist, which also applies to test files. +Framework: **PHPUnit**. Refer to `php-library-code-style.md` for the code style checklist, which also applies to +test files. ## Structure: Given/When/Then (BDD) @@ -62,13 +63,34 @@ Use `@And` for complementary preconditions or actions within the same scenario, 5. Never include conditional logic inside tests. 6. Include one logical concept per `@Then` block. 7. Maintain strict independence between tests. No inherited state. -8. For exception tests, place `@Then` (expectException) before `@When`. -9. Use domain-specific model classes in `tests/Models/` for test fixtures that represent domain concepts +8. Use domain-specific model classes in `tests/Models/` for test fixtures that represent domain concepts (e.g., `Amount`, `Invoice`, `Order`). -10. Use mock classes in `tests/Mocks/` (or `tests/Unit/Mocks/`) for test doubles of system boundaries - (e.g., `ClientMock`, `ExecutionCompletedMock`). -11. Exercise invariants and edge cases through the library's public entry point. Create a dedicated test class +9. Use mock classes in `tests/Mocks/` (or `tests/Unit/Mocks/`) for test doubles of system boundaries + (e.g., `ClientMock`, `ExecutionCompletedMock`). +10. Exercise invariants and edge cases through the library's public entry point. Create a dedicated test class for an internal model only when the condition cannot be reached through the public API. +11. Never use `/** @test */` annotation. Test methods are discovered by the `test` prefix in the method name. +12. Never use named arguments on PHPUnit assertions (`assertEquals`, `assertSame`, `assertTrue`, + `expectException`, etc.). Pass arguments positionally. + +## Test setup and fixtures + +1. **One annotation = one statement.** Each `@Given` or `@And` block contains exactly one annotation line + followed by one expression or assignment. Never place multiple variable declarations or object + constructions under a single annotation. +2. **No intermediate variables used only once.** If a value is consumed in a single place, inline it at the + call site. Chain method calls when the intermediate state is not referenced elsewhere + (e.g., `Money::of(...)->add(...)` instead of `$money = Money::of(...); $money->add(...);`). +3. **No private or helper methods in test classes.** The only non-test methods allowed are data providers. + If setup logic is complex enough to extract, it belongs in a dedicated fixture class, not in a + private method on the test class. +4. **Domain terms in variables and annotations.** Never use technical testing jargon (`$spy`, `$mock`, + `$stub`, `$fake`, `$dummy`) as variable or property names. Use the domain concept the object + represents: `$collection`, `$amount`, `$currency`, `$sortedElements`. Class names like + `ClientMock` or `GatewaySpy` are acceptable — the variable holding the instance is what matters. +5. **Annotations use domain language.** Write `/** @Given a collection of amounts */`, not + `/** @Given a mocked collection in test state */`. The annotation describes the domain + scenario, not the technical setup. ## Test organization @@ -82,11 +104,7 @@ tests/ └── bootstrap.php # Test bootstrap when needed ``` -- `tests/` or `tests/Unit/`: pure unit tests exercising the library's public API. -- `tests/Integration/`: tests requiring real external resources (e.g., Docker containers, databases). - Only present when the library interacts with infrastructure. -- `tests/Models/`: domain-specific fixture classes reused across test files. -- `tests/Mocks/` or `tests/Unit/Mocks/`: test doubles for system boundaries. +`tests/Integration/` is only present when the library interacts with infrastructure. ## Coverage and mutation testing diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 0000000..73e3c9a --- /dev/null +++ b/.editorconfig @@ -0,0 +1,18 @@ +root = true + +[*] +charset = utf-8 +end_of_line = lf +indent_size = 4 +indent_style = space +insert_final_newline = true +trim_trailing_whitespace = true + +[*.{yml,yaml}] +indent_size = 2 + +[Makefile] +indent_style = tab + +[*.md] +trim_trailing_whitespace = false diff --git a/.gitattributes b/.gitattributes index 8c85471..28337dc 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,13 +1,30 @@ -/tests export-ignore -/vendor export-ignore - -/LICENSE export-ignore -/Makefile export-ignore -/README.md export-ignore -/phpunit.xml export-ignore -/phpstan.neon.dist export-ignore -/infection.json.dist export-ignore - -/.github export-ignore -/.gitignore export-ignore -/.gitattributes export-ignore +# Auto detect text files and perform LF normalization +* text=auto eol=lf + +# ─── Diff drivers ──────────────────────────────────────────── +*.php diff=php +*.md diff=markdown + +# ─── Force LF ──────────────────────────────────────────────── +*.sh text eol=lf +Makefile text eol=lf + +# ─── Generated (skip diff and GitHub stats) ────────────────── +composer.lock -diff linguist-generated + +# ─── Export ignore (excluded from dist archive) ────────────── +/tests export-ignore +/vendor export-ignore +/rules export-ignore + +/.github export-ignore +/.gitignore export-ignore +/.gitattributes export-ignore + +/CLAUDE.md export-ignore +/LICENSE export-ignore +/Makefile export-ignore +/README.md export-ignore +/phpunit.xml export-ignore +/phpstan.neon.dist export-ignore +/infection.json.dist export-ignore \ No newline at end of file diff --git a/Makefile b/Makefile index 0a34e8b..1848900 100644 --- a/Makefile +++ b/Makefile @@ -7,10 +7,10 @@ ifeq ($(ARCH),arm64) endif DOCKER_RUN = docker run ${PLATFORM} -u root --rm -it --network=tiny-blocks --name test-lib \ - -v ${PWD}:/app \ - -v ${PWD}/tests/Integration/Database/Migrations:/test-adm-migrations \ - -v /var/run/docker.sock:/var/run/docker.sock \ - -w /app gustavofreze/php:8.5-alpine + -v ${PWD}:/app \ + -v ${PWD}/tests/Integration/Database/Migrations:/test-adm-migrations \ + -v /var/run/docker.sock:/var/run/docker.sock \ + -w /app gustavofreze/php:8.5-alpine RESET := \033[0m GREEN := \033[0;32m @@ -21,23 +21,20 @@ YELLOW := \033[0;33m .PHONY: configure configure: configure-test-environment ## Configure development environment @${DOCKER_RUN} composer update --optimize-autoloader + @${DOCKER_RUN} composer normalize .PHONY: test test: configure-test-environment ## Run all tests with coverage @${DOCKER_RUN} composer tests .PHONY: test-file -test-file: ## Run tests for a specific file (usage: make test-file FILE=path/to/file) +test-file: ## Run tests for a specific file (usage: make test-file FILE=ClassNameTest) @${DOCKER_RUN} composer test-file ${FILE} .PHONY: test-no-coverage test-no-coverage: configure-test-environment ## Run all tests without coverage @${DOCKER_RUN} composer tests-no-coverage -.PHONY: unit-test-no-coverage -unit-test-no-coverage: ## Run unit tests without coverage - @${DOCKER_RUN} composer run unit-tests-no-coverage - .PHONY: configure-test-environment configure-test-environment: @if ! docker network inspect tiny-blocks > /dev/null 2>&1; then \ @@ -50,7 +47,11 @@ review: ## Run static code analysis .PHONY: show-reports show-reports: ## Open static analysis reports (e.g., coverage, lints) in the browser - @sensible-browser report/coverage/coverage-html/index.html + @sensible-browser report/coverage/coverage-html/index.html report/coverage/mutation-report.html + +.PHONY: show-outdated +show-outdated: ## Show outdated direct dependencies + @${DOCKER_RUN} composer outdated --direct .PHONY: clean clean: ## Remove dependencies and generated artifacts @@ -66,7 +67,7 @@ help: ## Display this help message | awk 'BEGIN {FS = ":.*? ## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Testing$$(printf '$(RESET)')" - @grep -E '^(test|test-file|test-no-coverage|unit-test-no-coverage):.*?## .*$$' $(MAKEFILE_LIST) \ + @grep -E '^(test|test-file|test-no-coverage):.*?## .*$$' $(MAKEFILE_LIST) \ | awk 'BEGIN {FS = ":.*?## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Quality$$(printf '$(RESET)')" @@ -74,7 +75,7 @@ help: ## Display this help message | awk 'BEGIN {FS = ":.*?## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Reports$$(printf '$(RESET)')" - @grep -E '^(show-reports):.*?## .*$$' $(MAKEFILE_LIST) \ + @grep -E '^(show-reports|show-outdated):.*?## .*$$' $(MAKEFILE_LIST) \ | awk 'BEGIN {FS = ":.*?## "}; {printf "$(YELLOW)%-25s$(RESET) %s\n", $$1, $$2}' @echo "" @echo "$$(printf '$(GREEN)')Cleanup$$(printf '$(RESET)')" diff --git a/README.md b/README.md index 1e48365..b08abbe 100644 --- a/README.md +++ b/README.md @@ -36,11 +36,9 @@ ## Overview -Manage Docker containers programmatically, simplifying the creation, running, and interaction with containers. - -The library provides interfaces and implementations for adding network configurations, mapping ports, setting -environment variables, and executing commands inside containers. Designed to support **unit tests** and -**integration tests**, it enables developers to manage containerized environments with minimal effort. +Manages Docker containers programmatically for PHP, enabling integration tests and disposable infrastructure to be +orchestrated directly from code. Supports container definitions, port and volume mappings, environment variables, +readiness probes, shutdown hooks, and orphan reaping. ## Installation diff --git a/composer.json b/composer.json index f8a4c50..9a65af5 100644 --- a/composer.json +++ b/composer.json @@ -1,29 +1,34 @@ { "name": "tiny-blocks/docker-container", - "type": "library", + "description": "Manages Docker containers programmatically for PHP, aimed at integration tests and disposable infrastructure.", "license": "MIT", - "homepage": "https://github.com/tiny-blocks/docker-container", - "description": "Manage Docker containers programmatically, simplifying the creation, running, and interaction with containers.", - "prefer-stable": true, - "minimum-stability": "stable", - "keywords": [ - "psr", - "tests", - "docker", - "tiny-blocks", - "test-containers", - "docker-container" - ], + "type": "library", "authors": [ { "name": "Gustavo Freze de Araujo Santos", "homepage": "https://github.com/gustavofreze" } ], + "homepage": "https://github.com/tiny-blocks/docker-container", "support": { "issues": "https://github.com/tiny-blocks/docker-container/issues", "source": "https://github.com/tiny-blocks/docker-container" }, + "require": { + "php": "^8.5", + "symfony/process": "^8.0", + "tiny-blocks/collection": "^2.3", + "tiny-blocks/ksuid": "^1.5" + }, + "require-dev": { + "ergebnis/composer-normalize": "^2.51", + "infection/infection": "^0.32", + "phpstan/phpstan": "^2.1", + "phpunit/phpunit": "^13.1", + "squizlabs/php_codesniffer": "^4.0" + }, + "minimum-stability": "stable", + "prefer-stable": true, "autoload": { "psr-4": { "TinyBlocks\\DockerContainer\\": "src/" @@ -35,38 +40,24 @@ "TinyBlocks\\DockerContainer\\": "tests/Unit/" } }, - "require": { - "php": "^8.5", - "symfony/process": "^7.4", - "tiny-blocks/ksuid": "^1.5", - "tiny-blocks/collection": "^2.0" - }, - "require-dev": { - "ext-pdo": "*", - "phpunit/phpunit": "^11.5", - "phpstan/phpstan": "^2.1", - "dg/bypass-finals": "^1.9", - "infection/infection": "^0.32", - "squizlabs/php_codesniffer": "^4.0" - }, "config": { - "sort-packages": true, "allow-plugins": { + "ergebnis/composer-normalize": true, "infection/extension-installer": true - } + }, + "sort-packages": true }, "scripts": { - "test": "php -d memory_limit=2G ./vendor/bin/phpunit --configuration phpunit.xml tests", + "mutation-test": "php ./vendor/bin/infection --threads=max --logger-html=report/coverage/mutation-report.html --coverage=report/coverage", "phpcs": "php ./vendor/bin/phpcs --standard=PSR12 --extensions=php ./src", "phpstan": "php ./vendor/bin/phpstan analyse -c phpstan.neon.dist --quiet --no-progress", - "test-file": "php ./vendor/bin/phpunit --configuration phpunit.xml --no-coverage --filter", - "mutation-test": "php ./vendor/bin/infection --threads=max --logger-html=report/coverage/mutation-report.html --coverage=report/coverage", - "test-no-coverage": "php ./vendor/bin/phpunit --configuration phpunit.xml --no-coverage tests", - "unit-tests-no-coverage": "php ./vendor/bin/phpunit --configuration phpunit.xml --no-coverage --testsuite unit", "review": [ "@phpcs", "@phpstan" ], + "test": "php -d memory_limit=2G ./vendor/bin/phpunit --configuration phpunit.xml tests", + "test-file": "php ./vendor/bin/phpunit --configuration phpunit.xml --no-coverage --filter", + "test-no-coverage": "php ./vendor/bin/phpunit --configuration phpunit.xml --no-coverage --testsuite=unit", "tests": [ "@test", "@mutation-test" diff --git a/phpunit.xml b/phpunit.xml index d851f93..fee0f4e 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -2,7 +2,7 @@ toCommandLine()); + $process = new Process($command->toArguments()); try { if ($command instanceof CommandWithTimeout) { diff --git a/src/Internal/Commands/Command.php b/src/Internal/Commands/Command.php index 1c42c88..b9dec39 100644 --- a/src/Internal/Commands/Command.php +++ b/src/Internal/Commands/Command.php @@ -5,14 +5,18 @@ namespace TinyBlocks\DockerContainer\Internal\Commands; /** - * Represents a Docker CLI command that can be converted to a command-line string. + * Represents a Docker CLI command that exposes its tokenized argument list for + * direct use with Symfony Process's array form. */ interface Command { /** - * Converts the command to its command-line string representation. + * Converts the command to its argument-list representation. * - * @return string The full command-line string ready for execution. + * The first element is the executable, remaining elements are its arguments. No shell + * interpretation happens between elements, so values are passed through verbatim. + * + * @return array Ordered argument list. */ - public function toCommandLine(): string; + public function toArguments(): array; } diff --git a/src/Internal/Commands/DockerCopy.php b/src/Internal/Commands/DockerCopy.php index 12d3587..c06fb71 100644 --- a/src/Internal/Commands/DockerCopy.php +++ b/src/Internal/Commands/DockerCopy.php @@ -18,8 +18,8 @@ public static function from(ContainerId $id, CopyInstruction $instruction): Dock return new DockerCopy(id: $id, instruction: $instruction); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker cp %s', $this->instruction->toCopyArgument(id: $this->id)); + return ['docker', 'cp', ...$this->instruction->toCopyArguments(id: $this->id)]; } } diff --git a/src/Internal/Commands/DockerExecute.php b/src/Internal/Commands/DockerExecute.php index e1f46b0..3d7e45b 100644 --- a/src/Internal/Commands/DockerExecute.php +++ b/src/Internal/Commands/DockerExecute.php @@ -18,8 +18,8 @@ public static function from(Name $name, array $commands): DockerExecute return new DockerExecute(name: $name, commands: Collection::createFrom(elements: $commands)); } - public function toCommandLine(): string + public function toArguments(): array { - return trim(sprintf('docker exec %s %s', $this->name->value, $this->commands->joinToString(separator: ' '))); + return ['docker', 'exec', $this->name->value, ...$this->commands->toArray()]; } } diff --git a/src/Internal/Commands/DockerInspect.php b/src/Internal/Commands/DockerInspect.php index 03249b5..0adc08b 100644 --- a/src/Internal/Commands/DockerInspect.php +++ b/src/Internal/Commands/DockerInspect.php @@ -17,8 +17,8 @@ public static function from(ContainerId $id): DockerInspect return new DockerInspect(id: $id); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker inspect %s', $this->id->value); + return ['docker', 'inspect', $this->id->value]; } } diff --git a/src/Internal/Commands/DockerList.php b/src/Internal/Commands/DockerList.php index 43b97d2..d4f30b4 100644 --- a/src/Internal/Commands/DockerList.php +++ b/src/Internal/Commands/DockerList.php @@ -17,8 +17,8 @@ public static function from(Name $name): DockerList return new DockerList(name: $name); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker ps --all --quiet --filter name=^%s$', $this->name->value); + return ['docker', 'ps', '--all', '--quiet', '--filter', sprintf('name=^%s$', $this->name->value)]; } } diff --git a/src/Internal/Commands/DockerNetworkConnect.php b/src/Internal/Commands/DockerNetworkConnect.php index bc1f147..5718c06 100644 --- a/src/Internal/Commands/DockerNetworkConnect.php +++ b/src/Internal/Commands/DockerNetworkConnect.php @@ -15,8 +15,8 @@ public static function from(string $network, string $container): DockerNetworkCo return new DockerNetworkConnect(network: $network, container: $container); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker network connect %s %s 2>/dev/null || true', $this->network, $this->container); + return ['docker', 'network', 'connect', $this->network, $this->container]; } } diff --git a/src/Internal/Commands/DockerNetworkCreate.php b/src/Internal/Commands/DockerNetworkCreate.php index 3cef5a1..4a5ffb9 100644 --- a/src/Internal/Commands/DockerNetworkCreate.php +++ b/src/Internal/Commands/DockerNetworkCreate.php @@ -15,12 +15,8 @@ public static function from(string $network): DockerNetworkCreate return new DockerNetworkCreate(network: $network); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf( - 'docker network create --label %s %s 2>/dev/null || true', - DockerRun::MANAGED_LABEL, - $this->network - ); + return ['docker', 'network', 'create', '--label', DockerRun::MANAGED_LABEL, $this->network]; } } diff --git a/src/Internal/Commands/DockerNetworkPrune.php b/src/Internal/Commands/DockerNetworkPrune.php index 3b5875c..ee5e09f 100644 --- a/src/Internal/Commands/DockerNetworkPrune.php +++ b/src/Internal/Commands/DockerNetworkPrune.php @@ -15,8 +15,8 @@ public static function create(): DockerNetworkPrune return new DockerNetworkPrune(); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker network prune --force --filter label=%s', DockerRun::MANAGED_LABEL); + return ['docker', 'network', 'prune', '--force', '--filter', sprintf('label=%s', DockerRun::MANAGED_LABEL)]; } } diff --git a/src/Internal/Commands/DockerPull.php b/src/Internal/Commands/DockerPull.php index 89f87ee..65b56d8 100644 --- a/src/Internal/Commands/DockerPull.php +++ b/src/Internal/Commands/DockerPull.php @@ -15,8 +15,8 @@ public static function from(string $image): DockerPull return new DockerPull(image: $image); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker pull %s', $this->image); + return ['docker', 'pull', $this->image]; } } diff --git a/src/Internal/Commands/DockerReaper.php b/src/Internal/Commands/DockerReaper.php index 97bb4ba..cd3e66e 100644 --- a/src/Internal/Commands/DockerReaper.php +++ b/src/Internal/Commands/DockerReaper.php @@ -22,9 +22,29 @@ public static function from(string $reaperName, string $containerName, string $t ); } - public function toCommandLine(): string + public function toArguments(): array { - $script = sprintf( + return [ + 'docker', + 'run', + '--rm', + '-d', + '--name', + $this->reaperName, + '--label', + DockerRun::MANAGED_LABEL, + '-v', + '/var/run/docker.sock:/var/run/docker.sock', + 'docker:cli', + 'sh', + '-c', + $this->buildScript() + ]; + } + + private function buildScript(): string + { + return sprintf( implode(' ', [ 'while docker inspect %s >/dev/null 2>&1; do sleep 2; done;', 'docker rm -fv %s 2>/dev/null;', @@ -34,16 +54,5 @@ public function toCommandLine(): string $this->containerName, DockerRun::MANAGED_LABEL ); - - return sprintf( - implode(' ', [ - 'docker run --rm -d --name %s --label %s', - '-v /var/run/docker.sock:/var/run/docker.sock', - 'docker:cli sh -c %s' - ]), - $this->reaperName, - DockerRun::MANAGED_LABEL, - escapeshellarg($script) - ); } } diff --git a/src/Internal/Commands/DockerRemove.php b/src/Internal/Commands/DockerRemove.php index 3f7e350..a03bfe9 100644 --- a/src/Internal/Commands/DockerRemove.php +++ b/src/Internal/Commands/DockerRemove.php @@ -17,8 +17,8 @@ public static function from(ContainerId $id): DockerRemove return new DockerRemove(id: $id); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker rm --force --volumes %s', $this->id->value); + return ['docker', 'rm', '--force', '--volumes', $this->id->value]; } } diff --git a/src/Internal/Commands/DockerRun.php b/src/Internal/Commands/DockerRun.php index c9dcf70..5576903 100644 --- a/src/Internal/Commands/DockerRun.php +++ b/src/Internal/Commands/DockerRun.php @@ -23,53 +23,50 @@ public static function from(ContainerDefinition $definition, array $commands = [ return new DockerRun(commands: Collection::createFrom(elements: $commands), definition: $definition); } - public function toCommandLine(): string + public function toArguments(): array { $name = $this->definition->name->value; - $parts = Collection::createFrom(elements: [ - 'docker run --user root', - sprintf('--name %s', $name), - sprintf('--hostname %s', $name), - sprintf('--label %s', self::MANAGED_LABEL) - ]); - - $parts = $parts->merge( - other: $this->definition->portMappings->map( - transformations: static fn(PortMapping $port): string => $port->toArgument() - ) - ); + $arguments = [ + 'docker', + 'run', + '--user', + 'root', + '--name', + $name, + '--hostname', + $name, + '--label', + self::MANAGED_LABEL + ]; + + foreach ($this->definition->portMappings as $port) { + /** @var PortMapping $port */ + $arguments = [...$arguments, ...$port->toArguments()]; + } if (!is_null($this->definition->network)) { - $parts = $parts->add(sprintf('--network=%s', $this->definition->network)); + $arguments[] = sprintf('--network=%s', $this->definition->network); } - $parts = $parts->merge( - other: $this->definition->volumeMappings->map( - transformations: static fn(VolumeMapping $volume): string => $volume->toArgument() - ) - ); + foreach ($this->definition->volumeMappings as $volume) { + /** @var VolumeMapping $volume */ + $arguments = [...$arguments, ...$volume->toArguments()]; + } - $parts = $parts->add('--detach'); + $arguments[] = '--detach'; if ($this->definition->autoRemove) { - $parts = $parts->add('--rm'); + $arguments[] = '--rm'; } - $parts = $parts->merge( - other: $this->definition->environmentVariables->map( - transformations: static fn(EnvironmentVariable $environment): string => $environment->toArgument() - ) - ); - - $parts = $parts->add($this->definition->image->name); - - $commandString = $this->commands->joinToString(separator: ' '); - - if (!empty($commandString)) { - $parts = $parts->add($commandString); + foreach ($this->definition->environmentVariables as $environment) { + /** @var EnvironmentVariable $environment */ + $arguments = [...$arguments, ...$environment->toArguments()]; } - return trim($parts->joinToString(separator: ' ')); + $arguments[] = $this->definition->image->name; + + return [...$arguments, ...$this->commands->toArray()]; } } diff --git a/src/Internal/Commands/DockerStop.php b/src/Internal/Commands/DockerStop.php index f98a042..cd4051a 100644 --- a/src/Internal/Commands/DockerStop.php +++ b/src/Internal/Commands/DockerStop.php @@ -17,9 +17,9 @@ public static function from(ContainerId $id, int $timeoutInWholeSeconds): Docker return new DockerStop(id: $id, timeoutInWholeSeconds: $timeoutInWholeSeconds); } - public function toCommandLine(): string + public function toArguments(): array { - return sprintf('docker stop %s', $this->id->value); + return ['docker', 'stop', '--time', (string)$this->timeoutInWholeSeconds, $this->id->value]; } public function getTimeoutInWholeSeconds(): int diff --git a/src/Internal/Containers/Address/Ports.php b/src/Internal/Containers/Address/Ports.php index 1aa4148..368d9d5 100644 --- a/src/Internal/Containers/Address/Ports.php +++ b/src/Internal/Containers/Address/Ports.php @@ -7,44 +7,39 @@ use TinyBlocks\Collection\Collection; use TinyBlocks\DockerContainer\Contracts\Ports as ContainerPorts; use TinyBlocks\DockerContainer\Internal\Containers\HostEnvironment; -use TinyBlocks\Mapper\KeyPreservation; final readonly class Ports implements ContainerPorts { - private function __construct(private Collection $exposedPorts, private Collection $hostMappedPorts) + private function __construct(private array $exposedPorts, private array $hostMappedPorts) { } public static function from(Collection $exposedPorts, Collection $hostMappedPorts): Ports { return new Ports( - exposedPorts: $exposedPorts->filter(), - hostMappedPorts: $hostMappedPorts->filter() + exposedPorts: array_values(array_filter($exposedPorts->toArray())), + hostMappedPorts: array_values(array_filter($hostMappedPorts->toArray())) ); } public function hostPorts(): array { - return $this->hostMappedPorts->toArray(keyPreservation: KeyPreservation::DISCARD); + return $this->hostMappedPorts; } public function exposedPorts(): array { - return $this->exposedPorts->toArray(keyPreservation: KeyPreservation::DISCARD); + return $this->exposedPorts; } public function firstHostPort(): ?int { - $port = $this->hostMappedPorts->first(); - - return empty($port) ? null : (int)$port; + return $this->hostMappedPorts[0] ?? null; } public function firstExposedPort(): ?int { - $port = $this->exposedPorts->first(); - - return empty($port) ? null : (int)$port; + return $this->exposedPorts[0] ?? null; } public function getPortForConnection(): ?int diff --git a/src/Internal/Containers/ContainerInspection.php b/src/Internal/Containers/ContainerInspection.php index 15d2f3b..573e53d 100644 --- a/src/Internal/Containers/ContainerInspection.php +++ b/src/Internal/Containers/ContainerInspection.php @@ -44,18 +44,14 @@ public function toAddress(): Address $hostMappedPorts = Collection::createFrom( elements: array_reduce( - array_values($rawHostPorts), + $rawHostPorts, static function (array $ports, ?array $bindings): array { if (is_null($bindings)) { return $ports; } foreach ($bindings as $binding) { - $hostPort = (int)($binding['HostPort'] ?? 0); - - if ($hostPort > 0) { - $ports[] = $hostPort; - } + $ports[] = (int)($binding['HostPort'] ?? 0); } return $ports; diff --git a/src/Internal/Containers/ContainerLookup.php b/src/Internal/Containers/ContainerLookup.php index 5d1f6fb..efb3622 100644 --- a/src/Internal/Containers/ContainerLookup.php +++ b/src/Internal/Containers/ContainerLookup.php @@ -26,9 +26,9 @@ public function byId( $dockerInspect = DockerInspect::from(id: $id); $executionCompleted = $this->client->execute(command: $dockerInspect); - $inspectPayload = (array)json_decode($executionCompleted->getOutput(), true); + $inspectPayload = json_decode($executionCompleted->getOutput(), true); - if (empty(array_filter($inspectPayload))) { + if (!is_array($inspectPayload) || empty($inspectPayload[0]) || !is_array($inspectPayload[0])) { throw new DockerContainerNotFound(name: $definition->name); } diff --git a/src/Internal/Containers/Definitions/CopyInstruction.php b/src/Internal/Containers/Definitions/CopyInstruction.php index adb7895..dbf0f71 100644 --- a/src/Internal/Containers/Definitions/CopyInstruction.php +++ b/src/Internal/Containers/Definitions/CopyInstruction.php @@ -17,8 +17,8 @@ public static function from(string $pathOnHost, string $pathOnContainer): CopyIn return new CopyInstruction(pathOnHost: $pathOnHost, pathOnContainer: $pathOnContainer); } - public function toCopyArgument(ContainerId $id): string + public function toCopyArguments(ContainerId $id): array { - return sprintf('%s %s:%s', $this->pathOnHost, $id->value, $this->pathOnContainer); + return [$this->pathOnHost, sprintf('%s:%s', $id->value, $this->pathOnContainer)]; } } diff --git a/src/Internal/Containers/Definitions/EnvironmentVariable.php b/src/Internal/Containers/Definitions/EnvironmentVariable.php index 71308f1..91e2041 100644 --- a/src/Internal/Containers/Definitions/EnvironmentVariable.php +++ b/src/Internal/Containers/Definitions/EnvironmentVariable.php @@ -15,8 +15,8 @@ public static function from(string $key, string $value): EnvironmentVariable return new EnvironmentVariable(key: $key, value: $value); } - public function toArgument(): string + public function toArguments(): array { - return sprintf('--env %s=%s', $this->key, escapeshellarg($this->value)); + return ['--env', sprintf('%s=%s', $this->key, $this->value)]; } } diff --git a/src/Internal/Containers/Definitions/PortMapping.php b/src/Internal/Containers/Definitions/PortMapping.php index 869bd87..96d008a 100644 --- a/src/Internal/Containers/Definitions/PortMapping.php +++ b/src/Internal/Containers/Definitions/PortMapping.php @@ -15,8 +15,8 @@ public static function from(int $portOnHost, int $portOnContainer): PortMapping return new PortMapping(portOnHost: $portOnHost, portOnContainer: $portOnContainer); } - public function toArgument(): string + public function toArguments(): array { - return sprintf('--publish %d:%d', $this->portOnHost, $this->portOnContainer); + return ['--publish', sprintf('%d:%d', $this->portOnHost, $this->portOnContainer)]; } } diff --git a/src/Internal/Containers/Definitions/VolumeMapping.php b/src/Internal/Containers/Definitions/VolumeMapping.php index 15aeea7..2f02202 100644 --- a/src/Internal/Containers/Definitions/VolumeMapping.php +++ b/src/Internal/Containers/Definitions/VolumeMapping.php @@ -15,8 +15,8 @@ public static function from(string $pathOnHost, string $pathOnContainer): Volume return new VolumeMapping(pathOnHost: $pathOnHost, pathOnContainer: $pathOnContainer); } - public function toArgument(): string + public function toArguments(): array { - return sprintf('--volume %s:%s', $this->pathOnHost, $this->pathOnContainer); + return ['--volume', sprintf('%s:%s', $this->pathOnHost, $this->pathOnContainer)]; } } diff --git a/src/Internal/Containers/Environment/EnvironmentVariables.php b/src/Internal/Containers/Environment/EnvironmentVariables.php index 1ea8b7d..26f7ce6 100644 --- a/src/Internal/Containers/Environment/EnvironmentVariables.php +++ b/src/Internal/Containers/Environment/EnvironmentVariables.php @@ -9,17 +9,17 @@ final readonly class EnvironmentVariables implements ContainerEnvironmentVariables { - private function __construct(private Collection $variables) + private function __construct(private array $variables) { } public static function from(Collection $variables): EnvironmentVariables { - return new EnvironmentVariables(variables: $variables); + return new EnvironmentVariables(variables: $variables->toArray()); } public function getValueBy(string $key): string { - return (string)($this->variables->toArray()[$key] ?? ''); + return (string)($this->variables[$key] ?? ''); } } diff --git a/src/Internal/Containers/HostEnvironment.php b/src/Internal/Containers/HostEnvironment.php index 05c91ba..027e08d 100644 --- a/src/Internal/Containers/HostEnvironment.php +++ b/src/Internal/Containers/HostEnvironment.php @@ -13,6 +13,6 @@ public static function isInsideDocker(): bool public static function containerHostname(): string { - return (string) gethostname(); + return (string)gethostname(); } } diff --git a/src/Internal/Containers/Models/Name.php b/src/Internal/Containers/Models/Name.php index eb13a6c..42a98b2 100644 --- a/src/Internal/Containers/Models/Name.php +++ b/src/Internal/Containers/Models/Name.php @@ -14,7 +14,7 @@ private function __construct(public string $value) public static function from(?string $value): Name { - $value = empty($value) ? Ksuid::random()->getValue() : $value; + $value = is_null($value) || $value === '' ? Ksuid::random()->getValue() : $value; return new Name(value: $value); } diff --git a/src/Internal/Containers/ShutdownHook.php b/src/Internal/Containers/ShutdownHook.php index 165973b..fc08455 100644 --- a/src/Internal/Containers/ShutdownHook.php +++ b/src/Internal/Containers/ShutdownHook.php @@ -4,7 +4,7 @@ namespace TinyBlocks\DockerContainer\Internal\Containers; -final readonly class ShutdownHook +class ShutdownHook { public function register(callable $callback): void { diff --git a/src/Internal/Exceptions/DockerCommandExecutionFailed.php b/src/Internal/Exceptions/DockerCommandExecutionFailed.php index e980c32..33e5d34 100644 --- a/src/Internal/Exceptions/DockerCommandExecutionFailed.php +++ b/src/Internal/Exceptions/DockerCommandExecutionFailed.php @@ -28,6 +28,8 @@ public static function fromProcess(Process $process, Throwable $exception): Dock public static function fromCommand(Command $command, ExecutionCompleted $execution): DockerCommandExecutionFailed { - return new DockerCommandExecutionFailed(reason: $execution->getOutput(), command: $command->toCommandLine()); + $rendered = implode(' ', array_map('escapeshellarg', $command->toArguments())); + + return new DockerCommandExecutionFailed(reason: $execution->getOutput(), command: $rendered); } } diff --git a/src/Waits/Conditions/MySQL/MySQLReady.php b/src/Waits/Conditions/MySQL/MySQLReady.php index a2d0a84..9cfac1b 100644 --- a/src/Waits/Conditions/MySQL/MySQLReady.php +++ b/src/Waits/Conditions/MySQL/MySQLReady.php @@ -27,7 +27,14 @@ public function isReady(): bool ->getValueBy(key: 'MYSQL_ROOT_PASSWORD'); return $this->container - ->executeAfterStarted(commands: ['mysqladmin', 'ping', '-h', '127.0.0.1', "-p$rootPassword"]) + ->executeAfterStarted(commands: [ + 'env', + "MYSQL_PWD=$rootPassword", + 'mysqladmin', + 'ping', + '-h', + '127.0.0.1' + ]) ->isSuccessful(); } catch (Throwable) { return false; diff --git a/src/Waits/ContainerWaitForDependency.php b/src/Waits/ContainerWaitForDependency.php index 0f13385..bd591ba 100644 --- a/src/Waits/ContainerWaitForDependency.php +++ b/src/Waits/ContainerWaitForDependency.php @@ -9,6 +9,8 @@ final readonly class ContainerWaitForDependency implements ContainerWaitBeforeStarted { + private const int MICROSECONDS_PER_SECOND = 1_000_000; + private function __construct( private ContainerReady $condition, private int $timeoutInSeconds, @@ -30,17 +32,17 @@ public static function untilReady( public function waitBefore(): void { - $deadline = microtime(true) + $this->timeoutInSeconds; + $totalBudgetInMicroseconds = $this->timeoutInSeconds * self::MICROSECONDS_PER_SECOND; + $maxAttempts = max(1, intdiv($totalBudgetInMicroseconds, $this->pollIntervalInMicroseconds)); - $ready = $this->condition->isReady(); + for ($attempts = 0; $attempts < $maxAttempts; $attempts++) { + if ($this->condition->isReady()) { + return; + } - while (!$ready && microtime(true) < $deadline) { usleep($this->pollIntervalInMicroseconds); - $ready = $this->condition->isReady(); } - if (!$ready) { - throw new ContainerWaitTimeout(timeoutInSeconds: $this->timeoutInSeconds); - } + throw new ContainerWaitTimeout(timeoutInSeconds: $this->timeoutInSeconds); } } diff --git a/tests/Unit/FlywayDockerContainerTest.php b/tests/Unit/FlywayDockerContainerTest.php index 35f0ff1..16108ef 100644 --- a/tests/Unit/FlywayDockerContainerTest.php +++ b/tests/Unit/FlywayDockerContainerTest.php @@ -150,7 +150,7 @@ public function testWithSourceAutoDetectsSchemaFromMySQLContainer(): void /** @Then FLYWAY_SCHEMAS should be auto-detected from the MySQL database name */ self::assertCommandLineContains( - needle: "FLYWAY_SCHEMAS='products'", + needle: 'FLYWAY_SCHEMAS=products', commandLines: $this->client->getExecutedCommandLines() ); } @@ -184,7 +184,7 @@ public function testWithSourceSetsDefaultSchemaHistoryTable(): void /** @Then FLYWAY_TABLE should default to "schema_history" */ self::assertCommandLineContains( - needle: "FLYWAY_TABLE='schema_history'", + needle: 'FLYWAY_TABLE=schema_history', commandLines: $this->client->getExecutedCommandLines() ); } @@ -219,11 +219,11 @@ public function testWithSourceConfiguresJdbcUrlAndCredentials(): void /** @Then the docker run command should include the JDBC URL and credentials */ $commandLines = $this->client->getExecutedCommandLines(); self::assertCommandLineContains( - needle: "FLYWAY_URL='jdbc:mysql://source-db:3306/app_database", + needle: 'FLYWAY_URL=jdbc:mysql://source-db:3306/app_database', commandLines: $commandLines ); - self::assertCommandLineContains(needle: "FLYWAY_USER='admin'", commandLines: $commandLines); - self::assertCommandLineContains(needle: "FLYWAY_PASSWORD='secret'", commandLines: $commandLines); + self::assertCommandLineContains(needle: 'FLYWAY_USER=admin', commandLines: $commandLines); + self::assertCommandLineContains(needle: 'FLYWAY_PASSWORD=secret', commandLines: $commandLines); /** @And a MySQL readiness check should have been executed before Flyway started */ $mysqladminPingCount = count( @@ -266,7 +266,7 @@ public function testWithSchemaOverridesAutoDetectedSchema(): void /** @Then the overridden schema should be present in the command */ self::assertCommandLineContains( - needle: "FLYWAY_SCHEMAS='custom_schema'", + needle: 'FLYWAY_SCHEMAS=custom_schema', commandLines: $this->client->getExecutedCommandLines() ); } @@ -302,7 +302,7 @@ public function testWithTableOverridesDefaultTable(): void /** @Then the overridden table should be present in the command */ self::assertCommandLineContains( - needle: "FLYWAY_TABLE='flyway_history'", + needle: 'FLYWAY_TABLE=flyway_history', commandLines: $this->client->getExecutedCommandLines() ); } @@ -328,7 +328,7 @@ public function testWithMigrationsConfiguresCopyAndLocation(): void /** @Then the FLYWAY_LOCATIONS should point to the container migrations path */ $commandLines = $this->client->getExecutedCommandLines(); self::assertCommandLineContains( - needle: "FLYWAY_LOCATIONS='filesystem:/flyway/migrations'", + needle: 'FLYWAY_LOCATIONS=filesystem:/flyway/migrations', commandLines: $commandLines ); self::assertCommandLineContains(needle: 'docker cp /host/migrations', commandLines: $commandLines); @@ -354,7 +354,7 @@ public function testWithCleanDisabledSetsEnvironmentVariable(): void /** @Then FLYWAY_CLEAN_DISABLED should be set to true */ self::assertCommandLineContains( - needle: "FLYWAY_CLEAN_DISABLED='true'", + needle: 'FLYWAY_CLEAN_DISABLED=true', commandLines: $this->client->getExecutedCommandLines() ); } @@ -379,7 +379,7 @@ public function testWithConnectRetriesSetsEnvironmentVariable(): void /** @Then FLYWAY_CONNECT_RETRIES should be set to 10 */ self::assertCommandLineContains( - needle: "FLYWAY_CONNECT_RETRIES='10'", + needle: 'FLYWAY_CONNECT_RETRIES=10', commandLines: $this->client->getExecutedCommandLines() ); } @@ -404,7 +404,7 @@ public function testWithValidateMigrationNamingSetsEnvironmentVariable(): void /** @Then FLYWAY_VALIDATE_MIGRATION_NAMING should be set to true */ self::assertCommandLineContains( - needle: "FLYWAY_VALIDATE_MIGRATION_NAMING='true'", + needle: 'FLYWAY_VALIDATE_MIGRATION_NAMING=true', commandLines: $this->client->getExecutedCommandLines() ); } diff --git a/tests/Unit/GenericDockerContainerTest.php b/tests/Unit/GenericDockerContainerTest.php index 5d5ac50..33fc6b5 100644 --- a/tests/Unit/GenericDockerContainerTest.php +++ b/tests/Unit/GenericDockerContainerTest.php @@ -9,9 +9,9 @@ use PHPUnit\Framework\TestCase; use Test\Unit\Mocks\ClientMock; use Test\Unit\Mocks\InspectResponseFixture; +use Test\Unit\Mocks\ShutdownHookMock; use Test\Unit\Mocks\TestableGenericDockerContainer; use TinyBlocks\DockerContainer\GenericDockerContainer; -use TinyBlocks\DockerContainer\Internal\Containers\ShutdownHook; use TinyBlocks\DockerContainer\Internal\Exceptions\ContainerWaitTimeout; use TinyBlocks\DockerContainer\Internal\Exceptions\DockerCommandExecutionFailed; use TinyBlocks\DockerContainer\Internal\Exceptions\DockerContainerNotFound; @@ -239,6 +239,31 @@ public function testRunIfNotExistsCreatesNewContainer(): void self::assertSame(expected: 'test', actual: $started->getEnvironmentVariables()->getValueBy(key: 'APP_ENV')); } + public function testRunIfNotExistsTreatsWhitespaceOnlyListOutputAsMissingContainer(): void + { + /** @Given a container that does not exist according to a whitespace-only docker list response */ + $container = TestableGenericDockerContainer::createWith( + image: 'alpine:latest', + name: 'whitespace-list', + client: $this->client + ); + + /** @And the Docker list returns only whitespace */ + $this->client->withDockerListResponse(output: " \n\t "); + + /** @And the Docker daemon returns valid run and inspect responses */ + $this->client->withDockerRunResponse(output: InspectResponseFixture::containerId()); + $this->client->withDockerInspectResponse( + inspectResult: InspectResponseFixture::build(hostname: 'whitespace-list') + ); + + /** @When runIfNotExists is called */ + $started = $container->runIfNotExists(); + + /** @Then a new container should be created because the whitespace list output is trimmed */ + self::assertSame(expected: 'whitespace-list', actual: $started->getName()); + } + public function testRunIfNotExistsReturnsExistingContainer(): void { /** @Given a container that already exists */ @@ -337,6 +362,26 @@ public function testExceptionWhenRunFails(): void $container->run(); } + public function testExceptionWhenRunFailsRendersCommandWithShellEscapedArguments(): void + { + /** @Given a container that will fail to start */ + $container = TestableGenericDockerContainer::createWith( + image: 'invalid:image', + name: 'fail-render-test', + client: $this->client + ); + + /** @And the Docker daemon returns a failure */ + $this->client->withDockerRunResponse(output: 'boom', isSuccessful: false); + + /** @Then the failure message should render each argument shell-escaped */ + $this->expectException(DockerCommandExecutionFailed::class); + $this->expectExceptionMessageMatches("/'docker' 'run'/"); + + /** @When the container is started */ + $container->run(); + } + public function testExceptionWhenContainerInspectReturnsEmpty(): void { /** @Given a container whose inspect returns empty data */ @@ -459,7 +504,7 @@ public function testContainerWithMultipleHostPortMappings(): void hostname: 'multi-host-port', exposedPorts: ['80/tcp' => (object)[], '443/tcp' => (object)[]], hostPortBindings: [ - '80/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8080']], + '80/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8080']], '443/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8443']] ] ) @@ -474,6 +519,95 @@ public function testContainerWithMultipleHostPortMappings(): void self::assertSame(expected: 8080, actual: $started->getAddress()->getPorts()->firstHostPort()); } + public function testContainerWithMixedValidAndNullHostBindingsRetainsValidPorts(): void + { + /** @Given a container whose inspect payload mixes valid and null host bindings */ + $container = TestableGenericDockerContainer::createWith( + image: 'nginx:latest', + name: 'mixed-bindings', + client: $this->client + ) + ->withPortMapping(portOnHost: 8080, portOnContainer: 80) + ->withPortMapping(portOnHost: 8443, portOnContainer: 443); + + /** @And the Docker daemon returns an inspect response with a trailing null binding */ + $this->client->withDockerRunResponse(output: InspectResponseFixture::containerId()); + $this->client->withDockerInspectResponse( + inspectResult: InspectResponseFixture::build( + hostname: 'mixed-bindings', + exposedPorts: ['80/tcp' => (object)[], '443/tcp' => (object)[], '5432/tcp' => (object)[]], + hostPortBindings: [ + '80/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8080']], + '443/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8443']], + '5432/tcp' => null + ] + ) + ); + + /** @When the container is started */ + $started = $container->run(); + + /** @Then the host-mapped ports should retain all previously collected ports when a null binding follows */ + self::assertSame(expected: [8080, 8443], actual: $started->getAddress()->getPorts()->hostPorts()); + } + + public function testContainerWithBindingMissingHostPortIsSkipped(): void + { + /** @Given a container whose inspect payload includes a binding with no HostPort key */ + $container = TestableGenericDockerContainer::createWith( + image: 'alpine:latest', + name: 'no-host-port-key', + client: $this->client + ); + + /** @And the Docker daemon returns an inspect response with a partial binding */ + $this->client->withDockerRunResponse(output: InspectResponseFixture::containerId()); + $this->client->withDockerInspectResponse( + inspectResult: InspectResponseFixture::build( + hostname: 'no-host-port-key', + exposedPorts: ['80/tcp' => (object)[]], + hostPortBindings: [ + '80/tcp' => [['HostIp' => '0.0.0.0']] + ] + ) + ); + + /** @When the container is started */ + $started = $container->run(); + + /** @Then bindings lacking HostPort should be skipped without errors */ + self::assertEmpty($started->getAddress()->getPorts()->hostPorts()); + } + + public function testContainerWithZeroHostPortDropsItFromResult(): void + { + /** @Given a container whose inspect payload reports a binding with HostPort equal to zero */ + $container = TestableGenericDockerContainer::createWith( + image: 'alpine:latest', + name: 'zero-host-port', + client: $this->client + ); + + /** @And the Docker daemon returns an inspect response with HostPort zero alongside a valid binding */ + $this->client->withDockerRunResponse(output: InspectResponseFixture::containerId()); + $this->client->withDockerInspectResponse( + inspectResult: InspectResponseFixture::build( + hostname: 'zero-host-port', + exposedPorts: ['80/tcp' => (object)[], '443/tcp' => (object)[]], + hostPortBindings: [ + '80/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '0']], + '443/tcp' => [['HostIp' => '0.0.0.0', 'HostPort' => '8443']] + ] + ) + ); + + /** @When the container is started */ + $started = $container->run(); + + /** @Then only strictly positive host ports should be retained */ + self::assertSame(expected: [8443], actual: $started->getAddress()->getPorts()->hostPorts()); + } + public function testContainerWithExposedPortButNoHostBinding(): void { /** @Given a container with an exposed port but no host binding */ @@ -660,7 +794,7 @@ public function testRunIfNotExistsWithWaitBeforeRun(): void public function testExceptionWhenWaitBeforeRunTimesOut(): void { /** @Given a condition that never becomes ready */ - $condition = $this->createMock(ContainerReady::class); + $condition = $this->createStub(ContainerReady::class); $condition->method('isReady')->willReturn(false); /** @And a container with a wait-before-run that has a short timeout */ @@ -898,7 +1032,7 @@ public function testRunCommandLineIncludesEnvironmentVariable(): void /** @Then the docker run command should contain the environment variable argument */ $runCommand = $this->client->getExecutedCommandLines()[0]; - self::assertStringContainsString(needle: "--env APP_ENV='production'", haystack: $runCommand); + self::assertStringContainsString(needle: '--env APP_ENV=production', haystack: $runCommand); } public function testRunCommandLineIncludesNetwork(): void @@ -1159,8 +1293,7 @@ public function testRemoveCanBeCalledMultipleTimes(): void public function testStopOnShutdownRegistersRemove(): void { /** @Given a ShutdownHook that tracks registration */ - $shutdownHook = $this->createMock(ShutdownHook::class); - $shutdownHook->expects(self::once())->method('register'); + $shutdownHook = new ShutdownHookMock(); /** @And a running container using the tracked hook */ $container = TestableGenericDockerContainer::createWith( @@ -1183,7 +1316,7 @@ public function testStopOnShutdownRegistersRemove(): void $started->stopOnShutdown(); /** @Then the shutdown hook should have registered the remove callback */ - self::assertSame(expected: 'shutdown-test', actual: $started->getName()); + self::assertSame(1, $shutdownHook->getRegistrationCount()); } public function testRemoveOnReusedContainerIsNoOp(): void @@ -1213,25 +1346,29 @@ public function testRemoveOnReusedContainerIsNoOp(): void self::assertSame(expected: 'reused-remove', actual: $started->getName()); } + #[RunInSeparateProcess] public function testRunIfNotExistsSkipsReaperCreationWhenReaperAlreadyExists(): void { + require_once __DIR__ . '/Internal/Containers/Overrides/file_exists_inside_docker.php'; + /** @Given a container that already exists */ + $client = new ClientMock(); $container = TestableGenericDockerContainer::createWith( image: 'alpine:latest', name: 'reaper-skip', - client: $this->client + client: $client ); /** @And the Docker list returns an existing container */ - $this->client->withDockerListResponse(output: InspectResponseFixture::containerId()); + $client->withDockerListResponse(output: InspectResponseFixture::containerId()); /** @And the Docker inspect returns the container details */ - $this->client->withDockerInspectResponse( + $client->withDockerInspectResponse( inspectResult: InspectResponseFixture::build(hostname: 'reaper-skip') ); /** @And the reaper container already exists */ - $this->client->withDockerListResponse(output: 'existing-reaper-id'); + $client->withDockerListResponse(output: 'existing-reaper-id'); /** @When runIfNotExists is called */ $started = $container->runIfNotExists(); @@ -1239,15 +1376,18 @@ public function testRunIfNotExistsSkipsReaperCreationWhenReaperAlreadyExists(): /** @Then the container should be returned */ self::assertSame(expected: 'reaper-skip', actual: $started->getName()); - /** @And no reaper creation command should have been executed */ - $commandLines = $this->client->getExecutedCommandLines(); + /** @And the reused container should have probed for the reaper list to exist */ + $commandLines = $client->getExecutedCommandLines(); + $reaperListed = false; foreach ($commandLines as $commandLine) { + $reaperListed = $reaperListed || str_contains($commandLine, 'tiny-blocks-reaper-reaper-skip'); self::assertStringNotContainsString( needle: 'docker run --rm -d --name tiny-blocks-reaper', haystack: $commandLine ); } + self::assertTrue($reaperListed); } #[RunInSeparateProcess] diff --git a/tests/Unit/Internal/Commands/DockerCommandsTest.php b/tests/Unit/Internal/Commands/DockerCommandsTest.php index f9db0db..b522965 100644 --- a/tests/Unit/Internal/Commands/DockerCommandsTest.php +++ b/tests/Unit/Internal/Commands/DockerCommandsTest.php @@ -4,14 +4,21 @@ namespace Test\Unit\Internal\Commands; +use InvalidArgumentException; use PHPUnit\Framework\TestCase; +use TinyBlocks\DockerContainer\Internal\Commands\DockerInspect; +use TinyBlocks\DockerContainer\Internal\Commands\DockerList; +use TinyBlocks\DockerContainer\Internal\Commands\DockerNetworkConnect; use TinyBlocks\DockerContainer\Internal\Commands\DockerNetworkCreate; use TinyBlocks\DockerContainer\Internal\Commands\DockerNetworkPrune; use TinyBlocks\DockerContainer\Internal\Commands\DockerPull; +use TinyBlocks\DockerContainer\Internal\Commands\DockerReaper; use TinyBlocks\DockerContainer\Internal\Commands\DockerRemove; use TinyBlocks\DockerContainer\Internal\Commands\DockerRun; +use TinyBlocks\DockerContainer\Internal\Commands\DockerStop; use TinyBlocks\DockerContainer\Internal\Containers\Definitions\ContainerDefinition; use TinyBlocks\DockerContainer\Internal\Containers\Models\ContainerId; +use TinyBlocks\DockerContainer\Internal\Containers\Models\Name; final class DockerCommandsTest extends TestCase { @@ -20,11 +27,11 @@ public function testDockerPullGeneratesCorrectCommand(): void /** @Given a Docker pull command for a specific image */ $command = DockerPull::from(image: 'mysql:8.4'); - /** @When the command line is generated */ - $commandLine = $command->toCommandLine(); + /** @When the argument list is generated */ + $arguments = $command->toArguments(); - /** @Then the command should pull the specified image */ - self::assertSame(expected: 'docker pull mysql:8.4', actual: $commandLine); + /** @Then the arguments should represent pulling the specified image */ + self::assertSame(['docker', 'pull', 'mysql:8.4'], $arguments); } public function testDockerRemoveGeneratesCorrectCommand(): void @@ -33,11 +40,11 @@ public function testDockerRemoveGeneratesCorrectCommand(): void $containerId = ContainerId::from(value: '6acae5967be05d8441b4109eea3e4dec5e775068a2a99d95808afb21b2e0a2c8'); $command = DockerRemove::from(id: $containerId); - /** @When the command line is generated */ - $commandLine = $command->toCommandLine(); + /** @When the argument list is generated */ + $arguments = $command->toArguments(); - /** @Then the command should force-remove the container with its volumes */ - self::assertSame(expected: 'docker rm --force --volumes 6acae5967be0', actual: $commandLine); + /** @Then the arguments should force-remove the container with its volumes */ + self::assertSame(['docker', 'rm', '--force', '--volumes', '6acae5967be0'], $arguments); } public function testDockerNetworkCreateGeneratesCommandWithLabel(): void @@ -45,13 +52,13 @@ public function testDockerNetworkCreateGeneratesCommandWithLabel(): void /** @Given a Docker network create command */ $command = DockerNetworkCreate::from(network: 'my-network'); - /** @When the command line is generated */ - $commandLine = $command->toCommandLine(); + /** @When the argument list is generated */ + $arguments = $command->toArguments(); - /** @Then the command should create the network with the managed label */ + /** @Then the arguments should create the network with the managed label */ self::assertSame( - expected: 'docker network create --label tiny-blocks.docker-container=true my-network 2>/dev/null || true', - actual: $commandLine + ['docker', 'network', 'create', '--label', 'tiny-blocks.docker-container=true', 'my-network'], + $arguments ); } @@ -60,13 +67,13 @@ public function testDockerNetworkPruneGeneratesCommandFilteredByLabel(): void /** @Given a Docker network prune command */ $command = DockerNetworkPrune::create(); - /** @When the command line is generated */ - $commandLine = $command->toCommandLine(); + /** @When the argument list is generated */ + $arguments = $command->toArguments(); - /** @Then the command should prune only networks with the managed label */ + /** @Then the arguments should prune only networks with the managed label */ self::assertSame( - expected: 'docker network prune --force --filter label=tiny-blocks.docker-container=true', - actual: $commandLine + ['docker', 'network', 'prune', '--force', '--filter', 'label=tiny-blocks.docker-container=true'], + $arguments ); } @@ -79,13 +86,107 @@ public function testDockerRunIncludesManagedLabel(): void ); $command = DockerRun::from(definition: $definition); - /** @When the command line is generated */ - $commandLine = $command->toCommandLine(); + /** @When the argument list is generated */ + $arguments = $command->toArguments(); - /** @Then the command should include the managed label */ - self::assertStringContainsString( - needle: sprintf('--label %s', DockerRun::MANAGED_LABEL), - haystack: $commandLine + /** @Then the arguments should include the managed label flag and value */ + self::assertContains('--label', $arguments); + self::assertContains(DockerRun::MANAGED_LABEL, $arguments); + } + + public function testDockerInspectGeneratesCorrectCommand(): void + { + /** @Given a Docker inspect command for a specific container */ + $containerId = ContainerId::from(value: '6acae5967be05d8441b4109eea3e4dec5e775068a2a99d95808afb21b2e0a2c8'); + $command = DockerInspect::from(id: $containerId); + + /** @When the argument list is generated */ + $arguments = $command->toArguments(); + + /** @Then the arguments should invoke docker inspect on the container id */ + self::assertSame(['docker', 'inspect', '6acae5967be0'], $arguments); + } + + public function testDockerListGeneratesCorrectCommand(): void + { + /** @Given a Docker list command filtered by container name */ + $command = DockerList::from(name: Name::from(value: 'my-container')); + + /** @When the argument list is generated */ + $arguments = $command->toArguments(); + + /** @Then the arguments should list containers filtered by the exact name */ + self::assertSame( + ['docker', 'ps', '--all', '--quiet', '--filter', 'name=^my-container$'], + $arguments + ); + } + + public function testDockerNetworkConnectGeneratesCorrectCommand(): void + { + /** @Given a Docker network connect command */ + $command = DockerNetworkConnect::from(network: 'my-network', container: 'my-container'); + + /** @When the argument list is generated */ + $arguments = $command->toArguments(); + + /** @Then the arguments should connect the container to the network */ + self::assertSame(['docker', 'network', 'connect', 'my-network', 'my-container'], $arguments); + } + + public function testDockerStopGeneratesCorrectCommandWithStringTimeout(): void + { + /** @Given a Docker stop command with an integer timeout */ + $containerId = ContainerId::from(value: '6acae5967be05d8441b4109eea3e4dec5e775068a2a99d95808afb21b2e0a2c8'); + $command = DockerStop::from(id: $containerId, timeoutInWholeSeconds: 45); + + /** @When the argument list is generated */ + $arguments = $command->toArguments(); + + /** @Then the arguments should include the timeout as a string argument */ + self::assertSame(['docker', 'stop', '--time', '45', '6acae5967be0'], $arguments); + } + + public function testDockerReaperGeneratesCommandWithDockerAndScript(): void + { + /** @Given a Docker reaper command */ + $command = DockerReaper::from( + reaperName: 'tiny-blocks-reaper-db', + containerName: 'db', + testRunnerHostname: 'runner-host' ); + + /** @When the argument list is generated */ + $arguments = $command->toArguments(); + + /** @Then the command should start with docker run and set the reaper name */ + self::assertSame('docker', $arguments[0]); + self::assertSame('run', $arguments[1]); + self::assertContains('tiny-blocks-reaper-db', $arguments); + + /** @And the embedded shell script should poll, remove and prune by watching the runner hostname */ + $script = end($arguments); + self::assertStringContainsString('while docker inspect runner-host', $script); + self::assertStringContainsString('docker rm -fv db', $script); + self::assertStringContainsString(sprintf('label=%s', DockerRun::MANAGED_LABEL), $script); + } + + public function testContainerIdAcceptsExactMinimumLength(): void + { + /** @Given a container ID whose length is exactly the minimum */ + $containerId = ContainerId::from(value: '123456789012'); + + /** @Then the full string should be preserved as the container value */ + self::assertSame('123456789012', $containerId->value); + } + + public function testContainerIdRejectsElevenCharacterValue(): void + { + /** @Then an InvalidArgumentException should be thrown for a value one character below the minimum */ + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Container ID <12345678901> is too short. Minimum length is <12> characters.'); + + /** @When a container ID shorter than the minimum length is created */ + ContainerId::from(value: '12345678901'); } } diff --git a/tests/Unit/Internal/Containers/Address/PortsTest.php b/tests/Unit/Internal/Containers/Address/PortsTest.php index 75a056a..eb630a9 100644 --- a/tests/Unit/Internal/Containers/Address/PortsTest.php +++ b/tests/Unit/Internal/Containers/Address/PortsTest.php @@ -46,4 +46,21 @@ public function testGetPortForConnectionReturnsExposedPortWhenInsideDocker(): vo /** @Then it should return the container-internal exposed port */ self::assertSame(3306, $port); } + + public function testPortsDropsFalsyValuesAndReindexesSequentially(): void + { + /** @Given Ports built from collections containing zeros between valid port numbers */ + $ports = Ports::from( + exposedPorts: Collection::createFrom(elements: [0, 3306, 0, 8080]), + hostMappedPorts: Collection::createFrom(elements: [0, 49153, 0, 49154]) + ); + + /** @Then exposed ports should drop zero values and use sequential numeric keys from zero */ + self::assertSame([3306, 8080], $ports->exposedPorts()); + self::assertSame(3306, $ports->firstExposedPort()); + + /** @And host-mapped ports should drop zero values and use sequential numeric keys from zero */ + self::assertSame([49153, 49154], $ports->hostPorts()); + self::assertSame(49153, $ports->firstHostPort()); + } } diff --git a/tests/Unit/Internal/Containers/ContainerReaperTest.php b/tests/Unit/Internal/Containers/ContainerReaperTest.php index 09c15c4..c926613 100644 --- a/tests/Unit/Internal/Containers/ContainerReaperTest.php +++ b/tests/Unit/Internal/Containers/ContainerReaperTest.php @@ -26,5 +26,49 @@ public function testEnsureRunningForSkipsWhenOutsideDocker(): void /** @Then no Docker commands should have been executed */ self::assertEmpty($client->getExecutedCommandLines()); } + + #[RunInSeparateProcess] + public function testEnsureRunningForStartsReaperWhenReaperIsMissing(): void + { + require_once __DIR__ . '/Overrides/file_exists_inside_docker.php'; + + /** @Given a ContainerReaper with a mock client */ + $client = new ClientMock(); + $reaper = new ContainerReaper(client: $client); + + /** @And no existing reaper container is listed */ + $client->withDockerListResponse(output: ''); + + /** @When ensureRunningFor is called inside a Docker environment */ + $reaper->ensureRunningFor(containerName: 'test-container'); + + /** @Then a reaper container should have been started for the target container */ + self::assertStringContainsString( + 'docker run --rm -d --name tiny-blocks-reaper-test-container', + implode(PHP_EOL, $client->getExecutedCommandLines()) + ); + } + + #[RunInSeparateProcess] + public function testEnsureRunningForStartsReaperWhenListOutputIsOnlyWhitespace(): void + { + require_once __DIR__ . '/Overrides/file_exists_inside_docker.php'; + + /** @Given a ContainerReaper with a mock client */ + $client = new ClientMock(); + $reaper = new ContainerReaper(client: $client); + + /** @And the Docker list response contains only whitespace */ + $client->withDockerListResponse(output: " \n\t "); + + /** @When ensureRunningFor is called inside a Docker environment */ + $reaper->ensureRunningFor(containerName: 'whitespace-container'); + + /** @Then a reaper container should have been started for the target container */ + self::assertStringContainsString( + 'docker run --rm -d --name tiny-blocks-reaper-whitespace-container', + implode(PHP_EOL, $client->getExecutedCommandLines()) + ); + } } diff --git a/tests/Unit/Internal/Containers/Overrides/file_exists_inside_docker.php b/tests/Unit/Internal/Containers/Overrides/file_exists_inside_docker.php index 5b308c1..ed0ea37 100644 --- a/tests/Unit/Internal/Containers/Overrides/file_exists_inside_docker.php +++ b/tests/Unit/Internal/Containers/Overrides/file_exists_inside_docker.php @@ -7,4 +7,4 @@ function file_exists(string $filename): bool { return true; - } +} diff --git a/tests/Unit/Mocks/ClientMock.php b/tests/Unit/Mocks/ClientMock.php index 1736b03..b2c9d45 100644 --- a/tests/Unit/Mocks/ClientMock.php +++ b/tests/Unit/Mocks/ClientMock.php @@ -73,7 +73,7 @@ public function getExecutedCommandLines(): array public function execute(Command $command): ExecutionCompleted { - $this->executedCommandLines[] = $command->toCommandLine(); + $this->executedCommandLines[] = implode(' ', $command->toArguments()); if ($command instanceof CommandWithTimeout) { $command->getTimeoutInWholeSeconds(); @@ -92,15 +92,15 @@ public function execute(Command $command): ExecutionCompleted } [$output, $isSuccessful] = match (true) { - $command instanceof DockerRun => [ + $command instanceof DockerRun => [ array_shift($this->runResponses) ?? '', $this->runIsSuccessful ], - $command instanceof DockerList => [ + $command instanceof DockerList => [ ($listOutput = array_shift($this->listResponses) ?? ''), !empty($listOutput) ], - $command instanceof DockerInspect => [ + $command instanceof DockerInspect => [ json_encode([($inspectData = array_shift($this->inspectResponses))]), !empty($inspectData) ], diff --git a/tests/Unit/Mocks/CommandMock.php b/tests/Unit/Mocks/CommandMock.php index 26c6831..9503c17 100644 --- a/tests/Unit/Mocks/CommandMock.php +++ b/tests/Unit/Mocks/CommandMock.php @@ -12,8 +12,8 @@ public function __construct(private string $command) { } - public function toCommandLine(): string + public function toArguments(): array { - return $this->command; + return explode(' ', $this->command); } } diff --git a/tests/Unit/Mocks/CommandWithTimeoutMock.php b/tests/Unit/Mocks/CommandWithTimeoutMock.php index eff2bcd..3c94f96 100644 --- a/tests/Unit/Mocks/CommandWithTimeoutMock.php +++ b/tests/Unit/Mocks/CommandWithTimeoutMock.php @@ -12,9 +12,9 @@ public function __construct(private string $command, private int $timeoutInWhole { } - public function toCommandLine(): string + public function toArguments(): array { - return $this->command; + return explode(' ', $this->command); } public function getTimeoutInWholeSeconds(): int diff --git a/tests/Unit/Mocks/ShutdownHookMock.php b/tests/Unit/Mocks/ShutdownHookMock.php new file mode 100644 index 0000000..40a2469 --- /dev/null +++ b/tests/Unit/Mocks/ShutdownHookMock.php @@ -0,0 +1,23 @@ +registrations++; + } + + public function getRegistrationCount(): int + { + return $this->registrations; + } +} + diff --git a/tests/Unit/MySQLDockerContainerTest.php b/tests/Unit/MySQLDockerContainerTest.php index 35ad5a9..6557fc5 100644 --- a/tests/Unit/MySQLDockerContainerTest.php +++ b/tests/Unit/MySQLDockerContainerTest.php @@ -7,9 +7,9 @@ use PHPUnit\Framework\TestCase; use Test\Unit\Mocks\ClientMock; use Test\Unit\Mocks\InspectResponseFixture; +use Test\Unit\Mocks\ShutdownHookMock; use Test\Unit\Mocks\TestableMySQLDockerContainer; use TinyBlocks\DockerContainer\Contracts\MySQL\MySQLContainerStarted; -use TinyBlocks\DockerContainer\Internal\Containers\ShutdownHook; use TinyBlocks\DockerContainer\Internal\Exceptions\ContainerWaitTimeout; use TinyBlocks\DockerContainer\Internal\Exceptions\DockerCommandExecutionFailed; use TinyBlocks\DockerContainer\Waits\Conditions\ContainerReady; @@ -109,10 +109,19 @@ public function testRunMySQLContainerSuccessfully(): void self::assertStringNotContainsString(needle: '--rm', haystack: $runCommand); self::assertStringContainsString(needle: '--volume /var/lib/mysql:/var/lib/mysql', haystack: $runCommand); self::assertStringContainsString(needle: '--publish 3306:3306', haystack: $runCommand); - self::assertStringContainsString(needle: "MYSQL_USER='app_user'", haystack: $runCommand); - self::assertStringContainsString(needle: "MYSQL_PASSWORD='secret'", haystack: $runCommand); - self::assertStringContainsString(needle: "MYSQL_DATABASE='test_adm'", haystack: $runCommand); - self::assertStringContainsString(needle: "MYSQL_ROOT_PASSWORD='root'", haystack: $runCommand); + self::assertStringContainsString(needle: 'TZ=America/Sao_Paulo', haystack: $runCommand); + self::assertStringContainsString(needle: 'MYSQL_USER=app_user', haystack: $runCommand); + self::assertStringContainsString(needle: 'MYSQL_PASSWORD=secret', haystack: $runCommand); + self::assertStringContainsString(needle: 'MYSQL_DATABASE=test_adm', haystack: $runCommand); + self::assertStringContainsString(needle: 'MYSQL_ROOT_PASSWORD=root', haystack: $runCommand); + + /** @And the readiness probe should execute env-prefixed mysqladmin ping via docker exec */ + $readinessCommand = $commandLines[4]; + + self::assertStringContainsString( + needle: 'docker exec test-db env MYSQL_PWD=root mysqladmin ping -h 127.0.0.1', + haystack: $readinessCommand + ); /** @And the database setup should include CREATE DATABASE, GRANT, and FLUSH */ $setupCommand = $commandLines[5]; @@ -776,7 +785,7 @@ public function testMySQLContainerWithEnvironmentVariableDirectly(): void /** @And the docker run command should include the custom environment variable */ $runCommand = $this->client->getExecutedCommandLines()[0]; - self::assertStringContainsString(needle: "CUSTOM_KEY='custom_value'", haystack: $runCommand); + self::assertStringContainsString(needle: 'CUSTOM_KEY=custom_value', haystack: $runCommand); } public function testRunMySQLContainerWithPullImage(): void @@ -824,8 +833,7 @@ public function testRunMySQLContainerWithPullImage(): void public function testStopOnShutdownDelegatesToUnderlyingContainer(): void { /** @Given a ShutdownHook that tracks registration */ - $shutdownHook = $this->createMock(ShutdownHook::class); - $shutdownHook->expects(self::once())->method('register'); + $shutdownHook = new ShutdownHookMock(); /** @And a running MySQL container using the tracked hook */ $container = TestableMySQLDockerContainer::createWith( @@ -855,7 +863,7 @@ public function testStopOnShutdownDelegatesToUnderlyingContainer(): void $started->stopOnShutdown(); /** @Then the shutdown hook should have registered the remove callback */ - self::assertSame(expected: 'shutdown-db', actual: $started->getName()); + self::assertSame(1, $shutdownHook->getRegistrationCount()); } public function testRemoveDelegatesToUnderlyingContainer(): void diff --git a/tests/Unit/Waits/ContainerWaitForDependencyTest.php b/tests/Unit/Waits/ContainerWaitForDependencyTest.php index 8fa57de..59a6a75 100644 --- a/tests/Unit/Waits/ContainerWaitForDependencyTest.php +++ b/tests/Unit/Waits/ContainerWaitForDependencyTest.php @@ -48,7 +48,7 @@ public function testWaitBeforeRetriesUntilReady(): void public function testExceptionWhenWaitTimesOut(): void { /** @Given a condition that never becomes ready */ - $condition = $this->createMock(ContainerReady::class); + $condition = $this->createStub(ContainerReady::class); $condition->method('isReady')->willReturn(false); /** @Then a ContainerWaitTimeout exception should be thrown */ @@ -68,7 +68,7 @@ public function testCustomPollIntervalIsRespected(): void { /** @Given a condition that becomes ready after some retries */ $callCount = 0; - $condition = $this->createMock(ContainerReady::class); + $condition = $this->createStub(ContainerReady::class); $condition->method('isReady')->willReturnCallback(function () use (&$callCount): bool { $callCount++; return $callCount >= 3; @@ -88,4 +88,62 @@ public function testCustomPollIntervalIsRespected(): void self::assertLessThan(maximum: 1.0, actual: $elapsed); self::assertSame(expected: 3, actual: $callCount); } + + public function testWaitBeforeGuaranteesAtLeastOneReadinessCheckEvenWhenTimeoutIsZero(): void + { + /** @Given a condition that is immediately ready */ + $condition = $this->createStub(ContainerReady::class); + $condition->method('isReady')->willReturn(true); + + /** @When waiting with a zero-second timeout against a ready condition */ + ContainerWaitForDependency::untilReady( + condition: $condition, + timeoutInSeconds: 0, + pollIntervalInMicroseconds: 1 + )->waitBefore(); + + /** @Then the wait should complete without throwing */ + self::assertTrue(true); + } + + public function testWaitBeforeThrowsWhenSingleAttemptFailsAndConditionIsReadyOnSecondCheck(): void + { + /** @Given a condition that becomes ready only on the second check */ + $callCount = 0; + $condition = $this->createStub(ContainerReady::class); + $condition->method('isReady')->willReturnCallback(function () use (&$callCount): bool { + $callCount++; + return $callCount >= 2; + }); + + /** @Then a ContainerWaitTimeout should be thrown because a single attempt is exhausted */ + $this->expectException(ContainerWaitTimeout::class); + + /** @When waiting with a budget that allows exactly one readiness check */ + ContainerWaitForDependency::untilReady( + condition: $condition, + timeoutInSeconds: 0, + pollIntervalInMicroseconds: 1 + )->waitBefore(); + } + + public function testWaitBeforeSleepsBetweenReadinessChecks(): void + { + /** @Given a condition that only becomes ready after the configured poll interval elapses */ + $start = microtime(true); + $condition = $this->createStub(ContainerReady::class); + $condition->method('isReady')->willReturnCallback(static function () use ($start): bool { + return microtime(true) - $start >= 0.2; + }); + + /** @When waiting with a timeout that would expire instantly if sleeps were skipped */ + ContainerWaitForDependency::untilReady( + condition: $condition, + timeoutInSeconds: 2, + pollIntervalInMicroseconds: 100_000 + )->waitBefore(); + + /** @Then the wait should have taken at least the poll interval to observe readiness */ + self::assertGreaterThanOrEqual(minimum: 0.2, actual: microtime(true) - $start); + } } diff --git a/tests/Unit/Waits/ContainerWaitForTimeTest.php b/tests/Unit/Waits/ContainerWaitForTimeTest.php index a5f1727..299cf75 100644 --- a/tests/Unit/Waits/ContainerWaitForTimeTest.php +++ b/tests/Unit/Waits/ContainerWaitForTimeTest.php @@ -29,8 +29,8 @@ public function testWaitAfterPausesForSpecifiedDuration(): void /** @Given a wait-for-time of 1 second */ $wait = ContainerWaitForTime::forSeconds(seconds: 1); - /** @And a mock container started */ - $containerStarted = $this->createMock(ContainerStarted::class); + /** @And a container started stub */ + $containerStarted = $this->createStub(ContainerStarted::class); /** @When waiting after */ $start = microtime(true); diff --git a/tests/bootstrap.php b/tests/bootstrap.php deleted file mode 100644 index 6408be9..0000000 --- a/tests/bootstrap.php +++ /dev/null @@ -1,9 +0,0 @@ - Date: Tue, 21 Apr 2026 18:27:51 -0300 Subject: [PATCH 2/2] fix: Prevent shell injection and RCE risk in CI pipelines by passing commands as argument arrays to Symfony Process instead of shell strings. --- .gitattributes | 42 +++++++++++++++--------------------------- .gitignore | 23 ++++++++++++++++++----- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/.gitattributes b/.gitattributes index 28337dc..d641a25 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,30 +1,18 @@ -# Auto detect text files and perform LF normalization * text=auto eol=lf -# ─── Diff drivers ──────────────────────────────────────────── -*.php diff=php -*.md diff=markdown +*.php text diff=php -# ─── Force LF ──────────────────────────────────────────────── -*.sh text eol=lf -Makefile text eol=lf - -# ─── Generated (skip diff and GitHub stats) ────────────────── -composer.lock -diff linguist-generated - -# ─── Export ignore (excluded from dist archive) ────────────── -/tests export-ignore -/vendor export-ignore -/rules export-ignore - -/.github export-ignore -/.gitignore export-ignore -/.gitattributes export-ignore - -/CLAUDE.md export-ignore -/LICENSE export-ignore -/Makefile export-ignore -/README.md export-ignore -/phpunit.xml export-ignore -/phpstan.neon.dist export-ignore -/infection.json.dist export-ignore \ No newline at end of file +# Dev-only — excluded from the Packagist tarball +/.github export-ignore +/tests export-ignore +/.claude export-ignore +/.editorconfig export-ignore +/.gitattributes export-ignore +/.gitignore export-ignore +/phpunit.xml.dist export-ignore +/phpstan.neon.dist export-ignore +/phpcs.xml.dist export-ignore +/infection.json export-ignore +/Makefile export-ignore +/CONTRIBUTING.md export-ignore +/CHANGES.md export-ignore diff --git a/.gitignore b/.gitignore index 85fc064..bd5baa3 100644 --- a/.gitignore +++ b/.gitignore @@ -1,7 +1,20 @@ -.idea +# Agent/IDE +.claude/ +.idea/ +.vscode/ +.cursor/ -vendor -report +# Composer +/vendor/ +composer.lock -*.lock -.phpunit.* +# PHPUnit / coverage +.phpunit.cache/ +.phpunit.result.cache +report/ +coverage/ +build/ + +# OS +.DS_Store +Thumbs.db