feat: added task scheduler

[thorsten]

Summary by CodeRabbit

• New Features

  • Added an automated background task scheduler for session cleanup, search index optimization, statistics aggregation, and data backups.
  • Added a command‑line entry to run the scheduler with a JSON-formatted results output.

• Tests

  • Added comprehensive unit tests covering scheduler execution, individual task outcomes, and error-handling scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a TaskScheduler service and CLI entry that run four maintenance tasks (session cleanup, search index optimization, statistics aggregation, backup creation); normalizes search DB driver naming; registers the scheduler service; and adds PHPUnit tests for success and failure scenarios.

Changes

Cohort / File(s)	Summary
Task Scheduler Implementation phpmyfaq/src/phpMyFAQ/Scheduler/TaskScheduler.php	New class implementing four scheduled tasks with per-task methods (cleanupSessions, optimizeSearchIndex, aggregateStatistics, createBackup) and a run(): array aggregator; exceptions are caught, logged, and per-task structured results are returned.
Scheduler CLI Entry Point bin/scheduler.php	New CLI script enforcing run command, bootstraps and compiles DI container, retrieves phpmyfaq.scheduler.task-scheduler, invokes run(), and prints a pretty JSON results payload.
Service Registration phpmyfaq/src/services.php	Registers new public service phpmyfaq.scheduler.task-scheduler wired with phpmyfaq.configuration, phpmyfaq.admin.session, phpmyfaq.admin.backup, and phpmyfaq.faq.statistics.
Search Database Driver Resolution phpmyfaq/src/phpMyFAQ/Search.php	Adds resolveSearchDatabaseType() and getDatabaseDriverClassName() helpers and switches Search factory creation to use normalized DB driver identifiers instead of direct Database::getType().
Tests tests/phpMyFAQ/Scheduler/TaskSchedulerTest.php	New PHPUnit test suite covering run() success and partial-failure flows, and individual task failure handling with logging verification and dependency stubs.

Sequence Diagram

sequenceDiagram
    participant CLI as "bin/scheduler.php"
    participant DI as "DI Container"
    participant TS as "TaskScheduler"
    participant AS as "AdminSession"
    participant ES as "Elasticsearch/OpenSearch"
    participant Stats as "Statistics"
    participant Backup as "Backup"

    CLI->>DI: bootstrap & compile container
    CLI->>DI: get(service: phpmyfaq.scheduler.task-scheduler)
    DI-->>CLI: TaskScheduler instance
    CLI->>TS: run()

    TS->>AS: cleanupSessions() -> deleteSessions(0, cutoff)
    AS-->>TS: deletion result

    TS->>ES: optimizeSearchIndex() -> forcemerge (if enabled)
    ES-->>TS: merge result / error

    TS->>Stats: aggregateStatistics() -> query totals
    Stats-->>TS: totals

    TS->>Backup: createBackup() -> export(BACKUP_TYPE_DATA)
    Backup-->>TS: fileName / error

    TS-->>CLI: aggregated results array
    CLI->>CLI: output pretty JSON

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through logs at break of day,

Cleared old sessions, sent stale times away,
Merged the indices, counted every stat,
Bundled backups tidy in a hat,
The scheduler hums — a job well-played. 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: added task scheduler' accurately describes the main change—introducing a new task scheduler component with multiple scheduled tasks and related infrastructure.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/scheduler

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

tests/phpMyFAQ/Scheduler/TaskSchedulerTest.php (2)

51-81: Fault isolation test looks correct but may be asserting the wrong failure mode.

When deleteSessions throws, the exception is caught inside cleanupSessions() (lines 102-112 of TaskScheduler), so cleanupSessions() returns a normal array with success => false. The outer try/catch in run() (which sets the result to null) would only fire if something else threw after the inner catch.

Line 75 asserts assertIsArray(...) which confirms the inner catch handled it — this is correct behavior. But line 76 asserts assertFalse($result['sessionCleanup']['success']) which also confirms it. So the test is actually validating the inner exception handling of cleanupSessions(), not the fault-isolation in run(). If the goal is to test run()'s outer catch, you'd need an exception that escapes cleanupSessions() entirely (e.g., the logger itself throws).

This is fine as-is for regression coverage, but the test name slightly overstates what's being verified.

---

190-199: Helper doesn't wire a logger — callers that need one must remember to do so separately.

createConfigurationStub only wires get(). Tests that trigger error paths must separately configure getLogger() (and they do). Consider wiring a default no-op logger stub here so future test methods don't accidentally get a "method not configured" failure if they forget.

♻️ Suggested improvement

 private function createConfigurationStub(array $values): Configuration
 {
     $configuration = $this->createStub(Configuration::class);
     $configuration->method('get')->willReturnCallback(static fn(string $item): mixed => $values[$item] ?? null);
+    $configuration->method('getLogger')->willReturn($this->createStub(\Monolog\Logger::class));

     return $configuration;
 }

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In `@phpmyfaq/src/phpMyFAQ/Scheduler/TaskScheduler.php`:
- Around line 59-78: The cleanupSessions() method calls
$this->adminSession->deleteSessions(...) without catching exceptions; wrap that
call in a try/catch so any thrown exception is caught, logged via
$this->configuration->getLogger()->error() (include the exception message/trace
and the cutoffTimestamp/retentionSeconds context), and ensure $success is set to
false on exception before returning the same result array; this mirrors the
exception-handling pattern used in other task methods and prevents uncaught
exceptions from propagating out of cleanupSessions().
- Around line 140-148: aggregateStatistics() currently lets exceptions from
$this->statistics->totalFaqs() or $this->adminSession->getNumberOfSessions()
bubble up; wrap the calls in a try/catch (mirroring optimizeSearchIndex() and
createBackup()) so any Exception is caught and the method returns a failure
payload (e.g. 'success' => false, include 'error' => $e->getMessage(), and still
set 'generatedAt'), using aggregateStatistics(), totalFaqs(),
getNumberOfSessions(), $this->statistics and $this->adminSession to locate the
code to modify.
- Around line 46-54: run() currently evaluates cleanupSessions(),
optimizeSearchIndex(), aggregateStatistics(), and createBackup() eagerly so an
exception in any (notably cleanupSessions() or aggregateStatistics()) aborts the
whole scheduler; update run() in TaskScheduler to invoke each task inside its
own try/catch (catch \Throwable), call cleanupSessions(), optimizeSearchIndex(),
aggregateStatistics(), and createBackup() separately, log the caught exception
(use the class logger or error mechanism already used elsewhere), and populate
the returned array with either the task result or a failure marker/null so a
single task failure doesn’t prevent the others from running.

In `@phpmyfaq/src/phpMyFAQ/Search.php`:
- Around line 204-222: The getDatabaseDriverClassName() function contains an
unreachable fallback 'sqlite3' in "return end($classNameParts) ?: 'sqlite3'";
remove the dead fallback and return the last element directly (ensuring the
return is a string) so the function simply returns the class basename. Update
getDatabaseDriverClassName(DatabaseDriver $databaseDriver) to return the last
part of explode('\\', $databaseDriver::class) (e.g., cast the end() result to
string) and leave resolveSearchDatabaseType() unchanged.

🧹 Nitpick comments (4)

phpmyfaq/src/phpMyFAQ/Scheduler/TaskScheduler.php (1)

88-96: forcemerge with max_num_segments => 1 is resource-intensive.

Force-merging to a single segment is an expensive operation that can starve search performance on large indices while running. Consider using a higher segment count (e.g., 5) or making it configurable, and ensure this task runs during low-traffic windows.

bin/scheduler.php (2)

42-43: Mixing plain text and JSON output complicates machine parsing.

The "Task scheduler finished." message before the JSON payload means the output is not valid JSON. If this is intended for cron/automation, consider writing the status message to STDERR or including it inside the JSON payload.

♻️ Proposed fix

-echo "Task scheduler finished.\n";
-echo json_encode($results, JSON_PRETTY_PRINT) . PHP_EOL;
+fwrite(STDERR, "Task scheduler finished.\n");
+echo json_encode($results, JSON_PRETTY_PRINT) . PHP_EOL;

---

34-40: No error handling around container build or task execution.

If $container->compile() or $scheduler->run() throws, the script exits with an unhandled exception and exit code 255. Consider wrapping in a try/catch to log a clear error message and return a meaningful exit code.

tests/phpMyFAQ/Scheduler/TaskSchedulerTest.php (1)

21-49: Good coverage of the happy path; consider adding a test for cleanupSessions failure.

The run() test verifies the happy path well. However, there's no test for the cleanupSessions failure branch (when deleteSessions returns false), which would verify the warning log is triggered. Similarly, aggregateStatistics isn't tested in isolation.