Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 6

In my last post, I had managed to get a working system that would successfully convert blocks in a 1.9 backup file to a Moodle 2.3 restore. I need to do a bit more testing, and then look for a solution for blocks that have specific backup/restore needs.

Testing uncovered that trying to restore a 1.9 course that had multiple versions of the same block installed within it (such as multiple HTML blocks) would cause an error, indicating that a file already existed. Referring back to the new code, this is likely due to the new process_block() function. In that function, a new XML file is created using the name of the block. This means that if more than one of the same block is converted, the code would try and create more than one file with the same name. I need to change that code to make sure it creates a unique file name in this situation.

I change the code:

$this->open_xml_writer("course/blocks/{$data['name']}/block.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}/inforef.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}/roles.xml");

to:

$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/block.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/inforef.xml");
...
$this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/roles.xml");

By adding $instanceid to the file path, I guarantee that each XML file will have a unique name. A quick test proves that this is now working.

The next issue to tackle is what to do with blocks that have their own backup/restore needs. The one Moodle 2 block that seems to have this requirement is the block rss_client. It will need its own backup converter.

In the tracker Paul Nicholls started to tackle this by creating a new "blocks/rss_client/backup/moodle1/lib.php" file with an extended class. When he tested the first version, a bug was discovered in my moodle1_handlers_factory::get_plugin_handlers() function changes. If a block does have its own class extended, the file containing it is not included, meaning that the handler class is unknown and an error occurs during the conversion process. Paul has also provided a fix for this error using his own git repository. To fix my copy, I will use the power of git collaboration.

First, I need to be able to pull code from Paul's repository into my local one. I have already made this possible by adding his repository as a remote named "pauln" to my local one. The commit that fixes the code is identified, so I cherry-pick it into my repository with:

git fetch pauln
git cherry-pick 630756fe1063e96cc7a6057bd512aa4afa53bc61

This modifies the code from:

if ($type != "block") {
    if (!file_exists($handlerfile)) {
        continue;
    }
    require_once($handlerfile);
} else {
    if (!file_exists($handlerfile)) {
        $handlerclass = "moodle1_block_generic_handler";
    }
}

to:

if (file_exists($handlerfile)) {
    require_once($handlerfile);
} elseif ($type == 'block') {
    $handlerclass = "moodle1_block_generic_handler";
} else {
    continue;
}

Paul has also provided a new version of the rss_client block with the new handlers. Again, I cherry-pick using:

git cherry-pick d89ade28060ae858356ff4a42cf946a4c99878c0

This provides me with a working copy of the conversion code that will correctly add the extra functionality that the Moodle 2.3 rss_client block requires.

For the record, here is the code for the rss_client conversion handler:

class moodle1_block_rss_client_handler extends moodle1_block_handler {
    public function process_block(array $data) {
        parent::process_block($data);
        $instanceid = $data['id'];
        $contextid = $this->converter->get_contextid(CONTEXT_BLOCK, $data['id']);

        // Moodle 1.9 backups do not include sufficient data to restore feeds, so we need an empty shell rss_client.xml
        // for the restore process to find
        $this->open_xml_writer("course/blocks/{$data['name']}_{$instanceid}/rss_client.xml");
        $this->xmlwriter->begin_tag('block', array('id' => $instanceid, 'contextid' => $contextid, 'blockname' => 'rss_client'));
        $this->xmlwriter->begin_tag('rss_client', array('id' => $instanceid));
        $this->xmlwriter->full_tag('feeds', '');
        $this->xmlwriter->end_tag('rss_client');
        $this->xmlwriter->end_tag('block');
        $this->close_xml_writer();

        return $data;
    }
}

I don't think that this handler will be typical of 1.9 blocks that need to restore their own data though. As it turns out, although the rss_client block does store its own data (feed definitions) in its own table, the 1.9 code never backed that data up. So all 1.9 backups contain only the standard block information for these blocks. But, in Moodle 2.3, the restore operation expects that the feed definition data will be there, and if its not included (even empty) then it causes an error. To work around this, the conversion handler creates an empty version of the expected feed definition XML file for all 1.9 conversions. I will do an extra post later to show how a block that really has extra data would provide the correct conversion code.

There are no other core Moodle blocks that require any specific restore conversion code, so I am essentially done the coding that is required by the tracker issue. Next, I'm going to ready the code for use by the HQ integrators and complete the tracker data so that they can consider it for inclusion into core.

First things I need to do is make sure my git repo is in a good state. To start with, I will make sure that the main branch of Moodle my development branch was based off of is up to date with the latest upstream Moodle code:

git checkout MOODLE_23_STABLE
git fetch upstream
git pull upstream
git push origin MOODLE_23_STABLE

This part isn't necessary, but I want to make my development branch's name a little more manageable, so I rename my MOODLE_23_STABLE_MDL-32880 branch to MDL-32880_2.3. This is purely for optics.

Now, I want to compress all of the work I have done on this issue into one commit. This makes for easier documentation and understanding, since several of my commits were code tests that were later undone. The "rebase" function of git allows me to do this. From the branch MDL-32880_2.3, I issue the command:

git rebase -i MOODLE_23_STABLE

This command will merge all of my commits with the base branch specified, keeping them in my branch, but allowing me to compress some things. The "-i" portion stands for "interactive" and brings up an editor screen that allows me to change what happens:

pick 00cab4e MDL-32880 - Playing with ideas.
pick c7849a8 MDL-32880 - Playing with more ideas.
pick 5036ed8 MDL-32880: moodle1 backup converter: add basic block handler
pick b346f7e MDL-32880 - Adding generic block conversion handlers.
pick 6a3045e MDL-32880 - Named block XML file with instance, and corrected use of currentmod to currentblock.
pick 7b9e3ec MDL-32880: moodle1 backup converter: Include custom block handers if present
pick 6fbf1fb MDL-32880: Add moodle1 backup converter for rss_client block.
pick 1012e75 MDL-32880 - Trying to use convert API in specific block instance.
pick 0b86f82 Revert "MDL-32880 - Trying to use convert API in specific block instance."

# Rebase ce44bf4..2bf7777 onto ce44bf4
#
# Commands:
#  p, pick = use commit
#  r, reword = use commit, but edit the commit message
#  e, edit = use commit, but stop for amending
#  s, squash = use commit, but meld into previous commit
#  f, fixup = like "squash", but discard this commit's log message
#  x, exec = run command (the rest of the line) using shell
#
# If you remove a line here THAT COMMIT WILL BE LOST.
# However, if you remove everything, the rebase will be aborted.
#

This screen shows all of my commits in my development branch with "pick" and their information next to them. I want to compress all of these changes into one commit, so I change all but the first "pick" to "squash", and save the edits. Once I do this, I have told git to combine and merge all of the commits into one diff commit. The next thing git does is allow me to create a new commit message for the new combined commit, which I simplify to:
"MDL-32880 - Adding 1.9 block conversion to M2 restore."

Now my MDL-32880_2.3 development branch is up to date with the main Moodle code, and contains just one committed change containing all of the necessary changes for the block restore conversion. This will make it easier for others to review and include in their testing.

The last things that need to be done in order to have this considered for inclusion into core is to complete the rest of the tracker information. Refer to the tacker item MDL-32880 once again to see the results. In this case, Paul has completed all of the necessary information which includes:

• Testing instructions: Specific step-by-step instructions on how to verify that the code will do what it says. Paul has also attached a 1.9 backup file containing all core blocks.
• Pull from Repository: Git repository containing the new code.
• Pull X.X Branch: The name of the branch in the repository containing the code for the specified "X.X" version. For mine, it would be "MDL-32880_2.3".
• Pull X.X Diff URL: A link to the repository that specifies a "diff" between the codebase and the changes, so that reviewer can see a compare view. This is made easy in github, using the "branches" tab and selecting your base branch from the drop-down, and the selecting the specific branch to compare against. For mine, it produces this link: https://github.com/mchurchward/moodle/compare/MOODLE_23_STABLE...MDL-32880_2.3

 I will continue this series next by providing an example of a 1.9 block that does convert its own data.

Adding Moodle 1.9 Block Restore to Moodle 2.3 - Part 5

In my last post, I had determined that I could make things happen correctly through the correct use of the get_paths() function creating convert_path objects. This led me to believe that I could now plan out the changes necessary to complete this task.

If you have followed along with the MDL-32880 tracker issue, you will notice that there has been some activity from other participants. Particularly, Paul Nicholls has done some work in his repository and proposed some changes. While I won't use all of them, some of them will make my work easier.

I know that the moodle1_handlers_factory::get_handlers() function (in file "/backup/converter/moodle1/handlerlib.php") uses the get_plugin_handlers() function to load all handlers it finds for blocks. But, since it expects to find specific class files and functions for any block to convert, it doesn't do anything for blocks (since none of the blocks provide this function). Paul's solution involves adding the appropriate file and function to each block in core to make the conversions happen. I still believe I can do this for every existing block without having to create new class files for every block.

I have looked closely at the functions in "handlerlib.php", and my plan is to do almost all of the work in there. My plan is:

• Use the existing moodle1_block_handler class as the main handler. It will define all of the default, necessary processes for any block conversion.
• Extend that class as moodle1_block_generic_handler, for use by blocks that don't require their own conversion handlers. This extended class will simply call all of the default functions.
• Any block that requires more than the default conversions can extend and override this class with their own derived class, using the convention moodle1_block_[blockname]_handler.
• Modify the get_plugin_handlers function to load the generic or block specific handler depending on the block. If a specific block handler class exists it will load it, otherwise it will load the generic class.

For my plan, the best place to start is with the main handler, defining the default processes. This would be adding a get_paths and a process_block function to the moodle1_block_handler. At this point, I want to take a look at what Paul Nicholls has proposed.

In Paul's changes, he has provided a moodle1_block_handler::process_block() function that pretty much does everything we need for all of the blocks. He has also provided a specific get_paths() function in every block to define the use of the process_block function. I can use the first part, but I will create a generic get_paths() function instead.

To use Paul's process_block function, I could just cut and paste his code into my version of the file. But instead, I am going to use the strength of Open Source collaboration and pull his changes into mine using Git.

I already have a local version of my Git repository checkout from my "github" repository, with a working branch for this issue. My local repository also has the main Moodle repository added as a remote named "upstream" to allow me to get the latest Moodle code from HQ. I can add Paul's as a new "remote" to my local one allowing me to "pull" commits from his. From the tracker issue, Paul's repository is https://github.com/pauln/moodle/ and he has documented specific commits for his work. So, in my local Git repository I issue the commands:

git fetch upstream
git pull upstream MOODLE_23_STABLE

This makes sure I have all of the latest Moodle 2.3 code in my local repository before I do any more changes. Next, I issue:

git remote add pauln git://github.com/pauln/moodle.git
git fetch pauln

This attaches Paul's repository to mine as a remote, and gets his latest update information. Now, the next command is the important one. I can grab just the changes he made to moodle1_block_handler by cherry picking the commit that modified that code, using the specific commit hash associated with it. I issue:

git cherry-pick 76a7c44491120a696104cd5a4755890c1968777b

This pulls in Paul's changes that added the process_block function to the moodle1_block_handler and maintains his commit information so that his changes are credited to him. This allows me to use his work without having to duplicate it. Once complete, I have the function:

public function process_block(array $data) {
    $newdata = array();
    $instanceid     = $data['id'];
    $contextid = $this->converter->get_contextid(CONTEXT_BLOCK, $data['id']);

    $newdata['blockname'] = $data['name'];
    $newdata['parentcontextid'] = $this->converter->get_contextid(CONTEXT_COURSE, 0);
    $newdata['showinsubcontexts'] = 0;
    $newdata['pagetypepattern'] = $data['pagetype'].='-*';
    $newdata['subpagepattern'] = '$@NULL@$';
    $newdata['defaultregion'] = ($data['position']=='l')?'side-pre':'side-post';
    $newdata['defaultweight'] = $data['weight'];
    $newdata['configdata'] = $data['configdata'];

    // block.xml
    $this->open_xml_writer("course/blocks/{$data['name']}/block.xml");
    $this->xmlwriter->begin_tag('block', array('id' => $instanceid, 'contextid' => $contextid));

    foreach ($newdata as $field => $value) {
        $this->xmlwriter->full_tag($field, $value);
    }

    $this->xmlwriter->begin_tag('block_positions');
    $this->xmlwriter->begin_tag('block_position', array('id' => 1));
    $this->xmlwriter->full_tag('contextid', $newdata['parentcontextid']);
    $this->xmlwriter->full_tag('pagetype', $data['pagetype']);
    $this->xmlwriter->full_tag('subpage', '');
    $this->xmlwriter->full_tag('visible', $data['visible']);
    $this->xmlwriter->full_tag('region', $newdata['defaultregion']);
    $this->xmlwriter->full_tag('weight', $newdata['defaultweight']);
    $this->xmlwriter->end_tag('block_position');
    $this->xmlwriter->end_tag('block_positions');
    $this->xmlwriter->end_tag('block');
    $this->close_xml_writer();

    // inforef.xml
    $this->open_xml_writer("course/blocks/{$data['name']}/inforef.xml");
    $this->xmlwriter->begin_tag('inforef');
    // TODO: inforef contents if needed
    $this->xmlwriter->end_tag('inforef');
    $this->close_xml_writer();
     // roles.xml
    $this->open_xml_writer("course/blocks/{$data['name']}/roles.xml");
    $this->xmlwriter->begin_tag('roles');
    $this->xmlwriter->begin_tag('role_overrides');
    // TODO: role overrides if needed
    $this->xmlwriter->end_tag('role_overrides');
    $this->xmlwriter->begin_tag('role_assignments');
    // TODO: role assignments if needed
    $this->xmlwriter->end_tag('role_assignments');
    $this->xmlwriter->end_tag('roles');
    $this->close_xml_writer();

    return $data;
}

The other piece I need is a generic get_paths function, that creates convert_path objects for each block without having to add it to the specific block's code base. Since moodle1_block_handler is an extension of moodle1_plugin_handler, the name of the plugin (the block in this case) is stored in an object variable called pluginname. I can use this to create a default, block-aware get_paths function:

public function get_paths() {
    $blockname = strtoupper($this->pluginname);
    return array(
        new convert_path('block', "/MOODLE_BACKUP/COURSE/BLOCKS/BLOCK/{$blockname}"),
    );
}

This pretty much gives me everything I should need to convert default blocks to Moodle 2 restore data. Now, since moodle1_block_handler is an abstract class, I need to extend a new class that uses it so that it can be executed. This will be my moodle1_block_generic_handler class. I create:

/**
 * Base class for block generic handler
 */
class moodle1_block_generic_handler extends moodle1_block_handler {

}

At this point, that's all I need in  that class, since it will just be using the default functions.

The last thing I need to do is to modify the get_plugin_handlers function to load the proper handling class for each block it finds. Currently, it only loads a class if a specific handler exists for each block. I want to change it so that it loads a specific handler if it exists, otherwise it loads the generic class. I modify the existing get_plugin_handlers class as follows:

protected static function get_plugin_handlers($type, moodle1_converter $converter) {
    global $CFG;

    $handlers = array();
    $plugins = get_plugin_list($type);
    foreach ($plugins as $name => $dir) {
        $handlerfile  = $dir . '/backup/moodle1/lib.php';
        $handlerclass = "moodle1_{$type}_{$name}_handler";
        if ($type != "block") {
            if (!file_exists($handlerfile)) {
                continue;
            }
            require_once($handlerfile);
        } else {
            if (!file_exists($handlerfile)) {
                $handlerclass = "moodle1_block_generic_handler";
            }
        }

        if (!class_exists($handlerclass)) {
            throw new moodle1_convert_exception('missing_handler_class', $handlerclass);
        }
        $handlers[] = new $handlerclass($converter, $type, $name);
    }
    return $handlers;
}

Essentially, if the plugin type is not a block, continue doing what it always did. If it is a block, and there is no specific block handler defined, then load the generic block handler moodle1_block_generic_handler.

If you want to see the specific code I used, you can see it by the specific commits:
https://github.com/mchurchward/moodle/commit/5036ed80ad57c65c8cc18409e0b59307e0061c52
and
https://github.com/mchurchward/moodle/commit/b346f7e63c9f105dc27fc47707349b90e172ad52

With these changes, I rerun a test restore of a 1.9 backup file with blocks, and it works! The restored course contains the blocks that were in the 1.9 site. But I'm not done yet. I have to handle the blocks that have extra restore needs. That will be in the next post.