User: skip broken users in gallery (47791)

[schmitz-ilias]

This PR fixes 47791 by checking whether users exist in usr_data before instantiating them in ilUsersGalleryParticipants::getUsers.

[mjansenDatabay]

Should we really do this? This will lead to N additional SQL queries being fired.

[schmitz-ilias]

I'm not sure we can catch this any other way than with additional queries. As far as I can tell, the first time usr_data is read out in the call from the ticket is in the constructor of ilObjUser, immediately before the error occurs. As @kergomard said in the ticket, there is really no good way to get out of the failed instantiation of the user once we get to that point.

It would of course be possible to do the check with a single additional query. User would have to offer some kind of filterOutBrokenUsers utility method, so that we are able to do the check at scale. Not sure where that would go, adding another static method to ilObjUser seems ill advised.

I guess a different fix here would involve (periodically?) cleaning up broken users before they become a problem. Or did you have something else in mind?

[mjansenDatabay]

Hi Tim,

Thanks for the detailed explanation. I think your assessment is correct: within the current architecture there is probably no easy way to avoid the additional queries once we instantiate ilObjUser, because the consistency issue is only detected during object construction.

Personally, I find the idea of periodically checking and cleaning up broken records quite compelling. Since I do not expect ILIAS to adopt (component-spanning) database-level referential integrity constraints in the foreseeable future (the topic has been discussed multiple times over the years, including workshops and proposals documented in the "Feature Wiki"), I think periodic and manually triggerable integrity checks are probably the most pragmatic long-term solution.

In fact, I could imagine extending the "System Check" infrastructure in a more generic way. Instead of relying on hard-coded task definitions, components could contribute integrity checks and optional cleanup tasks through artifacts or future component integration mechanisms. A generic "Integrity Cleanup" framework could then detect and optionally repair/delete orphaned or otherwise inconsistent records across the system. Besides improving data integrity, this would likely also have a positive impact on the performance of long-running installations.

Regarding alternatives, I can think of a few options:

• As you also mentionend: Providing an API that can identify broken user records (or their IDs) with a single query. Or: Providing a utility that filters a given set of user IDs and removes invalid entries in a scalable way.
• Replacing ilErrorHandler::raiseError(..., FATAL) in ilObjUser with regular exceptions that consumers can catch and handle gracefully.
• Avoiding ilObjUser altogether in this context and introducing a dedicated gallery-facing entity (e.g. GalleryProfile) that exposes only the data required by the "Gallery" domain and can be loaded efficiently from a dedicated repository (ensuring the integrity of these provided entities).

That said, I fully agree that most of these ideas go well beyond the scope of a bug fix. I mainly wanted to point them out because performance and data consistency issues tend to become increasingly visible in larger and older installations.
In the hosting environments I am involved with (passively), we have observed measurable performance regressions when comparing ILIAS 8, 9 and 10 under otherwise comparable usage scenarios and data volumes. While the reasons are often multifaceted and not attributable to a single change, the overall trend is noticeable.
Beyond my own observations, similar concerns are raised within the "SIG ILIAS betreiben", suggesting that this is not merely an isolated experience. Changes that introduce additional database queries therefore tend to attract my attention.

Best regards,
Michael