Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Task 001: Integration Test for Boot-Callback Dialect Override

**Status**: completed
**Depends on**: none
**Retry count**: 0

## Description
Add an integration test in `packages/database-pgsql/tests/` that proves a downstream module can override the 4 dialect-specific bindings (`SqlGeneratorInterface`, `IntrospectorInterface`, `QueryBuilderInterface`, `QueryBuilderFactoryInterface`) via a `boot` callback while inheriting `PgSqlConnection` for `ConnectionInterface`. This locks in the architectural guarantee that makes shipping postgres-wire-compatible variant packages (CockroachDB, YugabyteDB, etc.) viable without splitting `marko/database-pgsql`.

## Context
- **Why a new test, not just expanding `ModuleBindingsTest`:** `tests/Module/ModuleBindingsTest.php` only inspects the manifest array statically. This task needs an actual container roundtrip to prove the override resolves at runtime.
- **Existing pattern to mirror:** Look at `packages/core/tests/` (especially anything that constructs `Container` + `ModuleManifest` directly) for the lightest-weight way to build a 2-module scenario without invoking full module discovery. Avoid spinning up a real filesystem-based discovery — instantiate `ModuleManifest` objects in the test and register them via `BindingRegistry` + boot-callback invocation.
- **Fixture classes:** The variant dialect's replacement classes don't need to be real implementations — minimal stubs implementing the 4 dialect interfaces are enough. Place them under `packages/database-pgsql/tests/Fixtures/` (or wherever the existing test convention places fixtures — check `packages/queue-database/tests/Fixtures/` for precedent).
- **Two-module scenario:**
1. Module A = the real `marko/database-pgsql` manifest (load it via `require .../module.php` and wrap in a `ModuleManifest` value object).
2. Module B = a fixture variant module with no static `bindings`, only a `boot` closure that calls `$container->bind()` for the 4 dialect interfaces.
- **Ordering requirement (must mirror Application::boot):** The test setup MUST register all static bindings from BOTH modules through `BindingRegistry` first, THEN invoke any boot callbacks. Resolving any of the 4 dialect interfaces from the container BEFORE the variant boot runs would defeat the test's purpose. Mirror the order in `packages/core/src/Application.php` lines 151-185.
- **Singleton-cache regression guard:** The override pattern depends on the 4 dialect interfaces NOT being declared in any module's `singletons` key (because `Container::resolve()` caches singleton instances at lines 141-143 and 211-213, and a cached instance would survive a later `bind()` call). Add an assertion that the loaded `marko/database-pgsql` manifest has no `singletons` entry for any of the 4 dialect interfaces — this guards against a future change to pgsql's `module.php` silently breaking variant packages.
- **Loud-error guarantee:** The test must also confirm that if Module B used the static `bindings` key instead of a `boot` callback, `BindingConflictException` is thrown. Construct `BindingRegistry` directly with two `ModuleManifest` instances of the same `source` (both `vendor`) declaring the same interface in `bindings`, and assert the exception. This documents *why* boot is the right mechanism.

## Requirements (Test Descriptions)
- [ ] `it resolves SqlGeneratorInterface to the variant override after boot runs`
- [ ] `it resolves IntrospectorInterface to the variant override after boot runs`
- [ ] `it resolves QueryBuilderInterface to the variant override after boot runs`
- [ ] `it resolves QueryBuilderFactoryInterface to the variant override after boot runs`
- [ ] `it still resolves ConnectionInterface to PgSqlConnection because the variant did not rebind it`
- [ ] `it throws BindingConflictException when a variant declares the override via static bindings instead of boot`
- [ ] `it asserts the pgsql manifest declares no singletons for the 4 dialect interfaces (regression guard so override pattern stays safe)`

## Acceptance Criteria
- New test file lives at `packages/database-pgsql/tests/Module/DialectOverrideTest.php` (or `tests/Feature/` if that better matches existing convention — verify before placing).
- Fixture classes implementing the 4 dialect interfaces live in `packages/database-pgsql/tests/Fixtures/Variant/`.
- All 6 requirements have passing tests under `composer test`.
- `./vendor/bin/phpcs packages/database-pgsql/tests/` clean.
- `./vendor/bin/php-cs-fixer fix packages/database-pgsql/tests/ --dry-run --diff --config=.php-cs-fixer.php` clean.
- Test does not depend on real PostgreSQL being available — pure container resolution only.

## Implementation Notes
(Left blank — filled in by programmer during implementation)
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Task 002: DI Concept Doc — Cross-Module Binding Override Section

**Status**: completed
**Depends on**: none
**Retry count**: 0

## Description
Add a new section to `docs/src/content/docs/concepts/dependency-injection.md` titled "Overriding another module's bindings" that explains the boot-callback pattern as the supported way for one module (especially a vendor-installed sibling driver) to rebind an interface that another vendor module has already bound. This is a generic architectural concept; the database/dialect use case is one application of it.

## Context
- **Why concept doc and not a package doc:** Boot-callback rebinding applies to any sibling driver scenario — cache variants, queue variants, mail variants, etc. The concept belongs at the DI level so any reader hitting "how do I override a vendor module's binding" finds it.
- **Current state of the DI doc (verified):** `concepts/dependency-injection.md` does NOT currently mention `boot` callbacks at all. Its sections are: Constructor Injection, Bindings, Singletons, Resolution Order, Auto-Resolution Example, What Won't Auto-Resolve, Next Steps. The new section therefore must introduce `boot` enough to explain the override, OR cross-link to `packages/core.md` for the full boot mechanics. The existing boot+env-conditional pattern lives in `packages/core.md` under "Environment-Specific Bindings" (line 135) — reuse that vocabulary and reference it; do not re-duplicate.
- **Mechanism to explain (concise, no over-elaboration):**
- Static `bindings` go through `BindingRegistry` which throws `BindingConflictException` if two same-priority modules bind the same interface — this protects against accidental conflicts.
- `boot` callbacks run after all static bindings are registered and call `$container->bind()` directly, which **intentionally** allows rebinding. This is the right tool when one module *deliberately* overrides another.
- Load order: boot callbacks honor module `sequence` (after/before) and `require` (composer dependency), so a variant module that `require`s its parent automatically boots after it. Use `sequence.after` only when there is no `require` relationship.
- **Singleton caveat:** If an interface is also declared in the parent module's `singletons` key and is resolved before the variant's boot runs, the cached instance will be returned despite the rebind. The override pattern is safe for the 4 pgsql dialect interfaces because none of them are singletons. Mention this gotcha briefly so readers applying the pattern elsewhere don't trip on it.
- **Show, don't tell:** Include a small worked example using the database-pgsql variant case so readers see the shape. Keep it short — the database guide will have the full worked example; this is just illustrative.
- **Cross-link:** Add a short pointer at the end to `packages/database.md` for the database-specific application of this pattern.
- **Anchor (locked):** The H2 must be exactly `## Overriding another module's bindings`. github-slugger (used by Astro/Starlight) produces the anchor `overriding-another-modules-bindings`. Task 003 hardcodes this anchor — do not change the title.

## Requirements (Test Descriptions)
*Docs-only task — "tests" are content-quality assertions verified by review.*

- [ ] `the new section is titled "Overriding another module's bindings" (exact title — drives the anchor)`
- [ ] `the section explains why static bindings conflict and why boot does not`
- [ ] `the section includes a minimal code example using ContainerInterface and $container->bind()`
- [ ] `the section notes that module sequence (after/before) and composer require control override timing when multiple modules touch the same binding`
- [ ] `the section warns that singleton bindings cache on first resolve, so the override only works reliably if the parent's binding is not declared as a singleton (or is not resolved before the override boot runs)`
- [ ] `the section cross-links to packages/database.md for the database-specific example`

## Acceptance Criteria
- New section added to `docs/src/content/docs/concepts/dependency-injection.md`. Since the DI doc has no existing boot-callback section, the natural placement is after the "Singletons" section and before "Resolution Order", OR appended near the end before "Next Steps". The implementer should pick whichever flows better with the surrounding narrative.
- `npm --prefix docs run build` passes with no new warnings.
- Code example uses correct namespaces. Note: the project uses `Marko\Core\Container\ContainerInterface` for boot-callback injection (see `packages/core.md` line 145 — boot closures type-hint `Container`/`ContainerInterface` from Marko, not PSR directly). Match that.
- Tone matches surrounding doc voice (declarative, no marketing language).

## Implementation Notes
(Left blank — filled in by programmer during implementation)
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Task 003: Database Guide — Wire-Compatible Variants Section

**Status**: completed
**Depends on**: 002
**Retry count**: 0

## Description
Add a new section to `docs/src/content/docs/packages/database.md` titled "Wire-compatible database variants" that explains the 5-binding split (1 wire connection + 4 dialect interfaces) and walks through a worked CockroachDB example showing how a variant package depends on `marko/database-pgsql`, inherits `PgSqlConnection`, and overrides the 4 dialect interfaces via a `boot` callback in its own `module.php`.

## Context
- **Audience:** Someone who wants to add support for a postgres-wire-compatible (or mysql-wire-compatible) database. They land on `database.md` looking for the contract and the extension story.
- **The 5-binding split to document:**
- `ConnectionInterface` — wire protocol (PDO connection, PostgreSQL/MySQL DSN). Inherited from the parent driver.
- `SqlGeneratorInterface` — dialect (DDL generation for schema diffs).
- `IntrospectorInterface` — dialect (reading existing schema from `information_schema` etc.).
- `QueryBuilderInterface` — dialect (SELECT/INSERT/UPDATE/DELETE SQL generation).
- `QueryBuilderFactoryInterface` — dialect (constructs query builders).
- **Worked CockroachDB example** — a complete `composer.json` + `module.php` for a hypothetical `marko/database-cockroachdb`:
- `composer.json` requires `marko/database-pgsql` (which transitively requires `marko/database`).
- `module.php` has no static `bindings` for the 4 dialect interfaces; instead a `boot` closure rebinds them to CockroachDB implementations.
- One-line comment on why `ConnectionInterface` is not in the boot closure (PgSqlConnection reused because cockroach speaks the pg wire protocol).
- **Reference, don't duplicate:** Link to `/docs/concepts/dependency-injection/#overriding-another-modules-bindings` for the mechanism. This section's job is the *database-specific application*, not re-explaining DI. The anchor is produced by task 002's H2 "Overriding another module's bindings" via github-slugger and is locked.
- **Section anchor (locked):** The new H2 must be exactly `## Wire-compatible database variants`. github-slugger produces the anchor `wire-compatible-database-variants`. Task 004 hardcodes this anchor — do not change the title.
- **Do not invent a real CockroachDB driver:** The example shows the wiring shape only. Use stub class names like `CockroachDbGenerator` without claiming they exist.

## Requirements (Test Descriptions)
*Docs-only task — "tests" are content-quality assertions verified by review.*

- [ ] `the new section is titled "Wire-compatible database variants" (exact title — drives the anchor used by task 004)`
- [ ] `the section enumerates the 5 bindings and labels which is wire and which 4 are dialect`
- [ ] `the section includes a complete CockroachDB-style composer.json snippet requiring marko/database-pgsql`
- [ ] `the section includes a complete module.php snippet showing a boot closure rebinding the 4 dialect interfaces`
- [ ] `the section explains why ConnectionInterface is not rebound`
- [ ] `the section cross-links to /docs/concepts/dependency-injection/#overriding-another-modules-bindings for the underlying mechanism`

## Acceptance Criteria
- New section added to `docs/src/content/docs/packages/database.md` in a sensible position (likely after the existing driver overview).
- `npm --prefix docs run build` passes with no new warnings.
- All code examples are valid PHP and use correct namespaces from the actual `marko/database` package.
- No claim that any specific variant package exists or is officially supported.

## Implementation Notes
(Left blank — filled in by programmer during implementation)
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Task 004: Database-PgSql Page — Cross-Link to Variants Guide

**Status**: completed
**Depends on**: 003
**Retry count**: 0

## Description
Add a short subsection to `docs/src/content/docs/packages/database-pgsql.md` that points readers to the "Wire-compatible database variants" section of `packages/database.md`. This makes the variant story discoverable from the driver page without duplicating content.

## Context
- **Why this is its own task:** Depends on task 003 — the link target must exist before this can land.
- **Subsection scope:** 2–4 sentences total. Title something like "Postgres-wire-compatible databases (CockroachDB, YugabyteDB, etc.)". One-line explanation that `PgSqlConnection` speaks pure PDO over the postgres wire protocol so other postgres-wire databases can reuse it, then a link to the database guide for the full pattern.
- **Placement:** Near the bottom of the page, after primary install/usage content but before any troubleshooting/FAQ section.
- **Do not duplicate the worked example.** Just the link and one-line motivation.

## Requirements (Test Descriptions)
*Docs-only task — "tests" are content-quality assertions verified by review.*

- [ ] `the new subsection has a clear heading mentioning postgres-wire-compatible databases`
- [ ] `the subsection is short (2-4 sentences, no duplicated code examples)`
- [ ] `the subsection links to /docs/packages/database/#wire-compatible-database-variants (anchor locked by task 003)`
- [ ] `the subsection notes that PgSqlConnection is reusable because it is dialect-clean`

## Acceptance Criteria
- New subsection added to `docs/src/content/docs/packages/database-pgsql.md` in the appropriate position (after "Driver-Specific Notes" or before "API Reference" — match existing flow).
- The cross-link uses the exact anchor `/docs/packages/database/#wire-compatible-database-variants` (locked by task 003).
- `npm --prefix docs run build` passes with no new warnings or broken-link errors.
- No code examples duplicated from `database.md`.

## Implementation Notes
(Left blank — filled in by programmer during implementation)
Loading