Component/Cron: init cron jobs via Component's seek

[nhaagen]

Move registration of cron-jobs to component-level with contribute / seek and remove xml-based processor.
Construction of CronJobs is still an issue (as stated in the roadmap already); I circumvented this with an additional flag in __construct, which was the easiest to stay in the scope of this PR.

[klees]

Hi @nhaagen,

thanks a lot for looking into this. Looks promising afaic.

Please answer the following questions:

• hello world: Why do we want this change in hello_world.php?
• BookingManager: Why do we need this $registration construction? When would $registration = false?
• Metrics: Could you explain a little, what the metrics for the cronjobs will do?

Please consider the following suggestions. You do not need to follow those, but but please indicate shortly why you prefer to do otherwise:

• named objective: Lets publish ilCronjobsRegisteredObjective as a named objective too. People could then be really sure that all cronjobs are registered indeed...

I guess we should do one iteration before handing this over to @mjansenDatabay. Also: Please remember to open an according PR for the test plugin repo.

Kind regards!

[thibsy]

Hi @nhaagen,

I was curious about the integration mechanism here, so I briefly looked over the changes. I have some questions in addition to @klees' review - could you answer them as well?

• why do contributors of cron job instances need to prepend components\ to the component class name? Is this important or just an aesthetic? Would it make sense to centralise this?
• I was having an issue with ILIAS\Language\Language::loadLanguageModule() calls inside the constructor of classes during the migration of the UI framework initialisation (#7969). Did you not run into the same issue here?
• why are you using a "mock" instance of ILIAS\Language\Language inside the new cron objectives? Isn't there a dedicated implementation for the setup available?

Thx for your time!

kind regards,
@thibsy

[nhaagen]

Hi @thibsy and @klees ,
sorry for the late response; I think, I addressed all issues.

CronJob is now constructed with Language and LoggerFactory; further initiallization can be done within ::init().
Language is no longer a Mock in the objective, and I removed the metrics altogether.

helo_world is just a fix - the filename has changed.

Looking forward to the next round,
best regards

[klees]

Hi @mjansenDatabay,

this moves the cronjobs to the new integration mechanisms of the Component Revision. If there's anything we can do to help moving this forward (e.g. have a call to answer questions) feel free to ping me.

Kind regards!

[klees]

I just realized that this needs rebasing due to introduction of namespaces in ILIAS\Cron.

@mjansenDatabay May I propose that you look into this and give feedback before rebasing and we only rebase shortly before merge?

[mjansenDatabay]

HI @klees ,

first, thanks for the PR @nhaagen and @klees !

I will put this on my stack and review it soon. I can't guarantee that it will be done by Friday, but I should get to it next week at the latest.

Once I've reviewed the relevant things in “Cron” (+ a few spot checks in “my” consuming components) and “cli” it would probably make sense for the relevant authorities to look at their components too, right?

Best regards,
Michael

[klees]

Hi @mjansenDatabay,

no need to hurry, this is a marathon, not a sprint =)

Changes in the various components are editorial from my POV, so I think we could use the "announce on JF and wait some time before merge" procedure.

Kind regards!

[mjansenDatabay]

Unfortunately, I still need a little more time, because this week too much of my working time was taken up with analyzing problems with ILIAS 9 upgrades of customer installations.

[klees]

Hi @mjansenDatabay,

no need to rush here.

Thanks for the feedback!

[mjansenDatabay]

Hi all,

first, thanks @nhaagen and @klees for providing the PR.

I reviewed the changes in "my" components (Auth, Certificate, Cron, Forum, LDAP, Mail) and the cron.php, and they LGTM. I only have a few remarks regarding the "Cron" component.

Please answer the following questions:

• Do we really need to change the internals of ilCronJobEntities? I agree that this is discussable, but if we want to change this, it should be put into another PR.

Please implement the following changes:

• Remove all added @inheritdoc comments in the "Cron" component.
• Make the jobs member variable in \ILIAS\Cron\CronRegistry "readonly".
• Make the registry member variable in \ilCronJobSetupAgent "readonly".
• Fix the undeclared property $this->cronjobs in \ilCronjobsRegisteredObjective::achieve.
• Fix the undeclared property $this->cronjobs in \ilCronJobSetupAgent::getNamedObjectives.
• For the sake of consistency, unregisterAllJobs in \ilCronJobRepositoryImpl` should be also moved to the interface.

One last thing: For me it is okay to use TRUNCATE in \ilCronJobRepositoryImpl::unregisterAllJobs, because it seems to be only executed in the setup context. TRUNCATE is a DDL statement and requires other permissions than a DELETE, although they are very similar. We have to be careful when introducing DDL statements to maintain the "Principle of Least Privilege".

Best regards,
Michael