Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major 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
    • Rank:
      1290

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

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy - integrated now

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

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

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

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

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

          Show
          Eloy Lafuente (stronk7) added a comment - Closing this, it's already part of upstream, thanks!
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          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: