We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 40358
    • 40 Posts
    I am in these days toying around with the slim branch in the Revolution repository at GitHub.

    First of all, a quick introduction to what we are trying to do there and what we are doing:
    Revolution has since its very first version (as far as I know) had an architect where the core class (named modX) is extending the xPDO class, e.i. the ORM library. In the slim branch we want to instead have modX extend the Slim app class. We also change xPDO to become a dependency, which will losen the coupling between the two. This is necessary to modernize the codebase to using Dependency Injection (DI) architectures.

    Why the old "extending xPDO" causes problems:
    In the current codebase (or 2.x if you like), you can do this:

    $obj = $xpdo->newObject('modUser');
    get_class($obj->xpdo); // the xpdo instance variable is an instance of xPDO


    But, you can also do this (from within the modX class):
    $obj = new modUser($this);
    get_class($obj->xpdo); // the xpdo instance variable is an instance of modX


    This leads to problems. Many places, the objects (such as modResponse, modRequest, and more) expects the modX instance to be available. Some classes can also be instantiated with both modX and xPDO passed in the constructor, such as modInputFilter and modOutputFilter. This is why I had to remove the typehinting in the constructor for these classes. The modUser class also have separated behavior defined if the xpdo variable is an modX instance.

    What this means:
    The tight coupling between the instances of various classes, and how they depend on the instance of modX has lead me to belive that Revolution 3.x can not be backwards compatible with earlier versions. The MAB-04 goal says:

    By refactoring MODX Revolution 3.x on top of Slim 3.x, MODX would instantly gain the benefits of an outstanding PHP microframework as a new foundation for future advancements and improve maintainability. By simply adapting Revolution's modX class as a Slim App instead of extending xPDO for this foundation, PSR-4 autoloading, proper dependency injection, PSR-7 HTTP messaging standards and the flexible middleware-centric architecture would automatically become part of the MODX core. Much backwards compatibility can be maintained while effectively jump starting the modernization of the existing codebase.

    Emphasis mine.

    I belive that this goal is unachievable. There is no way to get around the technical problems that follow the decoupling of modX and xPDO. I also belive that is will be difficult, if not possible to maintain much of the backwards compatibility either. Because the Revolution codebase does not adhere to common OOP best practices, the "internal" representation of objects and instances leaks everywhere. We can not just change modResponse to not have an instance of modX passed to it. Extras and project code depends on this instance and depends on methods available in the modX instance. If we change this to being xPDO, a LOT will break. This includes connectors, controllers and extending xPDO objects.

    Some of this could be avoided if the variables were boxed, and we used getters and setters for various tasks. This way we could "cheat" various problems, but I am still not convinced this would solve the problem entirely.

    We can for example see that a random extra (Collections used for example) depends on the modx instance in its controller(s). This would not work if we go all the way with DI and extending the Slim app.

    Because of this am I unsure how to proceed. Should we attempt to do this another way? Could serving the entire modX instance as a DI be a possible work around? This could make the existing code base work with more or less the current code, but the decoupling would get nowhere.

    Overarching ideas:
    Because we need to change so much of the original (current, 2.x) codebase, the changes will diverge so much between the slim branch (soon to be 3.x) and the 2.x branch that we will not be able to merge patches and changes from 2.x into 3.x in a very short amount of time. This is simply because too many changes in the 3.x branch are needed to get it to work with the Slim app approach.

    Because of this, we can ignore keeping most of the files in the core/modx/model directory and we can start working almost exclusively on the files in the core/src directory. Essentially we can move the current files in the model directory into src and split up the classes, add namespaces and the works. Because BC is not possible anyways, there is no reason to keep all the code in the model directory intact, as we an not benefit from the 2.x patches and changes.

    Alternative ideas:
    Otherwise we can make modX a service and pass it around. However, this approach will not decouple anything between modX and xPDO, and we are more or less where we are today once done.

    Things that needs to be decided/figured out:
    While working on the slim branch I came across numerous stuff that needs to be decided or figured out. Firstly we need to decide if any instance of modX should be passed anywhere at all. As I assume this is NO we will need to change a lot of code, especially concerning requests, response, controllers, connectors, and processors. Otherwise we need to decide on another approach to keep these things working like they do.

    We also need to split up the MODX\modX class into separate classes, many of which should work as typical utility classes. This includes methods for sanitizing etc. There is no need to keep this in the base class other than BC. We also need to decide if we should override __call for this functions and reroute them to the new classes. This could keep some BC for some of the functions.

    I also need to figure out what the modX::getOption and xPDO::getOption methods do, and what their purposes are. Why are there two of these? The same goes for modContext. How is this implemented and how does its inherited functions work? I have yet to understand this.

    I hope to get some comments and insight from other people (hopefully those who have played with the 3.x/slim branch) and/or other people in the MAB that know more details about the future roadmap for this project.
    • I think this is the first time I've seen anyone use something like this:
      $obj = new modUser($this);

      instead of the getObject/newObject approach. That doesn't seem to me like a very common thing people do.

      I also agree that full backwards compatibility with 2.x is going to be impossible. There's some major changes going on there, and while we can (and have) tried to automatically map old calls to new where possible (e.g. $modx->getObject automatically feeds it to $modx->getContainer('xpdo')->getObject), it's not going to be possible to cover everything.

      Things will break for 3.0, that's why it's 3.0 and not 2.7 tongue


      Section Overarching ideas::

      What I think we should do for 3.0 is get all non-model classes (modRequest, modResponse, modParser etc) out of core/model/modx/ and into new namespaced classes in core/src. modParser would become MODX\Parser, and perhaps we can preserve some BC by adding a class_alias or class definition for modParser.

      Model files should remain where they are for now, and perhaps eventually turned into proper xPDO 3 models (i.e. no more map files, but the table definitions being on the db-specific class files).

      Section Alternative ideas:

      I think that was discussed in the chat as either not possible or desired. Providing an upgrade path for this will be vital, which I'll share my thoughts on in a bit.

      My thoughts on Backwards Compatibility
      I have at least two dozen extras that I don't want to have to rewrite fully when 3.0 comes out. tongue

      Based on the work I've done on the slim branch (haven't in detail reviewed your latest yet) there's two main things that we need to make upgrading as smoothly as possible.

      1. Calls to the modX class that actually need to get to xPDO. There's already a __call method in my version that automatically resolves those.
      2. Having access to $this->xpdo (/$this->modx) from within a model class. This is unresolved for now, and the tricky bit.

      As it's going to be impossible to have full backwards compatibility, what I'd personally like to see is a staged breaking change. Instead of going from 2.7 (or 2.whatever) to 3.0 and having every addon break, I'd like to see 2.x include forward compatibility. If we know that in 3.0 the modX class will be available to models via, say, $this->getMODX() instead of $this->xpdo, then we could add a $this->getMODX() method to 2.x and instruct add-on developers to start using that early. The older approach could log deprecation warnings to an (optional?) log.

      Then, as much as reasonably possible, 3.0 would still allow $this->xpdo to work, but it would log warnings. In 3.1, we can then clean up (possibly yucky) code dedicated to the backwards compatibility.

      How to practically shape that is going to be difficult, but rather than attempting to have permanent backwards compatibility, I think a bit of extra effort for a temporary BC layer might be the answer. That code could be ugly/bad-oop, as it would only be temporarily to help add-ons adapt to containers and what not.
        Mark Hamstra • Developer spending his days working on Premium Extras and a MODX Site Dashboard with the ability to remotely upgrade MODX and extras to make the MODX world a little better.

        Tweet me @mark_hamstra, check my infrequent blog at markhamstra.com, my slightly more frequent ramblings at MODX.today or see code at Github.
        • 3749
        • 24,544 Posts
        I see the need for this, but as the author of 41+ extras, this conversation gives me the willies. I think most of them will be not only broken, but extremely difficult to fix. I hope I'm wrong. wink

        It sounds like Mark's suggestions would make things much easier, but I'd have to see them in action to know for sure.
          Did I help you? Buy me a beer
          Get my Book: MODX:The Official Guide
          MODX info for everyone: http://bobsguides.com/modx.html
          My MODX Extras
          Bob's Guides is now hosted at A2 MODX Hosting