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

          Attachments

            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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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 Mudrák 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