We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 38371
    • 42 Posts
    Description of Problem:

    The context of the problem requires 4 classes. Assume the following classes (see attachments for the schema file):

    1. modUser
    2. myUser
    3. myChildUser
    4. myComp

    Class myUser extends modUser and class myChildUser extends myUser. As can be seen from the schema, class myComp is a composite [1] of myUser.


    Class diagram of the classes involved.

    Steps to Reproduce:

    1. Upload "setup.php" (see attachments) to your web root and run it [2].
    2. Modify classes "myChildUser" and "myUser" as shown below.
    3. Run the Testing Snippet whose code is given below.

    myUser
    <?php
    
    class myUser extends modUser {
    
        public function __construct(xPDO & $xpdo) {
            parent::__construct($xpdo);
            $this->set('class_key', 'myUser');
        }
    }
    


    myChildUser
    <?php
    
    require_once MODX_CORE_PATH . 'components/mytest/model/mytest/myuser.class.php';
    
    class myChildUser extends myUser {
    
        public function __construct(xPDO & $xpdo) {
            parent::__construct($xpdo);
            $this->set('class_key', 'myChildUser');
        }
    }
    


    Testing Snippet
    <?php
    
    $modx->setLogLevel(xPDO::LOG_LEVEL_INFO);
    $modx->setLogTarget(XPDO_CLI_MODE ? 'ECHO' : 'HTML');
    $modx->log(xPDO::LOG_LEVEL_INFO, 'Hello!');
    
    $path = MODX_CORE_PATH . 'components/mytest/model/';
    $modx->addPackage('mytest', $path, 'mt_');
    
    /* Uncomment line below and comment the next one to make the problem disappear. */
    //$user = $modx->newObject('myUser');
    $user = $modx->newObject('myChildUser');
    
    $comp = $modx->newObject(
        'myComp',
        array(
            'name' => 'Name'
        )
    );
    $added = $user->addOne($comp, 'Comp');
    
    return  $added ? 'true' : 'false';
    


    Expected Outcome:

    The value of $added should be "true". Since class myChildUser extends myUser, I am under the assumption that it should inherit all of myUser's relationships, too.

    Uninstallation

    You can run "uninstall.php" to revert all changes performed on your MODX installation.


    • MODX Version: 2.2.7pl
    • PHP Version: 5.4.3
    • Database (MySQL, SQL Server, etc) Version: MySQL 5.5.24
    • Additional Server Info: WAMP 2.2E (32-bit)
    • Installed MODX Add-ons: None
    • Error Log Contents:
    • [...] (ERROR @ /index.php)
      No foreign key definition for parentClass: myChildUser using relation alias: Comp

    Update: Added the class diagram.
    Update: Added "setup.php" and "uninstall.php".
    Update: Flagged as answered. The solution is given below.

    I have flagged the question as answered. The solution that was finally implemented has the base classes of the Extra loaded in the Service class.

    myextra.class.php
    public function __construct(modX &$modx) {
        $this->modx = &$modx;
        $this->modx->addPackage('pkgName', 'path/to/extra/model/', 'tablePrefix_');
        $this->loadBaseClasses();
    }
    


    where the method myExtra::loadBaseClasses() loads all the classes that are used as parents of another class in the package. In our example, a base class is myUser.

    Subnotes

    [1]: http://rtfm.modx.com/display/xPDO20/Defining+Relationships#DefiningRelationships-CompositeRelationships
    [2]: Either from your console (e.g., "php setup.php") or from your web browser (e.g., http://mydomain/setup.php).

    This question has been answered by dunnock. See the first response.

    [ed. note: leffyrant last edited this post 13 years, 4 months ago.]
      • 38371
      • 42 Posts
      Cause of the Problem

      After some debugging, I have found that the cause of the problem is that the class myUser is not present in the modX::map [1].

      Expected Behavior

      I would expect that when creating a new instance of an object, like the following code:
      $user = $modx->newObject('myChildUser');
      

      that all of the ancestors of this class would be added to the modX::map. In the above example, I would expect myUser's map file to be added automatically to the modX::map. However, this is not the case.

      Proposed Workaround

      As a temporary solution, I have thought to modify the constructor of the myChildUser class in order to load the ancestor (i.e., myUser). So, the constructor would look like this:
      class myChildUser extends myUser {
      
          public function __construct(xPDO & $xpdo) {
              $xpdo->loadClass('myUser', MODX_CORE_PATH . 'components/mytest/model/mytest/');
              parent::__construct($xpdo);
              $this->set('class_key', 'myChildUser');
          }
      }
      


      But, I would like to hear from anyone on that matter. Specifically, I would greatly appreciate an answer to any of the following questions:

      • Is there anything I am missing? For example, is there already an alternative way to achieve a two (or more) levels inheritance?
      • Is this a bug in xPDO or am I simply on the wrong path? If the latter is true, then what should I be doing instead?

      Thanks in advance for any answers and sorry for the lengthy post:-)

      Subnotes

      [1]: This is where the map files (e.g., moduser.map.inc.php) are loaded when the modX::loadClass() method is called. [ed. note: leffyrant last edited this post 13 years, 5 months ago.]
      • discuss.answer
        • 42602
        • 81 Posts
        I guess the common way would be to just to loadClass() the parent class before newObject.

        Another common solution is to use service class to addPackage() within the constructor and the loadClass('myUser'); directly after addPackage().

        More about service classes can be found in the Doodle tutorial http://rtfm.modx.com/display/revolution20/Developing+an+Extra+in+MODX+Revolution#DevelopinganExtrainMODXRevolution-MakingtheDoodlesBaseClass or in xPDO documentation http://rtfm.modx.com/display/xPDO20/As+Service+Layer

        Even core MODx class has the definitions in the constructor as many of core classes do not extend directly from xPDOObject or xPDOSimpleObject but from modAccessibleSimpleObject etc... so they are loaded with loadClass() for future usage when loading extended objects.
          • 38371
          • 42 Posts
          First, I want to thank you for taking the time to reply.

          Quote from: dunnock at May 20, 2013, 05:28 AM
          I guess the common way would be to just to loadClass() the parent class before newObject.

          So, you mean something like this:
          $modx->loadClass('myUser', MODX_CORE_PATH . 'components/mytest/model/');
          $user = $modx->newObject('myChildUser');
          

          That seems to me as unexpected behavior. By that I mean that specific documentation should be added to the myChildUser class in order for any developer who would wish to use it to know about that special way of instantiating it.

          Furthermore, the loading of all the ancestor classes' relationships does not seem so special a behavior to me. On the contrary, given the inheritance property of object oriented programming, I believe that this loading is expected when creating a new instance of the myChildUser class.

          In the context of this solution, do you think that using this pattern is better than calling the modX::loadClass() from within the myChildUser constructor, as mentioned above (I include the code right below for easy reference)?
          class myChildUser extends myUser {
           
              public function __construct(xPDO & $xpdo) {
                  $xpdo->loadClass('myUser', MODX_CORE_PATH . 'components/mytest/model/mytest/');
                  parent::__construct($xpdo);
                  $this->set('class_key', 'myChildUser');
              }
          }
          


          Quote from: dunnock at May 20, 2013, 05:28 AM
          Another common solution is to use service class to addPackage() within the constructor and the loadClass('myUser'); directly after addPackage().

          If I understand correctly you mean something like the following.
          class myExtra {
              
              private $modx;
              
              public function __construct(modX $modx) {
                  $this->modx = $modx;
                  $this->modx->addPackage('mytest', MODX_CORE_PATH . 'components/mytest/model/');
                  $this->modx->loadClass('myUser', MODX_CORE_PATH . 'components/mytest/model/mytest/');
              }
          }
          

          That would mean that the myUser class would be loaded everytime my Extra [1] would run. However, this is not desirable because the Extra also performs tasks that are irrelevant to this class.

          Solution Enquiry

          I am leaning towards a solution that would include a patch to the xPDOObject class' constructor. I will post the code for contribution after testing it, but the algorithm will involve the following steps:

          1. Retrieve the ancestry of this class (xPDO::getAncestry()).
          2. Load the classes (xPDO::loadClass()) in the ancestry.

          This will occur right before the loading of the relationships (i.e., aggregates and composites) in the constructor.

          I will create a pull request and if the patch is accepted I will post here the solution.

          I would like to thank you again for your answer!

          Subnotes

          [1]: I know I have not mentioned anything about an Extra, but the problem presented in this post has come up during the development of an Extra. Thus, all my answers are justified in the context of a MODX Extra development process.
            • 42602
            • 81 Posts
            100% safe is that you add require_once('myuser.class.php'); to start of the mychilduser.class.php file.

            Problem loading the parent class on constructor of the child class is that you will still get fatal error. As the myUser class is not defined. So the inclusion of the parent has to be done before the child is included. loadClass() in the end is just fancy include.

            Need to ponder, the getAncestry definitely leads to right direction but just wrong place. To get it working correctly it should be set with loadClass() already. Check the ancestry from map (which hopefully is there) and then load them in correct order. In the end, you could simply load the class with any plugin on page render, it does not consume memory that much and is always available. Also xPDO should have additional default classes loaded functionality with the constructor. But looks like that the constant is wrong in it right now, (guess that should post bug report on it)
              • 38371
              • 42 Posts
              Quote from: dunnock at May 20, 2013, 11:09 AM
              100% safe is that you add require_once('myuser.class.php'); to start of the mychilduser.class.php file.

              Yes, you are right. I have done so, but did not post the whole code. The full code, as per your recommendation, is the following.
              require_once MODX_CORE_PATH . 'components/mytest/model/mytest/myuser.class.php';
              
              class myChildUser extends myUser {
              
                  public function __construct(xPDO & $xpdo) {
                      $xpdo->loadClass('myUser', MODX_CORE_PATH . 'components/mytest/model/mytest/');
                      parent::__construct($xpdo);
                      $this->set('class_key', 'myChildUser');
                  }
              }
              


              Quote from: dunnock at May 20, 2013, 11:09 AM
              Need to ponder, the getAncestry definitely leads to right direction but just wrong place. To get it working correctly it should be set with loadClass() already.


              I haven't thought of it before. I think this is a very good idea.
              Quote from: dunnock at May 20, 2013, 11:09 AM
              Also xPDO should have additional default classes loaded functionality with the constructor. But looks like that the constant is wrong in it right now, (guess that should post bug report on it)

              You mean the xPDO::OPT_BASE_CLASSES constant? If yes, I believe that this would have the effect of loading the myUser class every time my Extra would run. That would be similar to the situation with the service class mentioned above.

              Subnotes

              [1]: As close as a change to a core class can get.
                • 38371
                • 42 Posts
                By examining the code of the methods xPDOObject::getAggregates() and xPDOObject::getComposites() you can see that the descendants of a class are indeed loaded (i.e., the method xPDO::loadClass() is called).

                I think that it would be proper to load the ancestors through those methods, too. So, for example, the xPDO::getComposites() method would become:
                public function getComposites($className) {
                        $composites= array ();
                        if ($className= $this->loadClass($className)) {
                            if ($ancestry= $this->getAncestry($className)) {
                                for ($i= count($ancestry) - 1; $i >= 0; $i--) {
                                    $ancestorClass = $this->loadClass($ancestry[$i]);
                                    if ($ancestorClass && isset ($this->map[$ancestry[$i]]['composites'])) {
                                        $composites= array_merge($composites, $this->map[$ancestry[$i]]['composites']);
                                    }
                                }
                            }
                            if ($this->getInherit($className) === 'single') {
                                $descendants= $this->getDescendants($className);
                                if ($descendants) {
                                    foreach ($descendants as $descendant) {
                                        $descendantClass= $this->loadClass($descendant);
                                        if ($descendantClass && isset($this->map[$descendantClass]['composites'])) {
                                            $composites= array_merge($composites, array_diff_key($this->map[$descendantClass]['composites'], $composites));
                                        }
                                    }
                                }
                            }
                        }
                        return $composites;
                    }
                


                The additions are at lines 6 and 7. The ancestor class is loaded and, then, a check is performed before its composites are added.

                I will submit this patch and see if it is right or if the ancestors are not loaded for a reason. [ed. note: leffyrant last edited this post 13 years, 4 months ago.]
                  • 42602
                  • 81 Posts
                  Still in the wrong place as the loadClass() before the ancestry check would cause Fatal error.

                  This then again works, (note, just a demo use only wink ), xPDO::loadClass() around line 621 where the first comment //Check depency is set. This will try to load the class if it is not declared. Package needs to be set before this works.

                  if (!empty($path) || $ignorePkg) {
                      $class= $this->_loadClass($class, $fqn, $included, $path, $transient);
                  } elseif (isset ($this->packages[$this->package])) {
                      $pqn= $this->package . '.' . $fqn;
                      // Check depency
                      foreach ($this->classMap as $baseClass => $classes) {
                          if (array_search($classname, $classes) !== false && !class_exists($baseClass)) { $this->loadClass($baseClass); }
                      }
                      // End Check
                      if (!$pkgClass= $this->_loadClass($class, $pqn, $included, $this->packages[$this->package]['path'], $transient)) {
                          foreach ($this->packages as $pkg => $pkgDef) {
                              if ($pkg === $this->package) continue;
                              $pqn= $pkg . '.' . $fqn;
                              if ($pkgClass= $this->_loadClass($class, $pqn, $included, $pkgDef['path'], $transient)) {
                                  break;
                              }
                          }
                      }
                      $class= $pkgClass;
                  } else {
                      $class= false;
                  }


                  Added: Used your schema and tried the "fix". It worked fine and no need to have extra loadClass() methods or anything else really. Used next snippet: (do no care about flawed package path). Also instead of saving user first, just simply set the id to mimic the relation required. Had table for the comp setup and one row there which had the right user_id set for my user.

                  $vchart = $modx->addPackage('mytest', $modx->getOption('core_path') . 'components/myuser/model/model/', 'test_');
                  $usr = $modx->newObject('myChildUser');
                  $usr->set('id', 1);
                  $comp = $usr->getOne('Comp');
                  return "<pre>" . print_r($comp->toArray(), true);


                  Output was fetched from database correctly. [ed. note: dunnock last edited this post 13 years, 5 months ago.]
                    • 38371
                    • 42 Posts
                    Quote from: dunnock at May 21, 2013, 02:15 AM
                    Still in the wrong place as the loadClass() before the ancestry check would cause Fatal error.

                    I assume you are talking about lines 6 and 7 [1]. Yes, you are right. That is the case if you do not include class myUser from the file where the myChildUser is declared.

                    However, I include the said class, as it is shown in Reply #6. Nonetheless, you were right to presuppose that this is not the case because when the myUser is included, then the call to the xPDO::loadClass() (e.g., line 6 of Reply #7) is useless. This is due to the fact that the method xPDO::loadClass() needs the path of a class if the latter has already been included.

                    A path is not specified in the above scenario. As a result, the class myUser is not added to the modX::map.

                    Quote from: dunnock at May 21, 2013, 02:15 AM
                    This then again works, (note, just a demo use only wink ), xPDO::loadClass() around line 621 where the first comment //Check depency is set. This will try to load the class if it is not declared.

                    if (!empty($path) || $ignorePkg) {
                        $class= $this->_loadClass($class, $fqn, $included, $path, $transient);
                    } elseif (isset ($this->packages[$this->package])) {
                        $pqn= $this->package . '.' . $fqn;
                        // Check depency
                        foreach ($this->classMap as $baseClass => $classes) {
                            if (array_search($classname, $classes) !== false && !class_exists($baseClass)) { $this->loadClass($baseClass); }
                        }
                        // End Check
                        if (!$pkgClass= $this->_loadClass($class, $pqn, $included, $this->packages[$this->package]['path'], $transient)) {
                            foreach ($this->packages as $pkg => $pkgDef) {
                                if ($pkg === $this->package) continue;
                                $pqn= $pkg . '.' . $fqn;
                                if ($pkgClass= $this->_loadClass($class, $pqn, $included, $pkgDef['path'], $transient)) {
                                    break;
                                }
                            }
                        }
                        $class= $pkgClass;
                    } else {
                        $class= false;
                    }


                    Indeed, this solution works.

                    However, this solution made think of a problem that might arise after loading a class with this code. I believe this problem is present to the current MODX code base, too. But before mentioning this problem, allow me to comment on the code a little.

                    There are two things I would like to propose:

                    1. I think the existence check is unnecessary, since it is performed in the xPDO::loadClass() method, too.
                    2. A "break" statement could be added after a successful loading of a base class, since there can be only one such class (see Unique Model Class Name Constraint, below).

                    The code for the changes above would be:
                    foreach ($this->classMap as $baseClass => $classes) {
                        if (array_search($classname, $classes) !== false) {
                            if ($this->loadClass($baseClass) !== false) {
                                break;
                            }
                        }
                    }
                    


                    Unique Model Class Name Constraint

                    Don't you think that there is a deeper problem here? Please, correct me if I'm wrong, but, as it stands now, isn't the xPDO::newObject() method a little restrictive in the logic it follows in order to instantiate a class?

                    By that I mean that, you provide to this method the class name [2] and it calls the xPDO::loadClass() with that name. This situation requires that someone creates class names that are unique across all the classes that are loaded by a particular MODX installation (core and extra related ones).

                    So, for example, imagine that there are currently two packages, "pkg_a" and "pkg_b" added to the MODX instance [3]. If both of the packages have each a class named ClassA, then by writing:
                    $a = $modx->newObject('ClassA');
                    

                    MODX will load the ClassA declared in "pkg_a". That's because it stops searching for further declarations of a class after finding one.

                    Don't you think that a call like this:
                    $a = $modx->newObject('ClassA', 'packageA');
                    

                    would be more suitable?

                    Of course, the above still requires that the package "packageA" is loaded (i.e., a call to xPDO::addPackage()) before the above call.

                    Subnotes

                    [1]: If not, then you must be referring to lines 3 and 4. These lines were there already. They were not added.
                    [2]: I assume that the "fields" parameter is irrelevant to this discussion.
                    [3]: Assume that the "modx" and "om" packages are not added by default and, further, they are not required for this example.
                      • 42602
                      • 81 Posts
                      It is true that the issue is deeper down in the code base, like the issue if the extras for example would have two same classes. And when the system gets getting bigger, someday there will be definitely a class collision in way or another. Final solution would be namespacing when it comes to that, maybe xPDO 3 and MODx 3 will utilize them.

                      For the package (if leaving the potential class collision out of the occasion), as the second param is field values for newObject and rest of the object fetching methods usually have criteria. Alternative would be to have third parameter to have the package. Or if would really extend the issue to more reusable, when adding package the new package could be loaded to xPDO core like service and could be utilized $xpdo->packageName->getObject('myObject'); This would bloat it bit as would require one new class added to xPDO core.

                      My silly idea:

                      class xPDOPackage {
                          var $package;
                          var $path;
                      
                          function getObject() {}
                          function getObjectCollection() {}
                          function newObject() {}
                          ...
                      }
                      


                      Again too late for my brains to function smiley writing debug cmp for modx to run and analyze queries through one page. Now wondering what options adding to it.

                      Added: I guess this topic could be moved to xPDO smiley [ed. note: dunnock last edited this post 13 years, 4 months ago.]