Completed
Push — feature/player-elo ( 8ea10f...365309 )
by Vladimir
10:20
created

LeagueOverseerHookController::matchReportAction()   D

Complexity

Conditions 19
Paths 134

Size

Total Lines 87
Code Lines 59

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 64
CRAP Score 19.0013

Importance

Changes 0
Metric Value
dl 0
loc 87
ccs 64
cts 65
cp 0.9846
rs 4.4786
c 0
b 0
f 0
cc 19
eloc 59
nc 134
nop 2
crap 19.0013

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
use Monolog\Logger;
4
use Nelmio\ApiDocBundle\Annotation\ApiDoc;
5
use Symfony\Component\HttpFoundation\JsonResponse;
6
use Symfony\Component\HttpFoundation\Request;
7
8
class LeagueOverseerHookController extends PlainTextController
9
{
10
    /**
11
     * The API version of the server performing the request
12
     * @var int
13
     */
14
    private $version;
15
16
    /**
17
     * The parameter bag representing the $_GET or $_POST array
18
     * @var Symfony\Component\HttpFoundation\ParameterBag
19
     */
20
    private $params;
21
22
    /**
23
     * {@inheritdoc}
24
     */
25 22
    public function setUp()
26
    {
27 22
        $request = $this->getRequest();
28
29
        // To prevent abuse of the automated system, we need to make sure that
30
        // the IP making the request is one of the IPs we allowed in the config
31 22
        $allowedIPs = array_map('trim', $this->container->getParameter('bzion.api.allowed_ips'));
32 22
        $clientIP   = $request->getClientIp();
33
34 22
        if (!$this->isDebug() && // Don't care about IPs if we're in debug mode
35 22
           !in_array($clientIP, $allowedIPs)) {
36
            // If server making the request isn't an official server, then log the unauthorized attempt and kill the script
37
38
            $this->getLogger()->addNotice("Unauthorized access attempt from $clientIP");
39
            throw new ForbiddenException("Error: 403 - Forbidden");
40
        }
41
42
        // We will be looking at either $_POST or $_GET depending on the status, production or development
43 22
        $this->params = $request->request; // $_POST
44
45 22
        if (!$this->params->has('query')) {
46
            // There seems to be nothing in $_POST. If we are in debug mode
47
            // however, we might have a debug request with data in $_GET
48
            if ($this->isDebug() && $request->query->has('query')) {
49
                $this->params = $request->query; // $_GET
50
            } else {
51
                throw new BadRequestException();
52
            }
53
        }
54
55
        // After the first major rewrite of the league overseer plugin, the
56
        // API was introduced in order to provide backwards compatibility for
57
        // servers that have not updated to the latest version of the plugin.
58 22
        $this->version = $this->params->get('apiVersion', 0);
59 22
    }
60
61
    /**
62
     * @ApiDoc(
63
     *  description="Query the LeagueOverseer API",
64
     *  parameters={
65
     *      {"name"="query", "dataType"="string", "required"=true, "description"="query type"},
66
     *      {"name"="apiVersion", "dataType"="integer", "required"=false, "description"="LeagueOverseer API version"}
67
     *  }
68
     * )
69
     * @todo Test/improve/revoke support for API version 0
70
     */
71 22
    public function queryAction()
72
    {
73 22
        $matchReportQuery = $this->version == 1 ? 'reportMatch' : 'matchReport';
74 22
        $teamNameQuery = $this->version == 1 ? 'teamNameQuery' : 'teamName';
75 22
        $teamNameDumpQuery = $this->version == 1 ? 'teamDump' : 'teamInfoDump';
76
77 22
        switch ($this->params->get('query')) {
78 22
            case $matchReportQuery:
79 22
                return $this->forward('matchReport');
80
            case $teamNameQuery:
81
                return $this->forward('teamName');
82
            case $teamNameDumpQuery:
83
                return $this->forward('teamNameDump');
84
            default:
85
                throw new BadRequestException();
86
            }
87
    }
88
89
    public function teamNameAction()
90
    {
91
        if ($this->version < 1) {
92
            throw new BadRequestException();
93
        }
94
95
        $param = $this->version == 1 ? 'teamPlayers' : 'bzid';
96
97
        $bzid = $this->params->get($param);
98
        $team = Player::getFromBZID($bzid)->getTeam();
99
100
        $teamName = ($team->isValid()) ? preg_replace("/&[^\s]*;/", "", $team->getName()) : '';
101
102
        return new JsonResponse(array(
103
            // API v1 legacy support
104
            "bzid" => $bzid,     // Replaced with "team_name" in API v2+
105
            "team" => $teamName, // Replaced with "player_bzid" in API v2+
106
107
            // API v2+
108
            "player_bzid" => $bzid,
109
            "team_id"     => $team->getId(),
110
            "team_name"   => $teamName,
111
        ));
112
    }
113
114
    public function teamNameDumpAction()
115
    {
116
        if ($this->version < 1) {
117
            throw new BadRequestException();
118
        }
119
120
        // Create an array to store all teams and the BZIDs
121
        $teamArray = array();
122
123
        foreach (Team::getTeams() as $team) {
124
            $memberList = "";
125
126
            foreach ($team->getMembers() as $member) {
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Model as the method getMembers() does only exist in the following sub-classes of Model: Conversation, Team. 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...
127
                $memberList .= $member->getBZID() . ",";
128
            }
129
130
            $teamName    = preg_replace("/&[^\s]*;/", "", $team->getName());
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Model as the method getName() does only exist in the following sub-classes of Model: AliasModel, ApiKey, AvatarModel, Ban, Conversation, Country, Map, Match, News, NewsCategory, Page, Permission, Player, Role, Server, Team. 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...
131
            $teamID      = $team->getId();
132
            $teamMembers = rtrim($memberList, ",");
133
134
            $teamArray[] = array(
135
                // API v1 legacy support
136
                "team"    => $teamName,
137
                "members" => $teamMembers,
138
139
                // API v2+
140
                "team_id"      => $teamID,
141
                "team_name"    => $teamName,
142
                "team_members" => $teamMembers
143
            );
144
        }
145
146
        return new JsonResponse(array(
147
            // API v1 legacy support
148
            "teamDump" => &$teamArray,
149
150
            // API v2+
151
            "team_list" => &$teamArray
152
        ));
153
    }
154
155 22
    public function matchReportAction(Logger $log, Request $request)
156
    {
157 22
        $log->addNotice("Match data received from " . $request->getClientIp());
158 22
        $log->addDebug("Debug match data query: " . http_build_query($this->params->all()));
159
160 22
        $matchType = $this->params->get('matchType', Match::OFFICIAL);
161
162 22
        $teamOneBZIDs = $this->params->get('teamOnePlayers');
163 22
        $teamTwoBZIDs = $this->params->get('teamTwoPlayers');
164
165 22
        $teamOnePlayers = $this->bzidsToIdArray($teamOneBZIDs);
166 22
        $teamTwoPlayers = $this->bzidsToIdArray($teamTwoBZIDs);
167
168 22
        $teamOne = $teamTwo = null;
169
170 22
        if (Match::OFFICIAL === $matchType) {
171 21
            $teamOne = $this->getTeam($teamOnePlayers);
172 21
            $teamTwo = $this->getTeam($teamTwoPlayers);
173
174 21
            if ($teamOne->isValid() && $teamTwo->isValid() && $teamOne->isSameAs($teamTwo)) {
0 ignored issues
show
Bug introduced by
It seems like $teamTwo defined by $this->getTeam($teamTwoPlayers) on line 172 can be null; however, Model::isSameAs() does not accept null, maybe add an additional type check?

Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code:

/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
Loading history...
175 1
                $log->addNotice("The '" . $teamOne->getName() . "' team played against each other in an official match. Match invalidated.");
176 1
                throw new ForbiddenException("Holy sanity check, Batman! The same team can't play against each other in an official match.");
177
            }
178 21
        } elseif (Match::FUN === $matchType) {
179 1
            if (count($teamOnePlayers)  < 2 || count($teamTwoPlayers) < 2) {
180
                throw new ForbiddenException("You are not allowed to report a match with less than 2 players per team.");
181
            }
182 1
        }
183
184 21
        $map = Map::fetchFromAlias($this->params->get('mapPlayed'));
185
186 21
        $match = Match::enterMatch(
187 21
            ($teamOne !== null) ? $teamOne->getId() : null,
188 21
            ($teamTwo !== null) ? $teamTwo->getId() : null,
189 21
            $this->params->get('teamOneWins'),
190 21
            $this->params->get('teamTwoWins'),
191 21
            $this->params->get('duration'),
192 21
            null,
193 21
            $this->params->get('matchTime'),
194 21
            $teamOnePlayers,
195 21
            $teamTwoPlayers,
196 21
            $this->params->get('server'),
197 21
            $this->params->get('replayFile'),
198 21
            $map->getId(),
199 21
            $this->params->get('matchType'),
200 21
            $this->params->get('teamOneColor'),
201 21
            $this->params->get('teamTwoColor')
202 21
        );
203
204 21
        $log->addNotice("Match reported automatically", array(
205
            'winner' => array(
206 21
                'name'  => $match->getWinner()->getName(),
207 21
                'score' => $match->getScore($match->getWinner()),
208 21
            ),
209
            'loser' => array(
210 21
                'name'  => $match->getLoser()->getName(),
211 21
                'score' => $match->getScore($match->getLoser())
212 21
            ),
213 21
            'eloDiff' => $match->getEloDiff(),
214 21
            'type'    => $matchType,
215 21
            'map'     => $map->getName()
216 21
        ));
217
218 21
        $bzfsAnnouncement = $match->getName();
219
220 21
        if (!$match->getWinner()->supportsMatchCount() || !$match->getLoser()->supportsMatchCount()) {
221 13
            if ($match->getWinner()->supportsMatchCount()) {
222 4
                $bzfsAnnouncement .= sprintf("\n  %s: +%d", $match->getWinner()->getName(), $match->getEloDiff());
223 13
            } elseif ($match->getLoser()->supportsMatchCount()) {
224 4
                $diff = -$match->getEloDiff();
225 4
                $bzfsAnnouncement .= sprintf("\n  %s: %d", $match->getLoser()->getName(), $diff);
226 4
            }
227 13
        }
228
229 21
        if ($match->isOfficial()) {
230
            $inverseSymbol = (
231 20
                $match->getTeamMatchType() === Match::TEAM_V_TEAM && $match->isDraw() &&
232 6
                ($match->getWinner()->isSameAs($match->getTeamB()) || $match->getPlayerEloDiff(false) < 0)
233 20
            );
234
235 20
            $symbol = ($inverseSymbol) ? '-/+' : '+/-';
236 20
            $bzfsAnnouncement .= sprintf("\n  player elo: %s %d", $symbol, $match->getPlayerEloDiff());
237 20
        }
238
239
        // Output the match stats that will be sent back to BZFS
240 21
        return $bzfsAnnouncement;
241
    }
242
243
    /**
244
     * Convert a comma-separated list of bzids to player IDs so we can pass
245
     * them to Match::enterMatch()
246
     *
247
     * @param  string $players A comma-separated list of BZIDs
248
     * @return int[]  A list of Player IDs
249
     */
250 22
    private function bzidsToIdArray($players)
251
    {
252 22
        $players = explode(',', $players);
253
254 22
        foreach ($players as &$player) {
255 22
            $player = Player::getFromBZID($player)->getId();
256 22
        }
257
258 22
        return $players;
259
    }
260
261
    /**
262
     * Queries the database to get the team which a conversation of players belong to
263
     *
264
     * @param  int[] $players The IDs of players
265
     * @return Team  The team
266
     */
267 21
    private function getTeam($players)
268
    {
269 21
        $team = null;
270
271 21
        foreach ($players as $id) {
272 21
            $player = Player::get($id);
273
274 21
            if ($player->isTeamless()) {
275 12
                return Team::invalid();
276 21
            } elseif ($team == null) {
277 21
                $team = $player->getTeam();
278 21
            } elseif ($team->getId() != $player->getTeam()->getId()) {
279
                // This player is on a different team from the previous player!
280
                return Team::invalid();
281
            }
282 21
        }
283
284 17
        return $team;
285
    }
286
287
    /**
288
     * {@inheritdoc}
289
     */
290 22
    protected static function getLogChannel()
291
    {
292 22
        return 'api';
293
    }
294
}
295