diff --git a/.coderabbit.yaml b/.coderabbit.yaml index 6305f1b4..c343b8fc 100644 --- a/.coderabbit.yaml +++ b/.coderabbit.yaml @@ -64,6 +64,54 @@ reviews: - Ensure domain-layer code invoked by the job (outside the DynamicListAttr exception) remains free of persistence calls. - Batch flush operations where practical. + - path: "src/Domain/Search/**" + instructions: &elasticsearch_consistency | + You are reviewing code for the MySQL/Elasticsearch dual-write system (write-through DB + + Elasticsearch via `SearchIndexDoctrineListener`, with `*ConfigurableReader`/`*HybridReader` + classes able to read from either backend, and a retention/purge command that deletes DB rows + once confirmed in Elasticsearch). Flag anything that can let the two stores drift apart: + + - ❌ Bulk/raw deletes or updates on a `SearchIndexableInterface` entity's table that bypass + Doctrine's entity lifecycle - DQL `->delete()`/`->update()` queries, or DBAL + `executeStatement()`/raw SQL - never trigger `SearchIndexDoctrineListener::preRemove/postRemove` + (those only fire for `EntityManager::remove()`/`persist()` + `flush()`). Any such bulk + operation silently skips the Elasticsearch side. This is legitimate ONLY when it is an + explicit, reviewed "delete from MySQL but deliberately keep the Elasticsearch document" + retention/purge path (and even then, it must verify the row exists in Elasticsearch via a + real ES query *before* deleting - never delete on the assumption that dual-write "should + have" succeeded). + - ❌ A `SearchIndexableInterface` entity with a foreign key that has a DB-level + `onDelete: 'CASCADE'` (`#[ORM\JoinColumn(..., onDelete: 'CASCADE')]`) but no matching + Doctrine-level `cascade: ['remove']` on the owning/parent side's association. Without the + Doctrine-level cascade, deleting the parent lets the database silently drop child rows that + Doctrine never loads/removes individually, so the listener never fires for them and their + Elasticsearch documents are orphaned. + - ❌ Calls to `ElasticsearchClientInterface::index()`/`delete()`, or construction of + `IndexDocumentMessage`, using a hardcoded, reused, or non-monotonic revision instead of the + established wall-clock-microseconds pattern (see `SearchIndexDoctrineListener::nextRevision()`). + External versioning is what stops a delayed/retried write or delete from clobbering newer + state - a wrong revision can let a stale message win. + - ❌ New/changed `*ConfigurableReader` implementations that don't correctly gate on + `elasticsearch.enabled` and fall back to the Doctrine repository, or that silently mix data + from both backends for the same logical query/response without that being the explicit intent. + - ⚠️ Any consumer that needs a *complete*/unbounded history or aggregate (not a bounded recent + window) reading directly from the Doctrine repository for an entity that has a retention/purge + policy configured elsewhere - it will silently see only the retained window once older rows + are purged. Point out that it should read through the ES-backed reader/interface instead. + - ⚠️ Missing or incomplete tests around the above: a repository method that deletes/updates rows + for a `SearchIndexableInterface` entity should have a test proving it does (or intentionally + does not) go through the dual-write path, and any new cascade relationship should have a test + that actually removes the parent and asserts the child is gone too. + + - path: "src/**/*Elasticsearch*.php" + instructions: *elasticsearch_consistency + + - path: "src/**/Service/Search/**" + instructions: *elasticsearch_consistency + + - path: "src/Core/Doctrine/SearchIndexDoctrineListener.php" + instructions: *elasticsearch_consistency + auto_review: enabled: true base_branches: diff --git a/.env.dist b/.env.dist index 03df0e91..e66a76e4 100644 --- a/.env.dist +++ b/.env.dist @@ -52,6 +52,20 @@ BOUNCE_IMAP_PURGE_UNPROCESSED=0 # Messenger configuration for asynchronous processing MESSENGER_TRANSPORT_DSN=doctrine://default?auto_setup=true +SEARCH_TRANSPORT_DSN=doctrine://default?queue_name=search_index + +# Elasticsearch configuration +# Set to false to fall back to reading/writing these tables straight from MySQL - no Elasticsearch +# cluster required. See docs/ElasticsearchSearch.md. +ELASTICSEARCH_ENABLED=false +ELASTICSEARCH_HOSTS=http://127.0.0.1:9200 +ELASTICSEARCH_USERNAME= +ELASTICSEARCH_PASSWORD= +ELASTICSEARCH_CONNECT_TIMEOUT=2 +ELASTICSEARCH_REQUEST_TIMEOUT=5 +# ISO-8601 duration (e.g. P1M) for how long SubscriberHistory rows are kept in MySQL after being +# confirmed in Elasticsearch. Empty disables purging. See docs/ElasticsearchSearch.md. +ELASTICSEARCH_PURGE_SUBSCRIBER_HISTORY_RETENTION= # A secret key that's used to generate certain security-related tokens PHPLIST_SECRET=%s diff --git a/.env.test b/.env.test new file mode 100644 index 00000000..88b6c338 --- /dev/null +++ b/.env.test @@ -0,0 +1,3 @@ +PHPLIST_DATABASE_DRIVER=pdo_sqlite +PHPLIST_DATABASE_PATH=:memory: +SEARCH_TRANSPORT_DSN=sync:// diff --git a/.env.test.local.dist b/.env.test.local.dist deleted file mode 100644 index c9992c34..00000000 --- a/.env.test.local.dist +++ /dev/null @@ -1,9 +0,0 @@ -# Optional: copy this file to ".env.test.local" to run tests against an in-memory SQLite -# database instead of MySQL, so no database server is needed for `vendor/bin/phpunit`. -# -# Note: this file is not loaded automatically by PHPUnit CLI runs (this project's ApplicationKernel -# does not read .env files on its own); either export these as real environment variables before -# running phpunit, or wire them up via your own bootstrap/CI step. - -PHPLIST_DATABASE_DRIVER=pdo_sqlite -PHPLIST_DATABASE_PATH=:memory: \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 263488c7..86596523 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,6 +19,17 @@ jobs: ports: - 3306/tcp options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3 + elasticsearch: + image: docker.elastic.co/elasticsearch/elasticsearch:8.15.0 + env: + discovery.type: single-node + xpack.security.enabled: false + ES_JAVA_OPTS: -Xms256m -Xmx256m + ports: + - 9200/tcp + options: >- + --health-cmd="curl -sf http://localhost:9200/_cluster/health || exit 1" + --health-interval=10s --health-timeout=5s --health-retries=5 strategy: fail-fast: false matrix: @@ -68,6 +79,10 @@ jobs: php bin/console doctrine:schema:validate --skip-sync - name: Run units tests with phpunit run: vendor/bin/phpunit tests/Unit/ --testdox + - name: Initialize Elasticsearch indices + run: | + export ELASTICSEARCH_HOSTS=http://127.0.0.1:${{ job.services.elasticsearch.ports['9200'] }} + php bin/console phplist:search:init-indices - name: Run integration tests with phpunit run: | export PHPLIST_DATABASE_NAME=${{ env.DB_DATABASE }} @@ -75,6 +90,7 @@ jobs: export PHPLIST_DATABASE_PASSWORD=${{ env.DB_PASSWORD }} export PHPLIST_DATABASE_PORT=${{ job.services.mysql.ports['3306'] }} export PHPLIST_DATABASE_HOST=127.0.0.1 + export ELASTICSEARCH_HOSTS=http://127.0.0.1:${{ job.services.elasticsearch.ports['9200'] }} vendor/bin/phpunit tests/Integration/ - name: Running the system tests run: vendor/bin/phpunit tests/System/ --testdox; diff --git a/README.md b/README.md index 4b929df2..1f6a71cd 100755 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ this code. * [Mailer transports](docs/MailerTransports.md) - configuring Gmail, Amazon SES, Mailchimp, and SendGrid * [Asynchronous email sending](docs/AsyncEmailSending.md) - queuing email delivery with Symfony Messenger * [Graylog integration](docs/Graylog.md) - centralized log management +* [Elasticsearch-backed search for big tables](docs/ElasticsearchSearch.md) - dual-write to the database and Elasticsearch, reading from Elasticsearch only * [Generating class API docs](PHPDOC.md) - regenerating the phpDocumentor output ## Running the web server @@ -201,6 +202,9 @@ To extract translation strings from the source into an XLIFF catalog: ```bash php bin/console translation:extract --force en --format=xlf +php bin/console messenger:setup-transports +php bin/console messenger:consume async --limit=1 +php bin/console phplist:search:init-indices ``` ## Copyright diff --git a/composer.json b/composer.json index d0371f58..5c6f9e05 100644 --- a/composer.json +++ b/composer.json @@ -89,7 +89,8 @@ "phpdocumentor/reflection-docblock": "^5.2", "guzzlehttp/guzzle": "^7.4.5", "symfony/dotenv": "^6.4", - "symfony/doctrine-messenger": "^6.4" + "symfony/doctrine-messenger": "^6.4", + "elasticsearch/elasticsearch": "^8.9" }, "require-dev": { "phpunit/phpunit": "^9.5", diff --git a/config/packages/messenger.yaml b/config/packages/messenger.yaml index 93022618..4896be58 100644 --- a/config/packages/messenger.yaml +++ b/config/packages/messenger.yaml @@ -14,13 +14,30 @@ framework: check_delayed_interval: 60000 retry_strategy: max_retries: 3 - # milliseconds delay + # millisecond delay delay: 1000 multiplier: 2 max_delay: 0 failed: 'doctrine://default?queue_name=failed' + # Dedicated queue (same Doctrine connection/table, distinct queue_name) for Elasticsearch + # dual-write. auto_setup is deliberately disabled: the messenger_messages table is already + # created by the async_email transport, and no DDL must ever run while a Doctrine ORM + # transaction is open (see SearchIndexDoctrineListener, which dispatches from postFlush). + # On a fresh install that enables search before ever sending email, run + # `bin/console messenger:setup-transports` once. + # Configurable so tests can swap in 'sync://' (see .env.test) and index synchronously. + async_search: + dsn: '%env(SEARCH_TRANSPORT_DSN)%' + options: + auto_setup: false + retry_strategy: + max_retries: 5 + delay: 1000 + multiplier: 2 + max_delay: 30000 + routing: # Route your messages to the transports 'PhpList\Core\Domain\Messaging\Message\AsyncEmailMessage': async_email @@ -30,4 +47,5 @@ framework: 'PhpList\Core\Domain\Messaging\Message\CampaignProcessor\CampaignProcessorMessage': async_email 'PhpList\Core\Domain\Messaging\Message\CampaignProcessor\SyncCampaignProcessorMessage': sync 'PhpList\Core\Domain\Subscription\Message\DynamicTableMessage': sync + 'PhpList\Core\Domain\Search\Message\IndexDocumentMessage': async_search diff --git a/config/parameters.yml b/config/parameters.yml index aecc30ec..1edd106d 100644 --- a/config/parameters.yml +++ b/config/parameters.yml @@ -50,6 +50,17 @@ parameters: # Messenger configuration for asynchronous processing app.messenger_transport_dsn: '%env(MESSENGER_TRANSPORT_DSN)%' + app.search_transport_dsn: '%env(SEARCH_TRANSPORT_DSN)%' + + # Elasticsearch configuration + elasticsearch.enabled: '%env(bool:ELASTICSEARCH_ENABLED)%' + elasticsearch.hosts: '%env(csv:ELASTICSEARCH_HOSTS)%' + elasticsearch.username: '%env(ELASTICSEARCH_USERNAME)%' + elasticsearch.password: '%env(ELASTICSEARCH_PASSWORD)%' + elasticsearch.index_prefix: '%env(DATABASE_PREFIX)%' + elasticsearch.connect_timeout: '%env(int:ELASTICSEARCH_CONNECT_TIMEOUT)%' + elasticsearch.request_timeout: '%env(int:ELASTICSEARCH_REQUEST_TIMEOUT)%' + elasticsearch.purge.subscriber_history_retention: '%env(ELASTICSEARCH_PURGE_SUBSCRIBER_HISTORY_RETENTION)%' # A secret key that's used to generate certain security-related tokens secret: '%env(PHPLIST_SECRET)%' diff --git a/config/services.yml b/config/services.yml index 31a2b6f1..768a4ca4 100644 --- a/config/services.yml +++ b/config/services.yml @@ -55,6 +55,10 @@ services: arguments: $tablePrefix: '%database_prefix%' + PhpList\Core\Core\Doctrine\SearchIndexDoctrineListener: + arguments: + $enabled: '%elasticsearch.enabled%' + HTMLPurifier_Config: class: HTMLPurifier_Config factory: [ 'HTMLPurifier_Config', 'createDefault' ] diff --git a/config/services/commands.yml b/config/services/commands.yml index d9305748..32b431f6 100644 --- a/config/services/commands.yml +++ b/config/services/commands.yml @@ -12,6 +12,10 @@ services: resource: '../../src/Domain/Identity/Command' tags: ['console.command'] + PhpList\Core\Domain\Search\Command\: + resource: '../../src/Domain/Search/Command' + tags: ['console.command'] + PhpList\Core\Domain\Messaging\Command\ProcessBouncesCommand: arguments: $protocolProcessors: !tagged_iterator 'phplist.bounce_protocol_processor' diff --git a/config/services/elasticsearch.yml b/config/services/elasticsearch.yml new file mode 100644 index 00000000..e6da2c57 --- /dev/null +++ b/config/services/elasticsearch.yml @@ -0,0 +1,69 @@ +services: + _defaults: + autowire: true + autoconfigure: true + public: false + + _instanceof: + PhpList\Core\Domain\Search\Model\Interfaces\SearchIndexDefinitionInterface: + tags: ['phplist.search_index_definition'] + PhpList\Core\Domain\Search\Model\Interfaces\SearchReindexProviderInterface: + tags: ['phplist.search_reindex_provider'] + PhpList\Core\Domain\Search\Model\Interfaces\SearchPurgeProviderInterface: + tags: ['phplist.search_purge_provider'] + + Elastic\Elasticsearch\Client: + factory: ['PhpList\Core\Domain\Search\Client\ElasticsearchClientFactory', 'create'] + arguments: + $hosts: '%elasticsearch.hosts%' + $username: '%elasticsearch.username%' + $password: '%elasticsearch.password%' + $connectTimeout: '%elasticsearch.connect_timeout%' + $requestTimeout: '%elasticsearch.request_timeout%' + + PhpList\Core\Domain\Search\Client\ElasticsearchClientInterface: + alias: PhpList\Core\Domain\Search\Client\ElasticsearchClientAdapter + + PhpList\Core\Domain\Search\Client\ElasticsearchClientAdapter: ~ + + PhpList\Core\Domain\Search\Service\ElasticsearchIndexerInterface: + alias: PhpList\Core\Domain\Search\Service\ElasticsearchIndexer + + PhpList\Core\Domain\Search\Service\ElasticsearchIndexer: + arguments: + $indexPrefix: '%elasticsearch.index_prefix%' + + PhpList\Core\Domain\Search\Registry\SearchIndexDefinitionRegistry: + arguments: + $definitions: !tagged_iterator 'phplist.search_index_definition' + + PhpList\Core\Domain\Search\Registry\SearchReindexProviderRegistry: + arguments: + $providers: !tagged_iterator 'phplist.search_reindex_provider' + + PhpList\Core\Domain\Search\Registry\SearchPurgeProviderRegistry: + arguments: + $providers: !tagged_iterator 'phplist.search_purge_provider' + + PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryElasticsearchReader: + arguments: + $indexPrefix: '%elasticsearch.index_prefix%' + + PhpList\Core\Domain\Subscription\Service\Search\: + resource: '../../src/Domain/Subscription/Service/Search' + + PhpList\Core\Domain\Subscription\Service\Search\SubscriberHistoryPurgeProvider: + arguments: + $indexPrefix: '%elasticsearch.index_prefix%' + $retentionPeriod: '%elasticsearch.purge.subscriber_history_retention%' + + PhpList\Core\Domain\Messaging\Repository\UserMessageBounceElasticsearchReader: + arguments: + $indexPrefix: '%elasticsearch.index_prefix%' + + PhpList\Core\Domain\Messaging\Repository\UserMessageBounceElasticsearchHybridReader: + arguments: + $indexPrefix: '%elasticsearch.index_prefix%' + + PhpList\Core\Domain\Messaging\Service\Search\: + resource: '../../src/Domain/Messaging/Service/Search' diff --git a/config/services/messenger.yml b/config/services/messenger.yml index 38130f6e..16e02ff4 100644 --- a/config/services/messenger.yml +++ b/config/services/messenger.yml @@ -11,6 +11,12 @@ services: resource: '../../src/Domain/Subscription/MessageHandler' tags: [ 'messenger.message_handler' ] + # Register Search message handlers (e.g., IndexDocumentMessageHandler) + PhpList\Core\Domain\Search\MessageHandler\: + autowire: true + resource: '../../src/Domain/Search/MessageHandler' + tags: [ 'messenger.message_handler' ] + PhpList\Core\Domain\Messaging\MessageHandler\CampaignProcessor\CampaignProcessorMessageHandler: autowire: true autoconfigure: true diff --git a/config/services/repositories.yml b/config/services/repositories.yml index a0650b35..5ee7eb40 100644 --- a/config/services/repositories.yml +++ b/config/services/repositories.yml @@ -83,6 +83,16 @@ services: parent: PhpList\Core\Domain\Common\Repository\AbstractRepository arguments: - PhpList\Core\Domain\Subscription\Model\SubscriberHistory + + # Reads for SubscriberHistoryManager/SubscriberManager go through this configurable reader, which + # picks Elasticsearch or the database based on elasticsearch.enabled (ELASTICSEARCH_ENABLED) - see + # SubscriberHistoryConfigurableReader and docs/ElasticsearchSearch.md. + PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryConfigurableReader: + autowire: true + arguments: + $elasticsearchEnabled: '%elasticsearch.enabled%' + PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface: + alias: PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryConfigurableReader PhpList\Core\Domain\Subscription\Repository\UserBlacklistRepository: parent: PhpList\Core\Domain\Common\Repository\AbstractRepository arguments: @@ -117,6 +127,25 @@ services: parent: PhpList\Core\Domain\Common\Repository\AbstractRepository arguments: - PhpList\Core\Domain\Messaging\Model\UserMessageBounce + + # Reads for UserMessageBounceReaderInterface consumers go through this configurable reader, which + # picks Elasticsearch or the database based on elasticsearch.enabled (ELASTICSEARCH_ENABLED) - see + # UserMessageBounceConfigurableReader and docs/ElasticsearchSearch.md. + PhpList\Core\Domain\Messaging\Repository\UserMessageBounceConfigurableReader: + autowire: true + arguments: + $elasticsearchEnabled: '%elasticsearch.enabled%' + PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReaderInterface: + alias: PhpList\Core\Domain\Messaging\Repository\UserMessageBounceConfigurableReader + + # Same configurable swap for the reporting queries (getListBounceTotals/getCampaignBounceTotals) + # that join bounce data against Subscriber/Message. + PhpList\Core\Domain\Messaging\Repository\UserMessageBounceReportConfigurableReader: + autowire: true + arguments: + $elasticsearchEnabled: '%elasticsearch.enabled%' + PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReportReaderInterface: + alias: PhpList\Core\Domain\Messaging\Repository\UserMessageBounceReportConfigurableReader PhpList\Core\Domain\Messaging\Repository\UserMessageForwardRepository: parent: PhpList\Core\Domain\Common\Repository\AbstractRepository arguments: diff --git a/docs/ElasticsearchSearch.md b/docs/ElasticsearchSearch.md new file mode 100644 index 00000000..97f9af81 --- /dev/null +++ b/docs/ElasticsearchSearch.md @@ -0,0 +1,158 @@ +# Elasticsearch-backed Search for Big Tables + +This document explains the generic Elasticsearch dual-write/read infrastructure and its consumers: +`SubscriberHistory` (table `phplist_user_user_history`) and `UserMessageBounce` +(table `phplist_user_message_bounce`). + +## Overview + +Some tables grow too large for comfortable ad-hoc filtering/pagination straight off MySQL. For those +tables, phpList Core writes to the database **and** to Elasticsearch, but reads **only** from +Elasticsearch: + +- **Writes** stay exactly as they are today (Doctrine `persist()`/`remove()`). A generic Doctrine + event listener (`PhpList\Core\Core\Doctrine\SearchIndexDoctrineListener`) detects any entity that + implements `SearchIndexableInterface` and asynchronously dispatches an indexing/deletion message via + Symfony Messenger from Doctrine's `postFlush` event, i.e. after the ORM flush - not necessarily after + a real commit. Callers that wrap `flush()` in their own explicit transaction (e.g. + `SubscriberCsvImporter`) cause `postFlush` to fire before that outer transaction actually commits; see + "Consistency model" below for why this is still safe. +- **Reads** for those entities go through a dedicated reader interface (e.g. + `SubscriberHistoryReaderInterface`) that is aliased in DI to an Elasticsearch-backed implementation + instead of the Doctrine repository. + +This is deliberately generic: adding the next big table only requires implementing three small +interfaces (see "Adding a new searchable entity" below) - no changes to the dual-write plumbing. + +## Consistency model + +- Dual-write is **asynchronous**. Between a row being committed to MySQL and the `async_search` worker + processing its queued message, a read from Elasticsearch will not yet reflect that row. +- Reads **hard-fail** if Elasticsearch is unreachable - there is no fallback to the database. Any + Elasticsearch error is raised as `PhpList\Core\Domain\Search\Exception\SearchBackendUnavailableException`. +- Dispatch happens from `postFlush`, not `postPersist`/`postUpdate`/`postRemove` (those fire while the + flush is still in progress), so a row that ends up rolled back is never indexed. For a plain + `flush()` call with no surrounding transaction, `postFlush`'s own commit is the real commit, so + dispatch does happen after the row is durably committed. When a caller instead wraps `flush()` in + its own explicit transaction (e.g. `SubscriberCsvImporter`), `postFlush` fires before that outer + transaction commits - but the `async_search` transport is a Doctrine queue on the same connection, so + the queued message insert shares that same outer transaction: the row and its message still commit + or roll back together. The remaining crash window is narrower than "commit vs. dispatch" - it's + strictly between the outer transaction's real commit and the `async_search` worker consuming the + message; if a process dies in that window, that one row is missed until the next + `phplist:search:reindex` run. + +Consumers of `phplist/core` that build UI on top of these read paths should plan for both of the above +(e.g. a brief "just added" staleness window, and handling a 5xx-equivalent from a search-unavailable +condition) rather than assuming synchronous consistency with the database. + +## Configuration + +Set in `.env` (see `.env.dist`): + +```dotenv +ELASTICSEARCH_HOSTS=http://127.0.0.1:9200 +ELASTICSEARCH_USERNAME= +ELASTICSEARCH_PASSWORD= +ELASTICSEARCH_CONNECT_TIMEOUT=2 +ELASTICSEARCH_REQUEST_TIMEOUT=5 +``` + +`ELASTICSEARCH_HOSTS` accepts a comma-separated list for multi-node clusters. The index prefix is +applied to every logical index alias (e.g. alias `subscriber_history` becomes index +`phplist_subscriber_history` with the default prefix), the same convention as `DATABASE_PREFIX` for +MySQL tables. + +### Making Elasticsearch optional + +Set `ELASTICSEARCH_ENABLED=false` to run without an Elasticsearch cluster at all - no code changes, +no cluster required, and it's safe even if `ELASTICSEARCH_HOSTS` is unreachable or unset: + +- **Writes**: `SearchIndexDoctrineListener` becomes a no-op - nothing is ever queued to the + `async_search` transport, so no `IndexDocumentMessage` accumulates unconsumed. +- **Reads**: every reader interface (`SubscriberHistoryReaderInterface`, + `UserMessageBounceReaderInterface`, `UserMessageBounceReportReaderInterface`) is aliased to a small + `*ConfigurableReader` that picks the Doctrine repository instead of the Elasticsearch reader - see + `SubscriberHistoryConfigurableReader`/`UserMessageBounceConfigurableReader`/ + `UserMessageBounceReportConfigurableReader`. + +The `elasticsearch/elasticsearch` PHP client package is still a hard Composer dependency of +`phplist/core` either way - disabling the feature at runtime doesn't remove the need to have that +package installed, only the need to have a reachable cluster. + +## Queueing + +Indexing/deletion messages are routed to a dedicated `async_search` Messenger transport +(`config/packages/messenger.yaml`) - the same Doctrine-backed queue table used by `async_email`, but a +distinct `queue_name` so the two workloads don't compete or block each other. Run a worker for it in +addition to the email worker: + +```bash +bin/console messenger:consume async_search +``` + +As with `async_email`, run this as a background service (e.g. via Supervisor) in production. +`auto_setup` is disabled for this transport: the `messenger_messages` table must already exist (it's +created lazily by `async_email` on first use). On a fresh install that enables search before ever +sending an email, run `bin/console messenger:setup-transports` once. + +## Console commands + +```bash +# Create or update Elasticsearch indices (mappings) for every registered searchable entity. +# Safe to re-run - never drops or recreates an existing index. +bin/console phplist:search:init-indices [--index=] + +# Backfill Elasticsearch from the database. Safe to re-run (indexing is an upsert by id). +bin/console phplist:search:reindex [] [--batch-size=500] [--last-id=0] + +# Delete DB rows older than a configured retention period, once confirmed to exist in +# Elasticsearch. Skips (and warns about, without deleting) any row not yet found in ES. +bin/console phplist:search:purge [] [--batch-size=500] [--dry-run] +``` + +Run `phplist:search:init-indices` once per environment before the first `phplist:search:reindex`, and +again after adding a new searchable entity or changing a mapping. + +### Purging old rows (`phplist:search:purge`) + +Only entities with an opted-in purge provider and a non-empty retention period are eligible. Today +that's `SubscriberHistory`, controlled by `ELASTICSEARCH_PURGE_SUBSCRIBER_HISTORY_RETENTION` (an +ISO-8601 duration, e.g. `P1M`; empty/unset disables purging for it). Adding another entity means +implementing `SearchPurgeProviderInterface` (mirroring `SubscriberHistoryPurgeProvider`) and adding its +own retention parameter - it's auto-tagged and picked up the same way reindex providers are. + +**Before scheduling this command in production**, make sure Elasticsearch snapshots/backups are +configured. Once a row is purged from MySQL, `phplist:search:reindex` can no longer recover it if the +ES index is ever lost - the verify-before-delete step only protects against rows that *haven't* made it +into ES yet, not against losing the ES index itself afterwards. This command is not currently wired +into any cron/Supervisor schedule; that should happen only after the snapshot policy is in place. + +## Adding a new searchable entity + +1. Implement `PhpList\Core\Domain\Search\Model\Interfaces\SearchIndexableInterface` on the entity + (`getSearchIndexName()`, `getSearchDocumentId()`, `toSearchDocument()`). This is what makes + `SearchIndexDoctrineListener` dual-write it automatically - no other write-path changes needed. +2. Add an index definition (`SearchIndexDefinitionInterface`: alias + mapping + settings) under the + entity's own `Service/Search` folder, following + `PhpList\Core\Domain\Subscription\Service\Search\SubscriberHistoryIndexDefinition` - it's + auto-tagged and picked up by `phplist:search:init-indices` via DI `_instanceof` autoconfiguration + in `config/services/elasticsearch.yml`. +3. Add a reindex provider (`SearchReindexProviderInterface`: alias + `countAll()` + `fetchBatch()`) + following `SubscriberHistoryReindexProvider` - likewise auto-tagged, picked up by + `phplist:search:reindex`. +4. If reads should also move to Elasticsearch, introduce a reader interface for that entity (mirroring + `SubscriberHistoryReaderInterface`) and an Elasticsearch-backed implementation (mirroring + `SubscriberHistoryElasticsearchReader`). To keep Elasticsearch optional for the new entity too, also + add a `*ConfigurableReader` (mirroring `SubscriberHistoryConfigurableReader`) that picks between the + Doctrine repository and the Elasticsearch reader based on `elasticsearch.enabled`, and alias the + interface to that instead of aliasing directly to either backend. + +## Troubleshooting + +- **Reads throwing `SearchBackendUnavailableException`**: check Elasticsearch is reachable at + `ELASTICSEARCH_HOSTS` and that `phplist:search:init-indices` has been run. +- **New/updated rows not appearing in search results**: make sure a `messenger:consume async_search` + worker is running; check `bin/console messenger:failed:show` for stuck messages. +- **Data drifted between MySQL and Elasticsearch**: re-run `bin/console phplist:search:reindex ` + - it's a safe, idempotent full backfill. diff --git a/src/Core/Doctrine/SearchIndexDoctrineListener.php b/src/Core/Doctrine/SearchIndexDoctrineListener.php new file mode 100644 index 00000000..8dc84829 --- /dev/null +++ b/src/Core/Doctrine/SearchIndexDoctrineListener.php @@ -0,0 +1,161 @@ + */ + private array $pending = []; + + /** @var array keyed by spl_object_id() */ + private array $removalKeys = []; + + public function __construct( + private readonly MessageBusInterface $messageBus, + private readonly bool $enabled = true, + ) { + } + + public function postPersist(PostPersistEventArgs $args): void + { + if (!$this->enabled) { + return; + } + + $entity = $args->getObject(); + if (!$entity instanceof SearchIndexableInterface) { + return; + } + + $this->queue($entity, SearchOperation::Index, $entity->getSearchIndexName(), $entity->getSearchDocumentId()); + } + + public function postUpdate(PostUpdateEventArgs $args): void + { + if (!$this->enabled) { + return; + } + + $entity = $args->getObject(); + if (!$entity instanceof SearchIndexableInterface) { + return; + } + + $this->queue($entity, SearchOperation::Index, $entity->getSearchIndexName(), $entity->getSearchDocumentId()); + } + + public function preRemove(PreRemoveEventArgs $args): void + { + if (!$this->enabled) { + return; + } + + $entity = $args->getObject(); + if (!$entity instanceof SearchIndexableInterface) { + return; + } + + $this->removalKeys[spl_object_id($entity)] = [$entity->getSearchIndexName(), $entity->getSearchDocumentId()]; + } + + public function postRemove(PostRemoveEventArgs $args): void + { + if (!$this->enabled) { + return; + } + + $entity = $args->getObject(); + if (!$entity instanceof SearchIndexableInterface) { + return; + } + + $objectId = spl_object_id($entity); + [$indexName, $documentId] = $this->removalKeys[$objectId] + ?? [$entity->getSearchIndexName(), $entity->getSearchDocumentId()]; + unset($this->removalKeys[$objectId]); + + $this->queue($entity, SearchOperation::Delete, $indexName, $documentId); + } + + public function postFlush(PostFlushEventArgs $args): void + { + if ($this->pending === []) { + return; + } + + $messages = $this->pending; + $this->pending = []; + + foreach ($messages as $message) { + $this->messageBus->dispatch($message); + } + } + + private function queue( + SearchIndexableInterface $entity, + SearchOperation $operation, + string $indexName, + string $documentId, + ): void { + $key = $indexName . '|' . $documentId; + $document = $operation === SearchOperation::Index ? $entity->toSearchDocument() : []; + + $this->pending[$key] = new IndexDocumentMessage( + $indexName, + $documentId, + $document, + $operation, + $this->nextRevision(), + ); + } + + /** + * Wall-clock microseconds, not a per-process counter: a delayed Messenger retry carries the + * revision assigned when it was originally queued, and must stay comparable against revisions + * assigned by other PHP processes/workers for the same document so the indexer (via Elasticsearch + * external versioning) can tell a stale retry apart from a newer write. + */ + private function nextRevision(): int + { + return (int) (microtime(true) * 1_000_000); + } +} diff --git a/src/Domain/Analytics/Service/AnalyticsService.php b/src/Domain/Analytics/Service/AnalyticsService.php index e0ff6985..170a24be 100644 --- a/src/Domain/Analytics/Service/AnalyticsService.php +++ b/src/Domain/Analytics/Service/AnalyticsService.php @@ -10,8 +10,8 @@ use PhpList\Core\Domain\Analytics\Service\Manager\LinkTrackManager; use PhpList\Core\Domain\Analytics\Service\Manager\UserMessageViewManager; use PhpList\Core\Domain\Messaging\Model\Filter\MessageFilter; +use PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReaderInterface; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; -use PhpList\Core\Domain\Messaging\Repository\UserMessageBounceRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageForwardRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; @@ -22,7 +22,7 @@ public function __construct( private readonly LinkTrackManager $linkTrackManager, private readonly UserMessageViewManager $userMessageViewManager, private readonly MessageRepository $messageRepository, - private readonly UserMessageBounceRepository $messageBounceRepository, + private readonly UserMessageBounceReaderInterface $messageBounceReader, private readonly UserMessageForwardRepository $messageForwardRepository, private readonly SubscriberRepository $subscriberRepository, private readonly UserMessageRepository $userMessageRepository, @@ -68,7 +68,7 @@ public function getCampaignStatistics(int $limit = 50, int $lastId = 0): array } $uniqueClicks = count($uniqueClickers); - $bounces = $this->messageBounceRepository->getCountByMessageId($message->getId()); + $bounces = $this->messageBounceReader->getCountByMessageId($message->getId()); $forwards = $this->messageForwardRepository->getCountByMessageId($message->getId()); $sentDate = $message->getMetadata()->getSent(); $sentCount = $message->getMetadata()->getBounceCount() + $views; @@ -178,11 +178,11 @@ public function getSummaryStatistics(): array $sentTotal = $this->userMessageRepository->countSentBetween($thisMonthStart, $now); $openTotal = $this->userMessageViewRepository->countBetween($thisMonthStart, $now); - $bounceTotal = $this->messageBounceRepository->countBetween($thisMonthStart, $now); + $bounceTotal = $this->messageBounceReader->countBetween($thisMonthStart, $now); $sentTotalLastMonth = $this->userMessageRepository->countSentBetween($lastMonthStart, $lastMonthEnd); $openTotalLastMonth = $this->userMessageViewRepository->countBetween($lastMonthStart, $lastMonthEnd); - $bounceTotalLastMonth = $this->messageBounceRepository->countBetween($lastMonthStart, $lastMonthEnd); + $bounceTotalLastMonth = $this->messageBounceReader->countBetween($lastMonthStart, $lastMonthEnd); $openRate = $this->calculateRate($openTotal, $sentTotal); $openRateLastMonth = $this->calculateRate($openTotalLastMonth, $sentTotalLastMonth); diff --git a/src/Domain/Common/Model/PaginatedResult.php b/src/Domain/Common/Model/PaginatedResult.php index 83eec8f2..aa9bb1b7 100644 --- a/src/Domain/Common/Model/PaginatedResult.php +++ b/src/Domain/Common/Model/PaginatedResult.php @@ -6,7 +6,7 @@ use PhpList\Core\Domain\Common\Model\Interfaces\DomainModel; -/** @template T of DomainModel */ +/** @template-covariant T of DomainModel */ class PaginatedResult { /** @var list */ diff --git a/src/Domain/Messaging/Model/Filter/UserMessageBounceFilter.php b/src/Domain/Messaging/Model/Filter/UserMessageBounceFilter.php new file mode 100644 index 00000000..ada2752e --- /dev/null +++ b/src/Domain/Messaging/Model/Filter/UserMessageBounceFilter.php @@ -0,0 +1,53 @@ +userId = $userId; + $this->messageId = $messageId; + $this->bounceId = $bounceId; + $this->dateFrom = $dateFrom; + $this->setLastId($lastId); + $this->setLimit($limit); + } + + public function getUserId(): ?int + { + return $this->userId; + } + + public function getMessageId(): ?int + { + return $this->messageId; + } + + public function getBounceId(): ?int + { + return $this->bounceId; + } + + public function getDateFrom(): ?DateTimeImmutable + { + return $this->dateFrom; + } +} diff --git a/src/Domain/Messaging/Model/Interfaces/UserMessageBounceRecordInterface.php b/src/Domain/Messaging/Model/Interfaces/UserMessageBounceRecordInterface.php new file mode 100644 index 00000000..37e77171 --- /dev/null +++ b/src/Domain/Messaging/Model/Interfaces/UserMessageBounceRecordInterface.php @@ -0,0 +1,26 @@ +id; + } + + public function getUserId(): int + { + return $this->userId; + } + + public function getMessageId(): int + { + return $this->messageId; + } + + public function getBounceId(): int + { + return $this->bounceId; + } + + public function getCreatedAt(): DateTime + { + return $this->createdAt; + } +} diff --git a/src/Domain/Messaging/Model/UserMessageBounce.php b/src/Domain/Messaging/Model/UserMessageBounce.php index 2a7ef519..9a350f88 100644 --- a/src/Domain/Messaging/Model/UserMessageBounce.php +++ b/src/Domain/Messaging/Model/UserMessageBounce.php @@ -5,10 +5,13 @@ namespace PhpList\Core\Domain\Messaging\Model; use DateTime; +use DateTimeInterface; use Doctrine\ORM\Mapping as ORM; use PhpList\Core\Domain\Common\Model\Interfaces\DomainModel; use PhpList\Core\Domain\Common\Model\Interfaces\Identity; +use PhpList\Core\Domain\Messaging\Model\Interfaces\UserMessageBounceRecordInterface; use PhpList\Core\Domain\Messaging\Repository\UserMessageBounceRepository; +use PhpList\Core\Domain\Search\Model\Interfaces\SearchIndexableInterface; #[ORM\Entity(repositoryClass: UserMessageBounceRepository::class)] #[ORM\Table(name: 'user_message_bounce')] @@ -17,8 +20,16 @@ #[ORM\Index(name: 'phplist_user_message_bounce_umbindex', columns: ['user', 'message', 'bounce'])] #[ORM\Index(name: 'phplist_user_message_bounce_useridx', columns: ['user'])] // todo: #[ORM\Index(name: 'phplist_user_message_bounce_timeidx', columns: ['time'])] -class UserMessageBounce implements DomainModel, Identity +class UserMessageBounce implements + DomainModel, + Identity, + SearchIndexableInterface, + UserMessageBounceRecordInterface { + public const SEARCH_INDEX_NAME = 'user_message_bounce'; + // 1000 is enough, I think, but if we ever need more, we can implement pagination. + public const MAX_RESULTS_BY_USER = 1000; + #[ORM\Id] #[ORM\Column(type: 'integer')] #[ORM\GeneratedValue] @@ -84,4 +95,26 @@ public function setBounceId(int $bounceId): self $this->bounceId = $bounceId; return $this; } + + public function getSearchIndexName(): string + { + return self::SEARCH_INDEX_NAME; + } + + public function getSearchDocumentId(): string + { + return (string) $this->id; + } + + public function toSearchDocument(): array + { + return [ + 'id' => $this->id, + 'idSort' => $this->id, + 'userId' => $this->userId, + 'messageId' => $this->messageId, + 'bounceId' => $this->bounceId, + 'time' => $this->createdAt->format(DateTimeInterface::ATOM), + ]; + } } diff --git a/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReaderInterface.php b/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReaderInterface.php new file mode 100644 index 00000000..083be412 --- /dev/null +++ b/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReaderInterface.php @@ -0,0 +1,30 @@ + */ + public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedResult; + + /** @return UserMessageBounceRecordInterface[] */ + public function getByUserId(int $userId): array; + + public function getCountByMessageId(int $messageId): int; + + public function countBetween(DateTimeInterface $start, DateTimeInterface $end): int; + + public function existsByMessageIdAndUserId(int $messageId, int $subscriberId): bool; +} diff --git a/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReportReaderInterface.php b/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReportReaderInterface.php new file mode 100644 index 00000000..4e574628 --- /dev/null +++ b/src/Domain/Messaging/Repository/Interfaces/UserMessageBounceReportReaderInterface.php @@ -0,0 +1,31 @@ + + */ + public function getListBounceTotals(int $listId): array; + + /** @return array */ + public function getCampaignBounceTotals(?int $ownerId = null): array; +} diff --git a/src/Domain/Messaging/Repository/UserMessageBounceConfigurableReader.php b/src/Domain/Messaging/Repository/UserMessageBounceConfigurableReader.php new file mode 100644 index 00000000..0f927c4c --- /dev/null +++ b/src/Domain/Messaging/Repository/UserMessageBounceConfigurableReader.php @@ -0,0 +1,59 @@ +activeReader()->getFilteredAfterId($filter); + } + + /** @return UserMessageBounceRecordInterface[] */ + public function getByUserId(int $userId): array + { + return $this->activeReader()->getByUserId($userId); + } + + public function getCountByMessageId(int $messageId): int + { + return $this->activeReader()->getCountByMessageId($messageId); + } + + public function countBetween(DateTimeInterface $start, DateTimeInterface $end): int + { + return $this->activeReader()->countBetween($start, $end); + } + + public function existsByMessageIdAndUserId(int $messageId, int $subscriberId): bool + { + return $this->activeReader()->existsByMessageIdAndUserId($messageId, $subscriberId); + } + + private function activeReader(): UserMessageBounceReaderInterface + { + return $this->elasticsearchEnabled ? $this->elasticsearchReader : $this->databaseReader; + } +} diff --git a/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReader.php b/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReader.php new file mode 100644 index 00000000..9bc94e1f --- /dev/null +++ b/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReader.php @@ -0,0 +1,351 @@ + + */ + public function getListBounceTotals(int $listId): array + { + $rows = $this->entityManager->createQueryBuilder() + ->select( + 'subscriber.id AS subscriberId', + 'subscriber.email AS email', + 'subscriber.confirmed AS confirmed', + 'subscriber.blacklisted AS blacklisted' + ) + ->from(Subscriber::class, 'subscriber') + ->innerJoin(Subscription::class, 'subscription', 'ON', 'subscription.subscriber = subscriber') + ->where('IDENTITY(subscription.subscriberList) = :listId') + ->setParameter('listId', $listId) + ->groupBy('subscriber.id, subscriber.email, subscriber.confirmed, subscriber.blacklisted') + ->orderBy('subscriber.id', 'ASC') + ->getQuery() + ->getArrayResult(); + + if ($rows === []) { + return []; + } + + $subscriberIds = array_map(static fn (array $row): int => (int) $row['subscriberId'], $rows); + $totalsByUserId = $this->countsByTermsField('userId', $subscriberIds); + + $result = []; + foreach ($rows as $row) { + $subscriberId = (int) $row['subscriberId']; + $totalBounces = $totalsByUserId[$subscriberId] ?? 0; + + if ($totalBounces === 0) { + continue; + } + + $result[] = [ + 'subscriber_id' => $subscriberId, + 'email' => (string) $row['email'], + 'confirmed' => (bool) $row['confirmed'], + 'blacklisted' => (bool) $row['blacklisted'], + 'total_bounces' => $totalBounces, + ]; + } + + return $result; + } + + /** @return array */ + public function getCampaignBounceTotals(?int $ownerId = null): array + { + $queryBuilder = $this->entityManager->createQueryBuilder() + ->select('m.id AS messageId', 'm.content.subject AS subject') + ->from(Message::class, 'm') + ->orderBy('m.id', 'ASC'); + + if ($ownerId !== null) { + $queryBuilder + ->andWhere('IDENTITY(m.owner) = :ownerId') + ->setParameter('ownerId', $ownerId); + } + + /** @var array $rows */ + $rows = $queryBuilder->getQuery()->getArrayResult(); + + if ($rows === []) { + return []; + } + + $messageIds = array_map(static fn (array $row): int => (int) $row['messageId'], $rows); + $totalsByMessageId = $this->countsByTermsField('messageId', $messageIds); + + $result = []; + foreach ($rows as $row) { + $messageId = (int) $row['messageId']; + $totalBounces = $totalsByMessageId[$messageId] ?? 0; + + if ($totalBounces === 0) { + continue; + } + + $result[] = [ + 'message_id' => $messageId, + 'subject' => $row['subject'], + 'total_bounces' => $totalBounces, + ]; + } + + return $result; + } + + /** @return array */ + public function getPaginatedWithJoinNoRelation(int $fromId, int $limit): array + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['range' => ['idSort' => ['gt' => $fromId]]], + 'sort' => [['idSort' => 'asc']], + 'size' => $limit, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + if ($hits === []) { + return []; + } + + $records = array_map($this->hydrate(...), $hits); + $bounceIds = array_values(array_unique(array_map( + static fn (UserMessageBounceRecordInterface $record): int => $record->getBounceId(), + $records + ))); + + $bouncesById = $this->findBouncesByIdIndexedById($bounceIds); + + $result = []; + foreach ($records as $record) { + $bounce = $bouncesById[$record->getBounceId()] ?? null; + + if ($bounce === null) { + continue; + } + + $result[] = ['umb' => $record, 'bounce' => $bounce]; + } + + return $result; + } + + /** + * @return array + */ + public function getUserMessageHistoryWithBounces(Subscriber $subscriber): array + { + /** @var UserMessage[] $userMessages */ + $userMessages = $this->entityManager->createQueryBuilder() + ->select('um') + ->from(UserMessage::class, 'um') + ->where('um.user = :userId') + ->andWhere('um.status = :status') + ->setParameter('userId', $subscriber->getId()) + ->setParameter('status', 'sent') + ->orderBy('um.createdAt', 'DESC') + ->getQuery() + ->getResult(); + + if ($userMessages === []) { + return []; + } + + $docsByMessageId = $this->groupByMessageId($this->fetchDocsByUserId((int) $subscriber->getId())); + $bouncesById = $this->findBouncesByIdIndexedById($this->bounceIdsUsedIn($docsByMessageId)); + + $result = []; + foreach ($userMessages as $userMessage) { + $docs = $docsByMessageId[$userMessage->getMessage()->getId()] ?? []; + + if ($docs === []) { + $result[] = ['um' => $userMessage, 'umb' => null, 'b' => null]; + continue; + } + + foreach ($docs as $doc) { + $result[] = [ + 'um' => $userMessage, + 'umb' => $doc, + 'b' => $bouncesById[$doc->getBounceId()] ?? null, + ]; + } + } + + return $result; + } + + /** @return UserMessageBounceRecordInterface[] */ + private function fetchDocsByUserId(int $userId): array + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['term' => ['userId' => $userId]], + 'sort' => [['idSort' => 'desc']], + 'size' => 10000, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + + return array_map($this->hydrate(...), $hits); + } + + /** + * @param UserMessageBounceRecordInterface[] $docs + * @return array + */ + private function groupByMessageId(array $docs): array + { + $docsByMessageId = []; + foreach ($docs as $doc) { + $docsByMessageId[$doc->getMessageId()][] = $doc; + } + + return $docsByMessageId; + } + + /** + * @param array $docsByMessageId + * @return int[] + */ + private function bounceIdsUsedIn(array $docsByMessageId): array + { + $bounceIds = []; + foreach ($docsByMessageId as $docs) { + foreach ($docs as $doc) { + $bounceIds[] = $doc->getBounceId(); + } + } + + return array_values(array_unique($bounceIds)); + } + + /** + * @param int[] $bounceIds + * @return array + */ + private function findBouncesByIdIndexedById(array $bounceIds): array + { + if ($bounceIds === []) { + return []; + } + + $bouncesById = []; + /** @var Bounce $bounce */ + foreach ($this->entityManager->getRepository(Bounce::class)->findBy(['id' => $bounceIds]) as $bounce) { + $bouncesById[$bounce->getId()] = $bounce; + } + + return $bouncesById; + } + + /** @param array{_source: array} $hit */ + private function hydrate(array $hit): UserMessageBounceReadModel + { + $source = $hit['_source']; + + return new UserMessageBounceReadModel( + id: isset($source['id']) ? (int) $source['id'] : null, + userId: (int) $source['userId'], + messageId: (int) $source['messageId'], + bounceId: (int) $source['bounceId'], + createdAt: isset($source['time']) + ? (DateTime::createFromFormat(DATE_ATOM, $source['time']) ?: new DateTime()) + : new DateTime(), + ); + } + + /** + * Aggregates document counts by an exact-match field, restricted to a given set of ids - used to + * correlate bounce counts (Elasticsearch) with rows from a small, non-"big table" DB query + * (subscribers in a list, messages owned by an admin) without joining across data stores. + * + * @param int[] $ids + * @return array counts keyed by id + */ + private function countsByTermsField(string $field, array $ids): array + { + if ($ids === []) { + return []; + } + + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'size' => 0, + 'query' => ['bool' => ['filter' => [['terms' => [$field => $ids]]]]], + 'aggs' => [ + 'by_field' => [ + 'terms' => ['field' => $field, 'size' => count($ids)], + ], + ], + ], + ); + + $counts = []; + foreach ($response['aggregations']['by_field']['buckets'] ?? [] as $bucket) { + $counts[(int) $bucket['key']] = (int) $bucket['doc_count']; + } + + return $counts; + } + + private function resolvePhysicalIndexName(): string + { + return $this->indexPrefix . UserMessageBounce::SEARCH_INDEX_NAME; + } +} diff --git a/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchReader.php b/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchReader.php new file mode 100644 index 00000000..af8c7e1b --- /dev/null +++ b/src/Domain/Messaging/Repository/UserMessageBounceElasticsearchReader.php @@ -0,0 +1,166 @@ +getUserId() !== null) { + $mustFilters[] = ['term' => ['userId' => $filter->getUserId()]]; + } + + if ($filter->getMessageId() !== null) { + $mustFilters[] = ['term' => ['messageId' => $filter->getMessageId()]]; + } + + if ($filter->getBounceId() !== null) { + $mustFilters[] = ['term' => ['bounceId' => $filter->getBounceId()]]; + } + + if ($filter->getDateFrom() !== null) { + $mustFilters[] = ['range' => ['time' => ['gte' => $filter->getDateFrom()->format(DATE_ATOM)]]]; + } + + $mustFilters[] = ['range' => ['idSort' => ['gt' => $filter->getLastId()]]]; + + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['bool' => ['filter' => $mustFilters]], + 'sort' => [['idSort' => 'asc']], + 'size' => $filter->getLimit(), + 'track_total_hits' => true, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + $lastHit = $hits !== [] ? $hits[array_key_last($hits)] : null; + + return new PaginatedResult( + items: array_map($this->hydrate(...), $hits), + total: (int) ($response['hits']['total']['value'] ?? 0), + limit: $filter->getLimit(), + lastId: $lastHit !== null ? (int) $lastHit['_source']['idSort'] : $filter->getLastId(), + ); + } + + /** @return UserMessageBounceRecordInterface[] */ + public function getByUserId(int $userId): array + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['term' => ['userId' => $userId]], + 'sort' => [['idSort' => 'desc']], + 'size' => UserMessageBounce::MAX_RESULTS_BY_USER, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + + return array_map($this->hydrate(...), $hits); + } + + public function getCountByMessageId(int $messageId): int + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['term' => ['messageId' => $messageId]], + 'size' => 0, + 'track_total_hits' => true, + ], + ); + + return (int) ($response['hits']['total']['value'] ?? 0); + } + + public function countBetween(DateTimeInterface $start, DateTimeInterface $end): int + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['range' => ['time' => [ + 'gte' => $start->format(DateTimeInterface::ATOM), + 'lte' => $end->format(DateTimeInterface::ATOM), + ]]], + 'size' => 0, + 'track_total_hits' => true, + ], + ); + + return (int) ($response['hits']['total']['value'] ?? 0); + } + + public function existsByMessageIdAndUserId(int $messageId, int $subscriberId): bool + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['bool' => ['filter' => [ + ['term' => ['messageId' => $messageId]], + ['term' => ['userId' => $subscriberId]], + ]]], + 'size' => 0, + 'track_total_hits' => true, + ], + ); + + return ((int) ($response['hits']['total']['value'] ?? 0)) > 0; + } + + /** @param array{_source: array} $hit */ + private function hydrate(array $hit): UserMessageBounceReadModel + { + $source = $hit['_source']; + + return new UserMessageBounceReadModel( + id: isset($source['id']) ? (int) $source['id'] : null, + userId: (int) $source['userId'], + messageId: (int) $source['messageId'], + bounceId: (int) $source['bounceId'], + createdAt: isset($source['time']) + ? (DateTime::createFromFormat(DATE_ATOM, $source['time']) ?: new DateTime()) + : new DateTime(), + ); + } + + private function resolvePhysicalIndexName(): string + { + return $this->indexPrefix . UserMessageBounce::SEARCH_INDEX_NAME; + } +} diff --git a/src/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReader.php b/src/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReader.php new file mode 100644 index 00000000..7f154402 --- /dev/null +++ b/src/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReader.php @@ -0,0 +1,39 @@ +activeReader()->getListBounceTotals($listId); + } + + public function getCampaignBounceTotals(?int $ownerId = null): array + { + return $this->activeReader()->getCampaignBounceTotals($ownerId); + } + + private function activeReader(): UserMessageBounceReportReaderInterface + { + return $this->elasticsearchEnabled ? $this->elasticsearchReader : $this->databaseReader; + } +} diff --git a/src/Domain/Messaging/Repository/UserMessageBounceRepository.php b/src/Domain/Messaging/Repository/UserMessageBounceRepository.php index c677e5c1..4d99217d 100644 --- a/src/Domain/Messaging/Repository/UserMessageBounceRepository.php +++ b/src/Domain/Messaging/Repository/UserMessageBounceRepository.php @@ -5,20 +5,98 @@ namespace PhpList\Core\Domain\Messaging\Repository; use DateTimeInterface; +use InvalidArgumentException; +use PhpList\Core\Domain\Common\Model\Filter\FilterRequestInterface; +use PhpList\Core\Domain\Common\Model\PaginatedResult; use PhpList\Core\Domain\Common\Repository\AbstractRepository; use PhpList\Core\Domain\Common\Repository\CursorPaginationTrait; use PhpList\Core\Domain\Common\Repository\Interfaces\PaginatableRepositoryInterface; use PhpList\Core\Domain\Messaging\Model\Bounce; +use PhpList\Core\Domain\Messaging\Model\Filter\UserMessageBounceFilter; use PhpList\Core\Domain\Messaging\Model\Message; use PhpList\Core\Domain\Messaging\Model\UserMessage; use PhpList\Core\Domain\Messaging\Model\UserMessageBounce; +use PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReaderInterface; +use PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReportReaderInterface; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\Subscription; -class UserMessageBounceRepository extends AbstractRepository implements PaginatableRepositoryInterface +class UserMessageBounceRepository extends AbstractRepository implements + PaginatableRepositoryInterface, + UserMessageBounceReaderInterface, + UserMessageBounceReportReaderInterface { use CursorPaginationTrait; + /** + * @return PaginatedResult + * @throws InvalidArgumentException + */ + public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedResult + { + if (!$filter instanceof UserMessageBounceFilter) { + throw new InvalidArgumentException('Expected UserMessageBounceFilter.'); + } + + $lastId = $filter->getLastId(); + $limit = $filter->getLimit(); + $queryBuilder = $this->createQueryBuilder('umb'); + + if ($filter->getUserId() !== null) { + $queryBuilder->andWhere('umb.userId = :userId') + ->setParameter('userId', $filter->getUserId()); + } + + if ($filter->getMessageId() !== null) { + $queryBuilder->andWhere('umb.messageId = :messageId') + ->setParameter('messageId', $filter->getMessageId()); + } + + if ($filter->getBounceId() !== null) { + $queryBuilder->andWhere('umb.bounceId = :bounceId') + ->setParameter('bounceId', $filter->getBounceId()); + } + + if ($filter->getDateFrom() !== null) { + $queryBuilder->andWhere('umb.createdAt >= :dateFrom') + ->setParameter('dateFrom', $filter->getDateFrom()); + } + + $countQb = clone $queryBuilder; + $total = (int) $countQb + ->select('COUNT(DISTINCT umb.id)') + ->getQuery() + ->getSingleScalarResult(); + + /** @var list $items */ + $items = $queryBuilder + ->andWhere('umb.id > :lastId') + ->setParameter('lastId', $lastId) + ->orderBy('umb.id', 'ASC') + ->setMaxResults($limit) + ->getQuery() + ->getResult(); + + return new PaginatedResult( + items: $items, + total: $total, + limit: $limit, + lastId: $items !== [] ? $items[array_key_last($items)]->getId() : $lastId, + ); + } + + /** @return UserMessageBounce[] */ + public function getByUserId(int $userId): array + { + return $this->createQueryBuilder('umb') + ->andWhere('umb.userId = :userId') + ->setParameter('userId', $userId) + ->orderBy('umb.id', 'DESC') + ->setMaxResults(UserMessageBounce::MAX_RESULTS_BY_USER) + ->getQuery() + ->getResult(); + } + public function getCountByMessageId(int $messageId): int { return (int) $this->createQueryBuilder('umb') @@ -188,7 +266,7 @@ public function getUserMessageHistoryWithBounces(Subscriber $subscriber): array ->andWhere('um.status = :status') ->setParameter('userId', $subscriber->getId()) ->setParameter('status', 'sent') - ->orderBy('um.entered', 'DESC') + ->orderBy('um.createdAt', 'DESC') ->getQuery() ->getResult(); } diff --git a/src/Domain/Messaging/Service/Manager/BounceManager.php b/src/Domain/Messaging/Service/Manager/BounceManager.php index bae5e094..e8e7c497 100644 --- a/src/Domain/Messaging/Service/Manager/BounceManager.php +++ b/src/Domain/Messaging/Service/Manager/BounceManager.php @@ -89,6 +89,8 @@ public function linkUserMessageBounce( $userMessageBounce->setUserId($subscriberId); $userMessageBounce->setMessageId($messageId); + $this->userMessageBounceRepo->persist($userMessageBounce); + return $userMessageBounce; } diff --git a/src/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinition.php b/src/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinition.php new file mode 100644 index 00000000..29292169 --- /dev/null +++ b/src/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinition.php @@ -0,0 +1,37 @@ + [ + 'id' => ['type' => 'keyword'], + // Numeric mirror of `id`, used for range/sort in cursor pagination - `id` stays a + // keyword for exact-match filtering. + 'idSort' => ['type' => 'long'], + 'userId' => ['type' => 'keyword'], + 'messageId' => ['type' => 'keyword'], + 'bounceId' => ['type' => 'keyword'], + 'time' => ['type' => 'date'], + ], + ]; + } + + public function getSettings(): array + { + return []; + } +} diff --git a/src/Domain/Messaging/Service/Search/UserMessageBounceReindexProvider.php b/src/Domain/Messaging/Service/Search/UserMessageBounceReindexProvider.php new file mode 100644 index 00000000..34c9be22 --- /dev/null +++ b/src/Domain/Messaging/Service/Search/UserMessageBounceReindexProvider.php @@ -0,0 +1,40 @@ +repository->createQueryBuilder('umb') + ->select('COUNT(umb.id)') + ->getQuery() + ->getSingleScalarResult(); + } + + public function fetchBatch(int $lastId, int $batchSize): iterable + { + return $this->repository->createQueryBuilder('umb') + ->andWhere('umb.id > :lastId') + ->setParameter('lastId', $lastId) + ->orderBy('umb.id', 'ASC') + ->setMaxResults($batchSize) + ->getQuery() + ->toIterable(); + } +} diff --git a/src/Domain/Search/Client/ElasticsearchClientAdapter.php b/src/Domain/Search/Client/ElasticsearchClientAdapter.php new file mode 100644 index 00000000..b1bbefb0 --- /dev/null +++ b/src/Domain/Search/Client/ElasticsearchClientAdapter.php @@ -0,0 +1,114 @@ +call(function () use ($indexName, $documentId, $document, $revision): void { + try { + $this->client->index([ + 'index' => $indexName, + 'id' => $documentId, + 'body' => $document, + 'version' => $revision, + 'version_type' => 'external_gte', + ]); + } catch (ClientResponseException $exception) { + if ($exception->getCode() !== self::HTTP_CONFLICT) { + throw $exception; + } + } + }); + } + + public function delete(string $indexName, string $documentId, int $revision): void + { + $this->call(function () use ($indexName, $documentId, $revision): void { + try { + $this->client->delete([ + 'index' => $indexName, + 'id' => $documentId, + 'version' => $revision, + 'version_type' => 'external_gte', + ]); + } catch (ClientResponseException $exception) { + if (!in_array($exception->getCode(), [self::HTTP_NOT_FOUND, self::HTTP_CONFLICT], true)) { + throw $exception; + } + } + }); + } + + public function indexExists(string $indexName): bool + { + return $this->call(fn (): bool => $this->client->indices()->exists(['index' => $indexName])->asBool()); + } + + public function createIndex(string $indexName, array $mapping, array $settings): void + { + $this->call(function () use ($indexName, $mapping, $settings): void { + $body = ['mappings' => $mapping]; + if ($settings !== []) { + $body['settings'] = $settings; + } + + $this->client->indices()->create([ + 'index' => $indexName, + 'body' => $body, + ]); + }); + } + + public function updateMapping(string $indexName, array $mapping): void + { + $this->call(function () use ($indexName, $mapping): void { + $this->client->indices()->putMapping([ + 'index' => $indexName, + 'body' => $mapping, + ]); + }); + } + + public function search(string $indexName, array $query): array + { + return $this->call(fn (): array => $this->client->search([ + 'index' => $indexName, + 'body' => $query, + ])->asArray()); + } + + /** + * @template T + * @param callable(): T $operation + * @return T + * @throws SearchBackendUnavailableException + */ + private function call(callable $operation): mixed + { + try { + return $operation(); + } catch (Throwable $exception) { + throw new SearchBackendUnavailableException( + 'Elasticsearch operation failed: ' . $exception->getMessage(), + 0, + $exception, + ); + } + } +} diff --git a/src/Domain/Search/Client/ElasticsearchClientFactory.php b/src/Domain/Search/Client/ElasticsearchClientFactory.php new file mode 100644 index 00000000..1eff7b86 --- /dev/null +++ b/src/Domain/Search/Client/ElasticsearchClientFactory.php @@ -0,0 +1,32 @@ +setHosts($hosts); + + if (!empty($username)) { + $builder->setBasicAuthentication($username, $password ?? ''); + } + + $builder->setHttpClientOptions([ + 'max_connect_duration' => $connectTimeout, + 'timeout' => $requestTimeout, + ]); + + return $builder->build(); + } +} diff --git a/src/Domain/Search/Client/ElasticsearchClientInterface.php b/src/Domain/Search/Client/ElasticsearchClientInterface.php new file mode 100644 index 00000000..b7240c1f --- /dev/null +++ b/src/Domain/Search/Client/ElasticsearchClientInterface.php @@ -0,0 +1,54 @@ + $document + * @throws SearchBackendUnavailableException + */ + public function index(string $indexName, string $documentId, array $document, int $revision): void; + + /** + * Returns quietly (idempotent) if the document does not exist, or if $revision is older than the + * revision currently stored for this document. + * @throws SearchBackendUnavailableException + */ + public function delete(string $indexName, string $documentId, int $revision): void; + + /** @throws SearchBackendUnavailableException */ + public function indexExists(string $indexName): bool; + + /** + * @param array $mapping + * @param array $settings + * @throws SearchBackendUnavailableException + */ + public function createIndex(string $indexName, array $mapping, array $settings): void; + + /** + * @param array $mapping + * @throws SearchBackendUnavailableException + */ + public function updateMapping(string $indexName, array $mapping): void; + + /** + * @param array $query + * @return array Raw decoded ES response body. + * @throws SearchBackendUnavailableException + */ + public function search(string $indexName, array $query): array; +} diff --git a/src/Domain/Search/Command/InitSearchIndicesCommand.php b/src/Domain/Search/Command/InitSearchIndicesCommand.php new file mode 100644 index 00000000..a3e5e6dc --- /dev/null +++ b/src/Domain/Search/Command/InitSearchIndicesCommand.php @@ -0,0 +1,63 @@ +addOption( + 'index', + null, + InputOption::VALUE_REQUIRED, + 'Only create/update the index for this alias (e.g. subscriber_history)', + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $alias = $input->getOption('index'); + + $definitions = $alias !== null + ? array_filter([$this->registry->find($alias)]) + : $this->registry->getAll(); + + if ($definitions === []) { + $io->warning($alias !== null + ? sprintf('No index definition registered for alias "%s".', $alias) + : 'No index definitions registered.'); + + return Command::SUCCESS; + } + + foreach ($definitions as $definition) { + $this->indexer->createOrUpdateIndex($definition); + $io->writeln(sprintf('OK %s', $definition->getIndexAlias())); + } + + return Command::SUCCESS; + } +} diff --git a/src/Domain/Search/Command/PurgeSearchIndexedRowsCommand.php b/src/Domain/Search/Command/PurgeSearchIndexedRowsCommand.php new file mode 100644 index 00000000..02baf293 --- /dev/null +++ b/src/Domain/Search/Command/PurgeSearchIndexedRowsCommand.php @@ -0,0 +1,210 @@ +addArgument('alias', InputArgument::OPTIONAL, 'Purge only this alias (default: all configured)') + ->addOption('batch-size', null, InputOption::VALUE_REQUIRED, 'Rows per batch', self::DEFAULT_BATCH_SIZE) + ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Report counts without deleting anything'); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $alias = $input->getArgument('alias'); + $batchSize = (int) $input->getOption('batch-size'); + $dryRun = (bool) $input->getOption('dry-run'); + + if ($batchSize < 1) { + $io->error('The --batch-size option must be greater than zero.'); + return Command::FAILURE; + } + + if ($alias !== null) { + $provider = $this->registry->find($alias); + + if ($provider === null) { + $io->error(sprintf('No purge provider registered for alias "%s".', $alias)); + return Command::FAILURE; + } + + if ($provider->getRetentionPeriod() === null) { + $io->error(sprintf('No retention period configured for alias "%s".', $alias)); + return Command::FAILURE; + } + + $providers = [$provider]; + } else { + $providers = array_filter( + $this->registry->getAll(), + static fn (SearchPurgeProviderInterface $provider): bool => $provider->getRetentionPeriod() !== null, + ); + } + + if ($providers === []) { + $io->warning('No purge providers with a configured retention period.'); + return Command::SUCCESS; + } + + foreach ($providers as $provider) { + $this->purgeProvider($provider, $batchSize, $dryRun, $io); + } + + return Command::SUCCESS; + } + + private function purgeProvider( + SearchPurgeProviderInterface $provider, + int $batchSize, + bool $dryRun, + SymfonyStyle $io, + ): void { + $cutoff = (new DateTimeImmutable())->sub($provider->getRetentionPeriod()); + $io->writeln(sprintf( + '%s: purging rows older than %s', + $provider->getAlias(), + $cutoff->format(DateTimeImmutable::ATOM), + )); + + $lastId = 0; + $scanned = 0; + $deleted = 0; + $skipped = []; + + do { + $batch = [...$provider->fetchBatchOlderThan($cutoff, $lastId, $batchSize)]; + $countInBatch = count($batch); + + if ($countInBatch === 0) { + break; + } + + $result = $this->purgeBatch($provider, $batch, $dryRun); + $deleted += $result['deleted']; + array_push($skipped, ...$result['skipped']); + $scanned += $countInBatch; + $lastId = $result['lastId']; + } while ($countInBatch >= $batchSize); + + $this->reportResults($provider, $scanned, $deleted, $skipped, $dryRun, $io); + } + + /** + * @param SearchIndexableInterface[] $batch + * @return array{deleted: int, skipped: int[], lastId: int} + */ + private function purgeBatch(SearchPurgeProviderInterface $provider, array $batch, bool $dryRun): array + { + $docIds = array_map( + static fn (SearchIndexableInterface $entity): string => $entity->getSearchDocumentId(), + $batch, + ); + $confirmedIds = $this->confirmedInElasticsearch($provider->getSearchIndexName(), $docIds); + + $confirmedEntityIds = []; + $skipped = []; + foreach ($docIds as $docId) { + if (in_array($docId, $confirmedIds, true)) { + $confirmedEntityIds[] = (int) $docId; + } else { + $skipped[] = (int) $docId; + } + } + + $deleted = count($confirmedEntityIds); + if (!$dryRun && $confirmedEntityIds !== []) { + $deleted = $provider->deleteByIds($confirmedEntityIds); + } + + return [ + 'deleted' => $deleted, + 'skipped' => $skipped, + 'lastId' => (int) $docIds[array_key_last($docIds)], + ]; + } + + /** @param int[] $skipped */ + private function reportResults( + SearchPurgeProviderInterface $provider, + int $scanned, + int $deleted, + array $skipped, + bool $dryRun, + SymfonyStyle $io, + ): void { + if ($skipped !== []) { + $io->warning(sprintf( + '%s: %d row(s) older than cutoff were not found in Elasticsearch and were left in place: %s', + $provider->getAlias(), + count($skipped), + implode(', ', array_slice($skipped, 0, 10)) . (count($skipped) > 10 ? ', ...' : ''), + )); + } + + $io->success(sprintf( + '%s: scanned %d row(s), %s %d row(s)%s.', + $provider->getAlias(), + $scanned, + $dryRun ? 'would delete' : 'deleted', + $deleted, + $skipped !== [] ? sprintf(', skipped %d unconfirmed', count($skipped)) : '', + )); + } + + /** @param string[] $docIds @return string[] */ + private function confirmedInElasticsearch(string $indexName, array $docIds): array + { + if ($docIds === []) { + return []; + } + + $response = $this->client->search($indexName, [ + 'size' => count($docIds), + '_source' => false, + 'query' => [ + 'bool' => [ + 'filter' => [ + ['terms' => ['id' => array_map('intval', $docIds)]], + ], + ], + ], + ]); + + return array_map( + static fn (array $hit): string => (string) $hit['_id'], + $response['hits']['hits'] ?? [], + ); + } +} diff --git a/src/Domain/Search/Command/ReindexSearchCommand.php b/src/Domain/Search/Command/ReindexSearchCommand.php new file mode 100644 index 00000000..c87543cc --- /dev/null +++ b/src/Domain/Search/Command/ReindexSearchCommand.php @@ -0,0 +1,122 @@ +addArgument('alias', InputArgument::OPTIONAL, 'Reindex only this alias (default: all registered)') + ->addOption('batch-size', null, InputOption::VALUE_REQUIRED, 'Rows per batch', self::DEFAULT_BATCH_SIZE) + ->addOption( + 'last-id', + null, + InputOption::VALUE_REQUIRED, + 'Resume from this id (only meaningful with a single alias)', + 0, + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $alias = $input->getArgument('alias'); + $batchSize = (int) $input->getOption('batch-size'); + $lastId = (int) $input->getOption('last-id'); + + if ($batchSize < 1) { + $io->error('The --batch-size option must be greater than zero.'); + return Command::FAILURE; + } + + if ($alias === null && $lastId !== 0) { + $io->error('The --last-id option requires an index alias.'); + return Command::FAILURE; + } + + $providers = $alias !== null + ? array_filter([$this->registry->find($alias)]) + : $this->registry->getAll(); + + if ($providers === []) { + $io->warning($alias !== null + ? sprintf('No reindex provider registered for alias "%s".', $alias) + : 'No reindex providers registered.'); + + return Command::SUCCESS; + } + + foreach ($providers as $provider) { + $this->reindexProvider($provider, $lastId, $batchSize, $io); + } + + return Command::SUCCESS; + } + + private function reindexProvider( + SearchReindexProviderInterface $provider, + int $lastId, + int $batchSize, + SymfonyStyle $io, + ): void { + $total = $provider->countAll(); + $io->writeln(sprintf('%s: %d rows total', $provider->getAlias(), $total)); + $progressBar = $io->createProgressBar($total); + + $indexed = 0; + do { + $batch = $provider->fetchBatch($lastId, $batchSize); + $countInBatch = 0; + // Captured once per batch, before any of this batch's ES writes, rather than per document + // at send time: a batch can take a while to send (progress bar, ES round trips), and a + // per-document revision taken at send time could end up newer than a concurrent delete's + // revision for a row this batch already read earlier, resurrecting it in Elasticsearch. + $revision = (int) (microtime(true) * 1_000_000); + + foreach ($batch as $entity) { + $this->indexer->index( + $entity->getSearchIndexName(), + $entity->getSearchDocumentId(), + $entity->toSearchDocument(), + $revision, + ); + $lastId = (int) $entity->getSearchDocumentId(); + $countInBatch++; + $indexed++; + } + + $progressBar->setProgress($indexed); + } while ($countInBatch >= $batchSize); + + $progressBar->finish(); + $io->newLine(2); + $io->success(sprintf('%s: indexed %d rows.', $provider->getAlias(), $indexed)); + } +} diff --git a/src/Domain/Search/Exception/SearchBackendUnavailableException.php b/src/Domain/Search/Exception/SearchBackendUnavailableException.php new file mode 100644 index 00000000..da3dd01f --- /dev/null +++ b/src/Domain/Search/Exception/SearchBackendUnavailableException.php @@ -0,0 +1,16 @@ + $document */ + public function __construct( + private readonly string $indexName, + private readonly string $documentId, + private readonly array $document, + private readonly SearchOperation $operation, + private readonly int $revision, + ) { + } + + public function getIndexName(): string + { + return $this->indexName; + } + + public function getDocumentId(): string + { + return $this->documentId; + } + + /** @return array */ + public function getDocument(): array + { + return $this->document; + } + + public function getOperation(): SearchOperation + { + return $this->operation; + } + + public function getRevision(): int + { + return $this->revision; + } +} diff --git a/src/Domain/Search/MessageHandler/IndexDocumentMessageHandler.php b/src/Domain/Search/MessageHandler/IndexDocumentMessageHandler.php new file mode 100644 index 00000000..7896d4cc --- /dev/null +++ b/src/Domain/Search/MessageHandler/IndexDocumentMessageHandler.php @@ -0,0 +1,35 @@ +getOperation()) { + SearchOperation::Index => $this->indexer->index( + $message->getIndexName(), + $message->getDocumentId(), + $message->getDocument(), + $message->getRevision(), + ), + SearchOperation::Delete => $this->indexer->delete( + $message->getIndexName(), + $message->getDocumentId(), + $message->getRevision(), + ), + }; + } +} diff --git a/src/Domain/Search/Model/Interfaces/SearchIndexDefinitionInterface.php b/src/Domain/Search/Model/Interfaces/SearchIndexDefinitionInterface.php new file mode 100644 index 00000000..4d7df679 --- /dev/null +++ b/src/Domain/Search/Model/Interfaces/SearchIndexDefinitionInterface.php @@ -0,0 +1,21 @@ + */ + public function getMapping(): array; + + /** @return array */ + public function getSettings(): array; +} diff --git a/src/Domain/Search/Model/Interfaces/SearchIndexableInterface.php b/src/Domain/Search/Model/Interfaces/SearchIndexableInterface.php new file mode 100644 index 00000000..275ca8f0 --- /dev/null +++ b/src/Domain/Search/Model/Interfaces/SearchIndexableInterface.php @@ -0,0 +1,20 @@ + */ + public function toSearchDocument(): array; +} diff --git a/src/Domain/Search/Model/Interfaces/SearchPurgeProviderInterface.php b/src/Domain/Search/Model/Interfaces/SearchPurgeProviderInterface.php new file mode 100644 index 00000000..35e95763 --- /dev/null +++ b/src/Domain/Search/Model/Interfaces/SearchPurgeProviderInterface.php @@ -0,0 +1,32 @@ + */ + public function fetchBatchOlderThan(DateTimeInterface $cutoff, int $lastId, int $batchSize): iterable; + + /** @param int[] $ids @return int number of rows deleted */ + public function deleteByIds(array $ids): int; +} diff --git a/src/Domain/Search/Model/Interfaces/SearchReindexProviderInterface.php b/src/Domain/Search/Model/Interfaces/SearchReindexProviderInterface.php new file mode 100644 index 00000000..a8bcf955 --- /dev/null +++ b/src/Domain/Search/Model/Interfaces/SearchReindexProviderInterface.php @@ -0,0 +1,19 @@ + */ + public function fetchBatch(int $lastId, int $batchSize): iterable; +} diff --git a/src/Domain/Search/Model/SearchOperation.php b/src/Domain/Search/Model/SearchOperation.php new file mode 100644 index 00000000..3a905d26 --- /dev/null +++ b/src/Domain/Search/Model/SearchOperation.php @@ -0,0 +1,11 @@ + $definitions */ + public function __construct(iterable $definitions) + { + $this->definitions = $definitions instanceof \Traversable ? iterator_to_array($definitions) : $definitions; + } + + /** @return SearchIndexDefinitionInterface[] */ + public function getAll(): array + { + return $this->definitions; + } + + public function find(string $alias): ?SearchIndexDefinitionInterface + { + foreach ($this->definitions as $definition) { + if ($definition->getIndexAlias() === $alias) { + return $definition; + } + } + + return null; + } +} diff --git a/src/Domain/Search/Registry/SearchPurgeProviderRegistry.php b/src/Domain/Search/Registry/SearchPurgeProviderRegistry.php new file mode 100644 index 00000000..3cd3236e --- /dev/null +++ b/src/Domain/Search/Registry/SearchPurgeProviderRegistry.php @@ -0,0 +1,37 @@ + $providers */ + public function __construct(iterable $providers) + { + $this->providers = $providers instanceof Traversable ? iterator_to_array($providers) : $providers; + } + + /** @return SearchPurgeProviderInterface[] */ + public function getAll(): array + { + return $this->providers; + } + + public function find(string $alias): ?SearchPurgeProviderInterface + { + foreach ($this->providers as $provider) { + if ($provider->getAlias() === $alias) { + return $provider; + } + } + + return null; + } +} diff --git a/src/Domain/Search/Registry/SearchReindexProviderRegistry.php b/src/Domain/Search/Registry/SearchReindexProviderRegistry.php new file mode 100644 index 00000000..6f8a1c3c --- /dev/null +++ b/src/Domain/Search/Registry/SearchReindexProviderRegistry.php @@ -0,0 +1,36 @@ + $providers */ + public function __construct(iterable $providers) + { + $this->providers = $providers instanceof \Traversable ? iterator_to_array($providers) : $providers; + } + + /** @return SearchReindexProviderInterface[] */ + public function getAll(): array + { + return $this->providers; + } + + public function find(string $alias): ?SearchReindexProviderInterface + { + foreach ($this->providers as $provider) { + if ($provider->getAlias() === $alias) { + return $provider; + } + } + + return null; + } +} diff --git a/src/Domain/Search/Service/ElasticsearchIndexer.php b/src/Domain/Search/Service/ElasticsearchIndexer.php new file mode 100644 index 00000000..317d40c0 --- /dev/null +++ b/src/Domain/Search/Service/ElasticsearchIndexer.php @@ -0,0 +1,45 @@ +client->index($this->resolvePhysicalIndexName($indexAlias), $documentId, $document, $revision); + } + + public function delete(string $indexAlias, string $documentId, int $revision): void + { + $this->client->delete($this->resolvePhysicalIndexName($indexAlias), $documentId, $revision); + } + + public function createOrUpdateIndex(SearchIndexDefinitionInterface $definition): void + { + $indexName = $this->resolvePhysicalIndexName($definition->getIndexAlias()); + + if ($this->client->indexExists($indexName)) { + $this->client->updateMapping($indexName, $definition->getMapping()); + + return; + } + + $this->client->createIndex($indexName, $definition->getMapping(), $definition->getSettings()); + } + + private function resolvePhysicalIndexName(string $indexAlias): string + { + return $this->indexPrefix . $indexAlias; + } +} diff --git a/src/Domain/Search/Service/ElasticsearchIndexerInterface.php b/src/Domain/Search/Service/ElasticsearchIndexerInterface.php new file mode 100644 index 00000000..8e01097d --- /dev/null +++ b/src/Domain/Search/Service/ElasticsearchIndexerInterface.php @@ -0,0 +1,32 @@ + $document + * @param int $revision Monotonic per-document revision; writes older than the last applied + * revision for this document are dropped instead of applied (see ElasticsearchClientAdapter). + * @throws SearchBackendUnavailableException + */ + public function index(string $indexAlias, string $documentId, array $document, int $revision): void; + + /** + * @param int $revision Monotonic per-document revision; see index(). + * @throws SearchBackendUnavailableException + */ + public function delete(string $indexAlias, string $documentId, int $revision): void; + + /** + * Creates the index with its mapping/settings if absent, otherwise applies the mapping + * non-destructively (never drops/recreates an existing index). + * @throws SearchBackendUnavailableException + */ + public function createOrUpdateIndex(SearchIndexDefinitionInterface $definition): void; +} diff --git a/src/Domain/Subscription/Model/Interfaces/SubscriberHistoryRecordInterface.php b/src/Domain/Subscription/Model/Interfaces/SubscriberHistoryRecordInterface.php new file mode 100644 index 00000000..48292f55 --- /dev/null +++ b/src/Domain/Subscription/Model/Interfaces/SubscriberHistoryRecordInterface.php @@ -0,0 +1,33 @@ +id; + } + + public function getSubscriberId(): ?int + { + return $this->subscriberId; + } + + public function getIp(): ?string + { + return $this->ip; + } + + public function getCreatedAt(): ?DateTime + { + return $this->createdAt; + } + + public function getSummary(): ?string + { + return $this->summary; + } + + public function getDetail(): ?string + { + return $this->detail; + } + + public function getSystemInfo(): ?string + { + return $this->systemInfo; + } +} diff --git a/src/Domain/Subscription/Model/Subscriber.php b/src/Domain/Subscription/Model/Subscriber.php index 53299540..e29a9b7d 100644 --- a/src/Domain/Subscription/Model/Subscriber.php +++ b/src/Domain/Subscription/Model/Subscriber.php @@ -12,6 +12,7 @@ use PhpList\Core\Domain\Common\Model\Interfaces\DomainModel; use PhpList\Core\Domain\Common\Model\Interfaces\Identity; use PhpList\Core\Domain\Common\Model\Interfaces\ModificationDate; +use PhpList\Core\Domain\Subscription\Model\Interfaces\SubscriberHistoryRecordInterface; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; /** @@ -93,6 +94,18 @@ class Subscriber implements DomainModel, Identity, CreationDate, ModificationDat )] private Collection $attributes; + /** + * Doctrine-only bookkeeping, not part of the public API (see getHistory()/setHistory() for that). + * SubscriberHistory's FK has only a DB-level ON DELETE CASCADE - without this mapped association, + * removing a Subscriber straight through the EntityManager would let the database silently drop its + * SubscriberHistory rows without Doctrine ever loading/removing them individually, so + * SearchIndexDoctrineListener would never fire for those rows and their Elasticsearch documents + * would be orphaned. This cascade makes Doctrine remove them itself instead. + * @var Collection + */ + #[ORM\OneToMany(targetEntity: SubscriberHistory::class, mappedBy: 'subscriber', cascade: ['remove'])] + private Collection $historyRecords; + #[ORM\Column(name: 'optedin', type: 'boolean')] private bool $optedIn = false; @@ -114,7 +127,7 @@ class Subscriber implements DomainModel, Identity, CreationDate, ModificationDat #[ORM\Column(name: 'foreignkey', type: 'string', length: 100, nullable: true)] private ?string $foreignKey = null; - /** @var SubscriberHistory[] */ + /** @var SubscriberHistoryRecordInterface[] */ private array $history = []; public function __construct(string $email) @@ -122,6 +135,7 @@ public function __construct(string $email) $this->email = $email; $this->subscriptions = new ArrayCollection(); $this->attributes = new ArrayCollection(); + $this->historyRecords = new ArrayCollection(); $this->extraData = ''; $this->createdAt = new DateTime(); $this->updatedAt = new DateTime(); @@ -378,7 +392,7 @@ public function setForeignKey(?string $foreignKey): void } /** - * @return SubscriberHistory[] + * @return SubscriberHistoryRecordInterface[] */ public function getHistory(): array { @@ -386,7 +400,7 @@ public function getHistory(): array } /** - * @param SubscriberHistory[] $history + * @param SubscriberHistoryRecordInterface[] $history */ public function setHistory(array $history): void { diff --git a/src/Domain/Subscription/Model/SubscriberHistory.php b/src/Domain/Subscription/Model/SubscriberHistory.php index 08f4f974..91991c44 100644 --- a/src/Domain/Subscription/Model/SubscriberHistory.php +++ b/src/Domain/Subscription/Model/SubscriberHistory.php @@ -5,23 +5,33 @@ namespace PhpList\Core\Domain\Subscription\Model; use DateTime; +use DateTimeInterface; use Doctrine\ORM\Mapping as ORM; use PhpList\Core\Domain\Common\Model\Interfaces\DomainModel; use PhpList\Core\Domain\Common\Model\Interfaces\Identity; +use PhpList\Core\Domain\Search\Model\Interfaces\SearchIndexableInterface; +use PhpList\Core\Domain\Subscription\Model\Interfaces\SubscriberHistoryRecordInterface; use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; #[ORM\Entity(repositoryClass: SubscriberHistoryRepository::class)] #[ORM\Table(name: 'user_user_history')] #[ORM\Index(name: 'phplist_user_user_history_dateidx', columns: ['date'])] #[ORM\Index(name: 'phplist_user_user_history_userididx', columns: ['userid'])] -class SubscriberHistory implements DomainModel, Identity +class SubscriberHistory implements + DomainModel, + Identity, + SearchIndexableInterface, + SubscriberHistoryRecordInterface { + public const SEARCH_INDEX_NAME = 'subscriber_history'; + public const MAX_RESULTS_BY_USER = 1000; + #[ORM\Id] #[ORM\Column(type: 'integer')] #[ORM\GeneratedValue] private ?int $id = null; - #[ORM\ManyToOne(targetEntity: Subscriber::class)] + #[ORM\ManyToOne(targetEntity: Subscriber::class, inversedBy: 'historyRecords')] #[ORM\JoinColumn(name: 'userid', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')] private Subscriber $subscriber; @@ -56,6 +66,11 @@ public function getSubscriber(): Subscriber return $this->subscriber; } + public function getSubscriberId(): ?int + { + return $this->subscriber->getId(); + } + public function getIp(): ?string { return $this->ip; @@ -110,4 +125,28 @@ public function setSystemInfo(?string $systemInfo): self $this->systemInfo = $systemInfo; return $this; } + + public function getSearchIndexName(): string + { + return self::SEARCH_INDEX_NAME; + } + + public function getSearchDocumentId(): string + { + return (string) $this->id; + } + + public function toSearchDocument(): array + { + return [ + 'id' => $this->id, + 'idSort' => $this->id, + 'subscriberId' => $this->getSubscriberId(), + 'ip' => $this->ip, + 'date' => $this->createdAt?->format(DateTimeInterface::ATOM), + 'summary' => $this->summary, + 'detail' => $this->detail, + 'systemInfo' => $this->systemInfo, + ]; + } } diff --git a/src/Domain/Subscription/Repository/Interfaces/SubscriberHistoryReaderInterface.php b/src/Domain/Subscription/Repository/Interfaces/SubscriberHistoryReaderInterface.php new file mode 100644 index 00000000..3b86e400 --- /dev/null +++ b/src/Domain/Subscription/Repository/Interfaces/SubscriberHistoryReaderInterface.php @@ -0,0 +1,24 @@ + */ + public function getFilteredAfterId(FilterRequestInterface $filter): PaginatedResult; + + /** @return SubscriberHistoryRecordInterface[] */ + public function getBySubscriber(Subscriber $subscriber): array; +} diff --git a/src/Domain/Subscription/Repository/SubscriberHistoryConfigurableReader.php b/src/Domain/Subscription/Repository/SubscriberHistoryConfigurableReader.php new file mode 100644 index 00000000..c96af077 --- /dev/null +++ b/src/Domain/Subscription/Repository/SubscriberHistoryConfigurableReader.php @@ -0,0 +1,44 @@ +activeReader()->getFilteredAfterId($filter); + } + + /** @return SubscriberHistoryRecordInterface[] */ + public function getBySubscriber(Subscriber $subscriber): array + { + return $this->activeReader()->getBySubscriber($subscriber); + } + + private function activeReader(): SubscriberHistoryReaderInterface + { + return $this->elasticsearchEnabled ? $this->elasticsearchReader : $this->databaseReader; + } +} diff --git a/src/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReader.php b/src/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReader.php new file mode 100644 index 00000000..deb109fb --- /dev/null +++ b/src/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReader.php @@ -0,0 +1,116 @@ +getSubscriber() !== null) { + $mustFilters[] = ['term' => ['subscriberId' => $filter->getSubscriber()->getId()]]; + } + + if ($filter->getDateFrom() !== null) { + $mustFilters[] = ['range' => ['date' => ['gte' => $filter->getDateFrom()->format(DATE_ATOM)]]]; + } + + if ($filter->getIp() !== null) { + $mustFilters[] = ['term' => ['ip' => $filter->getIp()]]; + } + + if ($filter->getSummery() !== null) { + $mustFilters[] = ['term' => ['summary.keyword' => $filter->getSummery()]]; + } + + $mustFilters[] = ['range' => ['idSort' => ['gt' => $filter->getLastId()]]]; + + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['bool' => ['filter' => $mustFilters]], + 'sort' => [['idSort' => 'asc']], + 'size' => $filter->getLimit(), + 'track_total_hits' => true, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + $lastHit = $hits !== [] ? $hits[array_key_last($hits)] : null; + + return new PaginatedResult( + items: array_map($this->hydrate(...), $hits), + total: (int) ($response['hits']['total']['value'] ?? 0), + limit: $filter->getLimit(), + lastId: $lastHit !== null ? (int) $lastHit['_source']['idSort'] : $filter->getLastId(), + ); + } + + /** @return SubscriberHistoryRecordInterface[] */ + public function getBySubscriber(Subscriber $subscriber): array + { + $response = $this->client->search( + $this->resolvePhysicalIndexName(), + [ + 'query' => ['term' => ['subscriberId' => $subscriber->getId()]], + 'sort' => [['idSort' => 'desc']], + 'size' => SubscriberHistory::MAX_RESULTS_BY_USER, + ], + ); + + $hits = $response['hits']['hits'] ?? []; + + return array_map($this->hydrate(...), $hits); + } + + /** @param array{_source: array} $hit */ + private function hydrate(array $hit): SubscriberHistoryReadModel + { + $source = $hit['_source']; + + return new SubscriberHistoryReadModel( + id: isset($source['id']) ? (int) $source['id'] : null, + subscriberId: isset($source['subscriberId']) ? (int) $source['subscriberId'] : null, + ip: $source['ip'] ?? null, + createdAt: isset($source['date']) ? DateTime::createFromFormat(DATE_ATOM, $source['date']) ?: null : null, + summary: $source['summary'] ?? null, + detail: $source['detail'] ?? null, + systemInfo: $source['systemInfo'] ?? null, + ); + } + + private function resolvePhysicalIndexName(): string + { + return $this->indexPrefix . SubscriberHistory::SEARCH_INDEX_NAME; + } +} diff --git a/src/Domain/Subscription/Repository/SubscriberHistoryRepository.php b/src/Domain/Subscription/Repository/SubscriberHistoryRepository.php index 137faa84..107e5d84 100644 --- a/src/Domain/Subscription/Repository/SubscriberHistoryRepository.php +++ b/src/Domain/Subscription/Repository/SubscriberHistoryRepository.php @@ -4,6 +4,7 @@ namespace PhpList\Core\Domain\Subscription\Repository; +use DateTimeInterface; use InvalidArgumentException; use PhpList\Core\Domain\Common\Model\Filter\FilterRequestInterface; use PhpList\Core\Domain\Common\Model\PaginatedResult; @@ -13,8 +14,11 @@ use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberHistory; +use PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface; -class SubscriberHistoryRepository extends AbstractRepository implements PaginatableRepositoryInterface +class SubscriberHistoryRepository extends AbstractRepository implements + PaginatableRepositoryInterface, + SubscriberHistoryReaderInterface { use CursorPaginationTrait; @@ -82,7 +86,47 @@ public function getBySubscriber(Subscriber $subscriber): array ->andWhere('sh.subscriber = :subscriberId') ->setParameter('subscriberId', $subscriber->getId()) ->orderBy('sh.id', 'DESC') + ->setMaxResults(SubscriberHistory::MAX_RESULTS_BY_USER) ->getQuery() ->getResult(); } + + public function countOlderThan(DateTimeInterface $cutoff): int + { + return (int) $this->createQueryBuilder('sh') + ->select('COUNT(sh.id)') + ->andWhere('sh.createdAt < :cutoff') + ->setParameter('cutoff', $cutoff) + ->getQuery() + ->getSingleScalarResult(); + } + + /** @return iterable */ + public function fetchBatchOlderThan(DateTimeInterface $cutoff, int $lastId, int $batchSize): iterable + { + return $this->createQueryBuilder('sh') + ->andWhere('sh.id > :lastId') + ->andWhere('sh.createdAt < :cutoff') + ->setParameter('lastId', $lastId) + ->setParameter('cutoff', $cutoff) + ->orderBy('sh.id', 'ASC') + ->setMaxResults($batchSize) + ->getQuery() + ->toIterable(); + } + + /** @param int[] $ids */ + public function deleteByIds(array $ids): int + { + if ($ids === []) { + return 0; + } + + return $this->createQueryBuilder('sh') + ->delete() + ->andWhere('sh.id IN (:ids)') + ->setParameter('ids', $ids) + ->getQuery() + ->execute(); + } } diff --git a/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php b/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php index bd36422d..5338580b 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberHistoryManager.php @@ -10,39 +10,40 @@ use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Subscription\Model\Dto\ChangeSetDto; use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter; +use PhpList\Core\Domain\Subscription\Model\Interfaces\SubscriberHistoryRecordInterface; use PhpList\Core\Domain\Subscription\Model\Subscriber; use PhpList\Core\Domain\Subscription\Model\SubscriberHistory; -use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; +use PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface; use Symfony\Contracts\Translation\TranslatorInterface; class SubscriberHistoryManager { - private SubscriberHistoryRepository $repository; + private SubscriberHistoryReaderInterface $reader; private ClientIpResolver $clientIpResolver; private SystemInfoCollector $systemInfoCollector; private TranslatorInterface $translator; private EntityManagerInterface $entityManager; public function __construct( - SubscriberHistoryRepository $repository, + SubscriberHistoryReaderInterface $reader, ClientIpResolver $clientIpResolver, SystemInfoCollector $systemInfoCollector, TranslatorInterface $translator, EntityManagerInterface $entityManager, ) { - $this->repository = $repository; + $this->reader = $reader; $this->clientIpResolver = $clientIpResolver; $this->systemInfoCollector = $systemInfoCollector; $this->translator = $translator; $this->entityManager = $entityManager; } - /** @return SubscriberHistory[] */ + /** @return SubscriberHistoryRecordInterface[] */ public function getHistory(int $lastId, int $limit, SubscriberHistoryFilter $filter): array { $filter->setLastId($lastId)->setLimit($limit); - return $this->repository->getFilteredAfterId($filter)->getItems(); + return $this->reader->getFilteredAfterId($filter)->getItems(); } public function addHistory(Subscriber $subscriber, string $message, ?string $details = null): SubscriberHistory diff --git a/src/Domain/Subscription/Service/Manager/SubscriberManager.php b/src/Domain/Subscription/Service/Manager/SubscriberManager.php index e13980ae..189a2881 100644 --- a/src/Domain/Subscription/Service/Manager/SubscriberManager.php +++ b/src/Domain/Subscription/Service/Manager/SubscriberManager.php @@ -11,7 +11,7 @@ use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Dto\UpdateSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Subscriber; -use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; +use PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; use PhpList\Core\Domain\Subscription\Service\SubscriberDeletionService; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; @@ -25,7 +25,7 @@ public function __construct( private readonly SubscriberDeletionService $subscriberDeletionService, private readonly TranslatorInterface $translator, private readonly SubscriberHistoryManager $subscriberHistoryManager, - private readonly SubscriberHistoryRepository $subscriberHistoryRepository, + private readonly SubscriberHistoryReaderInterface $subscriberHistoryReader, ) { } @@ -55,7 +55,7 @@ public function getSubscriberDetails(int $subscriberId): ?Subscriber return null; } - $history = $this->subscriberHistoryRepository->getBySubscriber($subscriber); + $history = $this->subscriberHistoryReader->getBySubscriber($subscriber); $subscriber->setHistory($history); return $subscriber; diff --git a/src/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinition.php b/src/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinition.php new file mode 100644 index 00000000..52171429 --- /dev/null +++ b/src/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinition.php @@ -0,0 +1,44 @@ + [ + 'id' => ['type' => 'keyword'], + // Numeric mirror of `id`, used for range/sort in cursor pagination - `id` stays a + // keyword for exact-match filtering. + 'idSort' => ['type' => 'long'], + 'subscriberId' => ['type' => 'keyword'], + 'ip' => ['type' => 'keyword'], + 'date' => ['type' => 'date'], + 'summary' => [ + 'type' => 'text', + 'fields' => [ + 'keyword' => ['type' => 'keyword', 'ignore_above' => 256], + ], + ], + 'detail' => ['type' => 'text'], + 'systemInfo' => ['type' => 'text'], + ], + ]; + } + + public function getSettings(): array + { + return []; + } +} diff --git a/src/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProvider.php b/src/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProvider.php new file mode 100644 index 00000000..20efdd83 --- /dev/null +++ b/src/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProvider.php @@ -0,0 +1,51 @@ +retentionPeriod !== '' ? new DateInterval($this->retentionPeriod) : null; + } + + public function getSearchIndexName(): string + { + return $this->indexPrefix . SubscriberHistory::SEARCH_INDEX_NAME; + } + + public function countOlderThan(DateTimeInterface $cutoff): int + { + return $this->repository->countOlderThan($cutoff); + } + + public function fetchBatchOlderThan(DateTimeInterface $cutoff, int $lastId, int $batchSize): iterable + { + return $this->repository->fetchBatchOlderThan($cutoff, $lastId, $batchSize); + } + + public function deleteByIds(array $ids): int + { + return $this->repository->deleteByIds($ids); + } +} diff --git a/src/Domain/Subscription/Service/Search/SubscriberHistoryReindexProvider.php b/src/Domain/Subscription/Service/Search/SubscriberHistoryReindexProvider.php new file mode 100644 index 00000000..e32ced1d --- /dev/null +++ b/src/Domain/Subscription/Service/Search/SubscriberHistoryReindexProvider.php @@ -0,0 +1,40 @@ +repository->createQueryBuilder('sh') + ->select('COUNT(sh.id)') + ->getQuery() + ->getSingleScalarResult(); + } + + public function fetchBatch(int $lastId, int $batchSize): iterable + { + return $this->repository->createQueryBuilder('sh') + ->andWhere('sh.id > :lastId') + ->setParameter('lastId', $lastId) + ->orderBy('sh.id', 'ASC') + ->setMaxResults($batchSize) + ->getQuery() + ->toIterable(); + } +} diff --git a/tests/Integration/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReaderTest.php b/tests/Integration/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReaderTest.php new file mode 100644 index 00000000..4459a4da --- /dev/null +++ b/tests/Integration/Domain/Messaging/Repository/UserMessageBounceElasticsearchHybridReaderTest.php @@ -0,0 +1,318 @@ +loadSchema(); + + $this->client = $this->createMock(ElasticsearchClientInterface::class); + $this->reader = new UserMessageBounceElasticsearchHybridReader($this->client, 'phplist_', $this->entityManager); + } + + protected function tearDown(): void + { + $schemaTool = new SchemaTool($this->entityManager); + $schemaTool->dropDatabase(); + parent::tearDown(); + } + + public function testGetListBounceTotalsMergesElasticsearchCountsWithSubscriberData(): void + { + $admin = (new Administrator()) + ->setLoginName('admin') + ->setEmail('admin@example.com'); + $this->entityManager->persist($admin); + + $targetList = (new SubscriberList())->setName('Target list')->setOwner($admin); + $otherList = (new SubscriberList())->setName('Other list')->setOwner($admin); + $this->entityManager->persist($targetList); + $this->entityManager->persist($otherList); + + $subscriber1 = (new Subscriber('one@example.com'))->setConfirmed(true)->setBlacklisted(false); + $subscriber2 = (new Subscriber('two@example.com'))->setConfirmed(false)->setBlacklisted(true); + $subscriber3 = (new Subscriber('three@example.com'))->setConfirmed(true)->setBlacklisted(false); + $this->entityManager->persist($subscriber1); + $this->entityManager->persist($subscriber2); + $this->entityManager->persist($subscriber3); + $this->entityManager->flush(); + + $subscription1 = (new Subscription())->setSubscriber($subscriber1)->setSubscriberList($targetList); + $subscription2 = (new Subscription())->setSubscriber($subscriber2)->setSubscriberList($targetList); + $subscription3 = (new Subscription())->setSubscriber($subscriber3)->setSubscriberList($otherList); + $this->entityManager->persist($subscription1); + $this->entityManager->persist($subscription2); + $this->entityManager->persist($subscription3); + $this->entityManager->flush(); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query) use ($subscriber1, $subscriber2): bool { + return $query['query']['bool']['filter'][0]['terms']['userId'] === [ + $subscriber1->getId(), + $subscriber2->getId(), + ]; + }), + ) + ->willReturn([ + 'aggregations' => [ + 'by_field' => [ + 'buckets' => [ + ['key' => $subscriber1->getId(), 'doc_count' => 2], + ['key' => $subscriber2->getId(), 'doc_count' => 1], + ], + ], + ], + ]); + + $rows = $this->reader->getListBounceTotals($targetList->getId()); + + self::assertSame( + [ + [ + 'subscriber_id' => $subscriber1->getId(), + 'email' => 'one@example.com', + 'confirmed' => true, + 'blacklisted' => false, + 'total_bounces' => 2, + ], + [ + 'subscriber_id' => $subscriber2->getId(), + 'email' => 'two@example.com', + 'confirmed' => false, + 'blacklisted' => true, + 'total_bounces' => 1, + ], + ], + $rows + ); + } + + public function testGetCampaignBounceTotalsMergesElasticsearchCountsWithMessageData(): void + { + $admin = (new Administrator()) + ->setLoginName('admin') + ->setEmail('admin@example.com'); + $this->entityManager->persist($admin); + $this->entityManager->flush(); + + $message1 = $this->createMessage('Campaign one', $admin); + $message2 = $this->createMessage('Campaign two', $admin); + $this->entityManager->persist($message1); + $this->entityManager->persist($message2); + $this->entityManager->flush(); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query) use ($message1, $message2): bool { + return $query['query']['bool']['filter'][0]['terms']['messageId'] === [ + $message1->getId(), + $message2->getId(), + ]; + }), + ) + ->willReturn([ + 'aggregations' => [ + 'by_field' => [ + 'buckets' => [ + ['key' => $message1->getId(), 'doc_count' => 4], + ], + ], + ], + ]); + + $rows = $this->reader->getCampaignBounceTotals(); + + self::assertSame( + [ + [ + 'message_id' => $message1->getId(), + 'subject' => 'Campaign one', + 'total_bounces' => 4, + ], + ], + $rows + ); + } + + public function testGetPaginatedWithJoinNoRelationHydratesMatchingBounceEntitiesAndSkipsMissingOnes(): void + { + $bounce1 = new Bounce(status: 'new'); + $bounce2 = new Bounce(status: 'processed'); + $this->entityManager->persist($bounce1); + $this->entityManager->persist($bounce2); + $this->entityManager->flush(); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query): bool { + return $query['query'] === ['range' => ['idSort' => ['gt' => 0]]] + && $query['size'] === 10; + }), + ) + ->willReturn([ + 'hits' => [ + 'hits' => [ + ['_source' => [ + 'id' => 1, + 'idSort' => 1, + 'userId' => 5, + 'messageId' => 10, + 'bounceId' => $bounce1->getId(), + 'time' => '2026-01-01T00:00:00+00:00', + ]], + ['_source' => [ + 'id' => 2, + 'idSort' => 2, + 'userId' => 6, + 'messageId' => 11, + // No matching Bounce row - must be skipped, mirroring the SQL inner join. + 'bounceId' => 999999, + 'time' => '2026-01-02T00:00:00+00:00', + ]], + ['_source' => [ + 'id' => 3, + 'idSort' => 3, + 'userId' => 7, + 'messageId' => 12, + 'bounceId' => $bounce2->getId(), + 'time' => '2026-01-03T00:00:00+00:00', + ]], + ], + ], + ]); + + $rows = $this->reader->getPaginatedWithJoinNoRelation(0, 10); + + self::assertCount(2, $rows); + self::assertSame(1, $rows[0]['umb']->getId()); + self::assertSame($bounce1, $rows[0]['bounce']); + self::assertSame(3, $rows[1]['umb']->getId()); + self::assertSame($bounce2, $rows[1]['bounce']); + } + + public function testGetUserMessageHistoryWithBouncesMergesSentMessagesWithBounceDocs(): void + { + $admin = (new Administrator()) + ->setLoginName('admin') + ->setEmail('admin@example.com'); + $this->entityManager->persist($admin); + + $subscriber = new Subscriber('history@example.com'); + $this->entityManager->persist($subscriber); + $this->entityManager->flush(); + + $message1 = $this->createMessage('First', $admin); + $message2 = $this->createMessage('Second', $admin); + $this->entityManager->persist($message1); + $this->entityManager->persist($message2); + $this->entityManager->flush(); + + $bounce = new Bounce(status: 'new'); + $this->entityManager->persist($bounce); + $this->entityManager->flush(); + + $userMessage1 = new UserMessage($subscriber, $message1); + $userMessage1->setStatus(UserMessageStatus::Sent); + $userMessage2 = new UserMessage($subscriber, $message2); + $userMessage2->setStatus(UserMessageStatus::Sent); + $this->entityManager->persist($userMessage1); + $this->entityManager->persist($userMessage2); + $this->entityManager->flush(); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query) use ($subscriber): bool { + return $query['query'] === ['term' => ['userId' => $subscriber->getId()]]; + }), + ) + ->willReturn([ + 'hits' => [ + 'hits' => [ + ['_source' => [ + 'id' => 1, + 'idSort' => 1, + 'userId' => $subscriber->getId(), + 'messageId' => $message1->getId(), + 'bounceId' => $bounce->getId(), + 'time' => '2026-01-01T00:00:00+00:00', + ]], + ], + ], + ]); + + $rows = $this->reader->getUserMessageHistoryWithBounces($subscriber); + + self::assertCount(2, $rows); + + $rowsByMessageId = []; + foreach ($rows as $row) { + $rowsByMessageId[$row['um']->getMessage()->getId()] = $row; + } + + self::assertSame($bounce, $rowsByMessageId[$message1->getId()]['b']); + self::assertSame($bounce->getId(), $rowsByMessageId[$message1->getId()]['umb']->getBounceId()); + self::assertNull($rowsByMessageId[$message2->getId()]['b']); + self::assertNull($rowsByMessageId[$message2->getId()]['umb']); + } + + private function createMessage(string $subject, Administrator $owner): Message + { + return new Message( + new MessageFormat(true, 'text'), + new MessageSchedule(null, null, null, null, null), + new MessageMetadata(), + new MessageContent($subject), + new MessageOptions(), + $owner + ); + } +} diff --git a/tests/Integration/Domain/Messaging/Repository/UserMessageBounceRepositoryTest.php b/tests/Integration/Domain/Messaging/Repository/UserMessageBounceRepositoryTest.php index ce3a0e9f..d7c82326 100644 --- a/tests/Integration/Domain/Messaging/Repository/UserMessageBounceRepositoryTest.php +++ b/tests/Integration/Domain/Messaging/Repository/UserMessageBounceRepositoryTest.php @@ -8,6 +8,7 @@ use Doctrine\ORM\Tools\SchemaTool; use PhpList\Core\Domain\Identity\Model\Administrator; use PhpList\Core\Domain\Messaging\Model\Bounce; +use PhpList\Core\Domain\Messaging\Model\Filter\UserMessageBounceFilter; use PhpList\Core\Domain\Messaging\Model\UserMessageBounce; use PhpList\Core\Domain\Messaging\Repository\UserMessageBounceRepository; use PhpList\Core\Domain\Subscription\Model\Subscriber; @@ -136,4 +137,42 @@ public function testGetListBounceTotalsReturnsAggregatedBouncesPerSubscriberForL $rows ); } + + public function testGetFilteredAfterIdAdvancesCursorAcrossConsecutivePages(): void + { + $bounce = new Bounce(status: 'new'); + $this->entityManager->persist($bounce); + $this->entityManager->flush(); + + $umb1 = (new UserMessageBounce($bounce->getId(), new DateTime()))->setUserId(1)->setMessageId(10); + $umb2 = (new UserMessageBounce($bounce->getId(), new DateTime()))->setUserId(2)->setMessageId(11); + $this->entityManager->persist($umb1); + $this->entityManager->persist($umb2); + $this->entityManager->flush(); + + $firstPage = $this->repository->getFilteredAfterId(new UserMessageBounceFilter(lastId: 0, limit: 1)); + + self::assertCount(1, $firstPage->getItems()); + self::assertSame($umb1->getId(), $firstPage->getItems()[0]->getId()); + self::assertSame($umb1->getId(), $firstPage->getLastId()); + + $secondPage = $this->repository->getFilteredAfterId( + new UserMessageBounceFilter(lastId: $firstPage->getLastId(), limit: 1) + ); + + self::assertCount(1, $secondPage->getItems()); + self::assertSame($umb2->getId(), $secondPage->getItems()[0]->getId()); + self::assertNotSame( + $firstPage->getItems()[0]->getId(), + $secondPage->getItems()[0]->getId(), + ); + } + + public function testGetFilteredAfterIdKeepsInputLastIdWhenPageIsEmpty(): void + { + $result = $this->repository->getFilteredAfterId(new UserMessageBounceFilter(lastId: 999, limit: 10)); + + self::assertSame([], $result->getItems()); + self::assertSame(999, $result->getLastId()); + } } diff --git a/tests/Integration/Domain/Subscription/Repository/SubscriberHistoryRepositoryTest.php b/tests/Integration/Domain/Subscription/Repository/SubscriberHistoryRepositoryTest.php new file mode 100644 index 00000000..c576618f --- /dev/null +++ b/tests/Integration/Domain/Subscription/Repository/SubscriberHistoryRepositoryTest.php @@ -0,0 +1,138 @@ +loadSchema(); + + $this->repository = self::getContainer()->get(SubscriberHistoryRepository::class); + } + + protected function tearDown(): void + { + $schemaTool = new SchemaTool($this->entityManager); + $schemaTool->dropDatabase(); + parent::tearDown(); + } + + private function persistHistoryRow(DateTime $date): SubscriberHistory + { + $subscriber = new Subscriber('subscriber-' . uniqid('', true) . '@example.com'); + $this->entityManager->persist($subscriber); + $this->entityManager->flush(); + + $history = new SubscriberHistory($subscriber); + $reflection = new ReflectionProperty(SubscriberHistory::class, 'createdAt'); + $reflection->setAccessible(true); + $reflection->setValue($history, $date); + + $this->entityManager->persist($history); + $this->entityManager->flush(); + + return $history; + } + + public function testCountOlderThanOnlyCountsRowsBeforeCutoff(): void + { + $old = $this->persistHistoryRow(new DateTime('2020-01-01')); + $this->persistHistoryRow(new DateTime('2030-01-01')); + + $count = $this->repository->countOlderThan(new DateTime('2025-01-01')); + + self::assertSame(1, $count); + self::assertNotNull($old->getId()); + } + + public function testFetchBatchOlderThanReturnsOnlyMatchingRowsInIdOrder(): void + { + $old1 = $this->persistHistoryRow(new DateTime('2020-01-01')); + $old2 = $this->persistHistoryRow(new DateTime('2020-06-01')); + $this->persistHistoryRow(new DateTime('2030-01-01')); + + $batch = [...$this->repository->fetchBatchOlderThan(new DateTime('2025-01-01'), 0, 10)]; + + self::assertCount(2, $batch); + self::assertSame($old1->getId(), $batch[0]->getId()); + self::assertSame($old2->getId(), $batch[1]->getId()); + } + + public function testFetchBatchOlderThanRespectsLastIdCursor(): void + { + $old1 = $this->persistHistoryRow(new DateTime('2020-01-01')); + $old2 = $this->persistHistoryRow(new DateTime('2020-06-01')); + + $batch = [...$this->repository->fetchBatchOlderThan(new DateTime('2025-01-01'), $old1->getId(), 10)]; + + self::assertCount(1, $batch); + self::assertSame($old2->getId(), $batch[0]->getId()); + } + + public function testDeleteByIdsRemovesOnlyGivenRows(): void + { + $toDelete = $this->persistHistoryRow(new DateTime('2020-01-01')); + $toKeep = $this->persistHistoryRow(new DateTime('2020-06-01')); + + $deleted = $this->repository->deleteByIds([$toDelete->getId()]); + // bulk DQL delete bypasses the identity map + $this->entityManager->clear(); + + self::assertSame(1, $deleted); + self::assertSame(1, $this->repository->countOlderThan(new DateTime('2025-01-01'))); + self::assertNotNull($this->repository->find($toKeep->getId())); + self::assertNull($this->repository->find($toDelete->getId())); + } + + public function testDeleteByIdsWithEmptyArrayDeletesNothing(): void + { + $this->persistHistoryRow(new DateTime('2020-01-01')); + + $deleted = $this->repository->deleteByIds([]); + + self::assertSame(0, $deleted); + } + + public function testRemovingSubscriberCascadeDeletesItsHistoryRecords(): void + { + $subscriber = new Subscriber('cascade-' . uniqid('', true) . '@example.com'); + $this->entityManager->persist($subscriber); + $this->entityManager->flush(); + + $history = new SubscriberHistory($subscriber); + $this->entityManager->persist($history); + $this->entityManager->flush(); + $historyId = $history->getId(); + $subscriberId = $subscriber->getId(); + + // Removing the Subscriber directly (not via SubscriberDeletionService) must still cascade to + // SubscriberHistory through Doctrine, not rely solely on the DB-level ON DELETE CASCADE, so + // SearchIndexDoctrineListener::preRemove/postRemove fires for the history row too. Cascade + // remove only walks a *loaded* collection, so re-fetch the Subscriber fresh from the DB first, + // as any real caller doing this outside of SubscriberDeletionService's manual loop would. + $this->entityManager->clear(); + $fetchedSubscriber = $this->entityManager->find(Subscriber::class, $subscriberId); + $this->entityManager->remove($fetchedSubscriber); + $this->entityManager->flush(); + $this->entityManager->clear(); + + self::assertNull($this->repository->find($historyId)); + } +} diff --git a/tests/Unit/Core/Doctrine/SearchIndexDoctrineListenerTest.php b/tests/Unit/Core/Doctrine/SearchIndexDoctrineListenerTest.php new file mode 100644 index 00000000..5be2f4d4 --- /dev/null +++ b/tests/Unit/Core/Doctrine/SearchIndexDoctrineListenerTest.php @@ -0,0 +1,173 @@ +messageBus = $this->createMock(MessageBusInterface::class); + $this->objectManager = $this->createMock(EntityManagerInterface::class); + $this->listener = new SearchIndexDoctrineListener($this->messageBus); + } + + private function createIndexable(string $indexName, string $documentId, array $document): SearchIndexableInterface + { + $entity = $this->createMock(SearchIndexableInterface::class); + $entity->method('getSearchIndexName')->willReturn($indexName); + $entity->method('getSearchDocumentId')->willReturn($documentId); + $entity->method('toSearchDocument')->willReturn($document); + + return $entity; + } + + public function testPostPersistDoesNotDispatchBeforePostFlush(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus->expects($this->never())->method('dispatch'); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + } + + public function testPostFlushDispatchesBufferedIndexMessage(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus + ->expects($this->once()) + ->method('dispatch') + ->with($this->callback(function (IndexDocumentMessage $message): bool { + return $message->getIndexName() === 'subscriber_history' + && $message->getDocumentId() === '1' + && $message->getDocument() === ['id' => 1] + && $message->getOperation() === SearchOperation::Index + && $message->getRevision() > 0; + })) + ->willReturn(new Envelope(new stdClass())); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testRevisionsAreMonotonicallyIncreasingAcrossFlushes(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + $revisions = []; + + $this->messageBus + ->expects($this->exactly(2)) + ->method('dispatch') + ->with($this->callback(function (IndexDocumentMessage $message) use (&$revisions): bool { + $revisions[] = $message->getRevision(); + + return true; + })) + ->willReturn(new Envelope(new stdClass())); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + + $this->listener->postUpdate(new PostUpdateEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + + $this->assertCount(2, $revisions); + $this->assertGreaterThanOrEqual($revisions[0], $revisions[1]); + } + + public function testPostRemoveBuffersDeleteOperationWithEmptyDocument(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus + ->expects($this->once()) + ->method('dispatch') + ->with($this->callback(function (IndexDocumentMessage $message): bool { + return $message->getOperation() === SearchOperation::Delete + && $message->getDocument() === []; + })) + ->willReturn(new Envelope(new stdClass())); + + $this->listener->postRemove(new PostRemoveEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testMultipleTouchesInOneFlushDedupeToOneDispatch(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus->expects($this->once())->method('dispatch') + ->willReturn(new Envelope(new stdClass())); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $this->listener->postUpdate(new PostUpdateEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testPostFlushWithNothingBufferedDoesNotDispatch(): void + { + $this->messageBus->expects($this->never())->method('dispatch'); + + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testNonSearchIndexableEntityIsIgnored(): void + { + $entity = new stdClass(); + + $this->messageBus->expects($this->never())->method('dispatch'); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testPendingBufferIsClearedAfterDispatch(): void + { + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus->expects($this->once())->method('dispatch') + ->willReturn(new Envelope(new stdClass())); + + $this->listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + + // A second postFlush with nothing new queued must not re-dispatch the same message. + $this->listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } + + public function testDisabledListenerNeverQueuesOrDispatchesAnything(): void + { + $listener = new SearchIndexDoctrineListener($this->messageBus, enabled: false); + $entity = $this->createIndexable('subscriber_history', '1', ['id' => 1]); + + $this->messageBus->expects($this->never())->method('dispatch'); + + $listener->postPersist(new PostPersistEventArgs($entity, $this->objectManager)); + $listener->postUpdate(new PostUpdateEventArgs($entity, $this->objectManager)); + $listener->preRemove(new PreRemoveEventArgs($entity, $this->objectManager)); + $listener->postRemove(new PostRemoveEventArgs($entity, $this->objectManager)); + $listener->postFlush(new PostFlushEventArgs($this->objectManager)); + } +} diff --git a/tests/Unit/Domain/Analytics/Service/AnalyticsServiceTest.php b/tests/Unit/Domain/Analytics/Service/AnalyticsServiceTest.php index a7558747..3b4cb7f9 100644 --- a/tests/Unit/Domain/Analytics/Service/AnalyticsServiceTest.php +++ b/tests/Unit/Domain/Analytics/Service/AnalyticsServiceTest.php @@ -17,8 +17,8 @@ use PhpList\Core\Domain\Messaging\Model\Message; use PhpList\Core\Domain\Messaging\Model\Message\MessageContent; use PhpList\Core\Domain\Messaging\Model\Message\MessageMetadata; +use PhpList\Core\Domain\Messaging\Repository\Interfaces\UserMessageBounceReaderInterface; use PhpList\Core\Domain\Messaging\Repository\MessageRepository; -use PhpList\Core\Domain\Messaging\Repository\UserMessageBounceRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageForwardRepository; use PhpList\Core\Domain\Messaging\Repository\UserMessageRepository; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; @@ -31,7 +31,7 @@ class AnalyticsServiceTest extends TestCase private LinkTrackManager|MockObject $linkTrackManager; private UserMessageViewManager|MockObject $userMessageViewManager; private MessageRepository|MockObject $messageRepository; - private UserMessageBounceRepository|MockObject $userMessageBounceRepository; + private UserMessageBounceReaderInterface|MockObject $userMessageBounceReader; private UserMessageForwardRepository|MockObject $userMessageForwardRepository; private SubscriberRepository|MockObject $subscriberRepository; private UserMessageRepository|MockObject $userMessageRepository; @@ -42,7 +42,7 @@ protected function setUp(): void $this->linkTrackManager = $this->createMock(LinkTrackManager::class); $this->userMessageViewManager = $this->createMock(UserMessageViewManager::class); $this->messageRepository = $this->createMock(MessageRepository::class); - $this->userMessageBounceRepository = $this->createMock(UserMessageBounceRepository::class); + $this->userMessageBounceReader = $this->createMock(UserMessageBounceReaderInterface::class); $this->userMessageForwardRepository = $this->createMock(UserMessageForwardRepository::class); $this->subscriberRepository = $this->createMock(SubscriberRepository::class); $this->userMessageRepository = $this->createMock(UserMessageRepository::class); @@ -52,7 +52,7 @@ protected function setUp(): void $this->linkTrackManager, $this->userMessageViewManager, $this->messageRepository, - $this->userMessageBounceRepository, + $this->userMessageBounceReader, $this->userMessageForwardRepository, $this->subscriberRepository, $this->userMessageRepository, @@ -109,7 +109,7 @@ public function testGetCampaignStatistics(): void ->with($messageId) ->willReturn([$linkTrack1, $linkTrack2]); - $this->userMessageBounceRepository->expects(self::once()) + $this->userMessageBounceReader->expects(self::once()) ->method('getCountByMessageId') ->with($messageId) ->willReturn(3); @@ -291,7 +291,7 @@ public function testGetSummaryStatistics(): void $this->userMessageRepository->method('countSentBetween')->willReturnOnConsecutiveCalls(500, 400); $this->userMessageViewRepository->method('countBetween')->willReturnOnConsecutiveCalls(250, 160); - $this->userMessageBounceRepository->method('countBetween')->willReturnOnConsecutiveCalls(10, 8); + $this->userMessageBounceReader->method('countBetween')->willReturnOnConsecutiveCalls(10, 8); $result = $this->subject->getSummaryStatistics(); diff --git a/tests/Unit/Domain/Messaging/Repository/UserMessageBounceConfigurableReaderTest.php b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceConfigurableReaderTest.php new file mode 100644 index 00000000..9a2298f0 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceConfigurableReaderTest.php @@ -0,0 +1,76 @@ +databaseReader = $this->createMock(UserMessageBounceRepository::class); + $this->elasticsearchReader = $this->createMock(UserMessageBounceElasticsearchReader::class); + } + + public function testDelegatesToElasticsearchWhenEnabled(): void + { + $reader = new UserMessageBounceConfigurableReader($this->databaseReader, $this->elasticsearchReader, true); + + $this->elasticsearchReader->expects($this->once()) + ->method('getCountByMessageId') + ->with(42) + ->willReturn(7); + $this->databaseReader->expects($this->never())->method('getCountByMessageId'); + + $this->assertSame(7, $reader->getCountByMessageId(42)); + } + + public function testDelegatesToDatabaseWhenDisabled(): void + { + $reader = new UserMessageBounceConfigurableReader($this->databaseReader, $this->elasticsearchReader, false); + $start = new DateTime('2026-01-01'); + $end = new DateTime('2026-01-31'); + + $this->databaseReader->expects($this->once()) + ->method('countBetween') + ->with($start, $end) + ->willReturn(3); + $this->elasticsearchReader->expects($this->never())->method('countBetween'); + + $this->assertSame(3, $reader->countBetween($start, $end)); + } + + public function testExistsByMessageIdAndUserIdDelegatesToActiveReader(): void + { + $reader = new UserMessageBounceConfigurableReader($this->databaseReader, $this->elasticsearchReader, true); + + $this->elasticsearchReader->expects($this->once()) + ->method('existsByMessageIdAndUserId') + ->with(5, 9) + ->willReturn(true); + + $this->assertTrue($reader->existsByMessageIdAndUserId(5, 9)); + } + + public function testGetByUserIdDelegatesToActiveReader(): void + { + $reader = new UserMessageBounceConfigurableReader($this->databaseReader, $this->elasticsearchReader, false); + + $this->databaseReader->expects($this->once()) + ->method('getByUserId') + ->with(3) + ->willReturn([]); + + $this->assertSame([], $reader->getByUserId(3)); + } +} diff --git a/tests/Unit/Domain/Messaging/Repository/UserMessageBounceElasticsearchReaderTest.php b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceElasticsearchReaderTest.php new file mode 100644 index 00000000..f195ada3 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceElasticsearchReaderTest.php @@ -0,0 +1,231 @@ +client = $this->createMock(ElasticsearchClientInterface::class); + $this->reader = new UserMessageBounceElasticsearchReader($this->client, 'phplist_'); + } + + public function testGetFilteredAfterIdQueriesPrefixedIndexAndHydratesResults(): void + { + $filter = new UserMessageBounceFilter(lastId: 5, limit: 10); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query): bool { + return $query['size'] === 10 + && $query['query']['bool']['filter'][0] === ['range' => ['idSort' => ['gt' => 5]]]; + }), + ) + ->willReturn([ + 'hits' => [ + 'total' => ['value' => 1], + 'hits' => [ + ['_source' => [ + 'id' => 7, + 'idSort' => 7, + 'userId' => 3, + 'messageId' => 42, + 'bounceId' => 99, + 'time' => '2026-01-01T00:00:00+00:00', + ]], + ], + ], + ]); + + $result = $this->reader->getFilteredAfterId($filter); + + $this->assertSame(1, $result->getTotal()); + $this->assertCount(1, $result->getItems()); + $this->assertSame(7, $result->getItems()[0]->getId()); + $this->assertSame(3, $result->getItems()[0]->getUserId()); + $this->assertSame(42, $result->getItems()[0]->getMessageId()); + $this->assertSame(99, $result->getItems()[0]->getBounceId()); + } + + public function testGetFilteredAfterIdPaginatesAcrossTwoPagesWithoutRepeatingResults(): void + { + $firstFilter = new UserMessageBounceFilter(lastId: 0, limit: 1); + $expectedCursors = [0, 5]; + $call = 0; + + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query) use (&$call, $expectedCursors): bool { + $expectedCursor = $expectedCursors[$call]; + $call++; + + return $query['query']['bool']['filter'][0] === [ + 'range' => [ + 'idSort' => ['gt' => $expectedCursor] + ] + ]; + }), + ) + ->willReturnOnConsecutiveCalls( + [ + 'hits' => [ + 'total' => ['value' => 2], + 'hits' => [ + ['_source' => [ + 'id' => 5, + 'idSort' => 5, + 'userId' => 1, + 'messageId' => 10, + 'bounceId' => 20, + 'time' => '2026-01-01T00:00:00+00:00', + ]], + ], + ], + ], + [ + 'hits' => [ + 'total' => ['value' => 2], + 'hits' => [ + ['_source' => [ + 'id' => 8, + 'idSort' => 8, + 'userId' => 2, + 'messageId' => 11, + 'bounceId' => 21, + 'time' => '2026-01-02T00:00:00+00:00', + ]], + ], + ], + ], + ); + + $firstPage = $this->reader->getFilteredAfterId($firstFilter); + + $this->assertSame(5, $firstPage->getLastId()); + $this->assertSame(5, $firstPage->getItems()[0]->getId()); + + $secondFilter = new UserMessageBounceFilter(lastId: $firstPage->getLastId(), limit: 1); + $secondPage = $this->reader->getFilteredAfterId($secondFilter); + + $this->assertSame(8, $secondPage->getLastId()); + $this->assertSame(8, $secondPage->getItems()[0]->getId()); + $this->assertNotSame( + $firstPage->getItems()[0]->getId(), + $secondPage->getItems()[0]->getId(), + ); + } + + public function testGetFilteredAfterIdRejectsWrongFilterType(): void + { + $wrongFilter = $this->createMock(FilterRequestInterface::class); + + $this->expectException(InvalidArgumentException::class); + $this->reader->getFilteredAfterId($wrongFilter); + } + + public function testGetByUserIdSortsDescending(): void + { + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query): bool { + return $query['query'] === ['term' => ['userId' => 9]] + && $query['sort'] === [['idSort' => 'desc']]; + }), + ) + ->willReturn(['hits' => ['hits' => []]]); + + $result = $this->reader->getByUserId(9); + + $this->assertSame([], $result); + } + + public function testGetCountByMessageIdQueriesTotalHitsForMessage(): void + { + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query): bool { + return $query['query'] === ['term' => ['messageId' => 42]] + && $query['size'] === 0; + }), + ) + ->willReturn(['hits' => ['total' => ['value' => 7]]]); + + $this->assertSame(7, $this->reader->getCountByMessageId(42)); + } + + public function testCountBetweenQueriesTimeRange(): void + { + $start = new DateTime('2026-01-01 00:00:00'); + $end = new DateTime('2026-01-31 23:59:59'); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query) use ($start, $end): bool { + return $query['query'] === ['range' => ['time' => [ + 'gte' => $start->format(DATE_ATOM), + 'lte' => $end->format(DATE_ATOM), + ]]]; + }), + ) + ->willReturn(['hits' => ['total' => ['value' => 3]]]); + + $this->assertSame(3, $this->reader->countBetween($start, $end)); + } + + public function testExistsByMessageIdAndUserIdReturnsTrueWhenHitsExist(): void + { + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_user_message_bounce', + $this->callback(function (array $query): bool { + return $query['query']['bool']['filter'] === [ + ['term' => ['messageId' => 5]], + ['term' => ['userId' => 9]], + ]; + }), + ) + ->willReturn(['hits' => ['total' => ['value' => 1]]]); + + $this->assertTrue($this->reader->existsByMessageIdAndUserId(5, 9)); + } + + public function testExistsByMessageIdAndUserIdReturnsFalseWhenNoHits(): void + { + $this->client + ->method('search') + ->willReturn(['hits' => ['total' => ['value' => 0]]]); + + $this->assertFalse($this->reader->existsByMessageIdAndUserId(5, 9)); + } +} diff --git a/tests/Unit/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReaderTest.php b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReaderTest.php new file mode 100644 index 00000000..56a16a89 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Repository/UserMessageBounceReportConfigurableReaderTest.php @@ -0,0 +1,60 @@ +databaseReader = $this->createMock(UserMessageBounceRepository::class); + $this->elasticsearchReader = $this->createMock(UserMessageBounceElasticsearchHybridReader::class); + } + + public function testGetListBounceTotalsDelegatesToElasticsearchWhenEnabled(): void + { + $reader = new UserMessageBounceReportConfigurableReader( + $this->databaseReader, + $this->elasticsearchReader, + true, + ); + $expected = [['subscriber_id' => 1, 'email' => 'a@example.com', 'confirmed' => true, + 'blacklisted' => false, 'total_bounces' => 2]]; + + $this->elasticsearchReader->expects($this->once()) + ->method('getListBounceTotals') + ->with(10) + ->willReturn($expected); + $this->databaseReader->expects($this->never())->method('getListBounceTotals'); + + $this->assertSame($expected, $reader->getListBounceTotals(10)); + } + + public function testGetCampaignBounceTotalsDelegatesToDatabaseWhenDisabled(): void + { + $reader = new UserMessageBounceReportConfigurableReader( + $this->databaseReader, + $this->elasticsearchReader, + false, + ); + $expected = [['message_id' => 1, 'subject' => 'Hello', 'total_bounces' => 4]]; + + $this->databaseReader->expects($this->once()) + ->method('getCampaignBounceTotals') + ->with(7) + ->willReturn($expected); + $this->elasticsearchReader->expects($this->never())->method('getCampaignBounceTotals'); + + $this->assertSame($expected, $reader->getCampaignBounceTotals(7)); + } +} diff --git a/tests/Unit/Domain/Messaging/Service/Manager/BounceManagerTest.php b/tests/Unit/Domain/Messaging/Service/Manager/BounceManagerTest.php index 3a07b0a0..043aa5f4 100644 --- a/tests/Unit/Domain/Messaging/Service/Manager/BounceManagerTest.php +++ b/tests/Unit/Domain/Messaging/Service/Manager/BounceManagerTest.php @@ -130,6 +130,11 @@ public function testLinkUserMessageBounceFlushesAndSetsFields(): void $bounce->method('getId')->willReturn(77); $dt = new DateTimeImmutable('2024-05-01 12:34:56'); + + $this->userMessageBounceRepository->expects($this->once()) + ->method('persist') + ->with($this->isInstanceOf(UserMessageBounce::class)); + $umb = $this->manager->linkUserMessageBounce($bounce, $dt, 123, 456); $this->assertSame(77, $umb->getBounceId()); diff --git a/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinitionTest.php b/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinitionTest.php new file mode 100644 index 00000000..29214e51 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceIndexDefinitionTest.php @@ -0,0 +1,37 @@ +assertSame('user_message_bounce', $definition->getIndexAlias()); + } + + public function testMappingDeclaresExpectedFields(): void + { + $definition = new UserMessageBounceIndexDefinition(); + $properties = $definition->getMapping()['properties']; + + foreach (['id', 'idSort', 'userId', 'messageId', 'bounceId', 'time'] as $field) { + $this->assertArrayHasKey($field, $properties); + } + $this->assertSame('long', $properties['idSort']['type']); + $this->assertSame('keyword', $properties['id']['type']); + } + + public function testSettingsAreEmptyByDefault(): void + { + $definition = new UserMessageBounceIndexDefinition(); + + $this->assertSame([], $definition->getSettings()); + } +} diff --git a/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceReindexProviderTest.php b/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceReindexProviderTest.php new file mode 100644 index 00000000..8009a0d9 --- /dev/null +++ b/tests/Unit/Domain/Messaging/Service/Search/UserMessageBounceReindexProviderTest.php @@ -0,0 +1,19 @@ +createMock(UserMessageBounceRepository::class)); + + $this->assertSame('user_message_bounce', $provider->getAlias()); + } +} diff --git a/tests/Unit/Domain/Search/Command/PurgeSearchIndexedRowsCommandTest.php b/tests/Unit/Domain/Search/Command/PurgeSearchIndexedRowsCommandTest.php new file mode 100644 index 00000000..a3046cd0 --- /dev/null +++ b/tests/Unit/Domain/Search/Command/PurgeSearchIndexedRowsCommandTest.php @@ -0,0 +1,123 @@ +createMock(SearchIndexableInterface::class); + $row->method('getSearchDocumentId')->willReturn((string) $id); + + return $row; + } + + private function commandTesterWithProviders(SearchPurgeProviderInterface ...$providers): CommandTester + { + $this->client = $this->createMock(ElasticsearchClientInterface::class); + $registry = new SearchPurgeProviderRegistry($providers); + $command = new PurgeSearchIndexedRowsCommand($registry, $this->client); + + $application = new Application(); + $application->add($command); + + return new CommandTester($command); + } + + public function testDeletesOnlyRowsConfirmedInElasticsearch(): void + { + $confirmedRow = $this->makeFakeRow(1); + $unconfirmedRow = $this->makeFakeRow(2); + + $provider = $this->createMock(SearchPurgeProviderInterface::class); + $provider->method('getAlias')->willReturn('some_alias'); + $provider->method('getRetentionPeriod')->willReturn(new DateInterval('P1M')); + $provider->method('getSearchIndexName')->willReturn('phplist_some_alias'); + $provider->method('fetchBatchOlderThan')->willReturnOnConsecutiveCalls( + [$confirmedRow, $unconfirmedRow], + [], + ); + + $tester = $this->commandTesterWithProviders($provider); + $this->client->method('search')->willReturn([ + 'hits' => ['hits' => [['_id' => '1']]], + ]); + + $provider->expects($this->once())->method('deleteByIds')->with([1])->willReturn(1); + + $tester->execute([]); + + $output = $tester->getDisplay(); + $this->assertStringContainsString('not found in', $output); + $this->assertStringContainsString('deleted 1 row(s), skipped 1 unconfirmed', $output); + $this->assertSame(0, $tester->getStatusCode()); + } + + public function testDryRunDoesNotDeleteAnything(): void + { + $row = $this->makeFakeRow(1); + + $provider = $this->createMock(SearchPurgeProviderInterface::class); + $provider->method('getAlias')->willReturn('some_alias'); + $provider->method('getRetentionPeriod')->willReturn(new DateInterval('P1M')); + $provider->method('getSearchIndexName')->willReturn('phplist_some_alias'); + $provider->method('fetchBatchOlderThan')->willReturnOnConsecutiveCalls([$row], []); + + $tester = $this->commandTesterWithProviders($provider); + $this->client->method('search')->willReturn(['hits' => ['hits' => [['_id' => '1']]]]); + + $provider->expects($this->never())->method('deleteByIds'); + + $tester->execute(['--dry-run' => true]); + + $this->assertStringContainsString('would delete', $tester->getDisplay()); + $this->assertSame(0, $tester->getStatusCode()); + } + + public function testSkipsProvidersWithoutARetentionPeriod(): void + { + $provider = $this->createMock(SearchPurgeProviderInterface::class); + $provider->method('getAlias')->willReturn('some_alias'); + $provider->method('getRetentionPeriod')->willReturn(null); + + $tester = $this->commandTesterWithProviders($provider); + $provider->expects($this->never())->method('fetchBatchOlderThan'); + + $tester->execute([]); + + $this->assertStringContainsString( + 'No purge providers with a configured retention period', + $tester->getDisplay(), + ); + $this->assertSame(0, $tester->getStatusCode()); + } + + public function testFailsWhenAliasHasNoRetentionPeriodConfigured(): void + { + $provider = $this->createMock(SearchPurgeProviderInterface::class); + $provider->method('getAlias')->willReturn('some_alias'); + $provider->method('getRetentionPeriod')->willReturn(null); + + $tester = $this->commandTesterWithProviders($provider); + + $tester->execute(['alias' => 'some_alias']); + + $this->assertStringContainsString('No retention period configured', $tester->getDisplay()); + $this->assertSame(1, $tester->getStatusCode()); + } +} diff --git a/tests/Unit/Domain/Search/Fake/InMemoryVersionedElasticsearchClient.php b/tests/Unit/Domain/Search/Fake/InMemoryVersionedElasticsearchClient.php new file mode 100644 index 00000000..7b10e8a4 --- /dev/null +++ b/tests/Unit/Domain/Search/Fake/InMemoryVersionedElasticsearchClient.php @@ -0,0 +1,68 @@ + */ + private array $revisions = []; + + /** @var array|null> */ + private array $documents = []; + + public function index(string $indexName, string $documentId, array $document, int $revision): void + { + $key = $indexName . '|' . $documentId; + if (isset($this->revisions[$key]) && $revision < $this->revisions[$key]) { + return; + } + + $this->revisions[$key] = $revision; + $this->documents[$key] = $document; + } + + public function delete(string $indexName, string $documentId, int $revision): void + { + $key = $indexName . '|' . $documentId; + if (isset($this->revisions[$key]) && $revision < $this->revisions[$key]) { + return; + } + + $this->revisions[$key] = $revision; + $this->documents[$key] = null; + } + + /** @return array|null */ + public function getDocument(string $indexName, string $documentId): ?array + { + return $this->documents[$indexName . '|' . $documentId] ?? null; + } + + public function indexExists(string $indexName): bool + { + return true; + } + + public function createIndex(string $indexName, array $mapping, array $settings): void + { + } + + public function updateMapping(string $indexName, array $mapping): void + { + } + + public function search(string $indexName, array $query): array + { + return []; + } +} diff --git a/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerRevisionOrderingTest.php b/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerRevisionOrderingTest.php new file mode 100644 index 00000000..c830e565 --- /dev/null +++ b/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerRevisionOrderingTest.php @@ -0,0 +1,113 @@ + handler -> indexer -> client) rather than mocking away the + * exact ordering guarantee under test. + */ +class IndexDocumentMessageHandlerRevisionOrderingTest extends TestCase +{ + private InMemoryVersionedElasticsearchClient $client; + private IndexDocumentMessageHandler $handler; + + protected function setUp(): void + { + $this->client = new InMemoryVersionedElasticsearchClient(); + $this->handler = new IndexDocumentMessageHandler(new ElasticsearchIndexer($this->client, 'phplist_')); + } + + public function testDelayedRetryOfOlderUpdateDoesNotOverwriteNewerUpdate(): void + { + // The update at revision 100 is dispatched first but, say, the Messenger transport fails + // to deliver it until a later retry - meanwhile a newer update (revision 200) for the same + // document is dispatched and processed first. + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + ['id' => 1, 'summary' => 'Updated'], + SearchOperation::Index, + 200, + )); + + // The retry of the older message finally lands. + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + ['id' => 1, 'summary' => 'Original'], + SearchOperation::Index, + 100, + )); + + $this->assertSame( + ['id' => 1, 'summary' => 'Updated'], + $this->client->getDocument('phplist_subscriber_history', '1'), + ); + } + + public function testDelayedRetryOfOlderUpdateDoesNotResurrectDeletedDocument(): void + { + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + ['id' => 1, 'summary' => 'Original'], + SearchOperation::Index, + 100, + )); + + // A delete for the same document, at a newer revision, is processed first. + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + [], + SearchOperation::Delete, + 300, + )); + + // The delayed retry of the stale update finally lands and must not resurrect the document. + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + ['id' => 1, 'summary' => 'Original'], + SearchOperation::Index, + 150, + )); + + $this->assertNull($this->client->getDocument('phplist_subscriber_history', '1')); + } + + public function testNewerUpdateAfterADeleteIsStillApplied(): void + { + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + [], + SearchOperation::Delete, + 300, + )); + + ($this->handler)(new IndexDocumentMessage( + 'subscriber_history', + '1', + ['id' => 1, 'summary' => 'Recreated'], + SearchOperation::Index, + 400, + )); + + $this->assertSame( + ['id' => 1, 'summary' => 'Recreated'], + $this->client->getDocument('phplist_subscriber_history', '1'), + ); + } +} diff --git a/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerTest.php b/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerTest.php new file mode 100644 index 00000000..661ed121 --- /dev/null +++ b/tests/Unit/Domain/Search/MessageHandler/IndexDocumentMessageHandlerTest.php @@ -0,0 +1,51 @@ +indexer = $this->createMock(ElasticsearchIndexerInterface::class); + $this->handler = new IndexDocumentMessageHandler($this->indexer); + } + + public function testInvokeIndexesOnIndexOperation(): void + { + $document = ['id' => 1, 'summary' => 'hello']; + $message = new IndexDocumentMessage('subscriber_history', '1', $document, SearchOperation::Index, 100); + + $this->indexer + ->expects($this->once()) + ->method('index') + ->with('subscriber_history', '1', $document, 100); + $this->indexer->expects($this->never())->method('delete'); + + ($this->handler)($message); + } + + public function testInvokeDeletesOnDeleteOperation(): void + { + $message = new IndexDocumentMessage('subscriber_history', '1', [], SearchOperation::Delete, 100); + + $this->indexer + ->expects($this->once()) + ->method('delete') + ->with('subscriber_history', '1', 100); + $this->indexer->expects($this->never())->method('index'); + + ($this->handler)($message); + } +} diff --git a/tests/Unit/Domain/Search/Service/ElasticsearchIndexerTest.php b/tests/Unit/Domain/Search/Service/ElasticsearchIndexerTest.php new file mode 100644 index 00000000..6d8f300f --- /dev/null +++ b/tests/Unit/Domain/Search/Service/ElasticsearchIndexerTest.php @@ -0,0 +1,87 @@ +client = $this->createMock(ElasticsearchClientInterface::class); + $this->indexer = new ElasticsearchIndexer($this->client, 'phplist_'); + } + + public function testIndexAppliesIndexPrefix(): void + { + $this->client + ->expects($this->once()) + ->method('index') + ->with('phplist_subscriber_history', '42', ['id' => 42], 100); + + $this->indexer->index('subscriber_history', '42', ['id' => 42], 100); + } + + public function testDeleteAppliesIndexPrefix(): void + { + $this->client + ->expects($this->once()) + ->method('delete') + ->with('phplist_subscriber_history', '42', 100); + + $this->indexer->delete('subscriber_history', '42', 100); + } + + public function testCreateOrUpdateIndexCreatesWhenAbsent(): void + { + $definition = $this->createMock(SearchIndexDefinitionInterface::class); + $definition->method('getIndexAlias')->willReturn('subscriber_history'); + $definition->method('getMapping')->willReturn(['properties' => ['id' => ['type' => 'keyword']]]); + $definition->method('getSettings')->willReturn([]); + + $this->client + ->expects($this->once()) + ->method('indexExists') + ->with('phplist_subscriber_history') + ->willReturn(false); + + $this->client + ->expects($this->once()) + ->method('createIndex') + ->with('phplist_subscriber_history', $definition->getMapping(), []); + $this->client->expects($this->never())->method('updateMapping'); + + $this->indexer->createOrUpdateIndex($definition); + } + + public function testCreateOrUpdateIndexUpdatesMappingWhenPresent(): void + { + $definition = $this->createMock(SearchIndexDefinitionInterface::class); + $definition->method('getIndexAlias')->willReturn('subscriber_history'); + $definition->method('getMapping')->willReturn(['properties' => ['id' => ['type' => 'keyword']]]); + $definition->method('getSettings')->willReturn([]); + + $this->client + ->expects($this->once()) + ->method('indexExists') + ->with('phplist_subscriber_history') + ->willReturn(true); + + $this->client + ->expects($this->once()) + ->method('updateMapping') + ->with('phplist_subscriber_history', $definition->getMapping()); + $this->client->expects($this->never())->method('createIndex'); + + $this->indexer->createOrUpdateIndex($definition); + } +} diff --git a/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryConfigurableReaderTest.php b/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryConfigurableReaderTest.php new file mode 100644 index 00000000..4af5782c --- /dev/null +++ b/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryConfigurableReaderTest.php @@ -0,0 +1,64 @@ +databaseReader = $this->createMock(SubscriberHistoryRepository::class); + $this->elasticsearchReader = $this->createMock(SubscriberHistoryElasticsearchReader::class); + } + + public function testDelegatesToElasticsearchWhenEnabled(): void + { + $reader = new SubscriberHistoryConfigurableReader( + $this->databaseReader, + $this->elasticsearchReader, + true, + ); + $filter = $this->createMock(FilterRequestInterface::class); + $expected = new PaginatedResult([], 0, 50, 0); + + $this->elasticsearchReader->expects($this->once()) + ->method('getFilteredAfterId') + ->with($filter) + ->willReturn($expected); + $this->databaseReader->expects($this->never())->method('getFilteredAfterId'); + + $this->assertSame($expected, $reader->getFilteredAfterId($filter)); + } + + public function testDelegatesToDatabaseWhenDisabled(): void + { + $reader = new SubscriberHistoryConfigurableReader( + $this->databaseReader, + $this->elasticsearchReader, + false, + ); + $subscriber = $this->createMock(Subscriber::class); + $expected = []; + + $this->databaseReader->expects($this->once()) + ->method('getBySubscriber') + ->with($subscriber) + ->willReturn($expected); + $this->elasticsearchReader->expects($this->never())->method('getBySubscriber'); + + $this->assertSame($expected, $reader->getBySubscriber($subscriber)); + } +} diff --git a/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReaderTest.php b/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReaderTest.php new file mode 100644 index 00000000..a1c12bf5 --- /dev/null +++ b/tests/Unit/Domain/Subscription/Repository/SubscriberHistoryElasticsearchReaderTest.php @@ -0,0 +1,157 @@ +client = $this->createMock(ElasticsearchClientInterface::class); + $this->reader = new SubscriberHistoryElasticsearchReader($this->client, 'phplist_'); + } + + public function testGetFilteredAfterIdQueriesPrefixedIndexAndHydratesResults(): void + { + $filter = new SubscriberHistoryFilter(lastId: 5, limit: 10); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_subscriber_history', + $this->callback(function (array $query): bool { + return $query['size'] === 10 + && $query['query']['bool']['filter'][0] === ['range' => ['idSort' => ['gt' => 5]]]; + }), + ) + ->willReturn([ + 'hits' => [ + 'total' => ['value' => 1], + 'hits' => [ + ['_source' => [ + 'id' => 7, + 'idSort' => 7, + 'subscriberId' => 3, + 'ip' => '127.0.0.1', + 'date' => '2026-01-01T00:00:00+00:00', + 'summary' => 'Updated', + 'detail' => 'Detail', + 'systemInfo' => 'Info', + ]], + ], + ], + ]); + + $result = $this->reader->getFilteredAfterId($filter); + + $this->assertSame(1, $result->getTotal()); + $this->assertCount(1, $result->getItems()); + $this->assertSame(7, $result->getItems()[0]->getId()); + $this->assertSame(3, $result->getItems()[0]->getSubscriberId()); + $this->assertSame('Updated', $result->getItems()[0]->getSummary()); + } + + public function testGetFilteredAfterIdPaginatesAcrossTwoPagesWithoutRepeatingResults(): void + { + $firstFilter = new SubscriberHistoryFilter(lastId: 0, limit: 1); + + $this->client + ->expects($this->exactly(2)) + ->method('search') + ->willReturnOnConsecutiveCalls( + [ + 'hits' => [ + 'total' => ['value' => 2], + 'hits' => [ + ['_source' => [ + 'id' => 5, + 'idSort' => 5, + 'subscriberId' => 1, + 'ip' => '127.0.0.1', + 'date' => '2026-01-01T00:00:00+00:00', + 'summary' => 'First', + 'detail' => 'Detail', + 'systemInfo' => 'Info', + ]], + ], + ], + ], + [ + 'hits' => [ + 'total' => ['value' => 2], + 'hits' => [ + ['_source' => [ + 'id' => 8, + 'idSort' => 8, + 'subscriberId' => 2, + 'ip' => '127.0.0.1', + 'date' => '2026-01-02T00:00:00+00:00', + 'summary' => 'Second', + 'detail' => 'Detail', + 'systemInfo' => 'Info', + ]], + ], + ], + ], + ); + + $firstPage = $this->reader->getFilteredAfterId($firstFilter); + + $this->assertSame(5, $firstPage->getLastId()); + $this->assertSame(5, $firstPage->getItems()[0]->getId()); + + $secondFilter = new SubscriberHistoryFilter(lastId: $firstPage->getLastId(), limit: 1); + $secondPage = $this->reader->getFilteredAfterId($secondFilter); + + $this->assertSame(8, $secondPage->getLastId()); + $this->assertSame(8, $secondPage->getItems()[0]->getId()); + $this->assertNotSame( + $firstPage->getItems()[0]->getId(), + $secondPage->getItems()[0]->getId(), + ); + } + + public function testGetFilteredAfterIdRejectsWrongFilterType(): void + { + $wrongFilter = $this->createMock(FilterRequestInterface::class); + + $this->expectException(InvalidArgumentException::class); + $this->reader->getFilteredAfterId($wrongFilter); + } + + public function testGetBySubscriberSortsDescending(): void + { + $subscriber = $this->createMock(Subscriber::class); + $subscriber->method('getId')->willReturn(9); + + $this->client + ->expects($this->once()) + ->method('search') + ->with( + 'phplist_subscriber_history', + $this->callback(function (array $query): bool { + return $query['query'] === ['term' => ['subscriberId' => 9]] + && $query['sort'] === [['idSort' => 'desc']]; + }), + ) + ->willReturn(['hits' => ['hits' => []]]); + + $result = $this->reader->getBySubscriber($subscriber); + + $this->assertSame([], $result); + } +} diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php index f28dc08a..ce8aa7d1 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberHistoryManagerTest.php @@ -10,7 +10,7 @@ use PhpList\Core\Domain\Common\SystemInfoCollector; use PhpList\Core\Domain\Subscription\Model\Filter\SubscriberHistoryFilter; use PhpList\Core\Domain\Subscription\Model\SubscriberHistory; -use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; +use PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -18,14 +18,14 @@ class SubscriberHistoryManagerTest extends TestCase { - private SubscriberHistoryRepository|MockObject $subscriberHistoryRepository; + private SubscriberHistoryReaderInterface|MockObject $subscriberHistoryRepository; private SubscriberHistoryManager $subscriptionHistoryService; protected function setUp(): void { - $this->subscriberHistoryRepository = $this->createMock(SubscriberHistoryRepository::class); + $this->subscriberHistoryRepository = $this->createMock(SubscriberHistoryReaderInterface::class); $this->subscriptionHistoryService = new SubscriberHistoryManager( - repository: $this->subscriberHistoryRepository, + reader: $this->subscriberHistoryRepository, clientIpResolver: $this->createMock(ClientIpResolver::class), systemInfoCollector: $this->createMock(SystemInfoCollector::class), translator: $this->createMock(TranslatorInterface::class), diff --git a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php index d02ca66f..628f1553 100644 --- a/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php +++ b/tests/Unit/Domain/Subscription/Service/Manager/SubscriberManagerTest.php @@ -7,7 +7,7 @@ use Doctrine\ORM\EntityManagerInterface; use PhpList\Core\Domain\Subscription\Model\Dto\CreateSubscriberDto; use PhpList\Core\Domain\Subscription\Model\Subscriber; -use PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository; +use PhpList\Core\Domain\Subscription\Repository\Interfaces\SubscriberHistoryReaderInterface; use PhpList\Core\Domain\Subscription\Repository\SubscriberRepository; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager; use PhpList\Core\Domain\Subscription\Service\Manager\SubscriberManager; @@ -33,7 +33,7 @@ protected function setUp(): void subscriberDeletionService: $subscriberDeletionService, translator: new Translator('en'), subscriberHistoryManager: $this->createMock(SubscriberHistoryManager::class), - subscriberHistoryRepository: $this->createMock(SubscriberHistoryRepository::class), + subscriberHistoryReader: $this->createMock(SubscriberHistoryReaderInterface::class), ); } diff --git a/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinitionTest.php b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinitionTest.php new file mode 100644 index 00000000..065ec627 --- /dev/null +++ b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryIndexDefinitionTest.php @@ -0,0 +1,37 @@ +assertSame('subscriber_history', $definition->getIndexAlias()); + } + + public function testMappingDeclaresExpectedFields(): void + { + $definition = new SubscriberHistoryIndexDefinition(); + $properties = $definition->getMapping()['properties']; + + foreach (['id', 'idSort', 'subscriberId', 'ip', 'date', 'summary', 'detail', 'systemInfo'] as $field) { + $this->assertArrayHasKey($field, $properties); + } + $this->assertSame('long', $properties['idSort']['type']); + $this->assertSame('keyword', $properties['id']['type']); + } + + public function testSettingsAreEmptyByDefault(): void + { + $definition = new SubscriberHistoryIndexDefinition(); + + $this->assertSame([], $definition->getSettings()); + } +} diff --git a/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProviderTest.php b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProviderTest.php new file mode 100644 index 00000000..fb49fca8 --- /dev/null +++ b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryPurgeProviderTest.php @@ -0,0 +1,57 @@ +createMock(SubscriberHistoryRepository::class), + 'phplist_', + 'P1M', + ); + + $this->assertSame('subscriber_history', $provider->getAlias()); + } + + public function testGetRetentionPeriodReturnsNullWhenNotConfigured(): void + { + $provider = new SubscriberHistoryPurgeProvider( + $this->createMock(SubscriberHistoryRepository::class), + 'phplist_', + '', + ); + + $this->assertNull($provider->getRetentionPeriod()); + } + + public function testGetRetentionPeriodParsesConfiguredIsoDuration(): void + { + $provider = new SubscriberHistoryPurgeProvider( + $this->createMock(SubscriberHistoryRepository::class), + 'phplist_', + 'P1M', + ); + + $this->assertEquals(new DateInterval('P1M'), $provider->getRetentionPeriod()); + } + + public function testGetSearchIndexNameIncludesPrefix(): void + { + $provider = new SubscriberHistoryPurgeProvider( + $this->createMock(SubscriberHistoryRepository::class), + 'phplist_', + 'P1M', + ); + + $this->assertSame('phplist_subscriber_history', $provider->getSearchIndexName()); + } +} diff --git a/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryReindexProviderTest.php b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryReindexProviderTest.php new file mode 100644 index 00000000..28926eb5 --- /dev/null +++ b/tests/Unit/Domain/Subscription/Service/Search/SubscriberHistoryReindexProviderTest.php @@ -0,0 +1,19 @@ +createMock(SubscriberHistoryRepository::class)); + + $this->assertSame('subscriber_history', $provider->getAlias()); + } +}