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:
    • Rank:
      17414

      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).

        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: