Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.1
    • Fix Version/s: 2.1
    • Component/s: Backup
    • Testing Instructions:
      Hide
      • Run the unit tests (Admin -> Developer -> Unit tests) by specifying "backup/util/xml/parser".
      • TEST: All the tests pass ok
      • TEST: The number of tests is, at least 216

      (note this is one 2.1dev only integration)

      Show
      Run the unit tests (Admin -> Developer -> Unit tests) by specifying "backup/util/xml/parser". TEST: All the tests pass ok TEST: The number of tests is, at least 216 (note this is one 2.1dev only integration)
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      start_end_notifications

      Description

      It has been detected that the new start/end notifications were being dispatched in incorrect order some times.

      It was caused because we were using some parser internals to perform the notification. So the idea is to move the notifications to proper place, unrelated with parser and "ordered" for any reader (human or machine).

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Eloy - integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Eloy - integrated now
            Hide
            tsala Helen Foster added a comment -

            Tested on http://qa.moodle.net/

            1/1 test cases complete: 216 passes, 0 fails and 0 exceptions.

            Show
            tsala Helen Foster added a comment - Tested on http://qa.moodle.net/ 1/1 test cases complete: 216 passes, 0 fails and 0 exceptions.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Closing this, it's already part of upstream, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Closing this, it's already part of upstream, thanks!
            Hide
            mudrd8mz David Mudrak added a comment -

            Eloy, I can confirm the order is correct now. But I have noticed that now, the notify_path_start() is not executed if the path does not contain any final elements. Eg for

            <a>
              <b>
                <c>VALUE</c>
              </b>
            </a>

            the first start notification would be for "b started" because <a> contains just subpaths, not final elements. However, the notify_path_end() would be triggered for </a> normally. I just want to make sure this is expected/needed. If so, I will stop using notify_path_start() at all because I can't rely on it being executed...

            Show
            mudrd8mz David Mudrak added a comment - Eloy, I can confirm the order is correct now. But I have noticed that now, the notify_path_start() is not executed if the path does not contain any final elements. Eg for <a> <b> <c>VALUE</c> </b> </a> the first start notification would be for "b started" because <a> contains just subpaths, not final elements. However, the notify_path_end() would be triggered for </a> normally. I just want to make sure this is expected/needed. If so, I will stop using notify_path_start() at all because I can't rely on it being executed...
            Hide
            mudrd8mz David Mudrak added a comment -

            ... just thinking, is it related to the issue with the head-sub-branches? (that is when an element contents starts with a subbranch?)

            Show
            mudrd8mz David Mudrak added a comment - ... just thinking, is it related to the issue with the head-sub-branches? (that is when an element contents starts with a subbranch?)
            Hide
            mudrd8mz David Mudrak added a comment -

            Hmm and I seem to have yet another problem with the order. Let us have a structure like this (real example):

              <MOODLE_BACKUP>
                <COURSE>
                  <SECTION>
                    <ID>436</ID>
                    <MODS>
                      <MOD>
                        <ID>250</ID>
                        <ROLES_OVERRIDES>
                        </ROLES_OVERRIDES>
                      </MOD>
                    </MODS>
                  </SECTION>
                </COURSE>
              </MOODLE_BACKUP>

            and let us have the following paths registered:

            /MOODLE_BACKUP/COURSE/SECTIONS/SECTION
            /MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
            /MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES

            Then the event handler for the end of the .../ROLES_OVERRIDES path is executed first before the processing method of the .../MOD. Therefore I am unable to parse data progressively because I have no way how to get to the information of MOD's ID from the ROLES_OVERRIDES. The only way at the moment is to stash ROLES_OVERRIDES in a temporarily memory and actually process it (create a file in this case) later.

            IMHO the MOD's processing and event handling should happen first, no?
            when such a structure is parsed

            Show
            mudrd8mz David Mudrak added a comment - Hmm and I seem to have yet another problem with the order. Let us have a structure like this (real example): <MOODLE_BACKUP> <COURSE> <SECTION> <ID>436</ID> <MODS> <MOD> <ID>250</ID> <ROLES_OVERRIDES> </ROLES_OVERRIDES> </MOD> </MODS> </SECTION> </COURSE> </MOODLE_BACKUP> and let us have the following paths registered: /MOODLE_BACKUP/COURSE/SECTIONS/SECTION /MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD /MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES Then the event handler for the end of the .../ROLES_OVERRIDES path is executed first before the processing method of the .../MOD. Therefore I am unable to parse data progressively because I have no way how to get to the information of MOD's ID from the ROLES_OVERRIDES. The only way at the moment is to stash ROLES_OVERRIDES in a temporarily memory and actually process it (create a file in this case) later. IMHO the MOD's processing and event handling should happen first, no? when such a structure is parsed
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Yes, 100%.

            That is the sort of things I supposedly tested with helper_check_notifications_order_integrity() in parser tests, grrr. Going to check with your example...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Yes, 100%. That is the sort of things I supposedly tested with helper_check_notifications_order_integrity() in parser tests, grrr. Going to check with your example...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Side comment, just to be 100% sure... in the example above... have you noticed that the registered paths and the XML doesn't match? I mean, there is no "SECTIONS" tree in the XML file.

            I've added it here, to my tests, to continue investigating... just commenting in case that's related with your problem.

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Side comment, just to be 100% sure... in the example above... have you noticed that the registered paths and the XML doesn't match? I mean, there is no "SECTIONS" tree in the XML file. I've added it here, to my tests, to continue investigating... just commenting in case that's related with your problem. Ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Wow, this is crazy, I get these notifications:

            Ordered allnotifs:
                [0] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION
                [1] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION
                [2] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES
                [3] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [4] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [5] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [6] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION

            So, obviously, something is wrong there... going to debug a bit....

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Wow, this is crazy, I get these notifications: Ordered allnotifs: [0] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION [1] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION [2] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES [3] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [4] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [5] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [6] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION So, obviously, something is wrong there... going to debug a bit....
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited

            Ah, my friend, I think I've got it. Problem happens because ROLES_OVERRIDES is empty, afaik. I've added one simple ID tag below it and then I get this (correct) list of notifications:

            Ordered allnotifs:
                [0] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION
                [1] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION
                [2] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [3] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [4] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES
                [5] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES
                [6] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES
                [7] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD
                [8] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION

            So, at some point, when one path is "empty", we are skipping too many calls... debugging...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - - edited Ah, my friend, I think I've got it. Problem happens because ROLES_OVERRIDES is empty, afaik. I've added one simple ID tag below it and then I get this (correct) list of notifications: Ordered allnotifs: [0] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION [1] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION [2] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [3] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [4] => start:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES [5] => process:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES [6] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD/ROLES_OVERRIDES [7] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION/MODS/MOD [8] => end:/MOODLE_BACKUP/COURSE/SECTIONS/SECTION So, at some point, when one path is "empty", we are skipping too many calls... debugging...
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The solution for this case has been requested for integration @ MDL-27594, plz follow it there, thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The solution for this case has been requested for integration @ MDL-27594 , plz follow it there, thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Jul/11