From d7ee09d9f7f960b49e84e98ec33860d291e65ef5 Mon Sep 17 00:00:00 2001 From: Gemorroj Date: Fri, 10 Feb 2023 14:07:44 +0300 Subject: [PATCH 1/4] typos, use str_contains/is_writable and static usage for static methods --- README.md | 6 +++--- src/Lib/FileStorage.php | 18 +++++++++--------- src/Lib/ManticoreBackup.php | 10 +++++----- src/Lib/ManticoreClient.php | 2 +- src/Lib/ManticoreConfig.php | 4 ++-- src/Lib/OS.php | 6 +++--- src/func.php | 6 +++--- test/FileStorageTest.php | 6 +++--- test/ManticoreBackupTest.php | 4 ++-- test/ScriptTest.php | 2 +- 10 files changed, 32 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index c509a0c..e30af10 100644 --- a/README.md +++ b/README.md @@ -12,10 +12,10 @@ Read the [official documentation](https://manual.manticoresearch.com/dev/Securin |-|-| | bin | This directory contains various binaries to execute and also build script | | bin/run | This script is used for run the backup script in development and use it for debug purpose | -| bin/build | It buildes and prepare single binary in PHP Phar format to be ready to ship | +| bin/build | It builds and prepare single binary in PHP Phar format to be ready to ship | | build | This directory is ignored by git but built binary goes there | | src | All sources code goes here | -| src/lib | Library independed components | +| src/lib | Library independent components | | src/lib/func.php | All helper functions that required to use the script are here | | src/main.php | This is the main entrypoint for starting the logic | | test | All tests are here | @@ -39,7 +39,7 @@ The directory with name `backup-%date%` is created in the *--backup-dir* folder ## Building -To build the final executable you need to to run `bin/build`. The executable can be found then in the `./build` directory under `build/manticore-backup`. +To build the final executable you need to run `bin/build`. The executable can be found then in the `./build` directory under `build/manticore-backup`. We recommend using [manticore-executor](https://github.com/manticoresoftware/executor). In this case, the script will use the custom-built PHP binary with all required extensions to run the tool. If you are adding a new functionality which requires a specific PHP module make sure you update [manticore-executor](https://github.com/manticoresoftware/executor) as well. diff --git a/src/Lib/FileStorage.php b/src/Lib/FileStorage.php index 7ad92f8..d473666 100644 --- a/src/Lib/FileStorage.php +++ b/src/Lib/FileStorage.php @@ -48,7 +48,7 @@ public function __construct(?string $backupDir, protected bool $useCompression = * @return string * @throws \RuntimeException */ - public function getBackupDir(): ?string { + public function getBackupDir(): string { if (!isset($this->backupDir)) { throw new \RuntimeException('Backup dir is not initialized.'); } @@ -177,7 +177,7 @@ protected function copyDir(string $from, string $to): bool { } $rootDir = dirname($to); - if (!is_dir($rootDir) || !is_writeable($rootDir)) { + if (!is_dir($rootDir) || !is_writable($rootDir)) { throw new InvalidPathException('Cannot write to backup directory - "' . $rootDir . '"'); } @@ -193,7 +193,7 @@ protected function copyDir(string $from, string $to): bool { if ($file->isDir()) { // Create dir if it does not exist if (!is_dir($destDir)) { - $this->createDir($destDir, $file->getPath()); + $this::createDir($destDir, $file->getPath()); } continue; } @@ -270,7 +270,7 @@ protected function copyFile(string $from, string $to): bool { * Result of the operation */ public function copyPaths(array $paths, string $to, bool $preservePath = false): bool { - if (!is_dir($to) || !is_writeable($to)) { + if (!is_dir($to) || !is_writable($to)) { throw new InvalidPathException('Cannot write to backup directory - "' . $to . '"'); } @@ -284,7 +284,7 @@ public function copyPaths(array $paths, string $to, bool $preservePath = false): if ($preservePath) { $dir = is_file($path) ? dirname($dest) : $dest; if (!is_dir($dir)) { - $this->createDir($dir, dirname($path), true); + $this::createDir($dir, dirname($path), true); } } if (is_file($path)) { @@ -388,7 +388,7 @@ public static function deleteDir(string $dir, bool $removeSelf = true): void { } /** - * Get tmp directory for project related usage primarely in tests + * Get tmp directory for project related usage primarily in tests * @return string * The path to the temporary dir that contains only files created by us */ @@ -414,7 +414,7 @@ public function setBackupPathsUsingDir(string $dir): static { $result = []; $result['root'] = $destination; - // Now lets create additional directories + // Now let's create additional directories foreach (['data', 'config', 'state'] as $dir) { $path = $destination . DIRECTORY_SEPARATOR . $dir; $result[$dir] = $path; @@ -463,7 +463,7 @@ public function getBackupPaths(): array { $result = []; $result['root'] = $destination; - // Now lets create additional directories + // Now let's create additional directories foreach (['data', 'config', 'state'] as $dir) { $path = $destination . DIRECTORY_SEPARATOR . $dir; $result[$dir] = $path; @@ -515,7 +515,7 @@ public function cleanUp(): void { return; } - $this->deleteDir($this->backupPaths['root']); + $this::deleteDir($this->backupPaths['root']); } /** diff --git a/src/Lib/ManticoreBackup.php b/src/Lib/ManticoreBackup.php index 3d88c62..00f8fa1 100644 --- a/src/Lib/ManticoreBackup.php +++ b/src/Lib/ManticoreBackup.php @@ -163,7 +163,7 @@ protected static function store(ManticoreClient $client, FileStorage $storage, a ); $backupPath = $destination['data'] . DIRECTORY_SEPARATOR . $index; - $storage->createDir( + $storage::createDir( $backupPath, $config->dataDir . DIRECTORY_SEPARATOR . $index ); @@ -234,7 +234,7 @@ protected static function restore(FileStorage $storage): void { static::validateRestore($storage, $backup['state']); - // Valdiate indexes + // Validate indexes if (!is_dir($config->dataDir)) { metric('restore_config_dir_missing', 1); throw new \Exception('Failed to find data dir, make sure that it exists: ' . $config->dataDir); @@ -301,7 +301,7 @@ protected static function restoreState(FileStorage $storage, string $path): bool $from = $file->getRealPath(); $to = dirname($storage->getOriginRealPath($from)); if (!is_dir($to)) { - FileStorage::createDir($to, $from); + $storage::createDir($to, $from); } println(LogLevel::Debug, ' ' . $from . ' -> ' . $to); @@ -329,7 +329,7 @@ protected static function restoreData(FileStorage $storage, ManticoreConfig $con $from = $file->getRealPath(); $to = $config->dataDir . DIRECTORY_SEPARATOR . dirname($storage->getOriginRealPath($file->getRealPath())); if (!is_dir($to)) { - $storage->createDir($to, $file->getPath(), true); + $storage::createDir($to, $file->getPath(), true); } println(LogLevel::Debug, ' ' . $from . ' -> ' . $to); @@ -350,7 +350,7 @@ protected static function restoreData(FileStorage $storage, ManticoreConfig $con */ protected static function storeVersions(array $versions, string $backupDir): bool { $filePath = $backupDir . DIRECTORY_SEPARATOR . 'versions.json'; - return !!file_put_contents($filePath, json_encode($versions)); + return (bool)file_put_contents($filePath, json_encode($versions)); } /** diff --git a/src/Lib/ManticoreClient.php b/src/Lib/ManticoreClient.php index 9f5a630..7759428 100644 --- a/src/Lib/ManticoreClient.php +++ b/src/Lib/ManticoreClient.php @@ -135,7 +135,7 @@ public function freeze(array|string $tables): array { } /** - * This method unfreezes the index we fronzen before + * This method unfreezes the index we frozen before * * @param array|string $tables * Name of index to unfreeze or list of tables diff --git a/src/Lib/ManticoreConfig.php b/src/Lib/ManticoreConfig.php index 2a08da3..25a324d 100644 --- a/src/Lib/ManticoreConfig.php +++ b/src/Lib/ManticoreConfig.php @@ -106,7 +106,7 @@ protected function parseHostPort(string $value): void { return; } $listen = substr($value, 0, $httpPos); - if (false === strpos($listen, ':')) { + if (!str_contains($listen, ':')) { $this->port = (int)$listen; } else { $this->host = strtok($listen, ':'); @@ -147,7 +147,7 @@ public function getStatePaths(): array { */ public static function isDataDirValid(string $dataDir): bool { return OS::isWindows() - ? !!preg_match('|^[a-z]\:\\\\|ius', $dataDir) + ? (bool)preg_match('|^[a-z]\:\\\\|ius', $dataDir) : $dataDir[0] === '/' ; } diff --git a/src/Lib/OS.php b/src/Lib/OS.php index 69bafff..262247a 100644 --- a/src/Lib/OS.php +++ b/src/Lib/OS.php @@ -32,11 +32,11 @@ public static function isLinux(): bool { } /** - * Little helper to find the real path to executoable depending on running os + * Little helper to find the real path to executable depending on running os * * @param string $program * @return string - * The path to found executoable + * The path to found executable * @throws \Exception */ public static function which(string $program): string { @@ -57,6 +57,6 @@ public static function which(string $program): string { * @return bool */ public static function isRoot(): bool { - return !OS::isWindows() && function_exists('posix_getuid') && posix_getuid() === 0; + return !static::isWindows() && function_exists('posix_getuid') && posix_getuid() === 0; } } diff --git a/src/func.php b/src/func.php index a14f063..2b3c066 100644 --- a/src/func.php +++ b/src/func.php @@ -44,7 +44,7 @@ function validate_args(array $args): array { if (!isset($args['unlock'])) { if (!isset($options['backup-dir']) || !is_dir($options['backup-dir']) - || !is_writeable($options['backup-dir'])) { + || !is_writable($options['backup-dir'])) { throw new InvalidArgumentException( 'Failed to find backup dir to store backup: ' . ($options['backup-dir'] ?? 'none') ); @@ -61,7 +61,7 @@ function validate_args(array $args): array { } /** - * Little helper to conver bytes to human readable size + * Little helper to convert bytes to human readable size * * @param int $bytes * @param int $precision @@ -107,7 +107,7 @@ function get_input_args(): array { foreach ($argv as $arg) { $arg = strtok($arg, '='); - if (false === strpos($supportedArgs, '!' . $arg . '!')) { + if (!str_contains($supportedArgs, '!'.$arg.'!')) { throw new InvalidArgumentException('Unknown option: ' . $arg); } } diff --git a/test/FileStorageTest.php b/test/FileStorageTest.php index d513663..8578582 100644 --- a/test/FileStorageTest.php +++ b/test/FileStorageTest.php @@ -21,7 +21,7 @@ public function testDirCreated(): void { $storage = new FileStorage($tmpDir, false); $this->assertDirectoryDoesNotExist($dir); - $storage->createDir($dir); + $storage::createDir($dir); $this->assertDirectoryExists($dir); } @@ -33,12 +33,12 @@ public function testCopyPathsWithoutOwnership(): void { ]; $target = $tmpDir . DIRECTORY_SEPARATOR . 'target-path-' . uniqid(); $storage = new FileStorage($tmpDir, false); - $storage->createDir($target); + $storage::createDir($target); $this->expectException(InvalidPathException::class); $storage->copyPaths($paths, $target); - $storage->createDir($paths[0]); + $storage::createDir($paths[0]); $this->expectException(InvalidPathException::class); $storage->copyPaths($paths, $target); diff --git a/test/ManticoreBackupTest.php b/test/ManticoreBackupTest.php index 47045b1..9e75ee0 100644 --- a/test/ManticoreBackupTest.php +++ b/test/ManticoreBackupTest.php @@ -39,7 +39,7 @@ public function testStoreAllTables(): void { public function testStoreAllTablesToSymlinkPath(): void { [$config, $storage, $backupDir] = $this->initTestEnv(); $uniq = uniqid(); - $tmpDir = $storage->getTmpDir(); + $tmpDir = $storage::getTmpDir(); $baseDir = basename($backupDir); $realPath = "$tmpDir-$uniq/first/second/$baseDir"; mkdir($realPath, 0755, true); @@ -215,7 +215,7 @@ public function initTestEnv(): array { * @param array $tables * @return void */ - protected function assertBackupIsOK(ManticoreClient $client, string $backupDir, array $tables) { + protected function assertBackupIsOK(ManticoreClient $client, string $backupDir, array $tables): void { $dirs = glob($backupDir . DIRECTORY_SEPARATOR . '*'); $this->assertIsArray($dirs); diff --git a/test/ScriptTest.php b/test/ScriptTest.php index da6d36d..d0c863b 100644 --- a/test/ScriptTest.php +++ b/test/ScriptTest.php @@ -102,7 +102,7 @@ public function testRestoreArg(): void { } /** - * Helper function to validate that shell comand executed and return output + * Helper function to validate that shell command executed and return output * * @param string $arg * @return string From 3d91c8649a582261ec36a568c205077d321999e3 Mon Sep 17 00:00:00 2001 From: Gemorroj Date: Fri, 10 Feb 2023 14:10:32 +0300 Subject: [PATCH 2/4] use spaces for concatenation --- src/func.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/func.php b/src/func.php index 2b3c066..cbbd09d 100644 --- a/src/func.php +++ b/src/func.php @@ -107,7 +107,7 @@ function get_input_args(): array { foreach ($argv as $arg) { $arg = strtok($arg, '='); - if (!str_contains($supportedArgs, '!'.$arg.'!')) { + if (!str_contains($supportedArgs, '!' . $arg . '!')) { throw new InvalidArgumentException('Unknown option: ' . $arg); } } From 056a271831ff4212f11d1d577db067079809cc96 Mon Sep 17 00:00:00 2001 From: Gemorroj Date: Thu, 23 Feb 2023 14:47:55 +0300 Subject: [PATCH 3/4] fix the review --- src/Lib/FileStorage.php | 6 +++--- src/Lib/ManticoreBackup.php | 6 +++--- test/FileStorageTest.php | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Lib/FileStorage.php b/src/Lib/FileStorage.php index d473666..73d6427 100644 --- a/src/Lib/FileStorage.php +++ b/src/Lib/FileStorage.php @@ -193,7 +193,7 @@ protected function copyDir(string $from, string $to): bool { if ($file->isDir()) { // Create dir if it does not exist if (!is_dir($destDir)) { - $this::createDir($destDir, $file->getPath()); + static::createDir($destDir, $file->getPath()); } continue; } @@ -284,7 +284,7 @@ public function copyPaths(array $paths, string $to, bool $preservePath = false): if ($preservePath) { $dir = is_file($path) ? dirname($dest) : $dest; if (!is_dir($dir)) { - $this::createDir($dir, dirname($path), true); + static::createDir($dir, dirname($path), true); } } if (is_file($path)) { @@ -515,7 +515,7 @@ public function cleanUp(): void { return; } - $this::deleteDir($this->backupPaths['root']); + static::deleteDir($this->backupPaths['root']); } /** diff --git a/src/Lib/ManticoreBackup.php b/src/Lib/ManticoreBackup.php index 00f8fa1..c0dad20 100644 --- a/src/Lib/ManticoreBackup.php +++ b/src/Lib/ManticoreBackup.php @@ -163,7 +163,7 @@ protected static function store(ManticoreClient $client, FileStorage $storage, a ); $backupPath = $destination['data'] . DIRECTORY_SEPARATOR . $index; - $storage::createDir( + FileStorage::createDir( $backupPath, $config->dataDir . DIRECTORY_SEPARATOR . $index ); @@ -301,7 +301,7 @@ protected static function restoreState(FileStorage $storage, string $path): bool $from = $file->getRealPath(); $to = dirname($storage->getOriginRealPath($from)); if (!is_dir($to)) { - $storage::createDir($to, $from); + FileStorage::createDir($to, $from); } println(LogLevel::Debug, ' ' . $from . ' -> ' . $to); @@ -329,7 +329,7 @@ protected static function restoreData(FileStorage $storage, ManticoreConfig $con $from = $file->getRealPath(); $to = $config->dataDir . DIRECTORY_SEPARATOR . dirname($storage->getOriginRealPath($file->getRealPath())); if (!is_dir($to)) { - $storage::createDir($to, $file->getPath(), true); + FileStorage::createDir($to, $file->getPath(), true); } println(LogLevel::Debug, ' ' . $from . ' -> ' . $to); diff --git a/test/FileStorageTest.php b/test/FileStorageTest.php index 8578582..554487a 100644 --- a/test/FileStorageTest.php +++ b/test/FileStorageTest.php @@ -21,7 +21,7 @@ public function testDirCreated(): void { $storage = new FileStorage($tmpDir, false); $this->assertDirectoryDoesNotExist($dir); - $storage::createDir($dir); + FileStorage::createDir($dir); $this->assertDirectoryExists($dir); } @@ -33,12 +33,12 @@ public function testCopyPathsWithoutOwnership(): void { ]; $target = $tmpDir . DIRECTORY_SEPARATOR . 'target-path-' . uniqid(); $storage = new FileStorage($tmpDir, false); - $storage::createDir($target); + FileStorage::createDir($target); $this->expectException(InvalidPathException::class); $storage->copyPaths($paths, $target); - $storage::createDir($paths[0]); + FileStorage::createDir($paths[0]); $this->expectException(InvalidPathException::class); $storage->copyPaths($paths, $target); From 59f3de3fa7a53d9ec65e409185d6393f471ac9e5 Mon Sep 17 00:00:00 2001 From: Gemorroj Date: Thu, 23 Feb 2023 14:59:25 +0300 Subject: [PATCH 4/4] merge with main branch --- README.md | 4 ++-- src/Lib/FileStorage.php | 18 +++++++++--------- src/Lib/OS.php | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 9fbda16..1ddf93c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,7 @@ Read the [official documentation](https://manual.manticoresearch.com/dev/Securin | bin/run | This script is used for run the backup script in development and use it for debug purpose | | build | This directory is ignored by git but built binary goes there | | src | All sources code goes here | -| src/lib | Library independed components | +| src/lib | Library independent components | | src/lib/func.php | All helper functions that required to use the script are here | | src/main.php | This is the main entrypoint for starting the logic | | test | All tests are here | @@ -38,7 +38,7 @@ The directory with name `backup-%date%` is created in the *--backup-dir* folder ## Building -To build the final executable you need to to run `bin/build`. The executable can be found then in the `./build` directory under `build/manticore-backup`. +To build the final executable you need to run `bin/build`. The executable can be found then in the `./build` directory under `build/manticore-backup`. We recommend using [manticore-executor](https://github.com/manticoresoftware/executor). In this case, the script will use the custom-built PHP binary with all required extensions to run the tool. If you are adding a new functionality which requires a specific PHP module make sure you update [manticore-executor](https://github.com/manticoresoftware/executor) as well. diff --git a/src/Lib/FileStorage.php b/src/Lib/FileStorage.php index db3b02c..094eca3 100644 --- a/src/Lib/FileStorage.php +++ b/src/Lib/FileStorage.php @@ -48,7 +48,7 @@ public function __construct(?string $backupDir, protected bool $useCompression = * @return string * @throws \RuntimeException */ - public function getBackupDir(): ?string { + public function getBackupDir(): string { if (!isset($this->backupDir)) { throw new \RuntimeException('Backup dir is not initialized.'); } @@ -177,7 +177,7 @@ protected function copyDir(string $from, string $to): bool { } $rootDir = dirname($to); - if (!is_dir($rootDir) || !is_writeable($rootDir)) { + if (!is_dir($rootDir) || !is_writable($rootDir)) { throw new InvalidPathException('Cannot write to backup directory - "' . $rootDir . '"'); } @@ -191,7 +191,7 @@ protected function copyDir(string $from, string $to): bool { // Create dir if it does not exist if (!is_dir($destDir)) { - $this->createDir($destDir, $file->getPath(), true); + static::createDir($destDir, $file->getPath(), true); } // Skip directories @@ -271,7 +271,7 @@ protected function copyFile(string $from, string $to): bool { * Result of the operation */ public function copyPaths(array $paths, string $to, bool $preservePath = false): bool { - if (!is_dir($to) || !is_writeable($to)) { + if (!is_dir($to) || !is_writable($to)) { throw new InvalidPathException('Cannot write to backup directory - "' . $to . '"'); } @@ -285,7 +285,7 @@ public function copyPaths(array $paths, string $to, bool $preservePath = false): if ($preservePath) { $dir = is_file($path) ? dirname($dest) : $dest; if (!is_dir($dir)) { - $this->createDir($dir, dirname($path), true); + static::createDir($dir, dirname($path), true); } } if (is_file($path)) { @@ -389,7 +389,7 @@ public static function deleteDir(string $dir, bool $removeSelf = true): void { } /** - * Get tmp directory for project related usage primarely in tests + * Get tmp directory for project related usage primarily in tests * @return string * The path to the temporary dir that contains only files created by us */ @@ -415,7 +415,7 @@ public function setBackupPathsUsingDir(string $dir): static { $result = []; $result['root'] = $destination; - // Now lets create additional directories + // Now let's create additional directories foreach (['data', 'config', 'state'] as $dir) { $path = $destination . DIRECTORY_SEPARATOR . $dir; $result[$dir] = $path; @@ -464,7 +464,7 @@ public function getBackupPaths(): array { $result = []; $result['root'] = $destination; - // Now lets create additional directories + // Now let's create additional directories foreach (['data', 'config', 'state'] as $dir) { $path = $destination . DIRECTORY_SEPARATOR . $dir; $result[$dir] = $path; @@ -516,7 +516,7 @@ public function cleanUp(): void { return; } - $this->deleteDir($this->backupPaths['root']); + static::deleteDir($this->backupPaths['root']); } /** diff --git a/src/Lib/OS.php b/src/Lib/OS.php index 95d8304..7b152fc 100644 --- a/src/Lib/OS.php +++ b/src/Lib/OS.php @@ -32,11 +32,11 @@ public static function isLinux(): bool { } /** - * Little helper to find the real path to executoable depending on running os + * Little helper to find the real path to executable depending on running os * * @param string $program * @return string - * The path to found executoable + * The path to found executable * @throws \Exception */ public static function which(string $program): string {