Completed
Branch master (d86252)
by Laurens
04:22 queued 59s
created

Client   C

Complexity

Total Complexity 44

Size/Duplication

Total Lines 406
Duplicated Lines 0 %

Coupling/Cohesion

Components 1
Dependencies 20

Test Coverage

Coverage 90.67%

Importance

Changes 26
Bugs 2 Features 6
Metric Value
wmc 44
c 26
b 2
f 6
lcom 1
cbo 20
dl 0
loc 406
ccs 136
cts 150
cp 0.9067
rs 5.1558

16 Methods

Rating   Name   Duplication   Size   Complexity  
A __construct() 0 18 1
A setApiDetails() 0 4 1
A setConfig() 0 6 1
A getRefreshToken() 0 4 1
B get() 0 27 2
A setProxy() 0 4 1
A getCacheDir() 0 9 2
A getFilesFromReportRequest() 0 13 2
A submitGenerateReport() 0 11 2
A getReportRequest() 0 6 1
D waitForStatus() 0 35 9
A pollGenerateReport() 0 10 2
A fixFile() 0 15 3
A moveFirstFile() 0 7 1
A clearCache() 0 17 3
C parseSoapFault() 0 30 12

How to fix   Complexity   

Complex Class

Complex classes like Client 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 Client, and based on these observations, apply Extract Interface, too.

1
<?php
2
namespace Werkspot\BingAdsApiBundle\Api;
3
4
use BingAds\Proxy\ClientProxy;
5
use BingAds\Reporting\PollGenerateReportRequest;
6
use BingAds\Reporting\ReportRequest;
7
use BingAds\Reporting\ReportTimePeriod;
8
use BingAds\Reporting\SubmitGenerateReportRequest;
9
use Exception;
10
use SoapFault;
11
use SoapVar;
12
use Symfony\Component\Filesystem\Filesystem;
13
use Symfony\Component\Finder\Finder;
14
use Werkspot\BingAdsApiBundle\Api\Helper\Csv;
15
use Werkspot\BingAdsApiBundle\Api\Helper\File;
16
use Werkspot\BingAdsApiBundle\Api\Helper\Time;
17
use Werkspot\BingAdsApiBundle\Api\Report\ReportInterface;
18
use Werkspot\BingAdsApiBundle\Guzzle\OauthTokenService;
19
use Werkspot\BingAdsApiBundle\Model\AccessToken;
20
use Werkspot\BingAdsApiBundle\Model\ApiDetails;
21
22
class Client
23
{
24
    /**
25
     * @var array
26
     */
27
    private $config = [];
28
29
    /**
30
     * @var string
31
     */
32
    private $fileName;
33
34
    /**
35
     * @var ClientProxy
36
     */
37
    private $proxy;
38
39
    /**
40
     * @var array
41
     */
42
    public $report;
43
44
    /**
45
     * @var string
46
     */
47
    private $files;
48
49
    /**
50
     * @var OauthTokenService
51
     */
52
    private $oauthTokenService;
53
54
    /**
55
     * @var ApiDetails
56
     */
57
    private $apiDetails;
58
59
    /**
60
     * @var ClientProxy
61
     */
62
    private $clientProxy;
63
64
    /**
65
     * @var File
66
     */
67
    private $fileHelper;
68
69
    /**
70
     * @var Csv
71
     */
72
    private $csvHelper;
73
74
    /**
75
     * @var Time
76
     */
77
    private $timeHelper;
78
79
    /**
80
     * Client constructor.
81
     *
82
     * @param OauthTokenService $oauthTokenService
83
     * @param ApiDetails $apiDetails
84
     * @param ClientProxy $clientProxy
85
     * @param File $file
86
     * @param Csv $csv
87
     * @param Time $timeHelper
88
     */
89 12
    public function __construct(OauthTokenService $oauthTokenService, ApiDetails $apiDetails, ClientProxy $clientProxy, File $file, Csv $csv, Time $timeHelper)
90 1
    {
91 12
        $this->oauthTokenService = $oauthTokenService;
92 12
        $this->apiDetails = $apiDetails;
93 12
        $this->clientProxy = $clientProxy;
94 12
        $this->fileHelper = $file;
95 12
        $this->csvHelper = $csv;
96 12
        $this->timeHelper = $timeHelper;
97
98 12
        ini_set('soap.wsdl_cache_enabled', '0');
99 12
        ini_set('soap.wsdl_cache_ttl', '0');
100
101 12
        $this->fileName = 'report.zip';
102
103 12
        $this->report = [
104 12
            'GeoLocationPerformanceReport' => new Report\GeoLocationPerformanceReport(),
105
        ];
106 12
    }
107
108
    public function setApiDetails(ApiDetails $apiDetails)
109
    {
110
        $this->apiDetails = $apiDetails;
111
    }
112
113
    /**
114
     * Sets the configuration
115
     *
116
     * @param $config
117
     */
118 12
    public function setConfig($config)
119
    {
120 12
        $this->config = $config;
121 12
        $this->config['cache_dir'] = $this->config['cache_dir'] . '/' . 'BingAdsApiBundle'; //<-- important for the cache clear function
122 12
        $this->config['csv']['fixHeader']['removeColumnHeader'] = true; //-- fix till i know how to do this
123 12
    }
124
125 1
    public function getRefreshToken()
126
    {
127 1
        return $this->apiDetails->getRefreshToken();
128
    }
129
130
    /**
131
     * @param array $columns
132
     * @param string $name
133
     * @param $timePeriod
134
     * @param null|string $fileLocation
135
     *
136
     * @return array|string
137
     */
138 12
    public function get(array $columns, $name = 'GeoLocationPerformanceReport', $timePeriod = ReportTimePeriod::LastWeek, $fileLocation = null)
139
    {
140 12
        $tokens = $this->oauthTokenService->refreshToken(
141 12
            $this->apiDetails->getClientId(),
142 12
            $this->apiDetails->getSecret(),
143 12
            $this->apiDetails->getRedirectUri(),
144 12
            new AccessToken(null, $this->apiDetails->getRefreshToken())
145 12
        );
146
147 12
        $accessToken = $tokens->getAccessToken();
148 12
        $this->apiDetails->setRefreshToken($tokens->getRefreshToken());
149
150 12
        $report = $this->report[$name];
151 12
        $report->setTimePeriod($timePeriod);
152 12
        $report->setColumns($columns);
153 12
        $reportRequest = $report->getRequest();
154 12
        $this->setProxy($report::WSDL, $accessToken);
155 12
        $files = $this->getFilesFromReportRequest($reportRequest, $name, "{$this->getCacheDir()}/{$this->fileName}", $report);
0 ignored issues
show
Coding Style Best Practice introduced by
As per coding-style, please use concatenation or sprintf for the variable $this instead of interpolation.

It is generally a best practice as it is often more readable to use concatenation instead of interpolation for variables inside strings.

// Instead of
$x = "foo $bar $baz";

// Better use either
$x = "foo " . $bar . " " . $baz;
$x = sprintf("foo %s %s", $bar, $baz);
Loading history...
156
157 3
        if ($fileLocation !== null) {
158 1
            $this->moveFirstFile($fileLocation);
159
160 1
            return $fileLocation;
161
        } else {
162 2
            return $files;
163
        }
164
    }
165
166
    /**
167
     * @param string $wsdl
168
     * @param string $accessToken
169
     */
170 12
    private function setProxy($wsdl, $accessToken)
171
    {
172 12
        $this->proxy = $this->clientProxy->ConstructWithCredentials($wsdl, null, null, $this->apiDetails->getDevToken(), $accessToken);
173 12
    }
174
175
    /**
176
     * @return string
177
     */
178 12
    private function getCacheDir()
179
    {
180 12
        $fs = new Filesystem();
181 12
        if (!$fs->exists($this->config['cache_dir'])) {
182 1
            $fs->mkdir($this->config['cache_dir'], 0700);
183 1
        }
184
185 12
        return $this->config['cache_dir'];
186
    }
187
188
    /**
189
     * @param ReportRequest $reportRequest
190
     * @param string $name
191
     * @param string $downloadFile
192
     * @param ReportInterface $report
193
     *
194
     * @throws Exception
195
     *
196
     * @return array
0 ignored issues
show
Documentation introduced by
Should the return type not be array|string? Also, consider making the array more specific, something like array<String>, or String[].

This check compares the return type specified in the @return annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch.

If the return type contains the type array, this check recommends the use of a more specific type like String[] or array<String>.

Loading history...
197
     */
198 12
    private function getFilesFromReportRequest(ReportRequest $reportRequest, $name, $downloadFile, ReportInterface $report)
199
    {
200 12
        $reportRequestId = $this->submitGenerateReport($reportRequest, $name);
201 6
        $reportRequestStatus = $this->waitForStatus($reportRequestId);
202 3
        $reportDownloadUrl = $reportRequestStatus->ReportDownloadUrl;
203 3
        $zipFile = $this->fileHelper->getFile($reportDownloadUrl, $downloadFile);
204 3
        if ($zipFile !== false) {
205 3
            $this->files = $this->fileHelper->unZip($zipFile);
1 ignored issue
show
Bug introduced by
It seems like $zipFile defined by $this->fileHelper->getFi...loadUrl, $downloadFile) on line 203 can also be of type boolean; however, Werkspot\BingAdsApiBundle\Api\Helper\File::unZip() does only seem to accept string, maybe add an additional type check?

If a method or function can return multiple different values and unless you are sure that you only can receive a single value in this context, we recommend to add an additional type check:

/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

If this a common case that PHP Analyzer should handle natively, please let us know by opening an issue.

Loading history...
206 3
            $this->fixFile($report);
207 3
        }
208
209 3
        return $this->files;
210
    }
211
212
    /**
213
     * SubmitGenerateReport helper method calls the corresponding Bing Ads service operation
214
     * to request the report identifier. The identifier is used to check report generation status
215
     * before downloading the report.
216
     *
217
     * @param mixed  $report
218
     * @param string $name
219
     *
220
     * @return string ReportRequestId
221
     */
222 12
    private function submitGenerateReport($report, $name)
223
    {
224 12
        $request = new SubmitGenerateReportRequest();
225
        try {
226 12
            $request->ReportRequest = $this->getReportRequest($report, $name);
227
228 12
            return $this->proxy->GetService()->SubmitGenerateReport($request)->ReportRequestId;
229 6
        } catch (SoapFault $e) {
230 6
            $this->parseSoapFault($e);
231
        }
232
    }
233
234
    /**
235
     * @param mixed  $report
236
     * @param string $name
237
     *
238
     * @return SoapVar
239
     */
240 12
    private function getReportRequest($report, $name)
241
    {
242 12
        $name = "{$name}Request";
1 ignored issue
show
Coding Style Best Practice introduced by
As per coding-style, please use concatenation or sprintf for the variable $name instead of interpolation.

It is generally a best practice as it is often more readable to use concatenation instead of interpolation for variables inside strings.

// Instead of
$x = "foo $bar $baz";

// Better use either
$x = "foo " . $bar . " " . $baz;
$x = sprintf("foo %s %s", $bar, $baz);
Loading history...
243
244 12
        return new SoapVar($report, SOAP_ENC_OBJECT, $name, $this->proxy->GetNamespace());
245
    }
246
247
    /**
248
     * Check if the report is ready for download
249
     * if not wait 10 sec and retry. (up to 6,5 hour)
250
     * After 30 tries check every 1 minute
251
     * After 34 tries check every 5 minutes
252
     * After 39 tries check every 15 minutes
253
     * After 43 tries check every 30 minutes
254
     *
255
     * @param string  $reportRequestId
256
     * @param int     $count
257
     * @param int     $maxCount
258
     * @param int     $sleep
259
     * @param bool $incrementTime
260
     *
261
     * @throws Exceptions\ReportRequestErrorException
262
     * @throws Exceptions\RequestTimeoutException
263
     *
264
     * @return string
265
     */
266 6
    private function waitForStatus($reportRequestId, $count = 1, $maxCount = 48, $sleep = 10, $incrementTime = true)
267
    {
268 6
        if ($count > $maxCount) {
269 1
            throw new Exceptions\RequestTimeoutException("The request is taking longer than expected.\nSave the report ID ({$reportRequestId}) and try again later.");
0 ignored issues
show
Coding Style Best Practice introduced by
As per coding-style, please use concatenation or sprintf for the variable $reportRequestId instead of interpolation.

It is generally a best practice as it is often more readable to use concatenation instead of interpolation for variables inside strings.

// Instead of
$x = "foo $bar $baz";

// Better use either
$x = "foo " . $bar . " " . $baz;
$x = sprintf("foo %s %s", $bar, $baz);
Loading history...
270
        }
271
272 6
        $reportRequestStatus = $this->pollGenerateReport($reportRequestId);
273 5
        if ($reportRequestStatus->Status == 'Pending') {
274 1
            ++$count;
275 1
            $this->timeHelper->sleep($sleep);
276 1
            if ($incrementTime) {
277
                switch ($count) {
278 1
                    case 31: // after 5 minutes
279 1
                        $sleep = (1 * 60);
280 1
                        break;
281 1
                    case 35: // after 10 minutes
282 1
                        $sleep = (5 * 60);
283 1
                        break;
284 1
                    case 40: // after 30 minutes
285 1
                        $sleep = (15 * 60);
286 1
                        break;
287 1
                    case 44: // after 1,5 hours
1 ignored issue
show
Unused Code Comprehensibility introduced by
38% of this comment could be valid code. Did you maybe forget this after debugging?

Sometimes obsolete code just ends up commented out instead of removed. In this case it is better to remove the code once you have checked you do not need it.

The code might also have been commented out for debugging purposes. In this case it is vital that someone uncomments it again or your project may behave in very unexpected ways in production.

This check looks for comments that seem to be mostly valid code and reports them.

Loading history...
288 1
                        $sleep = (30 * 60);
289 1
                        break;
290
                }
291 1
            }
292 1
            $reportRequestStatus = $this->waitForStatus($reportRequestId, $count, $maxCount, $sleep, $incrementTime);
293
        }
294
295 4
        if ($reportRequestStatus->Status == 'Error') {
296 1
            throw new Exceptions\ReportRequestErrorException("The request failed. Try requesting the report later.\nIf the request continues to fail, contact support.", $reportRequestStatus->Status, $reportRequestId);
297
        }
298
299 3
        return $reportRequestStatus;
300
    }
301
302
    /**
303
     * Check the status of the report request. The guidance of how often to poll
304
     * for status is from every five to 15 minutes depending on the amount
305
     * of data being requested. For smaller reports, you can poll every couple
306
     * of minutes. You should stop polling and try again later if the request
307
     * is taking longer than an hour.
308
     *
309
     * @param string $reportRequestId
310
     *
311
     * @return string ReportRequestStatus
312
     */
313 6
    private function pollGenerateReport($reportRequestId)
314
    {
315 6
        $request = new PollGenerateReportRequest();
316 6
        $request->ReportRequestId = $reportRequestId;
317
        try {
318 6
            return $this->proxy->GetService()->PollGenerateReport($request)->ReportRequestStatus;
319 1
        } catch (SoapFault $e) {
320 1
            $this->parseSoapFault($e);
321
        }
322
    }
323
324
    /**
325
     * @param array|null $files
326
     *
327
     * @return self
328
     */
329 3
    private function fixFile(ReportInterface $report, array $files = null)
330
    {
331 3
        $files = (!$files) ? $this->files : $files;
332 3
        foreach ($files as $file) {
0 ignored issues
show
Bug introduced by
The expression $files of type string|array is not guaranteed to be traversable. How about adding an additional type check?

There are different options of fixing this problem.

  1. If you want to be on the safe side, you can add an additional type-check:

    $collection = json_decode($data, true);
    if ( ! is_array($collection)) {
        throw new \RuntimeException('$collection must be an array.');
    }
    
    foreach ($collection as $item) { /** ... */ }
    
  2. If you are sure that the expression is traversable, you might want to add a doc comment cast to improve IDE auto-completion and static analysis:

    /** @var array $collection */
    $collection = json_decode($data, true);
    
    foreach ($collection as $item) { /** .. */ }
    
  3. Mark the issue as a false-positive: Just hover the remove button, in the top-right corner of this issue for more options.

Loading history...
333 3
            $lines = file($file);
334 3
            $lines = $this->csvHelper->removeHeaders($lines, $this->config['csv']['fixHeader']['removeColumnHeader'], $report::FILE_HEADERS, $report::COLUMN_HEADERS);
335 3
            $lines = $this->csvHelper->removeLastLines($lines);
336 3
            $lines = $this->csvHelper->convertDateMDYtoYMD($lines);
337 3
            $fp = fopen($file, 'w');
338 3
            fwrite($fp, implode('', $lines));
339 3
            fclose($fp);
340 3
        }
341
342 3
        return $this;
343
    }
344
345
    /**
346
     * Move first file form array $this->files to the target location
347
     *
348
     * @param string $target
349
     *
350
     * @return self
351
     */
352 1
    private function moveFirstFile($target)
353
    {
354 1
        $fs = new Filesystem();
355 1
        $fs->rename($this->files[0], $target);
356
357 1
        return $this;
358
    }
359
360
    /**
361
     * Clear Bundle Cache directory
362
     *
363
     * @param bool $allFiles delete all files in bundles cache, if false deletes only extracted files ($this->files)
364
     *
365
     * @return self
366
     *
367
     * @codeCoverageIgnore
368
     */
369
    public function clearCache($allFiles = false)
370
    {
371
        $fileSystem = new Filesystem();
372
373
        if ($allFiles) {
374
            $finder = new Finder();
375
            $files = $finder->files()->in($this->config['cache_dir']);
376
        } else {
377
            $files = $this->files;
378
        }
379
380
        foreach ($files as $file) {
381
            $fileSystem->remove($file);
382
        }
383
384
        return $this;
385
    }
386
387
    /**
388
     * @param SoapFault $e
389
     *
390
     * @throws Exceptions\SoapInternalErrorException
391
     * @throws Exceptions\SoapInvalidCredentialsException
392
     * @throws Exceptions\SoapNoCompleteDataAvailableException
393
     * @throws Exceptions\SoapReportingServiceInvalidReportIdException
394
     * @throws Exceptions\SoapUnknownErrorException
395
     * @throws Exceptions\SoapUserIsNotAuthorizedException
396
     */
397 7
    private function parseSoapFault(SoapFault $e)
398
    {
399 7
        if (isset($e->detail->AdApiFaultDetail)) {
400 7
            $error = $e->detail->AdApiFaultDetail->Errors->AdApiError;
401 7
        } elseif (isset($e->detail->ApiFaultDetail)) {
402
            if (!empty($e->detail->ApiFaultDetail->BatchErrors)) {
403
                $error = $error = $e->detail->ApiFaultDetail->Errors->AdApiError;
404
            } elseif (!empty($e->detail->ApiFaultDetail->OperationErrors)) {
405
                $error = $e->detail->ApiFaultDetail->OperationErrors->OperationError;
406
            }
407
        }
408 7
        $errors = is_array($error) ? $error : ['error' => $error];
0 ignored issues
show
Bug introduced by
The variable $error does not seem to be defined for all execution paths leading up to this point.

If you define a variable conditionally, it can happen that it is not defined for all execution paths.

Let’s take a look at an example:

function myFunction($a) {
    switch ($a) {
        case 'foo':
            $x = 1;
            break;

        case 'bar':
            $x = 2;
            break;
    }

    // $x is potentially undefined here.
    echo $x;
}

In the above example, the variable $x is defined if you pass “foo” or “bar” as argument for $a. However, since the switch statement has no default case statement, if you pass any other value, the variable $x would be undefined.

Available Fixes

  1. Check for existence of the variable explicitly:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        if (isset($x)) { // Make sure it's always set.
            echo $x;
        }
    }
    
  2. Define a default value for the variable:

    function myFunction($a) {
        $x = ''; // Set a default which gets overridden for certain paths.
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
        }
    
        echo $x;
    }
    
  3. Add a value for the missing path:

    function myFunction($a) {
        switch ($a) {
            case 'foo':
                $x = 1;
                break;
    
            case 'bar':
                $x = 2;
                break;
    
            // We add support for the missing case.
            default:
                $x = '';
                break;
        }
    
        echo $x;
    }
    
Loading history...
409 7
        foreach ($errors as $error) {
410 7
            switch ($error->Code) {
411 7
                case 0:
412 2
                    throw new Exceptions\SoapInternalErrorException($error->Message, $error->Code);
413 5
                case 105:
414 1
                    throw new Exceptions\SoapInvalidCredentialsException($error->Message, $error->Code);
415 4
                case 106:
416 1
                    throw new Exceptions\SoapUserIsNotAuthorizedException($error->Message, $error->Code);
417 3
                case 2004:
418 1
                    throw new Exceptions\SoapNoCompleteDataAvailableException($error->Message, $error->Code);
419 2
                case 2100:
420 1
                    throw new Exceptions\SoapReportingServiceInvalidReportIdException($error->Message, $error->Code);
421 1
                default:
422 1
                    $errorMessage = "[{$error->Code}]\n{$error->Message}";
0 ignored issues
show
Coding Style Best Practice introduced by
As per coding-style, please use concatenation or sprintf for the variable $error instead of interpolation.

It is generally a best practice as it is often more readable to use concatenation instead of interpolation for variables inside strings.

// Instead of
$x = "foo $bar $baz";

// Better use either
$x = "foo " . $bar . " " . $baz;
$x = sprintf("foo %s %s", $bar, $baz);
Loading history...
423 1
                    throw new Exceptions\SoapUnknownErrorException($errorMessage, $error->Code);
424 1
            }
425
        }
426
    }
427
}
428