Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Exclude hidden files from recent recommendations #354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Exclude hidden files from recent recommendations #354

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 49 additions & 13 deletions lib/Service/RecentlyEditedFilesSource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
namespace OCA\Recommendations\Service;

use OCP\Files\Node;
use OCP\Files\NotFoundException;
use OCP\Files\StorageNotAvailableException;
use OCP\IL10N;
use OCP\IServerContainer;
Expand All @@ -45,26 +46,61 @@ public function __construct(IServerContainer $serverContainer,
$this->l10n = $l10n;
}

/**
* @return bool
*/
private function isNodeExcluded(Node $node, bool $showHidden): bool {
$next = $node;

while (!$showHidden) {
if ($next->getName()[0] == ".")
return true;

try {
$next = $next->getParent();
} catch (NotFoundException $e) {
break;
Copy link
Member

@ChristophWurst ChristophWurst Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

won't you run into an infinite loop here?

I think you would be better off with recursion. If the current node is hidden, you return false. If there is a parent you return $this->isNodeExcluded($node->getParent(), $showHidden);. Else you return false.

Copy link
Author

@blkqi blkqi Jan 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking a look!

The sequence ends when a dotfile is found (return true) or the root node is found (break). In other words, it is always bounded by the depth of the node. If there were a clearer way to identify the root node other than by exception handling then I would be more than happy to use it.

While I agree that recursion is generally a more natural approach to tree traversal problems, I don't think anything is lost in this context by iterating. And in the recursive case, we would want to introduce a new function so that isNodeExcluded can be adapted to other types of exclusions related to the target node (name, mimetype, tag, etc.).

}
}

return false;
}

/**
* @return array
*/
public function getMostRecentRecommendation(IUser $user, int $max): array {
$showHidden = (bool) $this->serverContainer->getConfig()->getUserValue($user->getUID(), 'files', 'show_hidden', false);
$userFolder = $this->serverContainer->getUserFolder($user->getUID());

return array_filter(array_map(function (Node $node) use ($userFolder) {
try {
return new RecommendedFile(
$userFolder->getRelativePath($node->getParent()->getPath()),
$node,
$node->getMTime(),
$this->l10n->t("Recently edited")
);
} catch (StorageNotAvailableException $e) {
return null;
$count = 0;
$limit = 2 * $max;
$offset = 0;
$results = [];

do {
$recentFiles = $userFolder->getRecent($limit, $offset);

foreach ($recentFiles as $node) {
if (!$this->isNodeExcluded($node, $showHidden)) {
try {
$results[] = new RecommendedFile(
$userFolder->getRelativePath($node->getParent()->getPath()),
$node,
$node->getMTime(),
$this->l10n->t("Recently edited")
);
} catch (StorageNotAvailableException $e) {
//pass
}
}
}
}, $userFolder->getRecent($max)), function ($entry) {
return $entry !== null;
});

$offset += $limit;
$count++;
} while ((count($results) < $max) && ($count < 5));

return array_slice($results, 0, $max);
}

}