Completed
Push — master ( 8158f1...90d7fd )
by Konstantinos
11:17 queued 06:55
created

LeagueOverseerHookController   B

Complexity

Total Complexity 40

Size/Duplication

Total Lines 283
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 15

Importance

Changes 5
Bugs 1 Features 4
Metric Value
wmc 40
c 5
b 1
f 4
lcom 1
cbo 15
dl 0
loc 283
rs 7.6556

8 Methods

Rating   Name   Duplication   Size   Complexity  
B setUp() 0 35 6
B queryAction() 0 17 7
B teamNameAction() 0 24 4
B teamNameDumpAction() 0 40 4
C matchReportAction() 0 83 11
A bzidsToIdArray() 0 10 2
B getTeam() 0 19 5
A getLogChannel() 0 4 1

How to fix   Complexity   

Complex Class

Complex classes like LeagueOverseerHookController often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use LeagueOverseerHookController, and based on these observations, apply Extract Interface, too.

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
    public function setUp()
26
    {
27
        $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
        $allowedIPs = array_map('trim', $this->container->getParameter('bzion.api.allowed_ips'));
32
        $clientIP   = $request->getClientIp();
33
34
        if (!$this->isDebug() && // Don't care about IPs if we're in debug mode
35
           !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
        $this->params = $request->request; // $_POST
44
45
        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
        $this->version = $this->params->get('apiVersion', 0);
59
    }
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
    public function queryAction()
72
    {
73
        $matchReportQuery = $this->version == 1 ? 'reportMatch' : 'matchReport';
74
        $teamNameQuery = $this->version == 1 ? 'teamNameQuery' : 'teamName';
75
        $teamNameDumpQuery = $this->version == 1 ? 'teamDump' : 'teamInfoDump';
76
77
        switch ($this->params->get('query')) {
78
            case $matchReportQuery:
79
                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
    public function matchReportAction(Logger $log, Request $request)
156
    {
157
        $log->addNotice("Match data received from " . $request->getClientIp());
158
        $log->addDebug("Debug match data query: " . http_build_query($this->params->all()));
159
160
        $matchType = $this->params->get('matchType', Match::OFFICIAL);
161
162
        $teamOneBZIDs = $this->params->get('teamOnePlayers');
163
        $teamTwoBZIDs = $this->params->get('teamTwoPlayers');
164
165
        $teamOnePlayers = $this->bzidsToIdArray($teamOneBZIDs);
166
        $teamTwoPlayers = $this->bzidsToIdArray($teamTwoBZIDs);
167
168
        if (Match::OFFICIAL === $matchType) {
169
            $teamOne = $this->getTeam($teamOnePlayers);
170
            $teamTwo = $this->getTeam($teamTwoPlayers);
171
172
            // If we fail to get the the team ID for either the teams or both reported teams are the same team, we cannot
173
            // report the match due to it being illegal.
174
175
            // An invalid team could be found in either or both teams, so we need to check both teams and log the match
176
            // failure respectively.
177
            $error = true;
178
            if (!$teamOne->isValid()) {
179
                $log->addNotice("The BZIDs ($teamOneBZIDs) were not found on the same team. Match invalidated.");
180
            } elseif (!$teamTwo->isValid()) {
181
                $log->addNotice("The BZIDs ($teamTwoBZIDs) were not found on the same team. Match invalidated.");
182
            } else {
183
                $error = false;
184
            }
185
186
            if ($error) {
187
                throw new ForbiddenException("An invalid player was found during the match. Please message a referee to manually report the match.");
188
            }
189
190
            if ($teamOne->isSameAs($teamTwo)) {
0 ignored issues
show
Bug introduced by
It seems like $teamTwo defined by $this->getTeam($teamTwoPlayers) on line 170 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...
191
                $log->addNotice("The '" . $teamOne->getName() . "' team played against each other in an official match. Match invalidated.");
192
                throw new ForbiddenException("Holy sanity check, Batman! The same team can't play against each other in an official match.");
193
            }
194
        } elseif (Match::FUN === $matchType) {
195
            if (count($teamOnePlayers)  < 2 || count($teamTwoPlayers) < 2) {
196
                throw new ForbiddenException("You are not allowed to report a match with less than 2 players per team.");
197
            }
198
        }
199
200
        $map = Map::fetchFromAlias($this->params->get('mapPlayed'));
201
202
        $match = Match::enterMatch(
203
            (isset($teamOne)) ? $teamOne->getId() : null,
204
            (isset($teamTwo)) ? $teamTwo->getId() : null,
205
            $this->params->get('teamOneWins'),
206
            $this->params->get('teamTwoWins'),
207
            $this->params->get('duration'),
208
            null,
209
            $this->params->get('matchTime'),
210
            $teamOnePlayers,
211
            $teamTwoPlayers,
212
            $this->params->get('server'),
213
            $this->params->get('port'),
214
            $this->params->get('replayFile'),
215
            $map->getId(),
216
            $this->params->get('matchType'),
217
            $this->params->get('teamOneColor'),
218
            $this->params->get('teamTwoColor')
219
        );
220
221
        $log->addNotice("Match reported automatically", array(
222
            'winner' => array(
223
                'name'  => $match->getWinner()->getName(),
224
                'score' => $match->getScore($match->getWinner()),
225
            ),
226
            'loser' => array(
227
                'name'  => $match->getLoser()->getName(),
228
                'score' => $match->getScore($match->getLoser())
229
            ),
230
            'eloDiff' => $match->getEloDiff(),
231
            'type'    => $matchType,
232
            'map'     => $map->getName()
233
        ));
234
235
        // Output the match stats that will be sent back to BZFS
236
        return $match->getName();
237
    }
238
239
    /**
240
     * Convert a comma-separated list of bzids to player IDs so we can pass
241
     * them to Match::enterMatch()
242
     *
243
     * @param  string $players A comma-separated list of BZIDs
244
     * @return int[]  A list of Player IDs
245
     */
246
    private function bzidsToIdArray($players)
247
    {
248
        $players = explode(',', $players);
249
250
        foreach ($players as &$player) {
251
            $player = Player::getFromBZID($player)->getId();
252
        }
253
254
        return $players;
255
    }
256
257
    /**
258
     * Queries the database to get the team which a conversation of players belong to
259
     *
260
     * @param  int[] $players The IDs of players
261
     * @return Team  The team
262
     */
263
    private function getTeam($players)
264
    {
265
        $team = null;
266
267
        foreach ($players as $id) {
268
            $player = Player::get($id);
269
270
            if ($player->isTeamless()) {
271
                return Team::invalid();
272
            } elseif ($team == null) {
273
                $team = $player->getTeam();
274
            } elseif ($team->getId() != $player->getTeam()->getId()) {
275
                // This player is on a different team from the previous player!
276
                return Team::invalid();
277
            }
278
        }
279
280
        return $team;
281
    }
282
283
    /**
284
     * {@inheritdoc}
285
     */
286
    protected static function getLogChannel()
287
    {
288
        return 'api';
289
    }
290
}
291