Completed
Push — issue/96 ( 5d401e...7b0d36 )
by Alex
02:40
created

XmlParser::parseNode()   A

Complexity

Conditions 3
Paths 3

Size

Total Lines 10
Code Lines 5

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 7
CRAP Score 3

Importance

Changes 0
Metric Value
dl 0
loc 10
ccs 7
cts 7
cp 1
rs 9.4285
c 0
b 0
f 0
cc 3
eloc 5
nc 3
nop 3
crap 3
1
<?php
2
/*
3
 * This file is part of the feed-io package.
4
 *
5
 * (c) Alexandre Debril <[email protected]>
6
 *
7
 * For the full copyright and license information, please view the LICENSE
8
 * file that was distributed with this source code.
9
 */
10
11
namespace FeedIo\Parser;
12
13
14
use DOMDocument;
15
use FeedIo\Parser;
16
use FeedIo\RuleSet;
17
use FeedIo\FeedInterface;
18
use FeedIo\Feed\ItemInterface;
19
use FeedIo\Feed\NodeInterface;
20
use FeedIo\ParserAbstract;
21
use FeedIo\Reader\Document;
22
use FeedIo\Parser\MissingFieldsException;
23
use FeedIo\Parser\UnsupportedFormatException;
24
use Psr\Log\LoggerInterface;
25
26
/**
27
 * Parses a DOM document if its format matches the parser's standard
28
 *
29
 * Depends on :
30
 *  - FeedIo\StandardAbstract
31
 *  - Psr\Log\LoggerInterface
32
 *
33
 */
34
class XmlParser extends ParserAbstract
35
{
36
37
    /**
38
     * @param $tagName
39
     * @return bool
40
     */
41 8
    public function isItem($tagName)
42
    {
43 8
        return (strtolower($this->standard->getItemNodeName()) === strtolower($tagName));
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class FeedIo\StandardAbstract as the method getItemNodeName() does only exist in the following sub-classes of FeedIo\StandardAbstract: FeedIo\Standard\Atom, FeedIo\Standard\Rdf, FeedIo\Standard\Rss, FeedIo\Standard\XmlAbstract. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
44
    }
45
46
    /**
47
     * @param  Document                       $document
48
     * @param  FeedInterface                  $feed
49
     * @return \FeedIo\FeedInterface
50
     * @throws Parser\MissingFieldsException
51
     * @throws Parser\UnsupportedFormatException
52
     */
53 8
    public function parseContent(Document $document, FeedInterface $feed)
54
    {
55 8
        $element = $this->standard->getMainElement($document->getDOMDocument());
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class FeedIo\StandardAbstract as the method getMainElement() does only exist in the following sub-classes of FeedIo\StandardAbstract: FeedIo\Standard\Atom, FeedIo\Standard\Rdf, FeedIo\Standard\Rss, FeedIo\Standard\XmlAbstract. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
56
57 8
        $this->parseNode($feed, $element, $this->standard->getFeedRuleSet());
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class FeedIo\StandardAbstract as the method getFeedRuleSet() does only exist in the following sub-classes of FeedIo\StandardAbstract: FeedIo\Standard\Atom, FeedIo\Standard\Rdf, FeedIo\Standard\Rss, FeedIo\Standard\XmlAbstract. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
58
59 8
        return $feed;
60
    }
61
62
    /**
63
     * @param  Document            $document
64
     * @param  array                  $mandatoryFields
65
     * @return $this
66
     * @throws MissingFieldsException
67
     */
68 11
    public function checkBodyStructure(Document $document, array $mandatoryFields)
69
    {
70 11
        $errors = array();
71
72 11
        $element = $document->getDOMDocument()->documentElement;
73 11
        foreach ($mandatoryFields as $field) {
74 6
            $list = $element->getElementsByTagName($field);
75 6
            if (0 === $list->length) {
76 2
                $errors[] = $field;
77 2
            }
78 11
        }
79
80 11
        if (!empty($errors)) {
81 2
            $message = "missing mandatory field(s) : ".implode(',', $errors);
82 2
            $this->logger->warning($message);
83 2
            throw new MissingFieldsException($message);
84
        }
85
86 9
        return $this;
87
    }
88
89
    /**
90
     * @param  NodeInterface $item
91
     * @param  \DOMElement   $element
92
     * @param  RuleSet       $ruleSet
93
     * @return NodeInterface
94
     */
95 9
    public function parseNode(NodeInterface $item, \DOMElement $element, RuleSet $ruleSet)
96
    {
97 9
        foreach ($element->childNodes as $node) {
98 8
            if ($node instanceof \DOMElement) {
99 8
                $this->handleNode($item, $node, $ruleSet);
100 8
            }
101 9
        }
102
103 9
        return $item;
104
    }
105
106
    /**
107
     * @param NodeInterface $item
108
     * @param \DOMElement $node
109
     * @param RuleSet $ruleSet
110
     * @return $this
111
     */
112 8
    protected function handleNode(NodeInterface $item, \DOMElement $node, RuleSet $ruleSet)
113
    {
114 8
        if ($this->isItem($node->tagName) && $item instanceof FeedInterface) {
115 7
            $newItem = $this->parseNode($item->newItem(), $node, $this->standard->getItemRuleSet());
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class FeedIo\StandardAbstract as the method getItemRuleSet() does only exist in the following sub-classes of FeedIo\StandardAbstract: FeedIo\Standard\Atom, FeedIo\Standard\Rdf, FeedIo\Standard\Rss, FeedIo\Standard\XmlAbstract. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

abstract class User
{
    /** @return string */
    abstract public function getPassword();
}

class MyUser extends User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different sub-classes of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
116 7
            $this->addValidItem($item, $newItem);
117 7
        } else {
118 8
            $rule = $ruleSet->get($node->tagName);
119 8
            $rule->setProperty($item, $node);
120
        }
121
122 8
        return $this;
123
    }
124
125
126
}