*** Check out Grokking Magento ***

Vinai Kopp

Magento Expert, Developer & Trainer

  • 10. The Config XML Converter Kata

    June 2, 2016

    Mage2Katas

    Finally it’s time to do some real TDD with unit tests again.
    The tests will be running nice and quick since we don’t need the Magento runtime environment to be bootstrapped :)

    This time we will convert the raw XML structure of our custom configuration file from the previous kata to a more usable array structure.

    Just for reference, you can find the unit conversion XML files for the custom config file mage2kata mini series in the linked gist.

    For reading the configuration file we are using an instance of
    \Magento\Framework\Config\Reader\Filesystem that we have configured to our needs with di.xml.
    One of the constructor arguments that this class receives is a
    \Magento\Framework\Config\ConverterInterface object.

    So far we are using the default converter instance the object manager provides, which happens to be a \Magento\Framework\View\Element\UiComponent\Config\Converter.
    The class converts the configuration XML into a very generic array representation, which is not very useful for our purposes.
    We want to replace it with a custom ConverterInterface implementation.

    Lets start writing it!

    First things first, time to create the test file in Test/Unit/Model/Config/UnitConversionConfigConverterTest.php. As always, we start with a testNothing() test method to check that our phpunit setup and PHPStorm run configuration are in place.

    <?php
     
    namespace Mage2Kata\DiConfig\Model\Config;
     
    class UnitConversionConfigConverterTest extends \PHPUnit_Framework_TestCase
    {
        public function testNothing()
        {
            $this->fail('in the foo');
        }
    }

    Note: In the video I’m taking smaller steps then the ones I’ve written down in this blog post. You will have to find the right granularity of testing during TDD for you yourself.

    Once this test successfully fails, we can start with a real assertion that the class under tests implements the right interface.

    <?php
     
    namespace Mage2Kata\DiConfig\Model\Config;
     
    use Magento\Framework\Config\ConverterInterface;
     
    class UnitConversionConfigConverterTest extends \PHPUnit_Framework_TestCase
    {
        /**
         * @var UnitConversionConfigConverter
         */
        private $converter;
     
        protected function setUp()
        {
            $this->converter = new UnitConversionConfigConverter();
        }
     
        public function testImplementsTheConfigConverterInterface()
        {
            $this->assertInstanceOf(ConverterInterface::class, $this->converter);
        }
    }

    To make this initial real test pass, we need to create the class and implement the interface.

    <?php
     
    namespace Mage2Kata\DiConfig\Model\Config;
     
    use Magento\Framework\Config\ConverterInterface;
     
    class UnitConversionConfigConverter implements ConverterInterface
    {
        /**
         * @param \DOMDocument $source
         * @return array[]
         */
        public function convert($source)
        {
        }
    }

    For real work that will make it into production I always check the input type and validity for untyped arguments of public methods.
    During this kata we will skip this step, and assume the input always is a \DOMDocument instance.

    Unfortunately we can’t add the \DOMDocument type to the signature because it has to match the signature of the implemented interface exactly. Every type we can add to a signature saves us having to write an if block and an exception in production code.

    But again, for this kata we will just skip the input validation.
    Instead, lets start with the return type.

    Lets add a test that defines our return type in an empty \DOMDocument is passed to the convert() method.

    public function testReturnsEmptyArrayForEmptyInput()
    {
        $source = new \DOMDocument();
        $source->loadXML('<empty/>');
        $this->assertSame([], $this->converter->convert($source));
    }

    To make it pass, lets just fake it:

    public function convert($source)
    {
        return [];
    }

    Since all tests are passing, we can refactor.
    In the test class I would like to extract the creation of the test \DOMDocument instance into a utility method, so we can reuse it in following tests.

    /**
     * @param string $xml
     * @return \DOMDocument
     */
    private function createDOMDocument($xml)
    {
        $source = new \DOMDocument();
        $source->loadXML($xml);
        return $source;
    }
     
    // ... setUp method and the first test ...
     
    public function testReturnsEmptyArrayForEmptyInput()
    {
        $this->assertSame([], $this->converter->convert($this->createDOMDocument('<empty/>')));
    }

    Still green after this refactoring? Yes? Good.

    At this point in time we have to think about the array structure we actually want. Given the following unit conversion configuration:

    <conversion_map>
        <unit id="mg" type="weight">
            <conversion to="g" factor="111"/>
            <conversion to="kg" factor="222"/>
        </unit>
        <unit id="g" type="weight">
            <conversion to="mg" factor="333"/>
            <conversion to="kg" factor="444"/>
        </unit>
    </conversion_map>

    the expected array structure should look like this:

    [
        'mg' => [
            'g'  => '111',
            'kg' => '222',
        ],
        'g'  => [
            'mg' => '333',
            'kg' => '444',
        ],
    |

    However, we don’t just want to add the above as a test method, because that would force us to implement the full solution in one single step. TDD is about taking many small and keeping each step as trivial as possible.

    Lets start by checking the returned array contains an array key for the given <unit id="xxx"> node.

    public function testContainsTheUnitsAsArrayKeys()
    {
        $xml = <<<XML
    <conversion_map>
        <unit id="mg"/>
    </conversion_map>
    XML;
        $this->assertArrayHasKey('mg', $this->converter->convert($this->createDOMDocument($xml)));
    }

    Back to our production code, what do we need to do to make the test pass?

    1. We need to get the root element of our \DOMDocument.
    2. We then get each child unit node
    3. Build an output array with the keys being the units id attribute values.

    Hm, maybe doing add this in one TDD step is too much. Our test is a too big step from where we currently are.

    Lets backtrack and mark our test as incomplete for now, so we can focus on each of these steps one at a time.
    Once we have completed them we can enable our current test again and complete it.

    public function testContainsTheUnitsAsArrayKeys()
    {
        $this->markTestIncomplete();
        // other test code...

    Lets add a test for getting the root node from the document. For now we will make it public. Once we know it works, we can change it to private.

    public function testReturnsTheRootNode()
    {
        $document = $this->createDOMDocument('<root/>');
        $rootNode = $this->converter->getRootNode($document);
    }

    Since the method getRootNode() doesn’t exist yet, the test fails. According to the rules of TDD we have to stop writing the test at this point and make it pass before we can continue writing the test.

    To make it pass we simply have to add the method getRootNode() to our converter.

    public function getRootNode(\DOMDocument $document)
    {
    }

    Now we are back to green and can add an assertion to our test method:

    $this->assertInstanceOf(\DOMElement::class, $rootNode);

    A \DOMDocument always has a single child element.
    So as a first solution, we can iterate over all children and return the first element, like so:

    public function getRootNode(\DOMDocument $document)
    {
        /** @var \DOMNode $childNode */
        foreach ($document->childNodes as $childNode) {
            if ($childNode->nodeType === \XML_ELEMENT_NODE) {
                return $childNode;
            }
        }
    }

    We simply return the first element node we find. Since the XML parser would complain if no root nodes or more then one root node where present, we don’t have to worry about those cases.

    Lets add one more assertion to our test to ensure we are really returning the root node:

    $this->assertSame('root', $rootNode->nodeName);

    And indeed, seems like we are good to continue. All green.

    Lets refactor.
    In the converter, since we are going to need to iterate over child elements for out further tasks, too, lets extract a method getAllChildElements(), and then we can use that to get the root node.

    But since this isn’t a pure refactoring (we are adding new functionality to our class), I want a test for that.

    public function testReturnsAllChildNodes()
    {
        $xml = <<<XXX
    <root>
        <child/>
        <child/>
        <child/>
    </root>
    XXX;
        $documentChildren = $this->converter->getAllChildElements($this->createDOMDocument($xml));
        $this->assertInternalType('array', $documentChildren);
        $this->assertCount(1, $documentChildren);
        $this->assertContainsOnlyInstancesOf(\DOMElement::class, $documentChildren);
        $this->assertCount(3, $this->converter->getAllChildElements($documentChildren[0]));
    }

    Here is what I used to make that pass:

    /**
     * @param \DOMNode $source
     * @return \DOMElement[]
     */
    public function getAllChildElements(\DOMNode $source)
    {
        return array_filter(iterator_to_array($source->childNodes), function (\DOMNode $childNode) {
            return $childNode->nodeType === \XML_ELEMENT_NODE;
        });
    }

    Now we can refactor the getRootNode() method to use our new getAllChildElements() method.

    public function getRootNode(\DOMDocument $document)
    {
        return $this->getAllChildElements($document)[0];
    }

    All still green? Great!

    Now lets make the two methods private since we don’t really want to expose them as part of the converter API. From now on the methods will be an internal implementation detail of the converter.
    We can delete the tests for those methods, we no longer need them.

    Time to remove the $this->markTestIncomplete() from the test method, ensure it’s still failing as before (of course it is), and start working on making it pass.
    Here is what I came up with to make it pass:

    public function convert($document)
    {
        $rootNode = $this->getRootNode($document);
        $result = [];
        foreach ($this->getAllChildElements($rootNode) as $childNode) {
            if ($childNode->nodeName === 'unit') {
                $unit = $childNode->attributes->getNamedItem('id')->nodeValue;
                $result[$unit] = true;
            }
        }
        return $result;
    }

    All tests are passing.
    However, that code is not very clean. Lets extract a method to get all child elements with a given name.

    private function getChildrenByName(\DOMElement $parent, $name)
    {
        return array_filter($this->getAllChildElements($parent), function (\DOMElement $child) use ($name){
            return $child->nodeName === $name;
        });
    }

    This allows us to remove one level of indention in our convert() method:

    public function convert($document)
    {
        $rootNode = $this->getRootNode($document);
        $result = [];
        foreach ($this->getChildrenByName($rootNode, 'unit') as $childNode) {
            $unit = $childNode->attributes->getNamedItem('id')->nodeValue;
            $result[$unit] = true;
        }
        return $result;
    }

    Very good.
    Time to return to our tests and expand the previous one. The next iteration of the test method checks if every <conversion> node adds a record to every unit.

    public function testAddsEachConversionBelowItsUnit()
    {
        $xml = <<<XML
    <conversion_map>
        <unit id="mg">
            <conversion to="g" factor="111"/>
            <conversion to="kg" factor="222"/>
        </unit>
    </conversion_map>
    XML;
        $result = $this->converter->convert($this->createDOMDocument($xml));
        $this->assertSame(['mg' => ['g' => '111', 'kg' => '222']], $result);
    }

    To make it pass we need to iterate over the conversion nodes and add each one to the $result array.

    public function convert($document)
    {
        $rootNode = $this->getRootNode($document);
        $result = [];
        foreach ($this->getChildrenByName($rootNode, 'unit') as $unitNode) {
            $unit = $unitNode->attributes->getNamedItem('id')->nodeValue;
            $result[$unit] = [];
            foreach ($this->getChildrenByName($unitNode, 'conversion') as $conversionNode) {
                $targetUnit = $conversionNode->attributes->getNamedItem('to')->nodeValue;
                $factor = $conversionNode->attributes->getNamedItem('factor')->nodeValue;
                $result[$unit][$targetUnit] = $factor;
            }
        }
        return $result;
    }

    And again all tests pass.
    Lets clean up this code.
    First, maybe we can move all that attribute value extraction into a utility method.

    private function getAttribute(\DOMElement $element, $name)
    {
        return $element->attributes->getNamedItem($name)->nodeValue;
    }

    Not a big difference, but every little bit that makes the code more readable helps.

    Now, can we get rid of that second level of indention?
    How about this:

    public function convert($document)
    {
        $rootNode = $this->getRootNode($document);
        $result = [];
        foreach ($this->getChildrenByName($rootNode, 'unit') as $unitNode) {
            $unit = $this->getAttribute($unitNode, 'id');
            $result[$unit] = $this->collectConversions($unitNode);
        }
        return $result;
    }
     
    private function collectConversions(\DOMElement $unitNode)
    {
        $conversions = [];
        foreach ($this->getChildrenByName($unitNode, 'conversion') as $conversionNode) {
            $targetUnit = $this->getAttribute($conversionNode, 'to');
            $factor = $this->getAttribute($conversionNode, 'factor');
            $conversions[$targetUnit] = $factor;
        }
        return $conversions;
    }

    Can we do the same thing for the units? Of course we can:

    public function convert($document)
    {
        $rootNode = $this->getRootNode($document);
        return $this->collectUnits($rootNode);
    }
     
    private function getRootNode(\DOMDocument $document)
    {
        return $this->getAllChildElements($document)[0];
    }
     
    private function collectUnits(\DOMElement $rootNode)
    {
        $result = [];
        foreach ($this->getChildrenByName($rootNode, 'unit') as $unitNode) {
            $unit = $this->getAttribute($unitNode, 'id');
            $result[$unit] = $this->collectConversions($unitNode);
        }
        return $result;
    }
     
    private function collectConversions(\DOMElement $unitNode)
    {
        $conversions = [];
        foreach ($this->getChildrenByName($unitNode, 'conversion') as $conversionNode) {
            $targetUnit = $this->getAttribute($conversionNode, 'to');
            $conversions[$targetUnit] = $this->getAttribute($conversionNode, 'factor');
        }
        return $conversions;
    }

    It almost seems we are done!

    Are we really? I don’t trust our code yet. Lets add test for a mode complex XML that would involve some merging.

    public function testOverwritesNodesWithTheSameUnits()
    {
        $xml = <<<XML
    <conversion_map>
        <unit id="mg">
            <conversion to="g" factor="111"/>
            <conversion to="kg" factor="222"/>
        </unit>
        <unit id="mg">
            <conversion to="g" factor="333"/>
        </unit>
    </conversion_map>
    XML;
        $result = $this->converter->convert($this->createDOMDocument($xml));
        $this->assertSame(['mg' => ['g' => '222', 'kg' => '333']], $result);
    }

    So the mg to g mapping is specified twice.
    We expect the second mapping value of 333 to be the one that ends up in our result array.

    Does it work?
    Oh no, the second <unit id="mg"> node completely overwrote previous node!

    In collectUnits(), instead of simply assigning $result[$unit], we need to check if the $unit already is defined. If so, we need to merge the new conversion values into the existing one.

    private function collectUnits(\DOMElement $rootNode)
    {
        $result = [];
        foreach ($this->getChildrenByName($rootNode, 'unit') as $unitNode) {
            $unit = $this->getAttribute($unitNode, 'id');
            $conversions = $this->collectConversions($unitNode);
            $result[$unit] = isset($result[$unit]) ? array_merge($result[$unit], $conversions) : $conversions;
        }
        return $result;
    }

    And with this we are done.

    I hope the blog post can somehow tranfer how the code slowly evolves in small steps during TDD. I think it is more easy to follow in the video, but I applaud you for reading all the way to the end of this post.

    Please let me know if you had any questions, comments or insights. I will do my best to answer.

    In the next episode we will finally put it all together and use our converter together with the existing DI configuration.

    Thank you for following along!

    comments powered by Disqus

Read Grokking Magento now!

"The best technical reading I had since years by @VinaiKopp"

Tim Bezhashvyly
Magento Developer