Automatic database upgrade for non-module objects?

I am working on migrating some modules and started with downloads. I discovered my site did not have icms_data_file and icms_data_urllink tables in the database because we never added them to the upgrade scripts when we added them to the installer (in version 1.3.0). The Downloads uses those tables and I'm working on adding them to the database and addressing this for anyone else who might be in the same situation.

If these were module tables, there is an automatic upgrade method that looks at the object and its variables and determines the name and structure for the tables and adds or applies any necessary changes. At this point, I haven't found similar methods for these objects - and there are quite a few. Avatars, comments, notifications, files, URLs, images and image categories, users, groups, group permissions, private messages, autotasks, blocks, ranks, templates, and probably a few more - I haven't gone through them all. They all seemed primed and ready for this approach - am I missing something?

The system module is a strange animal On the one side, it contains functionality that should be part of the IPF classes, on the other, it looks like a mini-core,with submodules that are similar to full-fledged modules, but not quite.

I discussed some time ago with Mekdrop that we should be downsizing the system module and spinning out some of those functionalities into separate modules (like comments, ratings, ...). That would solve some of those issues, but that's not for right away though.

It would be a major undertaking to rework this and I was hoping there was something to leverage that I wasn't seeing. In order to get this done and the keep this release moving forward, there's a couple of ways to proceed.

1. Do what's been done in other releases and include the table structure manually in the upgrade script (see modules/system/include/update-13.php)

2. Replicate the icms_db_legacy_updater_Handler::upgradeObjectItem() method and modify the new method (or just embed it in the upgrade script) to not rely on the object being part of a module

3. Find a way to modify the icms_db_legacy_updater_Handler::upgradeObjectItem() method to allow for other objects like these in different locations (the library folder), or at least for these 2 objects and their tables.

If we can get it done and take a small step in the right direction to deal with all the other components, that would be ideal.

Well, some experimenting has gotten me the results I was looking for - with a pretty small change to icms_db_legacy_updater_Handler::upgradeObjectItem(). Here's how the first lines look -

  function upgradeObjectItem($dirname, $item) {
        $module_handler = icms_getModuleHandler($item, $dirname, null, true); // this is changed

        if (!$module_handler) { // added this branch
                $module_handler = icms::handler($dirname . '_' . $item);
        }
        if (!$module_handler) {
                return false;
        }

Then, in the system module's upgrade file I added at the end

    if (!$abortUpdate) $newDbVersion = 45;
    
    if ($dbVersion < $newDbVersion) {
        $icmsDatabaseUpdater->automaticUpgrade('icms_data', array('file', 'urllink'));
        /* Finish up this portion of the db update */
        if (!$abortUpdate) {
            $icmsDatabaseUpdater->updateModuleDBVersion($newDbVersion, 'system');
            echo sprintf(_DATABASEUPDATER_UPDATE_OK, icms_conv_nr2local($newDbVersion)) . '<br />';
        }
    }        

After upgrading the system module, I had the 2 tables I needed. Updates of other IPF modules still work.

Any unexpected risks? Another few sets of eyes on this would be fantastic!

this looks like a very elegant and minimalistic change, kudos for that!

Taking the module upgrade route would also have been my choice.

I had a good solid foundation for the solution - thanks to all the work others had done before me. I would like to test a few other things and then I'll create a pull request to add this in. What do you think about making this a default action of the system module update, rather than just a step?

If it is feasable to make this automatic, it would be best.