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

Refactor lib/private #39249

Closed
wants to merge 3 commits into from
+140 −140
Closed

Refactor lib/private #39249

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
667fecd
Refactor lib/private
shdehnavi Jul 9, 2023
f837b57
Make adjustments based on the Psalm code scanning
shdehnavi Jul 9, 2023
98ef21f
Merge branch 'master' into refactor_lib_private_root_classes_part_3
skjnldsv May 2, 2024
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
15 changes: 8 additions & 7 deletions lib/private/PreviewManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private function getGenerator(): Generator {
* @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
* @since 11.0.0 - \InvalidArgumentException was added in 12.0.0
*/
public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null) {
public function getPreview(File $file, $width = -1, $height = -1, $crop = false, $mode = IPreview::MODE_FILL, $mimeType = null): ISimpleFile {
$previewConcurrency = $this->getGenerator()->getNumConcurrentPreviews('preview_concurrency_all');
$sem = Generator::guardWithSemaphore(Generator::SEMAPHORE_ID_ALL, $previewConcurrency);
try {
Expand All @@ -203,7 +203,7 @@ public function getPreview(File $file, $width = -1, $height = -1, $crop = false,
* @throws \InvalidArgumentException if the preview would be invalid (in case the original image is invalid)
* @since 19.0.0
*/
public function generatePreviews(File $file, array $specifications, $mimeType = null) {
public function generatePreviews(File $file, array $specifications, $mimeType = null): ISimpleFile {
return $this->getGenerator()->generatePreviews($file, $specifications, $mimeType);
}

Expand All @@ -213,7 +213,7 @@ public function generatePreviews(File $file, array $specifications, $mimeType =
* @param string $mimeType
* @return boolean
*/
public function isMimeSupported($mimeType = '*') {
public function isMimeSupported($mimeType = '*'): bool {
if (!$this->config->getSystemValueBool('enable_previews', true)) {
return false;
}
Expand Down Expand Up @@ -299,9 +299,9 @@ public function isAvailable(\OCP\Files\FileInfo $file): bool {
* - OC\Preview\SVG
* - OC\Preview\TIFF
*
* @return array
* @return array|null
*/
protected function getEnabledDefaultProvider() {
protected function getEnabledDefaultProvider(): ?array {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
protected function getEnabledDefaultProvider(): ?array {
protected function getEnabledDefaultProvider(): array {

I do not see where it returns null?
Docblock @return should be reverted to array as well or removed.

if ($this->defaultProviders !== null) {
return $this->defaultProviders;
}
Expand Down Expand Up @@ -335,8 +335,9 @@ protected function getEnabledDefaultProvider() {
*
* @param string $class
* @param string $mimeType
* @param array $options
*/
protected function registerCoreProvider($class, $mimeType, $options = []) {
protected function registerCoreProvider(string $class, string $mimeType, array $options = []): void {
if (in_array(trim($class, '\\'), $this->getEnabledDefaultProvider())) {
$this->registerProvider($mimeType, function () use ($class, $options) {
return new $class($options);
Expand All @@ -347,7 +348,7 @@ protected function registerCoreProvider($class, $mimeType, $options = []) {
/**
* Register the default providers (if enabled)
*/
protected function registerCoreProviders() {
protected function registerCoreProviders(): void {
if ($this->registeredCoreProviders) {
return;
}
Expand Down
25 changes: 14 additions & 11 deletions lib/private/RedisFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,23 @@ class RedisFactory {
public const REDIS_EXTRA_PARAMETERS_MINIMAL_VERSION = '5.3.0';

/** @var \Redis|\RedisCluster */
private $instance;

private SystemConfig $config;

private IEventLogger $eventLogger;
private \Redis|\RedisCluster $instance;
Copy link
Contributor

Choose a reason for hiding this comment

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

This cannot work because the property is not instanciated in constructor.
Most likely should be changed to:

Suggested change
private \Redis|\RedisCluster $instance;
private \Redis|\RedisCluster|null $instance;

And then the class should be adapted so that psalm is happy. The create method is used for creating the object.
The instanceof test in getInstance looks wrong as it does not test cluster.


/**
* RedisFactory constructor.
*
* @param SystemConfig $config
*/
public function __construct(SystemConfig $config, IEventLogger $eventLogger) {
$this->config = $config;
$this->eventLogger = $eventLogger;
public function __construct(
private SystemConfig $config,
private IEventLogger $eventLogger,
) {
}

private function create() {
/**
* @throws \Exception
*/
private function create(): void {
$isCluster = in_array('redis.cluster', $this->config->getKeys(), true);
$config = $isCluster
? $this->config->getValue('redis.cluster', [])
Expand Down Expand Up @@ -137,7 +137,7 @@ private function create() {
* @return array|null
* @throws \UnexpectedValueException
*/
private function getSslContext($config) {
private function getSslContext(array $config): ?array {
if (isset($config['ssl_context'])) {
if (!$this->isConnectionParametersSupported()) {
throw new \UnexpectedValueException(\sprintf(
Expand All @@ -150,7 +150,10 @@ private function getSslContext($config) {
return null;
}

public function getInstance() {
/**
* @throws \Exception
*/
public function getInstance(): \RedisCluster|\Redis {
if (!$this->isAvailable()) {
throw new \Exception('Redis support is not available');
}
Expand Down
23 changes: 10 additions & 13 deletions lib/private/Repair.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public function setRepairSteps(array $repairSteps): void {
/**
* Run a series of repair steps for common problems
*/
public function run() {
public function run(): void {
if (count($this->repairSteps) === 0) {
$this->dispatcher->dispatchTyped(new RepairInfoEvent('No repair steps available'));

Expand All @@ -132,10 +132,10 @@ public function run() {
/**
* Add repair step
*
* @param IRepairStep|string $repairStep repair step
* @param string|IRepairStep $repairStep repair step
* @throws \Exception
*/
public function addStep($repairStep) {
public function addStep(string|IRepairStep $repairStep): void {
if (is_string($repairStep)) {
try {
$s = \OC::$server->get($repairStep);
Expand Down Expand Up @@ -220,7 +220,7 @@ public static function getRepairSteps(): array {
*
* @return IRepairStep[]
*/
public static function getExpensiveRepairSteps() {
public static function getExpensiveRepairSteps(): array {
return [
new OldGroupMembershipShares(\OC::$server->getDatabaseConnection(), \OC::$server->getGroupManager()),
\OC::$server->get(ValidatePhoneNumber::class),
Expand All @@ -233,7 +233,7 @@ public static function getExpensiveRepairSteps() {
*
* @return IRepairStep[]
*/
public static function getBeforeUpgradeRepairSteps() {
public static function getBeforeUpgradeRepairSteps(): array {
/** @var ConnectionAdapter $connectionAdapter */
$connectionAdapter = \OC::$server->get(ConnectionAdapter::class);
$config = \OC::$server->getConfig();
Expand All @@ -252,23 +252,23 @@ public function debug(string $message): void {
/**
* @param string $message
*/
public function info($message) {
public function info($message): void {
// for now just emit as we did in the past
$this->dispatcher->dispatchTyped(new RepairInfoEvent($message));
}

/**
* @param string $message
*/
public function warning($message) {
public function warning($message): void {
// for now just emit as we did in the past
$this->dispatcher->dispatchTyped(new RepairWarningEvent($message));
}

/**
* @param int $max
*/
public function startProgress($max = 0) {
public function startProgress($max = 0): void {
// for now just emit as we did in the past
$this->dispatcher->dispatchTyped(new RepairStartEvent($max, $this->currentStep));
}
Expand All @@ -277,15 +277,12 @@ public function startProgress($max = 0) {
* @param int $step number of step to advance
* @param string $description
*/
public function advance($step = 1, $description = '') {
public function advance($step = 1, $description = ''): void {
// for now just emit as we did in the past
$this->dispatcher->dispatchTyped(new RepairAdvanceEvent($step, $description));
}

/**
* @param int $max
*/
public function finishProgress() {
public function finishProgress(): void {
// for now just emit as we did in the past
$this->dispatcher->dispatchTyped(new RepairFinishEvent());
}
Expand Down
14 changes: 7 additions & 7 deletions lib/private/Search.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
*/
class Search implements ISearch {
/** @var Provider[] */
private $providers = [];
private $registeredProviders = [];
private array $providers = [];
private array $registeredProviders = [];

/**
* Search all providers for $query
Expand All @@ -48,7 +48,7 @@ class Search implements ISearch {
* @param int $size, 0 = all
* @return array An array of OC\Search\Result's
*/
public function searchPaged($query, array $inApps = [], $page = 1, $size = 30) {
public function searchPaged($query, array $inApps = [], $page = 1, $size = 30): array {
$this->initProviders();
$results = [];
foreach ($this->providers as $provider) {
Expand All @@ -75,7 +75,7 @@ public function searchPaged($query, array $inApps = [], $page = 1, $size = 30) {
/**
* Remove all registered search providers
*/
public function clearProviders() {
public function clearProviders(): void {
$this->providers = [];
$this->registeredProviders = [];
}
Expand All @@ -84,7 +84,7 @@ public function clearProviders() {
* Remove one existing search provider
* @param string $provider class name of a OC\Search\Provider
*/
public function removeProvider($provider) {
public function removeProvider($provider): void {
$this->registeredProviders = array_filter(
$this->registeredProviders,
function ($element) use ($provider) {
Expand All @@ -100,14 +100,14 @@ function ($element) use ($provider) {
* @param string $class class name of a OC\Search\Provider
* @param array $options optional
*/
public function registerProvider($class, array $options = []) {
public function registerProvider($class, array $options = []): void {
$this->registeredProviders[] = ['class' => $class, 'options' => $options];
}

/**
* Create instances of all the registered search providers
*/
private function initProviders() {
private function initProviders(): void {
if (! empty($this->providers)) {
return;
}
Expand Down
Loading
Loading