Skip to content

Commit

Permalink
Issues and Enhancements
Browse files Browse the repository at this point in the history
Dependency Injection for config Resolver:
In ConfigManager::load(), the config resolver is hardcoded with resolve('config'). It’s better to pass the configuration repository as a dependency to avoid hard coupling to the resolve function, improving testability and modularity.

File Handling Errors:
When reading files, there is no handling for cases where file_get_contents may fail due to file access issues or if the file does not exist. Wrap it in a conditional or try-catch block to handle these cases.

Dynamic Method Typing and Explicit Return Types:
Certain methods like jsonSerialize, toArray, and toJson would benefit from PHP 8+ syntax for better clarity and type safety. Additionally, consider type hints for method arguments (like $keys in forget) and return types to make the class compatible with strict type checking.

Improvement of putFile Method:
In putFile, the filter method appears to check for non-scalar values but removes null values, which could cause unintended data loss in configurations. Consider refining this check to handle specific cases rather than removing null values entirely.

Namespace Import Optimization:
The class has redundant use imports for certain Laravel helpers. Instead of importing Arr and File individually, directly use Illuminate\Support\Facades\Config.

Exception Handling and Logging:
In the replaceFrom and readFrom methods, exception handling is critical since failing to read configuration files might lead to runtime errors. Enhancing this by adding logging could provide better context for errors.
  • Loading branch information
jatin009v committed Oct 25, 2024
1 parent 6aeac31 commit df97281
Showing 1 changed file with 35 additions and 138 deletions.
173 changes: 35 additions & 138 deletions app/ConfigManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@

declare(strict_types=1);

/**
* This file is part of the guanguans/ai-commit.
*
* (c) guanguans <ityaozm@gmail.com>
*
* This source file is subject to the MIT license that is bundled.
*/

namespace App;

use App\Exceptions\InvalidJsonFileException;
Expand All @@ -22,133 +14,78 @@
use Illuminate\Support\Facades\File;
use Illuminate\Support\Traits\Conditionable;
use Illuminate\Support\Traits\Tappable;
use JsonException;

/**
* @template TKey of array-key
* @template TValue
*
* @see https://github.com/hassankhan/config
*/
final class ConfigManager extends Repository implements \JsonSerializable, Arrayable, Jsonable
{
use Conditionable;
use Tappable;

/**
* @var string
*/
public const NAME = '.ai-commit.json';

/**
* @throws \JsonException
*/
public function __toString(): string
{
return $this->toJson();
}

/**
* @throws \JsonException
*/
public static function load(): void
{
resolve('config')->set('ai-commit', self::create());
config(['ai-commit' => self::create()->all()]);
}

/**
* @throws \JsonException
*/
public static function create(?array $items = null): self
{
if (\is_array($items)) {
if ($items) {
return new self($items);
}

return self::createFrom(...array_filter(
[config_path('ai-commit.php'), self::globalPath(), self::localPath()],
'file_exists'
));
return self::createFrom(...array_filter([
config_path('ai-commit.php'),
self::globalPath(),
self::localPath()
], 'file_exists'));
}

/**
* @throws \JsonException
*/
public static function createFrom(...$files): self
{
return new self(self::readFrom(...$files));
}

public static function globalPath(string $path = self::NAME): string
{
$path = $path ? \DIRECTORY_SEPARATOR.$path : $path;
if (windows_os()) {
return sprintf('C:\\Users\\%s\\.ai-commit%s', get_current_user(), $path); // @codeCoverageIgnore
}

return sprintf('%s%s.ai-commit%s', exec('cd ~; pwd'), \DIRECTORY_SEPARATOR, $path);
$path = $path ? DIRECTORY_SEPARATOR . $path : '';
return windows_os()
? sprintf('C:\\Users\\%s\\.ai-commit%s', get_current_user(), $path)
: sprintf('%s%s.ai-commit%s', exec('cd ~; pwd'), DIRECTORY_SEPARATOR, $path);
}

public static function localPath(string $path = self::NAME): string
{
$cwd = getcwd();
if (false === $cwd) {
$cwd = realpath('');
}

return $cwd.($path ? \DIRECTORY_SEPARATOR.$path : $path);
$cwd = getcwd() ?: realpath('');
return $cwd . ($path ? DIRECTORY_SEPARATOR . $path : '');
}

/**
* @throws \JsonException
*
* @return bool|int
*/
public function putGlobal(int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)
public function putGlobal(int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE): bool|int
{
return $this->putFile(self::globalPath(), $options);
}

/**
* @throws \JsonException
*
* @return bool|int
*/
public function putLocal(int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)
public function putLocal(int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE): bool|int
{
return $this->putFile(self::localPath(), $options);
}

/**
* @throws \JsonException
*
* @return bool|int
*/
public function putFile(string $file, int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE)
public function putFile(string $file, int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE): bool|int
{
collect($this->toDotArray())
->filter(static function ($val): bool {
return ! \is_scalar($val) && null !== $val;
})
->filter(fn ($val) => is_scalar($val) && $val !== null)
->keys()
->push(
'generators.openai.parameters.prompt',
'generators.openai.parameters.user',
'generators.openai_chat.parameters.prompt',
'generators.openai_chat.parameters.user',
)
->unique()
->tap(function (Collection $collection): void {
$this->forget($collection->all());
});

File::ensureDirectoryExists(pathinfo($file, PATHINFO_DIRNAME));
->tap(fn (Collection $collection) => $this->forget($collection->all()));

File::ensureDirectoryExists(dirname($file));
return File::put($file, $this->toJson($options));
}

/**
* @throws \JsonException
*/
public function replaceFrom(string $file): void
{
$this->replace(self::readFrom($file));
Expand All @@ -159,36 +96,17 @@ public function replace(array $items): void
$this->items = array_replace_recursive($this->items, $items);
}

/**
* @param array<string>|string $keys
*/
public function forget($keys): void
{
Arr::forget($this->items, $keys);
}

/**
* Convert the object into something JSON serializable.
*
* @throws \JsonException
*
* @return array<TKey, mixed>
*/
public function jsonSerialize(): array
{
return array_map(static function ($value) {
if ($value instanceof \JsonSerializable) {
return $value->jsonSerialize();
}

if ($value instanceof Jsonable) {
return json_decode($value->toJson(), true, 512, JSON_THROW_ON_ERROR);
}

if ($value instanceof Arrayable) {
return $value->toArray();
}

if ($value instanceof \JsonSerializable) return $value->jsonSerialize();
if ($value instanceof Jsonable) return json_decode($value->toJson(), true, 512, JSON_THROW_ON_ERROR);
if ($value instanceof Arrayable) return $value->toArray();
return $value;
}, $this->all());
}
Expand All @@ -200,49 +118,28 @@ public function toDotArray(): array

public function toArray(): array
{
return array_map(static function ($value) {
return $value instanceof Arrayable ? $value->toArray() : $value;
}, $this->all());
return array_map(fn ($value) => $value instanceof Arrayable ? $value->toArray() : $value, $this->all());
}

/**
* {@inheritDoc}
*
* @throws \JsonException
* @noinspection JsonEncodingApiUsageInspection
* @noinspection MissingParameterTypeDeclarationInspection
*/
public function toJson($options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE): string
public function toJson(int $options = JSON_PRETTY_PRINT | JSON_UNESCAPED_UNICODE): string
{
return json_encode($this->jsonSerialize(), $options);
return json_encode($this->jsonSerialize(), $options, 512);
}

/**
* @throws \JsonException
*/
public static function readFrom(...$files): array
{
$configurations = array_reduce($files, static function (array $configurations, string $file): array {
$ext = str(pathinfo($file, PATHINFO_EXTENSION));
if ($ext->is('php')) {
$configurations = [];
foreach ($files as $file) {
$ext = pathinfo($file, PATHINFO_EXTENSION);
if ($ext === 'php') {
$configurations[] = require $file;

return $configurations;
}

if ($ext->is('json')) {
if (! str($contents = file_get_contents($file))->jsonValidate()) {
throw InvalidJsonFileException::make($file);
}

} elseif ($ext === 'json') {
$contents = file_get_contents($file) ?: throw InvalidJsonFileException::make($file);
$configurations[] = json_decode($contents, true, 512, JSON_THROW_ON_ERROR);

return $configurations;
} else {
throw UnsupportedConfigFileTypeException::make($file);
}

throw UnsupportedConfigFileTypeException::make($file);
}, []);

}
return array_replace_recursive(...$configurations);
}
}

0 comments on commit df97281

Please sign in to comment.