Skip to content

Commit

Permalink
Remove inefficient fed share scanner
Browse files Browse the repository at this point in the history
Remove scanAll which relies on the "shareinfo" endpoint that returns the
full cache tree.
The latter can become big for big shares and result in timeouts.
Furthermode, the full tree would be retrieved again for each and every
detected change which can become expensive quickly.

Signed-off-by: Vincent Petry <[email protected]>
  • Loading branch information
PVince81 committed Jan 10, 2022
1 parent 91c7efa commit ce31914
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 82 deletions.
70 changes: 0 additions & 70 deletions apps/files_sharing/lib/External/Scanner.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,11 @@
use OCP\Files\NotFoundException;
use OCP\Files\StorageInvalidException;
use OCP\Files\StorageNotAvailableException;
use OCP\Http\Client\LocalServerException;
use Psr\Log\LoggerInterface;

class Scanner extends \OC\Files\Cache\Scanner {
/** @var \OCA\Files_Sharing\External\Storage */
protected $storage;

/** {@inheritDoc} */
public function scan($path, $recursive = self::SCAN_RECURSIVE, $reuse = -1, $lock = true) {
try {
if (!$this->storage->remoteIsOwnCloud()) {
return parent::scan($path, $recursive, $reuse, $lock);
}
} catch (LocalServerException $e) {
// Scanner doesn't have dependency injection
\OC::$server->get(LoggerInterface::class)
->warning('Trying to scan files inside invalid external storage: ' . $this->storage->getRemote() . ' for mountpoint ' . $this->storage->getMountPoint() . ' and id ' . $this->storage->getId());
return;
}

$this->scanAll();
}

/**
* Scan a single file and store it in the cache.
* If an exception happened while accessing the external storage,
Expand Down Expand Up @@ -81,56 +63,4 @@ public function scanFile($file, $reuseExisting = 0, $parentId = -1, $cacheData =
$this->storage->checkStorageAvailability();
}
}

/**
* Checks the remote share for changes.
* If changes are available, scan them and update
* the cache.
* @throws NotFoundException
* @throws StorageInvalidException
* @throws \Exception
*/
public function scanAll() {
try {
$data = $this->storage->getShareInfo();
} catch (\Exception $e) {
$this->storage->checkStorageAvailability();
throw new \Exception(
'Error while scanning remote share: "' .
$this->storage->getRemote() . '" ' .
$e->getMessage()
);
}
if ($data['status'] === 'success') {
$this->addResult($data['data'], '');
} else {
throw new \Exception(
'Error while scanning remote share: "' .
$this->storage->getRemote() . '"'
);
}
}

/**
* @param array $data
* @param string $path
*/
private function addResult($data, $path) {
$id = $this->cache->put($path, $data);
if (isset($data['children'])) {
$children = [];
foreach ($data['children'] as $child) {
$children[$child['name']] = true;
$this->addResult($child, ltrim($path . '/' . $child['name'], '/'));
}

$existingCache = $this->cache->getFolderContentsById($id);
foreach ($existingCache as $existingChild) {
// if an existing child is not in the new data, remove it
if (!isset($children[$existingChild['name']])) {
$this->cache->remove(ltrim($path . '/' . $existingChild['name'], '/'));
}
}
}
}
}
12 changes: 0 additions & 12 deletions apps/files_sharing/tests/External/ScannerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,6 @@ protected function setUp(): void {
$this->scanner = new Scanner($this->storage);
}

public function testScanAll() {
$this->storage->expects($this->any())
->method('getShareInfo')
->willReturn(['status' => 'success', 'data' => []]);

// FIXME add real tests, we are currently only checking for
// Declaration of OCA\Files_Sharing\External\Scanner::*() should be
// compatible with OC\Files\Cache\Scanner::*()
$this->scanner->scanAll();
$this->addToAssertionCount(1);
}

public function testScan() {
$this->storage->expects($this->any())
->method('getShareInfo')
Expand Down

0 comments on commit ce31914

Please sign in to comment.