Doctrine 1
  1. Doctrine 1
  2. DC-248

Manager-level record listeners not firing when retrieving models with actAs templates

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Can't Fix
    • Affects Version/s: 1.1.5
    • Fix Version/s: None
    • Component/s: Record
    • Labels:
      None

      Description

      I'm having a bit of trouble getting my record listener methods to fire properly, as described in the issue title. To reproduce:

      1. Create a model classes which uses one or more built-in actAs behaviors (timestampable, sluggable, etc.).
      2. Create two instances of Doctrine_Overloadable, one to serve as a record listener and one to serve as an event listener:

      class My_Record_Listener extends Doctrine_Overloadable
      {
        public function __call($methodName, $args)
        {
          echo "Doctrine_Record_Listener::$methodName" . PHP_EOL;
        }
      }
      
      class My_EventListener extends Doctrine_Overloadable
      {
        public function __call($methodName, $args)
        {
          echo "Doctrine_EventListener::$methodName" . PHP_EOL;
        }
      }
      

      3. Set up a test script that bootstraps your Doctrine manager and connection; make sure to add the two event listeners to the manager object as follows:

      $manager->addListener(new My_EventListener());
      $manager->addRecordListener(new Doctrine_Record_Listener());
      

      4. Then, once your connection is set up, retrieve an instance of the table for the model, and run something along these lines:

      $table = $conn->getTable('Model');
      
      echo "Running Doctrine_Table::findAll()..." . PHP_EOL;
      $records = $table->findAll();
      echo "...done." . PHP_EOL;
      

      5. When you run this test script, the output will look something like this:

      Running Doctrine_Table::findAll()...
      Doctrine_EventListener::preConnect
      Doctrine_EventListener::postConnect
      Doctrine_EventListener::preQuery
      Doctrine_EventListener::postQuery
      ...done.
      

      6. Adjust the model such that it no longer includes the behaviors (e.g., comment out the $this->actAs() lines). Now, if you run the script again, you'll get this output instead (assuming there are records in the database):

      Running Doctrine_Table::findAll()...
      Doctrine_Record_Listener::getOption
      Doctrine_Record_Listener::preDqlSelect
      Doctrine_EventListener::preConnect
      Doctrine_EventListener::postConnect
      Doctrine_EventListener::preQuery
      Doctrine_EventListener::postQuery
      Doctrine_Record_Listener::getOption
      Doctrine_Record_Listener::preHydrate
      Doctrine_Record_Listener::getOption
      Doctrine_Record_Listener::postHydrate
      Doctrine_Record_Listener::getOption
      Doctrine_Record_Listener::preHydrate
      Doctrine_Record_Listener::getOption
      Doctrine_Record_Listener::postHydrate
      ...done.
      

      In a nutshell, it looks as though registered record listener hooks aren't firing if the model has some actAs() behaviors.

      Is this by design? If so, is there a way to get those hooks to fire even when the behaviors are registered?

      Thanks!
      Adam

        Activity

        Hide
        Jonathan H. Wage added a comment -

        It's because of actAs, it is because the behavior you actAs adds its own record listener, which overrides the manager. I believe this is the real issue.

        Show
        Jonathan H. Wage added a comment - It's because of actAs, it is because the behavior you actAs adds its own record listener, which overrides the manager. I believe this is the real issue.
        Hide
        Adam Jensen added a comment -

        That sounds like what I'm seeing; however, I should also note that it works perfectly fine to add multiple behaviors (my test model acts as timestampable, sluggable, versionable, searchable and softdelete) ...all of their listeners fire exactly as expected; it's just when you add a listener manually to the manager that it fails.

        Given that, I suppose one possible workaround would be to bundle my new listener into a custom behavior ...but for that to work globally, I'd need to attach it at the schema level to every one of my models. It'd certainly be more convenient to be able to add it to the manager instead.

        In any case, thanks for looking into this!

        Show
        Adam Jensen added a comment - That sounds like what I'm seeing; however, I should also note that it works perfectly fine to add multiple behaviors (my test model acts as timestampable, sluggable, versionable, searchable and softdelete) ...all of their listeners fire exactly as expected; it's just when you add a listener manually to the manager that it fails. Given that, I suppose one possible workaround would be to bundle my new listener into a custom behavior ...but for that to work globally, I'd need to attach it at the schema level to every one of my models. It'd certainly be more convenient to be able to add it to the manager instead. In any case, thanks for looking into this!
        Hide
        Jonathan H. Wage added a comment -

        The multiple actAs all work because the listeners for each behavior are attached to the table level, overriding anything that was set at the connection or manager level. It works by going through the hierarchy checking if each level has listeners where table has the highest precedence. I think this was specifically designed this way so I am not sure if it can be "patched"..Will have to look.

        Show
        Jonathan H. Wage added a comment - The multiple actAs all work because the listeners for each behavior are attached to the table level, overriding anything that was set at the connection or manager level. It works by going through the hierarchy checking if each level has listeners where table has the highest precedence. I think this was specifically designed this way so I am not sure if it can be "patched"..Will have to look.
        Hide
        Benjamin Arthur Lupton added a comment - - edited

        I just spent 3 hours trying to figure out why my sanitise listener was not firing for specific tables, and finally came to this conclusion here after much debugging. As Wage has said, it is not the actAs but rather the addition of another listener on the template/record/table level.

        This is a real misleading "by design" if it is, as the documentation never mentions such an override so naturally you expect manager and connection listeners to continue to work. I will have to add a sanitise template and add it manually to records that use other templates with listeners for the meantime. But again, this is the most annoying bug I've ever experienced with doctrine so far, and has caused me to storm around the office in near rage prepared to throw doctrine out a window!

        If this won't be fixed (which I'm dearly hoping it will to prevent any unexpected issues for future developers), then please add a huge disclaimer in the documentation informing the users about this feature.

        For myself, I'm particularly shocked as I was using a global level listener for stripping html out of all the fields that weren't set as html fields, so when suddenly javascript was starting to run in particular records, it started getting to risky business! Going to be a long haul ensuring that previous sites implemented remain secure.

        Show
        Benjamin Arthur Lupton added a comment - - edited I just spent 3 hours trying to figure out why my sanitise listener was not firing for specific tables, and finally came to this conclusion here after much debugging. As Wage has said, it is not the actAs but rather the addition of another listener on the template/record/table level. This is a real misleading "by design" if it is, as the documentation never mentions such an override so naturally you expect manager and connection listeners to continue to work. I will have to add a sanitise template and add it manually to records that use other templates with listeners for the meantime. But again, this is the most annoying bug I've ever experienced with doctrine so far, and has caused me to storm around the office in near rage prepared to throw doctrine out a window! If this won't be fixed (which I'm dearly hoping it will to prevent any unexpected issues for future developers), then please add a huge disclaimer in the documentation informing the users about this feature. For myself, I'm particularly shocked as I was using a global level listener for stripping html out of all the fields that weren't set as html fields, so when suddenly javascript was starting to run in particular records, it started getting to risky business! Going to be a long haul ensuring that previous sites implemented remain secure.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Adam Jensen
          • Votes:
            2 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: