Issues in CFastTrackCache.php - All Issues - Inspection of "prepare to tag v0.7.15" - mosbth/cimage - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 1e5de9...21e538 )

by Mikael

created 2016-08-09 08:20 UTC

CFastTrackCache.php (6 issues)

Labels

Severity

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

<?php
/**
 * Enable a fast track cache with a json representation of the image delivery.
 *
 */
class CFastTrackCache
namespace YourVendor;

class YourClass { }
{
    /**
     * Cache is disabled to start with.
     */
    private $enabled = false;



    /**
     * Path to the cache directory.
     */
    private $path;



    /**
     * Filename of current cache item.
     */
    private $filename;



    /**
     * Container with items to store as cached item.
     */
    private $container;



    /**
     * Enable or disable cache.
     *
     * @param boolean $enable set to true to enable, false to disable
/**
 * @param array $germany
 * @param array $ireland
 */
function finale($germany, $island) {
    return "2:1";
}
     *
     * @return $this
     */
    public function enable($enabled)
    {
        $this->enabled = $enabled;
        return $this;
    }



    /**
     * Set the path to the cache dir which must exist.
     *
     * @param string $path to the cache dir.
     *
     * @throws Exception when $path is not a directory.
     *
     * @return $this
     */
    public function setCacheDir($path)
    {
        if (!is_dir($path)) {
            throw new Exception("Cachedir is not a directory.");
        }

        $this->path = rtrim($path, "/");

        return $this;
    }



    /**
     * Set the filename to store in cache, use the querystring to create that
     * filename.
     *
     * @param array $clear items to clear in $_GET when creating the filename.
     *
     * @return string as filename created.
     */
    public function setFilename($clear)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $query = $_GET;

        // Remove parts from querystring that should not be part of filename
        foreach ($clear as $value) {
            unset($query[$value]);
        }

        arsort($query);
        $queryAsString = http_build_query($query);

        $this->filename = md5($queryAsString);

        if (CIMAGE_DEBUG) {
            $this->container["query-string"] = $queryAsString;
        }

        return $this->filename;
    }



    /**
     * Add header items.
     *
     * @param string $header add this as header.
     *
     * @return $this
     */
    public function addHeader($header)
    {
        $this->container["header"][] = $header;
        return $this;
    }



    /**
     * Add header items on output, these are not output when 304.
     *
     * @param string $header add this as header.
     *
     * @return $this
     */
    public function addHeaderOnOutput($header)
    {
        $this->container["header-output"][] = $header;
        return $this;
    }



    /**
     * Set path to source image to.
     *
     * @param string $source path to source image file.
     *
     * @return $this
     */
    public function setSource($source)
    {
        $this->container["source"] = $source;
        return $this;
    }



    /**
     * Set last modified of source image, use to check for 304.
     *
     * @param string $lastModified
     *
     * @return $this
     */
    public function setLastModified($lastModified)
    {
        $this->container["last-modified"] = $lastModified;
        return $this;
    }



    /**
     * Get filename of cached item.
     *
     * @return string as filename.
     */
    public function getFilename()
    {
        return $this->path . "/" . $this->filename;
    }



    /**
     * Write current item to cache.
     *
     * @return boolean if cache file was written.
     */
    public function writeToCache()
    {
        if (!$this->enabled) {
            return false;
        }

        if (is_dir($this->path) && is_writable($this->path)) {
            $filename = $this->getFilename();
            return file_put_contents($filename, json_encode($this->container)) !== false;
        }

        return false;
    }



    /**
     * Output current item from cache, if available.
     *
     * @return void
     */
    public function output()
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        $filename = $this->getFilename();
        if (!is_readable($filename)) {
            return;
        }

        $item = json_decode(file_get_contents($filename), true);

        if (!is_readable($item["source"])) {
            return;
        }

        foreach ($item["header"] as $value) {
            header($value);
        }

        if (isset($_SERVER["HTTP_IF_MODIFIED_SINCE"])
            && strtotime($_SERVER["HTTP_IF_MODIFIED_SINCE"]) == $item["last-modified"]) {
            header("HTTP/1.0 304 Not Modified");
            if (CIMAGE_DEBUG) {
                trace(__CLASS__ . " 304");
            }
            exit;

        }

        foreach ($item["header-output"] as $value) {
            header($value);
        }

        if (CIMAGE_DEBUG) {
            trace(__CLASS__ . " 200");
        }
        readfile($item["source"]);
        exit;

    }
}


1			<?php
2			/**
3			* Enable a fast track cache with a json representation of the image delivery.
4			*
5			*/
6			class CFastTrackCache
			0 ignored issues – show Coding Style Compatibility introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this PSR1 recommends that each class must be in a namespace of at least one level to avoid collisions. You can fix this by adding a namespace to your class: namespace YourVendor; class YourClass { } When choosing a vendor namespace, try to pick something that is not too generic to avoid conflicts with other libraries. Loading history...
7			{
8			/**
9			* Cache is disabled to start with.
10			*/
11			private $enabled = false;
12
13
14
15			/**
16			* Path to the cache directory.
17			*/
18			private $path;
19
20
21
22			/**
23			* Filename of current cache item.
24			*/
25			private $filename;
26
27
28
29			/**
30			* Container with items to store as cached item.
31			*/
32			private $container;
33
34
35
36			/**
37			* Enable or disable cache.
38			*
39			* @param boolean $enable set to true to enable, false to disable
			0 ignored issues – show Documentation introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this There is no parameter named `$enable`. Did you maybe mean `$enabled`? This check looks for PHPDoc comments describing methods or function parameters that do not exist on the corresponding method or function. It has, however, found a similar but not annotated parameter which might be a good fit. Consider the following example. The parameter `$ireland` is not defined by the method `finale(...)`. /** * @param array $germany * @param array $ireland */ function finale($germany, $island) { return "2:1"; } The most likely cause is that the parameter was changed, but the annotation was not. Loading history...
40			*
41			* @return $this
42			*/
43			public function enable($enabled)
44			{
45			$this->enabled = $enabled;
46			return $this;
47			}
48
49
50
51			/**
52			* Set the path to the cache dir which must exist.
53			*
54			* @param string $path to the cache dir.
55			*
56			* @throws Exception when $path is not a directory.
57			*
58			* @return $this
59			*/
60			public function setCacheDir($path)
61			{
62			if (!is_dir($path)) {
63			throw new Exception("Cachedir is not a directory.");
64			}
65
66			$this->path = rtrim($path, "/");
67
68			return $this;
69			}
70
71
72
73			/**
74			* Set the filename to store in cache, use the querystring to create that
75			* filename.
76			*
77			* @param array $clear items to clear in $_GET when creating the filename.
78			*
79			* @return string as filename created.
80			*/
81			public function setFilename($clear)
			0 ignored issues – show Coding Style introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `setFilename` uses the super-global variable `$_GET` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
82			{
83			$query = $_GET;
84
85			// Remove parts from querystring that should not be part of filename
86			foreach ($clear as $value) {
87			unset($query[$value]);
88			}
89
90			arsort($query);
91			$queryAsString = http_build_query($query);
92
93			$this->filename = md5($queryAsString);
94
95			if (CIMAGE_DEBUG) {
96			$this->container["query-string"] = $queryAsString;
97			}
98
99			return $this->filename;
100			}
101
102
103
104			/**
105			* Add header items.
106			*
107			* @param string $header add this as header.
108			*
109			* @return $this
110			*/
111			public function addHeader($header)
112			{
113			$this->container["header"][] = $header;
114			return $this;
115			}
116
117
118
119			/**
120			* Add header items on output, these are not output when 304.
121			*
122			* @param string $header add this as header.
123			*
124			* @return $this
125			*/
126			public function addHeaderOnOutput($header)
127			{
128			$this->container["header-output"][] = $header;
129			return $this;
130			}
131
132
133
134			/**
135			* Set path to source image to.
136			*
137			* @param string $source path to source image file.
138			*
139			* @return $this
140			*/
141			public function setSource($source)
142			{
143			$this->container["source"] = $source;
144			return $this;
145			}
146
147
148
149			/**
150			* Set last modified of source image, use to check for 304.
151			*
152			* @param string $lastModified
153			*
154			* @return $this
155			*/
156			public function setLastModified($lastModified)
157			{
158			$this->container["last-modified"] = $lastModified;
159			return $this;
160			}
161
162
163
164			/**
165			* Get filename of cached item.
166			*
167			* @return string as filename.
168			*/
169			public function getFilename()
170			{
171			return $this->path . "/" . $this->filename;
172			}
173
174
175
176			/**
177			* Write current item to cache.
178			*
179			* @return boolean if cache file was written.
180			*/
181			public function writeToCache()
182			{
183			if (!$this->enabled) {
184			return false;
185			}
186
187			if (is_dir($this->path) && is_writable($this->path)) {
188			$filename = $this->getFilename();
189			return file_put_contents($filename, json_encode($this->container)) !== false;
190			}
191
192			return false;
193			}
194
195
196
197			/**
198			* Output current item from cache, if available.
199			*
200			* @return void
201			*/
202			public function output()
			0 ignored issues – show Coding Style introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this `output` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history...
203			{
204			$filename = $this->getFilename();
205			if (!is_readable($filename)) {
206			return;
207			}
208
209			$item = json_decode(file_get_contents($filename), true);
210
211			if (!is_readable($item["source"])) {
212			return;
213			}
214
215			foreach ($item["header"] as $value) {
216			header($value);
217			}
218
219			if (isset($_SERVER["HTTP_IF_MODIFIED_SINCE"])
220			&& strtotime($_SERVER["HTTP_IF_MODIFIED_SINCE"]) == $item["last-modified"]) {
221			header("HTTP/1.0 304 Not Modified");
222			if (CIMAGE_DEBUG) {
223			trace(__CLASS__ . " 304");
224			}
225			exit;
			0 ignored issues – show Coding Style Compatibility introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this The method `output()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
226			}
227
228			foreach ($item["header-output"] as $value) {
229			header($value);
230			}
231
232			if (CIMAGE_DEBUG) {
233			trace(__CLASS__ . " 200");
234			}
235			readfile($item["source"]);
236			exit;
			0 ignored issues – show Coding Style Compatibility introduced 2016-07-10 20:58 UTC by Report Bug Copy Issue Report Show Similar Issues like this The method `output()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
237			}
238			}
239

mosbth / cimage

Push — master ( 1e5de9...21e538 )

CFastTrackCache.php (6 issues)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like