Musixmatch::jsonpDecode() - Code Metrics - Inspection of "Musixmatch service integration to populate each so..." - phanan/koel - Measure and Improve Code Quality continuously with Scrutinizer

Test Setup Failed

Pull Request — master (#623)

by Alejandro Carstens

created 2017-07-18 00:28 UTC

Musixmatch::jsonpDecode() A

↳ Parent: Musixmatch

Complexity

Conditions	1
Paths	1

Size

Total Lines	4
Code Lines	2

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	4
rs	10
c	0
b	0
f	0
cc	1
eloc	2
nc	1
nop	1

<?php

namespace App\Services;

use App\Models\Song;
use Cache;
use GuzzleHttp\Client;
use Log;

class Musixmatch extends RESTfulService
{
    /**
     * Construct an instance of Musixmatch service.
     *
     * @param string      $key    The Musixmatch API key
     * @param Client|null $client The Guzzle HTTP client
     */
    public function __construct($key = null, Client $client = null)

    {
        parent::__construct(
            $key ?: config('koel.musixmatch.key'),
            null,
            'https://api.musixmatch.com/ws/1.1',
            $client ?: new Client()
        );
    }
    
    /**
     * Determine if our application is using Musixmatch.
     *
     * @return bool
     */
    public function enabled()
    {
        return (bool) config('koel.musixmatch.key');
    }
    
    /**
     * @param App\Models\Song $song
     * 
     * @return mixed|null
     */
    public function searchLyricsRelatedToSong(Song $song)
    {
        if($song->hasLyrics() == false)

        {
            if (!$song->artist->isUnknown() && !$song->artist->isVarious()) {

                $lyrics = $this->search($song->title, $song->artist->name);
            } else {
                $lyrics = $this->search($song->title);
            }
            
            return $song->updateSingle($song->title, $song->album->name, $song->artist->name, $lyrics, $song->track, (int) $song->compilationState);
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
        }
        
        return false;
    }
    
    /**
     * @param string $name
     * @param string $artistName
     * 
     * @return mixed|false
     */
    public function search(string $name, string $artistName = null)
    {
        if (!$this->enabled()) {
            return false;
        }
        
        $uri = sprintf('matcher.lyrics.get?format=jsonp&callback=callback&q_track=%s&q_artist=%s&apikey=%s',
            urlencode($name),
            urlencode($artistName),
            $this->key
        );
        
        try{
            $response = $this->jsonpDecode($this->get($uri));
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
            
            return $this->getLyrics($response);
        }catch(\Exception $e){
            Log::error($e);

            return false;
        }
    }
    
    /**
     * @param string $response
     * 
     * @return mixed
     */
    private function jsonpDecode($response)
    {
        return json_decode(substr(preg_replace("/[^(]*\((.*)\)/","$1",$response), 0, -1));
    }
    
    /**
     * @param JSON $response
     * 
     * @return string|null
     */
    private function getLyrics($response)
    {
        $lyrics = $response->message->body->lyrics->lyrics_body;
        
        if(strlen($lyrics))
        {
            // Correct Musixmatch spelling mistake (is for are).
            return substr($lyrics, 0, strpos($lyrics, "******* This Lyrics is NOT for Commercial use *******")).
                "*** This Lyrics are NOT for Commercial use ***";
        }
        
        return false;
    }
}

1		<?php
2
3		namespace App\Services;
4
5		use App\Models\Song;
6		use Cache;
7		use GuzzleHttp\Client;
8		use Log;
9
10		class Musixmatch extends RESTfulService
11		{
12		/**
13		* Construct an instance of Musixmatch service.
14		*
15		* @param string $key The Musixmatch API key
16		* @param Client\|null $client The Guzzle HTTP client
17		*/
18	View Code Duplication	public function __construct($key = null, Client $client = null)
		0 ignored issues – show Duplication introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
19		{
20		parent::__construct(
21		$key ?: config('koel.musixmatch.key'),
22		null,
23		'https://api.musixmatch.com/ws/1.1',
24		$client ?: new Client()
25		);
26		}
27
28		/**
29		* Determine if our application is using Musixmatch.
30		*
31		* @return bool
32		*/
33		public function enabled()
34		{
35		return (bool) config('koel.musixmatch.key');
36		}
37
38		/**
39		* @param App\Models\Song $song
40		*
41		* @return mixed\|null
42		*/
43		public function searchLyricsRelatedToSong(Song $song)
44		{
45		if($song->hasLyrics() == false)
		0 ignored issues – show Coding Style Best Practice introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report It seems like you are loosely comparing two booleans. Considering using the strict comparison `===` instead. When comparing two booleans, it is generally considered safer to use the strict comparison operator. Loading history...
46		{
47		if (!$song->artist->isUnknown() && !$song->artist->isVarious()) {
		0 ignored issues – show Bug introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The method `isUnknown()` does not exist on `App\Models\Artist`. Did you maybe mean `getIsUnknownAttribute()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history... Bug introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The method `isVarious()` does not exist on `App\Models\Artist`. Did you maybe mean `getIsVariousAttribute()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history...
48		$lyrics = $this->search($song->title, $song->artist->name);
49		} else {
50		$lyrics = $this->search($song->title);
51		}
52
53		return $song->updateSingle($song->title, $song->album->name, $song->artist->name, $lyrics, $song->track, (int) $song->compilationState);
		0 ignored issues – show Documentation introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report The property `compilationState` does not exist on `object<App\Models\Song>`. Since you implemented `__get`, maybe consider adding a @property annotation. Since your code implements the magic getter `_get`, this function will be called for any read access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?php /** * @property int $x * @property int $y * @property string $text */ class MyLabel { private $properties; private $allowedProperties = array('x', 'y', 'text'); public function __get($name) { if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) { return $properties[$name]; } else { return null; } } public function __set($name, $value) { if (in_array($name, $this->allowedProperties)) { $properties[$name] = $value; } else { throw new \LogicException("Property $name is not defined."); } } } If the property has read access only, you can use the @property-read annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history... Security Bug introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report It seems like `$lyrics` can also be of type `false`; however, `App\Models\Song::updateSingle()` does only seem to accept `string`, did you maybe forget to handle an error condition? Loading history...
54		}
55
56		return false;
57		}
58
59		/**
60		* @param string $name
61		* @param string $artistName
62		*
63		* @return mixed\|false
64		*/
65		public function search(string $name, string $artistName = null)
66		{
67		if (!$this->enabled()) {
68		return false;
69		}
70
71		$uri = sprintf('matcher.lyrics.get?format=jsonp&callback=callback&q_track=%s&q_artist=%s&apikey=%s',
72		urlencode($name),
73		urlencode($artistName),
74		$this->key
75		);
76
77		try{
78		$response = $this->jsonpDecode($this->get($uri));
		0 ignored issues – show Documentation introduced 2017-07-17 23:56 UTC by Report Bug Copy Issue Report `$this->get($uri)` is of type `object`, but the function expects a `string`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
79
80		return $this->getLyrics($response);
81		}catch(\Exception $e){
82		Log::error($e);
83
84		return false;
85		}
86		}
87
88		/**
89		* @param string $response
90		*
91		* @return mixed
92		*/
93		private function jsonpDecode($response)
94		{
95		return json_decode(substr(preg_replace("/[^(]\((.)\)/","$1",$response), 0, -1));
96		}
97
98		/**
99		* @param JSON $response
100		*
101		* @return string\|null
102		*/
103		private function getLyrics($response)
104		{
105		$lyrics = $response->message->body->lyrics->lyrics_body;
106
107		if(strlen($lyrics))
108		{
109		// Correct Musixmatch spelling mistake (is for are).
110		return substr($lyrics, 0, strpos($lyrics, "***** This Lyrics is NOT for Commercial use *****")).
111		"* This Lyrics are NOT for Commercial use *";
112		}
113
114		return false;
115		}
116		}

phanan / koel

Pull Request — master (#623)

Musixmatch::jsonpDecode() A

Complexity

Size

Duplication

Importance

Duplication Side-by-Side

Filter issues like