fix(apps): Remove jobs when an app gets disabled

[joshtrichards]

• Resolves: #

Summary

Automatically remove background jobs when an app gets disabled rather than waiting for the job to fail at the next run (where we currently disable it reactively).

Current behavior is reactive only and triggers an unnecessary warning that gets logged at the next attempt to run the job:

server/lib/private/BackgroundJob/JobList.php

Lines 289 to 299 in f9c03f7

	} catch (QueryException $e) {
	if (class_exists($row['class'])) {
	$class = $row['class'];
	$job = new $class();
	} else {
	$this->logger->warning('failed to create instance of background job: ' . $row['class'], ['app' => 'cron', 'exception' => $e]);
	// Remove job from disabled app or old version of an app
	$this->removeById($row['id']);
	return null;
	}
	}

(keeping that to catch apps that get disabled externally - or that fail to load in other ways; this PR just makes it so that apps that are disabled cleanly also have their background jobs removed).

TODO

• ...

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[come-nc]

Please do not add new methods to legacy classes, we are trying to get rid of them!

Most of OC_App is replaced by the AppManager.
In this case you can directly put the loop in the disableApp function I’d say, the code is quite short.
getJobList is also deprecated, should be \OCP\Server::get(IJobList::class), or DI of the joblist in the constructor. (I’d say Server::get is better here as it’s code that runs rarely)

[come-nc]

Forgot to say that I do think the idea is good and should be implemented. It’s cleaner than waiting for jobs to fail indeed.