Inspection of "remove unneeded is_dir from file cache gc" - nextcloud/server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#9584)

by Robin

created 2018-05-24 12:25 UTC

Status

Indentation +157 added lines, -157 removed lines patch added patch discarded remove patch

@@ -35,170 +35,170 @@
 block discarded – undo
 
 class File implements ICache {
 
-	/** @var View */
-	protected $storage;
+    /** @var View */
+    protected $storage;
 
-	/**
-	 * Returns the cache storage for the logged in user
-	 *
-	 * @return \OC\Files\View cache storage
-	 * @throws \OC\ForbiddenException
-	 * @throws \OC\User\NoUserException
-	 */
-	protected function getStorage() {
-		if (isset($this->storage)) {
-			return $this->storage;
-		}
-		if (\OC::$server->getUserSession()->isLoggedIn()) {
-			$rootView = new View();
-			$user = \OC::$server->getUserSession()->getUser();
-			Filesystem::initMountPoints($user->getUID());
-			if (!$rootView->file_exists('/' . $user->getUID() . '/cache')) {
-				$rootView->mkdir('/' . $user->getUID() . '/cache');
-			}
-			$this->storage = new View('/' . $user->getUID() . '/cache');
-			return $this->storage;
-		} else {
-			\OCP\Util::writeLog('core', 'Can\'t get cache storage, user not logged in', ILogger::ERROR);
-			throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in');
-		}
-	}
+    /**
+     * Returns the cache storage for the logged in user
+     *
+     * @return \OC\Files\View cache storage
+     * @throws \OC\ForbiddenException
+     * @throws \OC\User\NoUserException
+     */
+    protected function getStorage() {
+        if (isset($this->storage)) {
+            return $this->storage;
+        }
+        if (\OC::$server->getUserSession()->isLoggedIn()) {
+            $rootView = new View();
+            $user = \OC::$server->getUserSession()->getUser();
+            Filesystem::initMountPoints($user->getUID());
+            if (!$rootView->file_exists('/' . $user->getUID() . '/cache')) {
+                $rootView->mkdir('/' . $user->getUID() . '/cache');
+            }
+            $this->storage = new View('/' . $user->getUID() . '/cache');
+            return $this->storage;
+        } else {
+            \OCP\Util::writeLog('core', 'Can\'t get cache storage, user not logged in', ILogger::ERROR);
+            throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in');
+        }
+    }
 
-	/**
-	 * @param string $key
-	 * @return mixed|null
-	 * @throws \OC\ForbiddenException
-	 */
-	public function get($key) {
-		$result = null;
-		if ($this->hasKey($key)) {
-			$storage = $this->getStorage();
-			$result = $storage->file_get_contents($key);
-		}
-		return $result;
-	}
+    /**
+     * @param string $key
+     * @return mixed|null
+     * @throws \OC\ForbiddenException
+     */
+    public function get($key) {
+        $result = null;
+        if ($this->hasKey($key)) {
+            $storage = $this->getStorage();
+            $result = $storage->file_get_contents($key);
+        }
+        return $result;
+    }
 
-	/**
-	 * Returns the size of the stored/cached data
-	 *
-	 * @param string $key
-	 * @return int
-	 */
-	public function size($key) {
-		$result = 0;
-		if ($this->hasKey($key)) {
-			$storage = $this->getStorage();
-			$result = $storage->filesize($key);
-		}
-		return $result;
-	}
+    /**
+     * Returns the size of the stored/cached data
+     *
+     * @param string $key
+     * @return int
+     */
+    public function size($key) {
+        $result = 0;
+        if ($this->hasKey($key)) {
+            $storage = $this->getStorage();
+            $result = $storage->filesize($key);
+        }
+        return $result;
+    }
 
-	/**
-	 * @param string $key
-	 * @param mixed $value
-	 * @param int $ttl
-	 * @return bool|mixed
-	 * @throws \OC\ForbiddenException
-	 */
-	public function set($key, $value, $ttl = 0) {
-		$storage = $this->getStorage();
-		$result = false;
-		// unique id to avoid chunk collision, just in case
-		$uniqueId = \OC::$server->getSecureRandom()->generate(
-			16,
-			ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER
-		);
+    /**
+     * @param string $key
+     * @param mixed $value
+     * @param int $ttl
+     * @return bool|mixed
+     * @throws \OC\ForbiddenException
+     */
+    public function set($key, $value, $ttl = 0) {
+        $storage = $this->getStorage();
+        $result = false;
+        // unique id to avoid chunk collision, just in case
+        $uniqueId = \OC::$server->getSecureRandom()->generate(
+            16,
+            ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER
+        );
 
-		// use part file to prevent hasKey() to find the key
-		// while it is being written
-		$keyPart = $key . '.' . $uniqueId . '.part';
-		if ($storage and $storage->file_put_contents($keyPart, $value)) {
-			if ($ttl === 0) {
-				$ttl = 86400; // 60*60*24
-			}
-			$result = $storage->touch($keyPart, time() + $ttl);
-			$result &= $storage->rename($keyPart, $key);
-		}
-		return $result;
-	}
+        // use part file to prevent hasKey() to find the key
+        // while it is being written
+        $keyPart = $key . '.' . $uniqueId . '.part';
+        if ($storage and $storage->file_put_contents($keyPart, $value)) {
+            if ($ttl === 0) {
+                $ttl = 86400; // 60*60*24
+            }
+            $result = $storage->touch($keyPart, time() + $ttl);
+            $result &= $storage->rename($keyPart, $key);
+        }
+        return $result;
+    }
 
-	/**
-	 * @param string $key
-	 * @return bool
-	 * @throws \OC\ForbiddenException
-	 */
-	public function hasKey($key) {
-		$storage = $this->getStorage();
-		if ($storage && $storage->is_file($key) && $storage->isReadable($key)) {
-			return true;
-		}
-		return false;
-	}
+    /**
+     * @param string $key
+     * @return bool
+     * @throws \OC\ForbiddenException
+     */
+    public function hasKey($key) {
+        $storage = $this->getStorage();
+        if ($storage && $storage->is_file($key) && $storage->isReadable($key)) {
+            return true;
+        }
+        return false;
+    }
 
-	/**
-	 * @param string $key
-	 * @return bool|mixed
-	 * @throws \OC\ForbiddenException
-	 */
-	public function remove($key) {
-		$storage = $this->getStorage();
-		if (!$storage) {
-			return false;
-		}
-		return $storage->unlink($key);
-	}
+    /**
+     * @param string $key
+     * @return bool|mixed
+     * @throws \OC\ForbiddenException
+     */
+    public function remove($key) {
+        $storage = $this->getStorage();
+        if (!$storage) {
+            return false;
+        }
+        return $storage->unlink($key);
+    }
 
-	/**
-	 * @param string $prefix
-	 * @return bool
-	 * @throws \OC\ForbiddenException
-	 */
-	public function clear($prefix = '') {
-		$storage = $this->getStorage();
-		if ($storage and $storage->is_dir('/')) {
-			$dh = $storage->opendir('/');
-			if (is_resource($dh)) {
-				while (($file = readdir($dh)) !== false) {
-					if ($file != '.' and $file != '..' and ($prefix === '' || strpos($file, $prefix) === 0)) {
-						$storage->unlink('/' . $file);
-					}
-				}
-			}
-		}
-		return true;
-	}
+    /**
+     * @param string $prefix
+     * @return bool
+     * @throws \OC\ForbiddenException
+     */
+    public function clear($prefix = '') {
+        $storage = $this->getStorage();
+        if ($storage and $storage->is_dir('/')) {
+            $dh = $storage->opendir('/');
+            if (is_resource($dh)) {
+                while (($file = readdir($dh)) !== false) {
+                    if ($file != '.' and $file != '..' and ($prefix === '' || strpos($file, $prefix) === 0)) {
+                        $storage->unlink('/' . $file);
+                    }
+                }
+            }
+        }
+        return true;
+    }
 
-	/**
-	 * Runs GC
-	 * @throws \OC\ForbiddenException
-	 */
-	public function gc() {
-		$storage = $this->getStorage();
-		if ($storage) {
-			// extra hour safety, in case of stray part chunks that take longer to write,
-			// because touch() is only called after the chunk was finished
-			$now = time() - 3600;
-			$dh = $storage->opendir('/');
-			if (!is_resource($dh)) {
-				return null;
-			}
-			while (($file = readdir($dh)) !== false) {
-				if ($file != '.' and $file != '..') {
-					try {
-						$mtime = $storage->filemtime('/' . $file);
-						if ($mtime < $now) {
-							$storage->unlink('/' . $file);
-						}
-					} catch (\OCP\Lock\LockedException $e) {
-						// ignore locked chunks
-						\OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
-					} catch (\OCP\Files\ForbiddenException $e) {
-						\OC::$server->getLogger()->debug('Could not cleanup forbidden chunk "' . $file . '"', array('app' => 'core'));
-					} catch (\OCP\Files\LockNotAcquiredException $e) {
-						\OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
-					}
-				}
-			}
-		}
-	}
+    /**
+     * Runs GC
+     * @throws \OC\ForbiddenException
+     */
+    public function gc() {
+        $storage = $this->getStorage();
+        if ($storage) {
+            // extra hour safety, in case of stray part chunks that take longer to write,
+            // because touch() is only called after the chunk was finished
+            $now = time() - 3600;
+            $dh = $storage->opendir('/');
+            if (!is_resource($dh)) {
+                return null;
+            }
+            while (($file = readdir($dh)) !== false) {
+                if ($file != '.' and $file != '..') {
+                    try {
+                        $mtime = $storage->filemtime('/' . $file);
+                        if ($mtime < $now) {
+                            $storage->unlink('/' . $file);
+                        }
+                    } catch (\OCP\Lock\LockedException $e) {
+                        // ignore locked chunks
+                        \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
+                    } catch (\OCP\Files\ForbiddenException $e) {
+                        \OC::$server->getLogger()->debug('Could not cleanup forbidden chunk "' . $file . '"', array('app' => 'core'));
+                    } catch (\OCP\Files\LockNotAcquiredException $e) {
+                        \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
+                    }
+                }
+            }
+        }
+    }
 }

Please login to merge, or discard this patch.

		@@ -35,170 +35,170 @@
		block discarded – undo
35	35
36	36	class File implements ICache {
37	37
38		- /** @var View */
39		- protected $storage;
	38	+ /** @var View */
	39	+ protected $storage;
40	40
41		- /**
42		- * Returns the cache storage for the logged in user
43		- *
44		- * @return \OC\Files\View cache storage
45		- * @throws \OC\ForbiddenException
46		- * @throws \OC\User\NoUserException
47		- */
48		- protected function getStorage() {
49		- if (isset($this->storage)) {
50		- return $this->storage;
51		- }
52		- if (\OC::$server->getUserSession()->isLoggedIn()) {
53		- $rootView = new View();
54		- $user = \OC::$server->getUserSession()->getUser();
55		- Filesystem::initMountPoints($user->getUID());
56		- if (!$rootView->file_exists('/' . $user->getUID() . '/cache')) {
57		- $rootView->mkdir('/' . $user->getUID() . '/cache');
58		- }
59		- $this->storage = new View('/' . $user->getUID() . '/cache');
60		- return $this->storage;
61		- } else {
62		- \OCP\Util::writeLog('core', 'Can\'t get cache storage, user not logged in', ILogger::ERROR);
63		- throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in');
64		- }
65		- }
	41	+ /**
	42	+ * Returns the cache storage for the logged in user
	43	+ *
	44	+ * @return \OC\Files\View cache storage
	45	+ * @throws \OC\ForbiddenException
	46	+ * @throws \OC\User\NoUserException
	47	+ */
	48	+ protected function getStorage() {
	49	+ if (isset($this->storage)) {
	50	+ return $this->storage;
	51	+ }
	52	+ if (\OC::$server->getUserSession()->isLoggedIn()) {
	53	+ $rootView = new View();
	54	+ $user = \OC::$server->getUserSession()->getUser();
	55	+ Filesystem::initMountPoints($user->getUID());
	56	+ if (!$rootView->file_exists('/' . $user->getUID() . '/cache')) {
	57	+ $rootView->mkdir('/' . $user->getUID() . '/cache');
	58	+ }
	59	+ $this->storage = new View('/' . $user->getUID() . '/cache');
	60	+ return $this->storage;
	61	+ } else {
	62	+ \OCP\Util::writeLog('core', 'Can\'t get cache storage, user not logged in', ILogger::ERROR);
	63	+ throw new \OC\ForbiddenException('Can\t get cache storage, user not logged in');
	64	+ }
	65	+ }
66	66
67		- /**
68		- * @param string $key
69		- * @return mixed\|null
70		- * @throws \OC\ForbiddenException
71		- */
72		- public function get($key) {
73		- $result = null;
74		- if ($this->hasKey($key)) {
75		- $storage = $this->getStorage();
76		- $result = $storage->file_get_contents($key);
77		- }
78		- return $result;
79		- }
	67	+ /**
	68	+ * @param string $key
	69	+ * @return mixed\|null
	70	+ * @throws \OC\ForbiddenException
	71	+ */
	72	+ public function get($key) {
	73	+ $result = null;
	74	+ if ($this->hasKey($key)) {
	75	+ $storage = $this->getStorage();
	76	+ $result = $storage->file_get_contents($key);
	77	+ }
	78	+ return $result;
	79	+ }
80	80
81		- /**
82		- * Returns the size of the stored/cached data
83		- *
84		- * @param string $key
85		- * @return int
86		- */
87		- public function size($key) {
88		- $result = 0;
89		- if ($this->hasKey($key)) {
90		- $storage = $this->getStorage();
91		- $result = $storage->filesize($key);
92		- }
93		- return $result;
94		- }
	81	+ /**
	82	+ * Returns the size of the stored/cached data
	83	+ *
	84	+ * @param string $key
	85	+ * @return int
	86	+ */
	87	+ public function size($key) {
	88	+ $result = 0;
	89	+ if ($this->hasKey($key)) {
	90	+ $storage = $this->getStorage();
	91	+ $result = $storage->filesize($key);
	92	+ }
	93	+ return $result;
	94	+ }
95	95
96		- /**
97		- * @param string $key
98		- * @param mixed $value
99		- * @param int $ttl
100		- * @return bool\|mixed
101		- * @throws \OC\ForbiddenException
102		- */
103		- public function set($key, $value, $ttl = 0) {
104		- $storage = $this->getStorage();
105		- $result = false;
106		- // unique id to avoid chunk collision, just in case
107		- $uniqueId = \OC::$server->getSecureRandom()->generate(
108		- 16,
109		- ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER
110		- );
	96	+ /**
	97	+ * @param string $key
	98	+ * @param mixed $value
	99	+ * @param int $ttl
	100	+ * @return bool\|mixed
	101	+ * @throws \OC\ForbiddenException
	102	+ */
	103	+ public function set($key, $value, $ttl = 0) {
	104	+ $storage = $this->getStorage();
	105	+ $result = false;
	106	+ // unique id to avoid chunk collision, just in case
	107	+ $uniqueId = \OC::$server->getSecureRandom()->generate(
	108	+ 16,
	109	+ ISecureRandom::CHAR_DIGITS . ISecureRandom::CHAR_LOWER . ISecureRandom::CHAR_UPPER
	110	+ );
111	111
112		- // use part file to prevent hasKey() to find the key
113		- // while it is being written
114		- $keyPart = $key . '.' . $uniqueId . '.part';
115		- if ($storage and $storage->file_put_contents($keyPart, $value)) {
116		- if ($ttl === 0) {
117		- $ttl = 86400; // 606024
118		- }
119		- $result = $storage->touch($keyPart, time() + $ttl);
120		- $result &= $storage->rename($keyPart, $key);
121		- }
122		- return $result;
123		- }
	112	+ // use part file to prevent hasKey() to find the key
	113	+ // while it is being written
	114	+ $keyPart = $key . '.' . $uniqueId . '.part';
	115	+ if ($storage and $storage->file_put_contents($keyPart, $value)) {
	116	+ if ($ttl === 0) {
	117	+ $ttl = 86400; // 606024
	118	+ }
	119	+ $result = $storage->touch($keyPart, time() + $ttl);
	120	+ $result &= $storage->rename($keyPart, $key);
	121	+ }
	122	+ return $result;
	123	+ }
124	124
125		- /**
126		- * @param string $key
127		- * @return bool
128		- * @throws \OC\ForbiddenException
129		- */
130		- public function hasKey($key) {
131		- $storage = $this->getStorage();
132		- if ($storage && $storage->is_file($key) && $storage->isReadable($key)) {
133		- return true;
134		- }
135		- return false;
136		- }
	125	+ /**
	126	+ * @param string $key
	127	+ * @return bool
	128	+ * @throws \OC\ForbiddenException
	129	+ */
	130	+ public function hasKey($key) {
	131	+ $storage = $this->getStorage();
	132	+ if ($storage && $storage->is_file($key) && $storage->isReadable($key)) {
	133	+ return true;
	134	+ }
	135	+ return false;
	136	+ }
137	137
138		- /**
139		- * @param string $key
140		- * @return bool\|mixed
141		- * @throws \OC\ForbiddenException
142		- */
143		- public function remove($key) {
144		- $storage = $this->getStorage();
145		- if (!$storage) {
146		- return false;
147		- }
148		- return $storage->unlink($key);
149		- }
	138	+ /**
	139	+ * @param string $key
	140	+ * @return bool\|mixed
	141	+ * @throws \OC\ForbiddenException
	142	+ */
	143	+ public function remove($key) {
	144	+ $storage = $this->getStorage();
	145	+ if (!$storage) {
	146	+ return false;
	147	+ }
	148	+ return $storage->unlink($key);
	149	+ }
150	150
151		- /**
152		- * @param string $prefix
153		- * @return bool
154		- * @throws \OC\ForbiddenException
155		- */
156		- public function clear($prefix = '') {
157		- $storage = $this->getStorage();
158		- if ($storage and $storage->is_dir('/')) {
159		- $dh = $storage->opendir('/');
160		- if (is_resource($dh)) {
161		- while (($file = readdir($dh)) !== false) {
162		- if ($file != '.' and $file != '..' and ($prefix === '' \|\| strpos($file, $prefix) === 0)) {
163		- $storage->unlink('/' . $file);
164		- }
165		- }
166		- }
167		- }
168		- return true;
169		- }
	151	+ /**
	152	+ * @param string $prefix
	153	+ * @return bool
	154	+ * @throws \OC\ForbiddenException
	155	+ */
	156	+ public function clear($prefix = '') {
	157	+ $storage = $this->getStorage();
	158	+ if ($storage and $storage->is_dir('/')) {
	159	+ $dh = $storage->opendir('/');
	160	+ if (is_resource($dh)) {
	161	+ while (($file = readdir($dh)) !== false) {
	162	+ if ($file != '.' and $file != '..' and ($prefix === '' \|\| strpos($file, $prefix) === 0)) {
	163	+ $storage->unlink('/' . $file);
	164	+ }
	165	+ }
	166	+ }
	167	+ }
	168	+ return true;
	169	+ }
170	170
171		- /**
172		- * Runs GC
173		- * @throws \OC\ForbiddenException
174		- */
175		- public function gc() {
176		- $storage = $this->getStorage();
177		- if ($storage) {
178		- // extra hour safety, in case of stray part chunks that take longer to write,
179		- // because touch() is only called after the chunk was finished
180		- $now = time() - 3600;
181		- $dh = $storage->opendir('/');
182		- if (!is_resource($dh)) {
183		- return null;
184		- }
185		- while (($file = readdir($dh)) !== false) {
186		- if ($file != '.' and $file != '..') {
187		- try {
188		- $mtime = $storage->filemtime('/' . $file);
189		- if ($mtime < $now) {
190		- $storage->unlink('/' . $file);
191		- }
192		- } catch (\OCP\Lock\LockedException $e) {
193		- // ignore locked chunks
194		- \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
195		- } catch (\OCP\Files\ForbiddenException $e) {
196		- \OC::$server->getLogger()->debug('Could not cleanup forbidden chunk "' . $file . '"', array('app' => 'core'));
197		- } catch (\OCP\Files\LockNotAcquiredException $e) {
198		- \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
199		- }
200		- }
201		- }
202		- }
203		- }
	171	+ /**
	172	+ * Runs GC
	173	+ * @throws \OC\ForbiddenException
	174	+ */
	175	+ public function gc() {
	176	+ $storage = $this->getStorage();
	177	+ if ($storage) {
	178	+ // extra hour safety, in case of stray part chunks that take longer to write,
	179	+ // because touch() is only called after the chunk was finished
	180	+ $now = time() - 3600;
	181	+ $dh = $storage->opendir('/');
	182	+ if (!is_resource($dh)) {
	183	+ return null;
	184	+ }
	185	+ while (($file = readdir($dh)) !== false) {
	186	+ if ($file != '.' and $file != '..') {
	187	+ try {
	188	+ $mtime = $storage->filemtime('/' . $file);
	189	+ if ($mtime < $now) {
	190	+ $storage->unlink('/' . $file);
	191	+ }
	192	+ } catch (\OCP\Lock\LockedException $e) {
	193	+ // ignore locked chunks
	194	+ \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
	195	+ } catch (\OCP\Files\ForbiddenException $e) {
	196	+ \OC::$server->getLogger()->debug('Could not cleanup forbidden chunk "' . $file . '"', array('app' => 'core'));
	197	+ } catch (\OCP\Files\LockNotAcquiredException $e) {
	198	+ \OC::$server->getLogger()->debug('Could not cleanup locked chunk "' . $file . '"', array('app' => 'core'));
	199	+ }
	200	+ }
	201	+ }
	202	+ }
	203	+ }
204	204	}

nextcloud / server

Pull Request — master (#9584)

Status

Category

Indentation +157 added lines, -157 removed lines patch added patch discarded remove patch