Skip to content

Commit 1db0dde

Browse files
PVince81PVince81
authored
Merge pull request #25768 from nextcloud/workflow-use-cache-for-mimetype
Use mimetype from cache for workflow checks
2 parents c21d3c0 + cb4648d commit 1db0dde

File tree

2 files changed

+202
-6
lines changed

2 files changed

+202
-6
lines changed

‎apps/workflowengine/lib/Check/FileMimeType.php‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
*/
2727
namespace OCA\WorkflowEngine\Check;
2828

29+
use OC\Files\Storage\Local;
2930
use OCA\WorkflowEngine\Entity\File;
3031
use OCP\Files\IMimeTypeDetector;
3132
use OCP\Files\Storage\IStorage;
@@ -76,7 +77,7 @@ public function setFileInfo(IStorage $storage, string $path, bool $isDir = false
7677
}
7778

7879
/**
79-
* The mimetype is only cached if the file exists. Otherwise files access
80+
* The mimetype is only cached if the file has a valid mimetype. Otherwise files access
8081
* control will cache "application/octet-stream" for all the target node on:
8182
* rename, move, copy and all other methods which create a new item
8283
*
@@ -91,7 +92,7 @@ public function setFileInfo(IStorage $storage, string $path, bool $isDir = false
9192
* @return string
9293
*/
9394
protected function cacheAndReturnMimeType(string $storageId, ?string $path, string $mimeType): string {
94-
if ($path !== null && $this->storage->file_exists($path)) {
95+
if ($path !== null && $mimeType !== 'application/octet-stream') {
9596
$this->mimeType[$storageId][$path] = $mimeType;
9697
}
9798

@@ -122,12 +123,15 @@ protected function getActualValue() {
122123
if ($this->mimeType[$this->storage->getId()][$this->path] !== null) {
123124
return $this->mimeType[$this->storage->getId()][$this->path];
124125
}
125-
126-
if ($this->storage->is_dir($this->path)) {
127-
return $this->cacheAndReturnMimeType($this->storage->getId(), $this->path, 'httpd/unix-directory');
126+
$cacheEntry = $this->storage->getCache()->get($this->path);
127+
if ($cacheEntry && $cacheEntry->getMimeType() !== 'application/octet-stream') {
128+
return $this->cacheAndReturnMimeType($this->storage->getId(), $this->path, $cacheEntry->getMimeType());
128129
}
129130

130-
if ($this->storage->file_exists($this->path) && $this->storage->filesize($this->path)) {
131+
if ($this->storage->file_exists($this->path) &&
132+
$this->storage->filesize($this->path) &&
133+
$this->storage->instanceOfStorage(Local::class)
134+
) {
131135
$path = $this->storage->getLocalFile($this->path);
132136
$mimeType = $this->mimeTypeDetector->detectContent($path);
133137
return $this->cacheAndReturnMimeType($this->storage->getId(), $this->path, $mimeType);

‎apps/workflowengine/tests/Check/FileMimeTypeTest.php‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
/**
5+
* @copyright Copyright (c) 2021 Robin Appelman <robin@icewind.nl>
6+
*
7+
* @license GNU AGPL version 3 or any later version
8+
*
9+
* This program is free software: you can redistribute it and/or modify
10+
* it under the terms of the GNU Affero General Public License as
11+
* published by the Free Software Foundation, either version 3 of the
12+
* License, or (at your option) any later version.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU Affero General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU Affero General Public License
20+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
21+
*
22+
*/
23+
24+
namespace OCA\WorkflowEngine\Tests\Check;
25+
26+
use OC\Files\Storage\Temporary;
27+
use OCA\WorkflowEngine\Check\FileMimeType;
28+
use OCP\Files\IMimeTypeDetector;
29+
use OCP\IL10N;
30+
use OCP\IRequest;
31+
use Test\TestCase;
32+
33+
class TemporaryNoLocal extends Temporary {
34+
public function instanceOfStorage($className) {
35+
if ($className === '\OC\Files\Storage\Local') {
36+
return false;
37+
} else {
38+
return parent::instanceOfStorage($className);
39+
}
40+
}
41+
}
42+
43+
/**
44+
* @group DB
45+
*/
46+
class FileMimeTypeTest extends TestCase {
47+
/** @var IL10N */
48+
private $l10n;
49+
/** @var IRequest */
50+
private $request;
51+
/** @var IMimeTypeDetector */
52+
private $mimeDetector;
53+
54+
private $extensions = [
55+
'.txt' => 'text/plain-path-detected',
56+
];
57+
58+
private $content = [
59+
'text-content' => 'text/plain-content-detected',
60+
];
61+
62+
protected function setUp(): void {
63+
parent::setUp();
64+
65+
$this->l10n = $this->createMock(IL10N::class);
66+
$this->request = $this->createMock(IRequest::class);
67+
$this->mimeDetector = $this->createMock(IMimeTypeDetector::class);
68+
$this->mimeDetector->method('detectPath')
69+
->willReturnCallback(function ($path) {
70+
foreach ($this->extensions as $extension => $mime) {
71+
if (strpos($path, $extension) !== false) {
72+
return $mime;
73+
}
74+
}
75+
return 'application/octet-stream';
76+
});
77+
$this->mimeDetector->method('detectContent')
78+
->willReturnCallback(function ($path) {
79+
$body = file_get_contents($path);
80+
foreach ($this->content as $match => $mime) {
81+
if (strpos($body, $match) !== false) {
82+
return $mime;
83+
}
84+
}
85+
return 'application/octet-stream';
86+
});
87+
}
88+
89+
public function testUseCachedMimetype() {
90+
$storage = new Temporary([]);
91+
$storage->mkdir('foo');
92+
$storage->file_put_contents('foo/bar.txt', 'asd');
93+
$storage->getScanner()->scan('');
94+
95+
96+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
97+
$check->setFileInfo($storage, 'foo/bar.txt');
98+
99+
$this->assertTrue($check->executeCheck('is', 'text/plain'));
100+
}
101+
102+
public function testNonCachedNotExists() {
103+
$storage = new Temporary([]);
104+
105+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
106+
$check->setFileInfo($storage, 'foo/bar.txt');
107+
108+
$this->assertTrue($check->executeCheck('is', 'text/plain-path-detected'));
109+
}
110+
111+
public function testNonCachedLocal() {
112+
$storage = new Temporary([]);
113+
$storage->mkdir('foo');
114+
$storage->file_put_contents('foo/bar.txt', 'text-content');
115+
116+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
117+
$check->setFileInfo($storage, 'foo/bar.txt');
118+
119+
$this->assertTrue($check->executeCheck('is', 'text/plain-content-detected'));
120+
}
121+
122+
public function testNonCachedNotLocal() {
123+
$storage = new TemporaryNoLocal([]);
124+
$storage->mkdir('foo');
125+
$storage->file_put_contents('foo/bar.txt', 'text-content');
126+
127+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
128+
$check->setFileInfo($storage, 'foo/bar.txt');
129+
130+
$this->assertTrue($check->executeCheck('is', 'text/plain-path-detected'));
131+
}
132+
133+
public function testFallback() {
134+
$storage = new Temporary([]);
135+
136+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
137+
$check->setFileInfo($storage, 'unknown');
138+
139+
$this->assertTrue($check->executeCheck('is', 'application/octet-stream'));
140+
}
141+
142+
public function testFromCacheCached() {
143+
$storage = new Temporary([]);
144+
$storage->mkdir('foo');
145+
$storage->file_put_contents('foo/bar.txt', 'asd');
146+
$storage->getScanner()->scan('');
147+
148+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
149+
$check->setFileInfo($storage, 'foo/bar.txt');
150+
151+
$this->assertTrue($check->executeCheck('is', 'text/plain'));
152+
153+
$storage->getCache()->clear();
154+
155+
$this->assertTrue($check->executeCheck('is', 'text/plain'));
156+
157+
$newCheck = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
158+
$newCheck->setFileInfo($storage, 'foo/bar.txt');
159+
$this->assertTrue($newCheck->executeCheck('is', 'text/plain-path-detected'));
160+
}
161+
162+
public function testExistsCached() {
163+
$storage = new TemporaryNoLocal([]);
164+
$storage->mkdir('foo');
165+
$storage->file_put_contents('foo/bar.txt', 'text-content');
166+
167+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
168+
$check->setFileInfo($storage, 'foo/bar.txt');
169+
170+
$this->assertTrue($check->executeCheck('is', 'text/plain-content-detected'));
171+
$storage->unlink('foo/bar.txt');
172+
$this->assertTrue($check->executeCheck('is', 'text/plain-content-detected'));
173+
174+
$newCheck = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
175+
$newCheck->setFileInfo($storage, 'foo/bar.txt');
176+
$this->assertTrue($newCheck->executeCheck('is', 'text/plain-path-detected'));
177+
}
178+
179+
public function testNonExistsNotCached() {
180+
$storage = new TemporaryNoLocal([]);
181+
182+
$check = new FileMimeType($this->l10n, $this->request, $this->mimeDetector);
183+
$check->setFileInfo($storage, 'foo/bar.txt');
184+
185+
$this->assertTrue($check->executeCheck('is', 'text/plain-path-detected'));
186+
187+
$storage->mkdir('foo');
188+
$storage->file_put_contents('foo/bar.txt', 'text-content');
189+
190+
$this->assertTrue($check->executeCheck('is', 'text/plain-content-detected'));
191+
}
192+
}

0 commit comments

Comments
 (0)