Repository navigation
BackgroundTasks: Migrate legacy BackgroundTasks_ - #12294
Conversation
| /** | ||
| * Stored background task buckets reference their tasks and values by class name. | ||
| */ | ||
| public function step_2(): void |
There was a problem hiding this comment.
Hi @thojou I think we should no go this may by (again) coupling components together, in this case using a mechanism via database. I't would be much easier to provide such implementations using the pattern we already have during bootstrapping of components. shall we have a call concerning this?
Well, I understand that this simply involves swapping out the existing class names, so that existing tasks continue to work after the update. But I don’t think that fully justifies an update step in the File component writing BackgroundTask into tables – do you know what I mean? That’s quite a hidden dependency.
An alternative would be (and this would be nice anyway) for Jobs and Tasks to register via the bootstrapping process in BackgroundTask; this would make the dependency between the components in the bootstrapping process directly visible. Jobs and Tasks could have an optional, deprecated ‘migrationName’ method or something similar for a release, which would define the mapping to old class names.
| * Description of class class | ||
| * @author Stefan Meyer <smeyer.ilias@gmx.de> | ||
| */ | ||
| class ilCopyDefinition extends AbstractValue |
There was a problem hiding this comment.
This is more general than just File I think, maybe this belongs to something like Repository or where the Copy-Process itself lives.
|
@thojou I’ve just had another idea and I’ll get back to you as soon as possible with a PR for your branch |
|
I prepared my suggestions as a PR against your branch: thojou#8 Instead of rewriting the stored class names, the tasks and values are contributed to BackgroundTasks via the bootstrap, and FileServices keeps the former names as deprecated subclasses until ILIAS 13. |
|
Okay, then we will wait until the new proposal arrives :). -- UPDATE -- Ah okay, it is already here :-). |
|
Hey @chfsx, Thanks for your changes once again. I've integrated the commits into this PR and adjusted my nitpick. Regarding @alex40724, @smeyer-ilias – What do you think? Should this job move to the Repository component? I'll merge this PR today. @chfsx, do we leave the Kind regards, |
fcb12f8 to
08fc791
Compare
08fc791 to
4961da6
Compare
Move the setup agent, its config and objectives, and the SOAP administration class into components/ILIAS/BackgroundTasks/src under the ILIAS\BackgroundTasks namespace. Delete the old files from BackgroundTasks_ and the empty class.ilBackgroundTasksDB80.php. ilBackgroundTasksDBHotfixes10 keeps its global class name, because the database step log stores executed steps by that class name.
Delete @inheritdoc blocks and @return or @throws tags that restate the interface or the method body. Delete the @var tag for $io in ConfigStoredObjective, which named a class that does not exist. Drop the two imports that only these docblocks used.
Move ilBTControllerGUI and ilBTPopOverGUI into components/ILIAS/BackgroundTasks/src/Implementation/UI as ControllerGUI and PopOverGUI, next to the StateTranslator trait, which keeps its namespace. BTNotificationProvider moves to src/Provider and keeps its class name, so its GlobalScreen provider id does not change. Move background_task_refresh.js and its asset contribution from BackgroundTasks_ to BackgroundTasks. The public path stays assets/js/background_task_refresh.js. Move service.xml with the component id bgtk, point its baseclass entry at ControllerGUI, and drop the gsprovider entry for ilBTGSProvider, a class that does not exist. BackgroundTasks_ keeps the download jobs and an empty component class, because cli/build_bootstrap.php expects one in every component directory.
The ZIP download jobs used by Container and WorkspaceFolder now live in FileServices under the namespace ILIAS\FileServices\Download. The old files in BackgroundTasks_ are deleted. No class alias remains. Background task buckets store the task class name in the database. A pending ZIP download that was created before the update cannot load after it and BackgroundTasks deletes that bucket. The user starts the download again.
The moved download jobs carried placeholder class docblocks and step comments that repeat the next line. CollectFilesJob::$targets becomes $duplicate_count_by_target, so the name says what the deleted comment said.
Background task buckets store the class name of each task and value in il_bt_task and il_bt_value. Without this step, the async worker cannot load a scheduled ZIP download that names ilCopyDefinition. It stops on that bucket at every run and leaves it in the queue. The new database update step replaces the seven old names with the new ones in the class_name and type columns.
The setup, SOAP, notification UI, and download job classes have moved out of BackgroundTasks_. Only an empty Component class, PRIVACY.md, and ROADMAP.md were left. cli/build_bootstrap.php discovers components by directory, so deleting the directory removes the component. The setup label in ConfigStoredObjective and the IRSS storage audit now name BackgroundTasks.
No class ilBackgroundTaskHub exists in the tree. The @ilCtrl_Calls entry on ilObjFolderGUI pointed at nothing.
The component directory is gone, so the authority block that pointed at it is gone too.
The copyright checker requires this header at the top of every JS file. The moved refresh script never had one.
…d jobs" This reverts commit 869199b.
…trap Buckets store their tasks and values by class name, so BackgroundTasks depends on classes of other components without that dependency being visible anywhere. Components now contribute their tasks and values as ILIAS\BackgroundTasks\Task and ILIAS\BackgroundTasks\Value. The Registry keeps the contributions as prototypes, and the task factory and the persistence clone them instead of building them through the injector. Classes nobody contributed, e.g. tasks of plugins, are still built through the injector. Init populates the Registry as $DIC['bt.registry'].
The download jobs receive their dependencies through the constructor
and are contributed to BackgroundTasks under their new names. The
former names stay as empty, deprecated subclasses, contributed as well,
so buckets created before the update still load. This replaces the
database step rewriting il_bt_task and il_bt_value. The former names
will be removed with ILIAS 13.
The global download limit is a setting of File, so FileServices defines
DownloadLimit and File implements it with its General settings. The
repository tree is not wired through the bootstrap yet; the jobs reach
it through an internal RepositoryTreeProxy until it is. Checking access
of the current user goes through Access::checkAccess(), so the jobs no
longer need the user. CheckSumOfFileSizesJob no longer creates the
unused ilSetting("fold").
The background task registry was stored in the DIC under the alias `bt.registry`. Registration, the fallback in `ilInitialisation`, and `BackgroundTaskServices::registry()` now use `ILIAS\BackgroundTasks\Registry\Registry` as the container key, the same way other modern services are registered.
e132529 to
04758df
Compare
Hey all,
The technical board asked me if we could remove BackgroundTasks_ and migrate the remaining code to their respective new components.
Why
BackgroundTasks_is the leftover half of the ILIAS 10 split. Background tasks are a framework plus the ILIAS integration, and the shared folder-download jobs are file features. This pull request moves both homes and deletes the old component.What changed
background_task_refresh.jsmove intoBackgroundTasksunder PSR-4. The missingilBTGSProviderentry is dropped.ILIAS\FileServices\Download. Container and WorkspaceFolder call the new names.ilFileServicesDatabaseObjective::step_2rewrites the stored class names inil_bt_taskandil_bt_value, so a scheduled ZIP download still loads.components/ILIAS/BackgroundTasks_/is deleted. The authority block indocs/development/maintenance.mdgoes with it, and the deadilBackgroundTaskHubctrl call is removed fromilObjFolderGUI.Scope
ilBackgroundTasksDBHotfixes10keeps its class name, so its database steps do not run again.Tradeoffs
The download jobs land in FileServices. File was the other candidate. FileServices matches the existing
src/Uploadlayout and is the home agreed with that component's maintainer.Blast Radius
Anyone watching a running background task uses the notification UI. Anyone downloading a folder as a ZIP uses the moved jobs. Setup and the SOAP worker construct the new classes. Without the database step, the async worker stops on a stored job that still names an old class.
Verification
php8.3 -lpasses on the moved PHP files.vendor. The hotfix class name is unchanged.step_2ran twice on a scratch copy ofil_bt_taskandil_bt_value. It rewrote the seven old names and left every other row unchanged. The scratch database was dropped.