Moodle
  1. Moodle
  2. MDL-27594

XML start/end parsing modifications correct order on empty paths

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1
    • Component/s: Backup
    • Labels:
    • Testing Instructions:
      Hide
      • Enable DEBUG = developer
      • Run the unit tests (Admin -> Developer -> Unit tests) by specifying "backup/util/xml/parser".
      • TEST: All tests pass ok.
      • TEST: The number of tests is, at least, 248
      • TEST: Restore some 2.x course, it should finish as usual
      Show
      Enable DEBUG = developer Run the unit tests (Admin -> Developer -> Unit tests) by specifying "backup/util/xml/parser". TEST: All tests pass ok. TEST: The number of tests is, at least, 248 TEST: Restore some 2.x course, it should finish as usual
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      This is one followup of MDL-27475, where David discovered one real example of start/process/end notifications not happening in correct order under some circumstances (empty chunks).

      The fix changes completely the way start/end notifications were dispatched previously, by implementing one simple stack of pending notifications and dispatching them when necessary.

      The fix adds 2 new test cases covering the trouble-maker situation (exaggerated!), without modifying previous tests, so no change is expected in behavior at all (BC ok).

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Sam Hemelryk added a comment -

            Thanks Eloy integrated now.

            Show
            Sam Hemelryk added a comment - Thanks Eloy integrated now.
            Hide
            Sam Hemelryk added a comment -

            And passed tested during integration

            Show
            Sam Hemelryk added a comment - And passed tested during integration
            Hide
            Aparup Banerjee added a comment -

            (this doesn't seem to be working for me in MDL-27440 )

            Show
            Aparup Banerjee added a comment - (this doesn't seem to be working for me in MDL-27440 )
            Hide
            Eloy Lafuente (stronk7) added a comment -

            One new commit has been added to the branch above, it simply takes out one incorrect and unnecessary condition and introduces some minor changes in the tests to make them stronger.

            The new commit, for reference, is: https://github.com/stronk7/moodle/commit/d4dd06591bce0921fbbd4844def92570df59d5f1

            Merging again the branch should do the job if I'm not wrong.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - One new commit has been added to the branch above, it simply takes out one incorrect and unnecessary condition and introduces some minor changes in the tests to make them stronger. The new commit, for reference, is: https://github.com/stronk7/moodle/commit/d4dd06591bce0921fbbd4844def92570df59d5f1 Merging again the branch should do the job if I'm not wrong. Ciao
            Hide
            Aparup Banerjee added a comment -

            i've rebased onto david's backup-convert branch, it works for me now thanks!

            Show
            Aparup Banerjee added a comment - i've rebased onto david's backup-convert branch, it works for me now thanks!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I've added the extra commit to integration.git (SamH not available today).

            I've rerun the tests and executed one complete course restore. Everything went ok.

            Ciao

            Show
            Eloy Lafuente (stronk7) added a comment - I've added the extra commit to integration.git (SamH not available today). I've rerun the tests and executed one complete course restore. Everything went ok. Ciao
            Hide
            Eloy Lafuente (stronk7) added a comment -

            And this is now part of Moodle upstream, many thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - And this is now part of Moodle upstream, many thanks!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                0 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: