refactor: API controllers instantiate engines with new instead of DI container #176

New Issue

Open

opened 2026-06-29 14:22:02 +00:00 by jmiller · 0 comments

jmiller commented 2026-06-29 14:22:02 +00:00

Owner

Copy Link

Copy Source

Summary

Both API controllers (api/src/Controller/BackupsController.php and api/src/Controller/SnapshotsController.php) create engine instances with new BackupEngine(), new SnapshotEngine(), and new SnapshotRestoreEngine() directly. This bypasses the DI container, making the engines untestable and preventing constructor injection of the database driver (related to #165).

The same pattern exists in AjaxController for SteppedBackupEngine and SteppedRestoreEngine.

Locations

• api/src/Controller/BackupsController.php:41 — $engine = new BackupEngine();
• api/src/Controller/SnapshotsController.php:121 — $engine = new SnapshotEngine();
• api/src/Controller/SnapshotsController.php:170 — $engine = new SnapshotRestoreEngine();
• src/Controller/AjaxController.php — new SteppedBackupEngine(), new SteppedRestoreEngine()

What to do

• Register engine classes in the DI container via services/provider.php
• Inject engines into controllers via constructor or factory method
• Update API and admin controllers to use injected instances
• This enables #165 (engines can receive DatabaseInterface via DI)

Why

Direct new instantiation couples controllers to concrete classes, prevents unit testing with mocks, and blocks the DI-based database migration tracked in #165.

## Summary Both API controllers (`api/src/Controller/BackupsController.php` and `api/src/Controller/SnapshotsController.php`) create engine instances with `new BackupEngine()`, `new SnapshotEngine()`, and `new SnapshotRestoreEngine()` directly. This bypasses the DI container, making the engines untestable and preventing constructor injection of the database driver (related to #165). The same pattern exists in `AjaxController` for `SteppedBackupEngine` and `SteppedRestoreEngine`. ## Locations - `api/src/Controller/BackupsController.php:41` — `$engine = new BackupEngine();` - `api/src/Controller/SnapshotsController.php:121` — `$engine = new SnapshotEngine();` - `api/src/Controller/SnapshotsController.php:170` — `$engine = new SnapshotRestoreEngine();` - `src/Controller/AjaxController.php` — `new SteppedBackupEngine()`, `new SteppedRestoreEngine()` ## What to do - [ ] Register engine classes in the DI container via `services/provider.php` - [ ] Inject engines into controllers via constructor or factory method - [ ] Update API and admin controllers to use injected instances - [ ] This enables #165 (engines can receive `DatabaseInterface` via DI) ## Why Direct `new` instantiation couples controllers to concrete classes, prevents unit testing with mocks, and blocks the DI-based database migration tracked in #165.

jmiller added the component: api label 2026-06-29 14:22:02 +00:00

Sign in to join this conversation.

Labels

Clear labels

component: admin

Admin UI and views

component: api

REST API

component: engine

Backup/restore engine

component: remote

Remote storage (FTP/GDrive)

component: scheduler

Scheduled tasks

bug

Something is not working

duplicate

This issue or pull request already exists

enhancement

New feature

help wanted

Need some help

invalid

Something is wrong

question

More information is needed

wontfix

This won't be fixed

No labels component: api

Priority Medium

Type Feature

Milestone

No items

No Milestone

Assignees

Clear assignees

jmiller (Jonathan Miller)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: MokoConsulting/MokoSuiteBackup#176