Skip to content

Add support for php-mysql-replication v9.0.0 + fix table-specific event routing#21

Merged
huangdijia merged 1 commit intohuangdijia:mainfrom
cyppe:feature/support-php-mysql-replication-v9
Feb 12, 2026
Merged

Add support for php-mysql-replication v9.0.0 + fix table-specific event routing#21
huangdijia merged 1 commit intohuangdijia:mainfrom
cyppe:feature/support-php-mysql-replication-v9

Conversation

@cyppe
Copy link
Contributor

@cyppe cyppe commented Feb 9, 2026

Summary

  • Widens krowinski/php-mysql-replication constraint from ^8.0 to ^8.0 || ^9.0
  • Fixes a bug in Trigger::dispatch() where table-specific event routing never actually worked

The bug

The dispatch() method checks if the event has a table map using is_callable([$event, 'getTableMap']), then calls $event->getTableMap()->getDatabase() and ->getTable(). The problem is these getter methods don't exist in v8.x (or v9.0.0) -- both versions use public readonly properties on TableMap (->database, ->table).

Because is_callable silently returns false, the entire block is skipped. This means any route registered with a specific table (e.g. $trigger->on('mydb.users', 'write', ...)) never fires. Only wildcard routes (*.*.*) work.

Fix

Replaced the broken callable check with $event instanceof RowsDTO and access the properties directly:

// Before (broken)
if (is_callable([$event, 'getTableMap'])) {
    $database = $event->getTableMap()->getDatabase();
    $table = $event->getTableMap()->getTable();

// After (works on both v8.x and v9.0.0)
if ($event instanceof RowsDTO) {
    $database = $event->tableMap->database;
    $table = $event->tableMap->table;

Why v9.0.0 works without other changes

The public API surface consumed by laravel-trigger (EventDTO, RowsDTO, TableMap, ConfigBuilder, BinLogCurrent, EventSubscribers, etc.) is identical between v8.x and v9.0.0. No other files need changes.

Summary by CodeRabbit

  • Chores

    • Broadened dependency compatibility to support both v8.0 and v9.0 of the replication library.
  • Refactor

    • Improved event dispatch to access table metadata more directly for row events.
  • New Features

    • Start-up now listens for termination signals (SIGTERM/SIGINT) and exits immediately for quicker shutdown.

@coderabbitai
Copy link

coderabbitai bot commented Feb 9, 2026

📝 Walkthrough

Walkthrough

Updates dependency constraint for krowinski/php-mysql-replication and changes event handling and CLI start command: Trigger::dispatch now uses an instanceof RowsDTO and reads tableMap properties; StartCommand adds signal handlers (SIGTERM/SIGINT) and a typed return for handle().

Changes

Cohort / File(s) Summary
Dependency Update
composer.json
Broadened krowinski/php-mysql-replication version constraint from ^8.0 to `^8.0
Event Handling Logic
src/Trigger.php
Replaced runtime method checks with an instanceof RowsDTO; when event is RowsDTO, reads database and table from $event->tableMap properties instead of calling getter methods.
CLI / Signal Handling
src/Console/StartCommand.php
handle() now returns int (: int) and invokes new listenForSignals() which installs SIGTERM/SIGINT handlers (uses pcntl_async_signals if available) that log and exit(0) on signal.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through composer and code today,
I widened versions, nudged checks away.
Tables read from maps, signals now heard—
A tiny rabbit patch, swift as a word. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the two main changes: supporting php-mysql-replication v9.0.0 and fixing table-specific event routing in Trigger::dispatch().
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 PHPStan (2.1.38)

At least one path must be specified to analyse.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment on lines 130 to 135
pcntl_signal(SIGTERM, function () {
$this->info('Received SIGTERM, shutting down...');
$this->shouldTerminate = true;
});

pcntl_signal(SIGINT, function () {

This comment was marked as outdated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer relevant — the SIGTERM/SIGINT signal handling code was accidentally included in this branch from a separate PR (#22). It has now been removed from this branch.

The pcntl_signal guard concern you raised is valid and has been addressed in PR #22 where that code belongs. Thanks!

@cyppe cyppe force-pushed the feature/support-php-mysql-replication-v9 branch 2 times, most recently from 8ce5e5b to 6607b19 Compare February 12, 2026 06:47
@huangdijia huangdijia merged commit 486be0b into huangdijia:main Feb 12, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants