added db update steps to remove remnants of digibooks from rbac tables

[schmitz-ilias]

Fixed https://mantis.ilias.de/view.php?id=32969: Removed all remnants of digibooks from the rbac tables (along with the entry of the digibook type from object_data to avoid dependencies).

[schmitz-ilias]

@klees I hope this usage of DBUpdateSteps is correct, just based on the documentation I wasn't quite sure about this.

[klees]

@schmitz-ilias Could you specify a little what you are asking for or what you have been missing in the docu?

[schmitz-ilias]

@klees The docu states that database update steps should only be used for schema updates, and this particular case (just deleting a few rows in a few tables, not really changing the structure of the database) doesn't seem like it deserves to be called a schema update. Maybe I'm misunderstanding the terminology, but this left me unsure whether to use steps or a more general objective.

[klees]

From my POV this sentence in the documentation pretty much captures it:

Schema updates need to be performed after an update, before the installation goes into production again, because ILIAS relies on the database to have a certain structure. So these updates should be light and run quickly. Migrations are concerned with potentially heavy tasks on the database, that might be performed in the background while the system already is productive again.

So the basic questions to decide if some desired db operation is a migration or something for the db update steps are:

1. Is this a potentially long running operation or is it light and runs quickly?
2. Can we perform this while the system is in production again?

There might be some gray area there, but the general idea is: Users should be able to move their installation to production again quickly and tasks, especially the long running ones, that can be performed while in production, should be performed in production as a migration.

Maybe I will elaborate the documentation a little along these lines in the documentation.

For your case here I would say: This basically looks like a migration. It is potentially long running, especially because you iterate over results and start subsequent queries. As far as I see, the task could well be performed in production because it is a cleanup operation. So I would say: It clearly is a migration.

If 1. would be "long running" but 2. would be "no", we would have two options: either, make the operation faster, potentially a lot. If this is not possible for some reason, as for the migration of the File object, we would need to adjust the system accordingly, so that it can deal with unfinished migrations. This is what @chfsx in fact did for the File object.

In your case I think that it should be possible to move the iterations in PHP to the SQL statements so that we do not lose time in the IO between MySQL and PHP. The steps might then be fast enough to answer 1. with "light and quicky", so 2. gets irrelevant. Keep in mind, though, that you do not know which exact data users have in their installation and that this might have a heavy effect on the effective runtime of your statements. If you cannot speed up the steps enough (for most installations) I would suggest to introduce an according migration for your tasks.

I hope this helps, feel free to ask further questions, of course.

[smeyer-ilias]

@klees Thanks for your feedback. Maybe we have a different understanding of schema updates. My interpretation of schema updates are database steps that change the table structure (tables, columns). Quoted from the documentation:

A few words of warning: Make sure to understand, that this mechanism really is about schema updates. Do not perform other kinds of updates (e.g. the migrations, creating files, ...) with this. There is a more general mechanism (the Objectives) to do this.

Regarding the performance: step 1 iterates over the type definition (object_data.type = 'typ' and title='dbk') not over (old) dbk-objects. The number of type definitions is equal 1. Similar arguments are valid for step 2.

Nevertheless, replacing the SQL queries with a single delete statement could be less error-prone.

[schmitz-ilias]

Thanks for the feedback @klees, I replaced the iterations in PHP with slightly more elaborate SQL queries.

[klees]

@smeyer-ilias No, we indeed have the same understanding of "schema updates" =)

I think this is a question of means and ends... The end is, that setup update is performed veeery quickly, so installations only have a very short downtime (if any at all). The means is to separate "migrations" from "schema updates" and write documentation accordingly.

I could have written something like "db updates that run veeery quickly" in the documentation. But this would not be a good criterion, since "quick" is very subjective and dependent on the installation. So I wrote "schema updates", which is a lot clearer but might be a little to strict, as there certainly are db-ops that aren't schema updates but still are very very likely to run super fast.

I won't (and can't) look into every possible db update. And if setup update runs quick, no one else will too. But: If there are db steps that run slowly and they are found we can say: "Please, put them into the migration, as pointed out in the documentation."

I hope this make some sense.

[kergomard]

@alex40724 , you were responsible for the digitbooks, right?