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

Refactor lib/private #39245

Closed
wants to merge 3 commits into from
Closed

Refactor lib/private #39245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fcb2c1b
Refactor lib/private
shdehnavi Jul 8, 2023
0251187
Make adjustments based on the Psalm code scanning
shdehnavi Jul 8, 2023
aafc51a
Make adjustments based on the Psalm code scanning
shdehnavi Jul 8, 2023
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
20 changes: 8 additions & 12 deletions lib/private/TagManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,30 +42,26 @@
* @template-implements IEventListener<UserDeletedEvent>
*/
class TagManager implements ITagManager, IEventListener {
private TagMapper $mapper;
private IUserSession $userSession;
private IDBConnection $connection;
private LoggerInterface $logger;

public function __construct(TagMapper $mapper, IUserSession $userSession, IDBConnection $connection, LoggerInterface $logger) {
$this->mapper = $mapper;
$this->userSession = $userSession;
$this->connection = $connection;
$this->logger = $logger;
public function __construct(
private TagMapper $mapper,
private IUserSession $userSession,
private IDBConnection $connection,
private LoggerInterface $logger,
) {
}

/**
* Create a new \OCP\ITags instance and load tags from db.
*
* @see \OCP\ITags
* @param string $type The type identifier e.g. 'contact' or 'event'.
* @param array $defaultTags An array of default tags to be used if none are stored.
* @param boolean $includeShared Whether to include tags for items shared with this user by others.
* @param string $userId user for which to retrieve the tags, defaults to the currently
* @param string|null $userId user for which to retrieve the tags, defaults to the currently
* logged in user
* @return \OCP\ITags
*
* since 20.0.0 $includeShared isn't used anymore
* @see \OCP\ITags
*/
public function load($type, $defaultTags = [], $includeShared = false, $userId = null) {
if (is_null($userId)) {
Expand Down
59 changes: 26 additions & 33 deletions lib/private/Tags.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,6 @@ class Tags implements ITags {
* Used for storing objectid/categoryname pairs while rescanning.
*/
private static array $relations = [];
private string $type;
private string $user;
private IDBConnection $db;
private LoggerInterface $logger;
private array $tags = [];

/**
Expand All @@ -62,11 +58,6 @@ class Tags implements ITags {
*/
private array $owners = [];

/**
* The Mapper we are using to communicate our Tag objects to the database.
*/
private TagMapper $mapper;

/**
* The sharing backend for objects of $this->type. Required if
* $this->includeShared === true to determine ownership of items.
Expand All @@ -86,14 +77,16 @@ class Tags implements ITags {
*
* since 20.0.0 $includeShared isn't used anymore
*/
public function __construct(TagMapper $mapper, string $user, string $type, LoggerInterface $logger, IDBConnection $connection, array $defaultTags = []) {
$this->mapper = $mapper;
$this->user = $user;
$this->type = $type;
public function __construct(
private TagMapper $mapper,
private string $user,
private string $type,
private LoggerInterface $logger,
private IDBConnection $db,
array $defaultTags = [],
) {
$this->owners = [$this->user];
$this->tags = $this->mapper->loadTags($this->owners, $this->type);
$this->db = $connection;
$this->logger = $logger;

if (count($defaultTags) > 0 && count($this->tags) === 0) {
$this->addMultiple($defaultTags, true);
Expand All @@ -116,7 +109,7 @@ public function isEmpty(): bool {
* @param string $id The ID of the tag that is going to be mapped
* @return array|false
*/
public function getTag(string $id) {
public function getTag(string $id): bool|array {
$key = $this->getTagById($id);
if ($key !== false) {
return $this->tagMap($this->tags[$key]);
Expand Down Expand Up @@ -175,7 +168,7 @@ function ($tag) use ($user) {
* @return array|false of tags id as key to array of tag names
* or false if an error occurred
*/
public function getTagsForObjects(array $objIds) {
public function getTagsForObjects(array $objIds): bool|array {
$entries = [];

try {
Expand Down Expand Up @@ -213,15 +206,15 @@ public function getTagsForObjects(array $objIds) {
}

/**
* Get the a list if items tagged with $tag.
* Get the list if items tagged with $tag.
*
* Throws an exception if the tag could not be found.
*
* @param string $tag Tag id or name.
* @return int[]|false An array of object ids or false on error.
* @throws \Exception
*/
public function getIdsForTag($tag) {
public function getIdsForTag($tag): array|bool {
$tagId = false;
if (is_numeric($tag)) {
$tagId = $tag;
Expand Down Expand Up @@ -291,7 +284,7 @@ public function hasTag(string $name): bool {
* @param string $name A string with a name of the tag
* @return false|int the id of the added tag or false on error.
*/
public function add(string $name) {
public function add(string $name): bool|int {
$name = trim($name);

if ($name === '') {
Expand Down Expand Up @@ -478,7 +471,7 @@ public function purgeObjects(array $ids): bool {
*
* @return array|false An array of object ids.
*/
public function getFavorites() {
public function getFavorites(): array|bool {
if (!$this->userHasTag(ITags::TAG_FAVORITE, $this->user)) {
return [];
}
Expand All @@ -501,7 +494,7 @@ public function getFavorites() {
* @param int $objid The id of the object
* @return boolean
*/
public function addToFavorites($objid) {
public function addToFavorites($objid): bool {
if (!$this->userHasTag(ITags::TAG_FAVORITE, $this->user)) {
$this->add(ITags::TAG_FAVORITE);
}
Expand All @@ -514,7 +507,7 @@ public function addToFavorites($objid) {
* @param int $objid The id of the object
* @return boolean
*/
public function removeFromFavorites($objid) {
public function removeFromFavorites($objid): bool {
return $this->unTag($objid, ITags::TAG_FAVORITE);
}

Expand All @@ -525,7 +518,7 @@ public function removeFromFavorites($objid) {
* @param string $tag The id or name of the tag
* @return boolean Returns false on error.
*/
public function tagAs($objid, $tag) {
public function tagAs($objid, $tag): bool {
if (is_string($tag) && !is_numeric($tag)) {
$tag = trim($tag);
if ($tag === '') {
Expand Down Expand Up @@ -565,7 +558,7 @@ public function tagAs($objid, $tag) {
* @param string $tag The id or name of the tag
* @return boolean
*/
public function unTag($objid, $tag) {
public function unTag($objid, $tag): bool {
if (is_string($tag) && !is_numeric($tag)) {
$tag = trim($tag);
if ($tag === '') {
Expand Down Expand Up @@ -601,7 +594,7 @@ public function unTag($objid, $tag) {
* @param string[]|integer[] $names An array of tags (names or IDs) to delete
* @return bool Returns false on error
*/
public function delete($names) {
public function delete($names): bool {
if (!is_array($names)) {
$names = [$names];
}
Expand Down Expand Up @@ -647,7 +640,7 @@ public function delete($names) {
}

// case-insensitive array_search
protected function array_searchi($needle, $haystack, $mem = 'getName') {
protected function array_searchi($needle, $haystack, $mem = 'getName'): bool|int|string {
if (!is_array($haystack)) {
return false;
}
Expand All @@ -664,7 +657,7 @@ function ($tag) use ($mem) {
* @param string $name The tag name to look for.
* @return string|bool The tag's id or false if no matching tag is found.
*/
private function getTagId($name) {
private function getTagId(string $name): bool|string {
$key = $this->array_searchi($name, $this->tags);
if ($key !== false) {
return $this->tags[$key]->getId();
Expand All @@ -676,21 +669,21 @@ private function getTagId($name) {
* Get a tag by its name.
*
* @param string $name The tag name.
* @return integer|bool The tag object's offset within the $this->tags
* @return bool|int|string The tag object's offset within the $this->tags
* array or false if it doesn't exist.
*/
private function getTagByName($name) {
private function getTagByName(string $name): bool|int|string {
return $this->array_searchi($name, $this->tags, 'getName');
}

/**
* Get a tag by its ID.
*
* @param string $id The tag ID to look for.
* @return integer|bool The tag object's offset within the $this->tags
* @return bool|int|string The tag object's offset within the $this->tags
* array or false if it doesn't exist.
*/
private function getTagById($id) {
private function getTagById(string $id): bool|int|string {
return $this->array_searchi($id, $this->tags, 'getId');
}

Expand All @@ -701,7 +694,7 @@ private function getTagById($id) {
* @param Tag $tag The tag that is going to be mapped
* @return array
*/
private function tagMap(Tag $tag) {
private function tagMap(Tag $tag): array {
return [
'id' => $tag->getId(),
'name' => $tag->getName(),
Expand Down
37 changes: 16 additions & 21 deletions lib/private/TempManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,18 @@

class TempManager implements ITempManager {
/** @var string[] Current temporary files and folders, used for cleanup */
protected $current = [];
protected array $current = [];
/** @var string i.e. /tmp on linux systems */
protected $tmpBaseDir;
/** @var LoggerInterface */
protected $log;
/** @var IConfig */
protected $config;
/** @var IniGetWrapper */
protected $iniGetWrapper;
protected string $tmpBaseDir;

/** Prefix */
public const TMP_PREFIX = 'oc_tmp_';

public function __construct(LoggerInterface $logger, IConfig $config, IniGetWrapper $iniGetWrapper) {
$this->log = $logger;
$this->config = $config;
$this->iniGetWrapper = $iniGetWrapper;
public function __construct(
protected LoggerInterface $log,
protected IConfig $config,
protected IniGetWrapper $iniGetWrapper,
) {
$this->tmpBaseDir = $this->getTempBaseDir();
}

Expand All @@ -66,7 +61,7 @@ public function __construct(LoggerInterface $logger, IConfig $config, IniGetWrap
* @param string $postFix Postfix appended to the temporary file name, may be user controlled
* @return string
*/
private function buildFileNameWithSuffix($absolutePath, $postFix = '') {
private function buildFileNameWithSuffix(string $absolutePath, string $postFix = ''): string {
if ($postFix !== '') {
$postFix = '.' . ltrim($postFix, '.');
$postFix = str_replace(['\\', '/'], '', $postFix);
Expand All @@ -84,7 +79,7 @@ private function buildFileNameWithSuffix($absolutePath, $postFix = '') {
*/
public function getTemporaryFile($postFix = '') {
if (is_writable($this->tmpBaseDir)) {
// To create an unique file and prevent the risk of race conditions
// To create a unique file and prevent the risk of race conditions
// or duplicated temporary files by other means such as collisions
// we need to create the file using `tempnam` and append a possible
// postfix to it later
Expand Down Expand Up @@ -148,14 +143,14 @@ public function getTemporaryFolder($postFix = '') {
/**
* Remove the temporary files and folders generated during this request
*/
public function clean() {
public function clean(): void {
$this->cleanFiles($this->current);
}

/**
* @param string[] $files
*/
protected function cleanFiles($files) {
protected function cleanFiles(array $files): void {
foreach ($files as $file) {
if (file_exists($file)) {
try {
Expand All @@ -176,7 +171,7 @@ protected function cleanFiles($files) {
/**
* Remove old temporary files and folders that were failed to be cleaned
*/
public function cleanOld() {
public function cleanOld(): void {
$this->cleanFiles($this->getOldFiles());
}

Expand All @@ -185,7 +180,7 @@ public function cleanOld() {
*
* @return string[]
*/
protected function getOldFiles() {
protected function getOldFiles(): array {
$cutOfTime = time() - 3600;
$files = [];
$dh = opendir($this->tmpBaseDir);
Expand All @@ -209,7 +204,7 @@ protected function getOldFiles() {
* @return string Path to the temporary directory or null
* @throws \UnexpectedValueException
*/
public function getTempBaseDir() {
public function getTempBaseDir(): string {
if ($this->tmpBaseDir) {
return $this->tmpBaseDir;
}
Expand Down Expand Up @@ -254,7 +249,7 @@ public function getTempBaseDir() {
* @param mixed $directory
* @return bool
*/
private function checkTemporaryDirectory($directory) {
private function checkTemporaryDirectory(mixed $directory): bool {
// suppress any possible errors caused by is_writable
// checks missing or invalid path or characters, wrong permissions etc
try {
Expand All @@ -274,7 +269,7 @@ private function checkTemporaryDirectory($directory) {
*
* @param string $directory
*/
public function overrideTempBaseDir($directory) {
public function overrideTempBaseDir(string $directory): void {
$this->tmpBaseDir = $directory;
}
}
Loading
Loading