Visa källa för Backup 2.0 - Converters review 2011-04

[[Development:Backup 2.0|Backup 2.0]] > [[Development:Backup 2.0 - Provide upwards compatibility of Moodle 1.9.x backups|Backup 1.9 => 2.x main page]] > Review 2011-04
{{Template:Development:Backup 2.0}}{{Moodle_2.1}}

This page summarizes the results of the review performed (on April 2011) over the code available at https://github.com/mrmark/moodle related to the implementation of a general solution for the creation and support of moodle restore converters and its first implementation as one Moodle 1.9 => 2.x converter.

=== References ===

* MDL-22414 - the main tracker issue for this task.
* [[Development:Backup 2.0 - Provide upwards compatibility of Moodle 1.9.x backups|Backup 1.9 => 2.x article]] in Moodle Docs.
* [http://tracker.moodle.org/secure/attachment/23241/Moodle1.9to2.0Restore.pdf PDF with initial implementation plan].
* [https://github.com/mrmark/moodle/compare/master...converters_wip Development branch changes] 

=== Nomenclature ===

* The review results are shown below as one list of numbered points, briefly explained, with link to corresponding MDL-xxxx (subtask of MDL-22414), where any discussion / agreement / coding should happen.
* All the MDL-xxxx issue created will begin with R04xx to note they have been ignited as part of this review process and the point they correspond to.
* Each point below includes one short "status" message in order to know the immediate actions that must be performed on it.
* New points can be added to the review as long as discussion / agreement happens, if necessary.

=== Global summary ===

* Overall, all code seems to be in correct places and be correct code.
* The main infrastructure parts (base/plan/moodle1 converters and plan/task/step architectures) are ok, though their APIs would need some improvements detailed below.
* There are a lot of stuff missing / todo. Files, users, enrolments, plugins, tricky activities, comments, grades...
* We need to push-push-push (4 weeks to freeze!).

=== The list ===

==== Related to integration with restore_controller & general behavior ====

* R0401: status: to code (MDL-27376) Right now, the to_moodle2_format() performs instantiation of all available converters + simple loop to get the conversions to perform. Instead, it should be possible to use exclusively static methods, returning origin format, target format and cost of the conversion, and then finding the best path or so. Note this is trivial right now but can be a useful general approach if new converters arrive (core and 3rd part).

* R0402: status: to code (MDL-27377) The can_convert() function can be static too, passing tempdir as param. Also, the same function (or another one, mandatory) should check if all the PHP/server requirements are satisfied or no (some converters can need extra stuff available in order to be able to perform conversions).

* R0403: status: to discuss. After each conversion, the original source directory is deleted and replaced by the converted one. This makes things complex to trace or perform repeated conversions and also, doesn't observe the $CFG->keeptempdirectoriesonbackup completely. We should be able to keep temp stuff undeleted if desired and, perhaps too, be able to instruct the restore controller about tempdir changes.

* R0404: status: to code. Exceptions: It needs to be decided if we are going to use own converter_exception/s or restore_exception/s and use them constantly along all the code base.

* R0405: status: to code. Logging: Converters stuff must inherit / use restore_controller logging facilities.

==== Related to the converters/moodle1 infrastructure ====

* R0406: status: to discuss. All the parser / processor / path_elelement stuff is part of the abstract plan_converter. That stuff should be moved to moodle1_converter as far as it is highly dependent / tidied with the conversion particularities.

* R0407: status: to discuss (not important). About converters, should class names and file names match, i.e. converter.class.php => moodle1_converter.class.php

* R0408: status: to discuss. On restore, we have both after_restore() methods available in steps (executed when the step execution ends) and in tasks (executed when the whole plan execution ends). Do we need this duality in plan_converter, or is it enough with current one. Right now it seems that we are using a lot these methods to do the work, and process_xxx() should be used instead (together with next point).

* R0409: status: to code. Since some weeks ago the notify_path_start/end() methods are available in the xml processor, should we start using them to call some stuff in converter, able to dispatch to start_xxxx() and end_xxxx() methods in the converter steps. It would help a lot with both starting/closing XML tags, creating and closing xml files, or any other pre/post stuff needing to be executed at certain stages.

* R0410: status: to discuss. The deprecated/new/renamed elements / mutate_datum stuff. Perhaps it should be moved from steps to path_elements instead, so each path element autocontains the basic transformations to be performed on data and they are applied automatically. It seems a better place for that definitions. Or, alternatively, we'll need deprecated/new/renamed/mutate_xxxx() methods, one for each path.

* R0411: status: to code. After looking current uses of the xml writer, it seems clear that we need some extra methods, able to print one tag completely, push/pop last open tag and other commodities in order to make coding easier.

* R0412: status: to code. The "trick" in the parser about dynamically modify /MOD paths seems ok (I cannot imagine another way to implement it). Similar magic will be needed for blocks and surely, the start/end implementation above will need a similar way to handle/dispatch processing.

* R0413: status: to code. Some raw accesses to the backup_ids_temp DB stuff must be modified to use helper functions. Also, the use of the table in get_context() can be really slow (text comparison). Look for some alternative.

* R0414: status: to code. The convert_file() method uses the backup_ids_temp to get sequential fileid values. It should be enough (completely safe), to use one static (in memory) generator instead. No concurrence problems at all IMO.

* R0415: status: to discuss. Support for plugins is a must as far as at least qtypes require it. Note that question types are going to suffer one major revamp on Moodle 2.1, so their final architecture is unknown right now (different from both 1.9 and 2.0).

* R0416: status: to code (tiny detail). In current moodle1_conversion, original_site_identifier should be generated only if there is one original identifier. Right now it saves one "?" that can lead to wrong results detecting same sites.

==== Related to coding in general ====

* <strike>R0417</strike>: status: done. Copyright messages are required on each new file. It must be the standard one with GPL3 msg and MR/Mark/whatever copyright.

* R0418: status: to code. We should have as many tests as possible, specially testing all the infrastructure stuff, in order to guarantee it continues working as expected after any refactor / modification. Right now it's practically inexistent.

* R0419: status: to do periodically. We need to keep the main repo for all the project "converters_wip" in sync (merged) with current moodle.git master branch in order to ensure stuff will land properly (conflicts-free) and converters will be able to use any new feature / improvement available.

==== Related to pending & tricky areas ====

* R0420: status: to code. There are a lot of areas which coding hasn't been ignited yet (surely to pending work in the list above, don't worry). Here there is a brief ordered list, annotating the ones that will be really tricky:
** Users and enrollments: All the users with their complete information must be saved to new users.xml file and each process_xxx() method must, continuously, be annotating needed uses in order to generate proper inforef.xml files. Enrolments will be all moved to manual ones (this needs confirmation).
** Workshop activity: Completely revamped in 2.0. It has proper subplugins. Upgrade code needs to be deeply analyzed and XML differences too to know how to implement it. (contact: David).
** Wiki activity: Also brand new in 2.0. Upgrade needs analysis and XML differences too. (contact: Dongsheng).
** Resource activity: Split into  file/folder/page/url activities. The structure is simple (no user data) but the split makes it really tricky. Also very file-intensive. Upgrade code needs analysis and XML differences (contact: Petr).
** Files: 1.9 files can had site/course files (apart from the ones "owned" by the module @ moddata). They must be annotated and moved to new files.xml & storage. There is some code already but is not used in the general converter helper stuff (should be moodle1 converter specific instead?).
** Gradebook: There are important differences between how 1.9 and 2.0 backup information is handled. Scales / outcomes / grade items need annotation in inforef.xml files and also there are new xml files designed to store specifically grades (categories, items, grades...) information. Need analysis. (contact: Andrew).

====After review, organization====

This section will be used to annotate any agreement / conclusion / organization for the next weeks in case the Tracker isn't enough to do so. Feel free to use it, beloved colleagues, always using the R04xx nomenclature for referencing exact stuff, plz.

=== See also ===

* Go back to [[Development:Backup 2.0|Backup 2.0]] > [[Development:Backup 2.0 - Provide upwards compatibility of Moodle 1.9.x backups|Backup 1.9 => 2.x main page]].