diff --git a/AGENTS.md b/AGENTS.md index 300be0f..59179db 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,7 +93,7 @@ Releases follow SemVer on this contract from the first tag: a break of it ships - No package ships free functions or a Composer `files` entry. A stateless function family is a final class with public static entry points, protected static helpers and a protected constructor. An operation that carries state across its steps is an object, and constant data is a constant. On a final class, a protected helper with a single call site is inlined, unless it is a `match` target, a hook callback or an override, or carries a `@phpstan-assert*` tag. Logic several classes share has one implementation: when its callers are in one package, an `@internal` final collaborator or an `@internal` public static on the class that owns the concept, and when they span packages, the smallest public API in the lowest package that needs it. A protected copy needs the owner's ruling. A lone call to a WordPress, WooCommerce or PHP API is not a copy, and neither is an idiom WPCS needs to see at each call site; the framework knowledge wrapped around such a call is. A copy also stays where no single home loads for every caller, as between PHP 7.4 by-path code and autoloaded code. The by-path classes register hooks with static closures, which bind each registration without static state, and a failed boot or boot check hooks its notice as a static closure; every other hook callback is a public named method. A framework method takes a callable only for consumer code it runs inside its own envelope, as the boot composition, migration steps and the uninstall cleanup, or for per-field logic in field arrays, and spells it `callable`; a helper's definition comes from a definition object, and an envelope inside the framework is written inline, with the rule it applies in one home. - A name imported from `Psr\` takes a `Psr` alias, as in `use Psr\Log\LoggerInterface as PsrLoggerInterface;`, so every use shows that the type is a PSR interface or class. Only PHP's own functions and constants take a leading `\`, which lets opcache compile the functions it special-cases; WordPress, WooCommerce and Action Scheduler functions and constants are unqualified, and classes are imported or fully qualified. - Every element outside `tests/` has a docblock: one third-person sentence naming the role, such as "Returns …", plus a second paragraph only for a non-obvious why. Property and constant docblocks open with "The …", constructors read "Constructor.", and an override reads `{@inheritDoc}`. Every other function and method docblock ends with `@return`, `@return void` included. Tag values align at column 10, a file docblock's `@package` sits in its own group after `@since` and `@version`, a function or method that declares a `global` documents it with `@global` in its own group between `@version` and `@param`, each `@param` stays on one line, `@throws` reads "Thrown when …", `@since` names the release that added the element, and `@version` the last release that changed its behavior or signature. -- A hook name is `{prefix}/area/name`, with `/` between levels and `_` between words, and its last level says what kind of hook it is, because WordPress tells actions from filters by no name form: a past participle for an event (`{prefix}/boot/succeeded`), `before_` or `after_` for a position, an imperative for a scheduled or command hook (`{option}/migration/run`), a predicate (`is_`, `has_`, `can_`, `should_`) for a boolean filter (`{prefix}/admin_notice/should_show`), and the noun of the value for any other filter, plural for a list. A hook built from a consumer's id is `{id}/{surface}/{leaf}` (`{id}/object_form/fields`), and any other dynamic part goes last (`dws_github_updater/config/{slug}`). +- A hook name is `{prefix}/area/name`, with `/` between levels and `_` between words, and its last level says what kind of hook it is, because WordPress tells actions from filters by no name form: a past participle for an event (`{prefix}/boot/succeeded`), `before_` or `after_` for a position, an imperative for a scheduled or command hook (`run`), a predicate (`is_`, `has_`, `can_`, `should_`) for a boolean filter (`{prefix}/admin_notice/should_show`), and the noun of the value for any other filter, plural for a list. A hook built from an id or option name the consumer passes takes it whole as its first level, `{id}/{surface}/{leaf}` (`{id}/object_form/fields`, `{option}/migration/run`), because the id also names a nonce, an option or a DOM id, and any other dynamic part goes last (`dws_github_updater/config/{slug}`). - Every `do_action()` and `apply_filters()` with a public hook name carries a WordPress hook docblock directly above the call: "Fires …" or "Filters …", `@since`, `@version` and one `@param` per argument, which PHPStan checks against the call's arguments. - Classes and traits outside `tests/` group members in `// region NAME` blocks named, in order, TRAITS, FIELDS AND CONSTANTS, MAGIC METHODS, GETTERS, SETTERS, INHERITED METHODS, METHODS, FACTORY METHODS, HOOKS and HELPERS; exception classes, interfaces and enums carry none. - Inline comments state a non-obvious why in one capitalized sentence, and every file describes the present, never the change. A `phpcs:ignore` trails its line, names the sniff and gives a one-clause reason; a `@phpstan-ignore` names its identifier. diff --git a/packages/bootstrap/src/Updater.php b/packages/bootstrap/src/Updater.php index e448238..26f34f3 100644 --- a/packages/bootstrap/src/Updater.php +++ b/packages/bootstrap/src/Updater.php @@ -138,7 +138,7 @@ protected static function get_updater_config( string $plugin_file ): ?array { ); /** - * Filters the GitHub updater's configuration for the plugin whose text domain ends the hook name; a key the filter leaves out keeps its default. + * Filters the GitHub updater's configuration for the plugin whose text domain ends the hook name. A key the filter leaves out keeps its default. * * @since 2.0.0 * @version 2.0.0 diff --git a/packages/core/src/Migration/MigrationRunner.php b/packages/core/src/Migration/MigrationRunner.php index 152d5b7..6da1fad 100644 --- a/packages/core/src/Migration/MigrationRunner.php +++ b/packages/core/src/Migration/MigrationRunner.php @@ -62,6 +62,16 @@ final class MigrationRunner implements ComponentInterface { */ protected Lock $lock; + /** + * The hook that runs the steps in the background, built once so that an option too long to name it fails at construction. + * + * @since 2.0.0 + * @version 2.0.0 + * + * @var ScheduledHook + */ + protected ScheduledHook $scheduled_hook; + /** * The token that names this runner's hold on the lock. * @@ -100,7 +110,7 @@ final class MigrationRunner implements ComponentInterface { * @param int $batch_time_limit Optional. The seconds, from 1, that a request spends on steps and batches before it pauses. Default is 20. * @param PsrClockInterface|null $clock Optional. The clock. Default is the system clock. * - * @throws InvalidArgumentException Thrown when the option name cannot name the lock. + * @throws InvalidArgumentException Thrown when the option name cannot name the lock or the scheduled hook. */ public function __construct( protected string $plugin_file, @@ -111,10 +121,11 @@ public function __construct( protected int $batch_time_limit = 20, ?PsrClockInterface $clock = null ) { - $this->clock = $clock ?? new SystemClock(); - $this->lock = new Lock( "{$option}_lock", self::LOCK_TTL, $this->clock ); - $this->owner = wp_generate_uuid4(); - $this->logger = new GuardedLogger( $logger, $option ); + $this->clock = $clock ?? new SystemClock(); + $this->lock = new Lock( "{$option}_lock", self::LOCK_TTL, $this->clock ); + $this->scheduled_hook = new ScheduledHook( $this->get_hook() ); + $this->owner = wp_generate_uuid4(); + $this->logger = new GuardedLogger( $logger, $option ); } // endregion @@ -442,8 +453,7 @@ protected function unlock(): void { * @return void */ protected function schedule_continuation(): void { - $hook = new ScheduledHook( $this->get_hook() ); - $scheduled = doing_action( $this->get_hook() ) ? $this->scheduler->schedule_single( $hook, \time() + 1 ) : $this->scheduler->enqueue( $hook ); + $scheduled = doing_action( $this->get_hook() ) ? $this->scheduler->schedule_single( $this->scheduled_hook, \time() + 1 ) : $this->scheduler->enqueue( $this->scheduled_hook ); if ( true !== $scheduled ) { $this->logger->warning( 'Could not schedule the next data migration run: {error}', diff --git a/tests/Integration/ConsumerA/ConsumerABootTest.php b/tests/Integration/ConsumerA/ConsumerABootTest.php index b54a932..b2bf6fc 100644 --- a/tests/Integration/ConsumerA/ConsumerABootTest.php +++ b/tests/Integration/ConsumerA/ConsumerABootTest.php @@ -22,7 +22,7 @@ public function test_the_plugin_boots_with_its_scheduler_and_admin_notices_as_ke self::assertNull( $request['died'] ); self::assertStringContainsString( 'id="wpfooter"', $request['body'] ); self::assertSame( 'booted', $request['records']['consumer_a']['status'] ); - self::assertSame( 1, $request['records']['consumer_a']['initialized'] ); + self::assertSame( 1, $request['records']['consumer_a']['boot_succeeded'] ); self::assertSame( array( self::SCOPED . 'Core\\Scheduling\\Scheduler', self::SCOPED . 'Core\\AdminNotice\\AdminNotices', 'Acme\\ConsumerA\\ProductData', 'Acme\\ConsumerA\\Exports' ), $request['records']['consumer_a']['resolved'] ); self::assertTrue( $request['records']['export_hooked'] ); self::assertFalse( $request['records']['unloaded_include'] ); diff --git a/tests/Integration/ConsumerA/ConsumerAFailureTest.php b/tests/Integration/ConsumerA/ConsumerAFailureTest.php index 4ebd78c..48087bc 100644 --- a/tests/Integration/ConsumerA/ConsumerAFailureTest.php +++ b/tests/Integration/ConsumerA/ConsumerAFailureTest.php @@ -49,7 +49,7 @@ public function test_a_register_hooks_throw_after_a_relaxation_keeps_the_safegua self::assertStringContainsString( 'id="wpfooter"', $request['body'] ); self::assertStringContainsString( self::FAILURE_NOTICE, $request['body'] ); self::assertSame( 'failed', $request['records']['consumer_a']['status'] ); - self::assertSame( 0, $request['records']['consumer_a']['initialized'] ); + self::assertSame( 0, $request['records']['consumer_a']['boot_succeeded'] ); self::assertFalse( $request['records']['can_export'] ); } diff --git a/tests/Integration/Consumers/ConsumersCoexistenceTest.php b/tests/Integration/Consumers/ConsumersCoexistenceTest.php index 9e16ead..c4ab536 100644 --- a/tests/Integration/Consumers/ConsumersCoexistenceTest.php +++ b/tests/Integration/Consumers/ConsumersCoexistenceTest.php @@ -61,7 +61,7 @@ public function test_both_plugins_boot_with_their_own_components_in_either_load_ self::assertStringContainsString( 'id="wpfooter"', $request['body'] ); foreach ( self::ROOTS as $consumer => $roots ) { self::assertSame( 'booted', $request['records'][ $consumer ]['status'] ); - self::assertSame( 1, $request['records'][ $consumer ]['initialized'] ); + self::assertSame( 1, $request['records'][ $consumer ]['boot_succeeded'] ); self::assertSame( $roots, $request['records'][ $consumer ]['resolved'] ); } } diff --git a/tests/Integration/Core/Migration/MigrationRunnerTest.php b/tests/Integration/Core/Migration/MigrationRunnerTest.php index 7144a1b..c978bbe 100644 --- a/tests/Integration/Core/Migration/MigrationRunnerTest.php +++ b/tests/Integration/Core/Migration/MigrationRunnerTest.php @@ -7,6 +7,7 @@ use DeepWebSolutions\Framework\Core\Scheduling\Scheduler; use DeepWebSolutions\Framework\Core\Scheduling\SchedulerInterface; use DeepWebSolutions\Framework\Core\Storage\OptionRows; +use DeepWebSolutions\Framework\Shared\Exception\InvalidArgumentException; use DeepWebSolutions\Framework\Shared\Exception\RuntimeException; use DeepWebSolutions\Framework\Tests\Integration\Support\CreatesUsersTrait; use DeepWebSolutions\Framework\Tests\Integration\Support\IsolatesHooksTrait; @@ -116,6 +117,13 @@ public function test_steps_below_the_stored_revision_never_run(): void { self::assertSame( 3, MigrationRunner::get_revision( $this->option ) ); } + public function test_an_option_whose_scheduled_hook_would_pass_191_bytes_is_refused(): void { + new MigrationRunner( self::PLUGIN_FILE, \str_repeat( 'a', 177 ), array(), $this->logger, $this->make_wp_cron_scheduler() ); + + $this->expectException( InvalidArgumentException::class ); + new MigrationRunner( self::PLUGIN_FILE, \str_repeat( 'a', 178 ), array(), $this->logger, $this->make_wp_cron_scheduler() ); + } + public static function current_revisions(): array { return array( 'database-string' => array( true ), diff --git a/tests/Integration/Personas/CorePersonaTest.php b/tests/Integration/Personas/CorePersonaTest.php index a8084e1..fb4b991 100644 --- a/tests/Integration/Personas/CorePersonaTest.php +++ b/tests/Integration/Personas/CorePersonaTest.php @@ -22,7 +22,7 @@ public function test_the_plugin_boots_its_components_and_runs_its_migration_with self::assertFalse( $request['records']['woocommerce_present'] ); self::assertSame( 'booted', $request['records']['persona_core']['status'] ); self::assertSame( array( 'Acme\\PersonaCore\\Scoped\\DeepWebSolutions\\Framework\\Core\\AdminNotice\\AdminNotices', 'Acme\\PersonaCore\\Greeting' ), $request['records']['persona_core']['resolved'] ); - self::assertSame( 1, $request['records']['persona_core']['initialized'] ); + self::assertSame( 1, $request['records']['persona_core']['boot_succeeded'] ); self::assertSame( 'Acme\\PersonaCore\\Scoped\\DeepWebSolutions\\Framework\\Core\\Migration\\MigrationRunner', $request['records']['persona_core']['migrations'] ); self::assertTrue( $request['records']['greeting_shortcode'] ); self::assertSame( array( 'greeting' => 'Hello' ), get_option( 'acme_pc_settings' ) ); diff --git a/tests/Integration/Support/consumer-request.php b/tests/Integration/Support/consumer-request.php index f59963c..7325f7f 100644 --- a/tests/Integration/Support/consumer-request.php +++ b/tests/Integration/Support/consumer-request.php @@ -59,7 +59,7 @@ static function ( int $level, string $message, string $file, int $line ) use ( $ 'status' => $plugin?->boot->status, 'resolved' => $plugin?->kernel?->resolved, 'skipped' => $plugin?->kernel?->skipped, - 'initialized' => did_action( "{$prefix}/boot/succeeded" ), + 'boot_succeeded' => did_action( "{$prefix}/boot/succeeded" ), 'functions_loaded' => function_exists( "{$prefix}_plugin" ), 'migrations' => \is_object( $plugin ) && property_exists( $plugin, 'migrations' ) && \is_object( $plugin->migrations ) ? \get_class( $plugin->migrations ) : null, );