Skip to content

Commit 3e63780

Browse files
c15k0c15k0
committed
Refactor API helpers and raise coverage with focused unit tests
1 parent fd53a8c commit 3e63780

11 files changed

Lines changed: 1002 additions & 145 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
# PSFS Core
22

33
[![Build Status](https://scrutinizer-ci.com/g/psfs/core/badges/build.png?b=master)](https://scrutinizer-ci.com/g/psfs/core/build-status/master)
4+
[![Security Pipeline](https://github.com/psfs/core/actions/workflows/security-pipeline.yml/badge.svg)](https://github.com/psfs/core/actions/workflows/security-pipeline.yml)
45
[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/psfs/core/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/psfs/core/?branch=master)
56
[![Code Coverage](https://scrutinizer-ci.com/g/psfs/core/badges/coverage.png?b=master)](https://scrutinizer-ci.com/g/psfs/core/?branch=master)
67
[![Packagist Stable Version](https://img.shields.io/packagist/v/psfs/core)](https://packagist.org/packages/psfs/core)
7-
[![Development Line](https://img.shields.io/badge/dev--master-2.2.x--dev-0A7BBB)](https://github.com/psfs/core/tree/master)
8-
[![PHP 8.3](https://img.shields.io/badge/PHP-8.3-777BB4.svg?logo=php&logoColor=white)](https://www.php.net/releases/8.3/en.php)
8+
[![Minimum PHP Version](http://img.shields.io/badge/php-%3E%3D%208-8892BF.svg)](https://php.net/)
9+
[![License](https://poser.pugx.org/propel/propel/license.svg)](https://packagist.org/packages/psfs/core)
910

1011
PSFS is a lightweight PHP framework for MVC/API applications (Twig + Propel + Symfony components).
1112

‎src/base/config/Config.php‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -244,18 +244,7 @@ public static function save(array $data, $extra = null)
244244
$finalData = self::saveExtraParams($data);
245245
$saved = false;
246246
try {
247-
$finalData = array_filter($finalData, function ($key, $value) {
248-
if (in_array($key, self::$required, true)) {
249-
return true;
250-
}
251-
252-
// Keep explicit false/0 flags (security toggles rely on them).
253-
if (is_bool($value) || is_int($value) || is_float($value)) {
254-
return true;
255-
}
256-
257-
return $value !== null && $value !== '';
258-
}, ARRAY_FILTER_USE_BOTH);
247+
$finalData = array_filter($finalData, [self::class, 'shouldPersistConfigEntry'], ARRAY_FILTER_USE_BOTH);
259248
ksort($finalData, SORT_NATURAL | SORT_FLAG_CASE);
260249
$instance = self::getInstance();
261250
$saved = $instance->repository->save($finalData);
@@ -335,6 +324,20 @@ public static function clearConfigFiles(): bool
335324
return $done;
336325
}
337326

327+
private static function shouldPersistConfigEntry(mixed $value, string $key): bool
328+
{
329+
if (in_array($key, self::$required, true)) {
330+
return true;
331+
}
332+
333+
// Keep explicit false/0 flags (security toggles rely on them).
334+
if (is_bool($value) || is_int($value) || is_float($value)) {
335+
return true;
336+
}
337+
338+
return $value !== null && $value !== '';
339+
}
340+
338341
protected function createRepository(): ConfigRepositoryInterface
339342
{
340343
$configPath = CONFIG_DIR . DIRECTORY_SEPARATOR . self::CONFIG_FILE;

‎src/base/types/helpers/FileHelper.php‎

Lines changed: 8 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44

55
use Symfony\Component\Filesystem\Exception\IOException;
66
use Symfony\Component\Filesystem\Filesystem;
7+
use PSFS\base\types\traits\Helper\FileAtomicTrait;
78

89
/**
910
* @package PSFS\base\types\helpers
1011
*/
1112
class FileHelper
1213
{
14+
use FileAtomicTrait;
1315
/**
1416
* @param mixed $data
1517
* @param string $path
@@ -29,36 +31,14 @@ public static function writeFile(string $path, mixed $data): int|bool
2931
*/
3032
public static function writeFileAtomic(string $path, mixed $data, int $flags = 0): bool
3133
{
32-
$dir = dirname($path);
33-
if (file_exists($dir) && !is_dir($dir)) {
34-
return false;
35-
}
36-
if (!is_dir($dir) && mkdir($dir, 0775, true) === false && !is_dir($dir)) {
34+
if (!self::ensureParentDirectory($path)) {
3735
return false;
3836
}
3937
if (is_dir($path)) {
4038
return false;
4139
}
4240
$existingMode = file_exists($path) ? (fileperms($path) & 0777) : 0644;
43-
$tmpPath = tempnam($dir, '.tmp-psfs-');
44-
if (false === $tmpPath) {
45-
return false;
46-
}
47-
$bytes = file_put_contents($tmpPath, $data, $flags | LOCK_EX);
48-
if (false === $bytes) {
49-
if (file_exists($tmpPath)) {
50-
unlink($tmpPath);
51-
}
52-
return false;
53-
}
54-
if (rename($tmpPath, $path) === false) {
55-
if (file_exists($tmpPath)) {
56-
unlink($tmpPath);
57-
}
58-
return false;
59-
}
60-
chmod($path, $existingMode > 0 ? $existingMode : 0644);
61-
return true;
41+
return self::writeTempAndSwap($path, $data, $flags, $existingMode);
6242
}
6343

6444
/**
@@ -71,35 +51,14 @@ public static function copyFileAtomic(string $source, string $target): bool
7151
if (!file_exists($source)) {
7252
return false;
7353
}
74-
$dir = dirname($target);
75-
if (file_exists($dir) && !is_dir($dir)) {
76-
return false;
77-
}
78-
if (!is_dir($dir) && mkdir($dir, 0775, true) === false && !is_dir($dir)) {
54+
if (!self::ensureParentDirectory($target)) {
7955
return false;
8056
}
8157
if (is_dir($target)) {
8258
return false;
8359
}
8460
$mode = fileperms($source) & 0777;
85-
$tmpPath = tempnam($dir, '.tmp-psfs-');
86-
if (false === $tmpPath) {
87-
return false;
88-
}
89-
if (copy($source, $tmpPath) === false) {
90-
if (file_exists($tmpPath)) {
91-
unlink($tmpPath);
92-
}
93-
return false;
94-
}
95-
if (rename($tmpPath, $target) === false) {
96-
if (file_exists($tmpPath)) {
97-
unlink($tmpPath);
98-
}
99-
return false;
100-
}
101-
chmod($target, $mode > 0 ? $mode : 0644);
102-
return true;
61+
return self::copyTempAndSwap($source, $target, $mode);
10362
}
10463

10564
/**
@@ -125,26 +84,10 @@ public static function deleteFile(string $path): bool
12584
*/
12685
public static function withExclusiveLock(string $lockPath, callable $callback): mixed
12786
{
128-
$dir = dirname($lockPath);
129-
if (file_exists($dir) && !is_dir($dir)) {
87+
if (!self::ensureParentDirectory($lockPath)) {
13088
return null;
13189
}
132-
if (!is_dir($dir) && mkdir($dir, 0775, true) === false && !is_dir($dir)) {
133-
return null;
134-
}
135-
$handle = fopen($lockPath, 'c+');
136-
if (false === $handle) {
137-
return null;
138-
}
139-
try {
140-
if (!flock($handle, LOCK_EX)) {
141-
return null;
142-
}
143-
return $callback();
144-
} finally {
145-
flock($handle, LOCK_UN);
146-
fclose($handle);
147-
}
90+
return self::withExclusiveFileLock($lockPath, $callback);
14891
}
14992

15093
/**

‎src/base/types/helpers/InjectorHelper.php‎

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,25 +25,35 @@ public static function extractVariables(ReflectionClass $reflector)
2525
{
2626
$variables = [];
2727
foreach ($reflector->getProperties(ReflectionProperty::IS_PUBLIC) as $property) {
28-
$doc = $property->getDocComment() ?: '';
29-
$instanceType = self::extractVarType($doc, $property);
30-
if (null !== $instanceType) {
31-
$isRequired = self::checkIsRequired($doc, $property);
32-
$label = self::getLabel($doc, $property);
33-
$values = self::getValues($doc, $property);
34-
$variables[$property->getName()] = InjectorDefinitionHelper::buildVariableDefinition(
35-
$instanceType,
36-
$isRequired,
37-
$label
38-
);
39-
if (!empty($values)) {
40-
$variables[$property->getName()]['enum'] = $values;
41-
}
28+
$definition = self::buildVariableDefinition($property);
29+
if (is_array($definition)) {
30+
$variables[$property->getName()] = $definition;
4231
}
4332
}
4433
return $variables;
4534
}
4635

36+
private static function buildVariableDefinition(ReflectionProperty $property): ?array
37+
{
38+
$doc = $property->getDocComment() ?: '';
39+
$instanceType = self::extractVarType($doc, $property);
40+
if (null === $instanceType) {
41+
return null;
42+
}
43+
44+
$definition = InjectorDefinitionHelper::buildVariableDefinition(
45+
$instanceType,
46+
self::checkIsRequired($doc, $property),
47+
self::getLabel($doc, $property)
48+
);
49+
$values = self::getValues($doc, $property);
50+
if (!empty($values)) {
51+
$definition['enum'] = $values;
52+
}
53+
54+
return $definition;
55+
}
56+
4757
/**
4858
* @param ReflectionClass $reflector
4959
* @param integer $type
@@ -171,7 +181,10 @@ public static function getValues($doc, ReflectionProperty $property = null)
171181
if (is_array($values)) {
172182
return $values;
173183
}
174-
return false !== strpos($values, '|') ? explode('|', $values) : $values;
184+
if (is_string($values) && false !== strpos($values, '|')) {
185+
return explode('|', $values);
186+
}
187+
return $values;
175188
}
176189

177190
/**
@@ -200,12 +213,17 @@ public static function constructInjectableInstance($variable, $singleton, $class
200213
$property->getDocComment(),
201214
$property
202215
) : $classNameSpace;
203-
if (true === $singleton && method_exists($varInstanceType, 'getInstance')) {
204-
$instance = $varInstanceType::getInstance();
205-
} else {
206-
$instance = new $varInstanceType();
216+
217+
return self::buildInjectableInstance((string)$varInstanceType, (bool)$singleton);
218+
}
219+
220+
private static function buildInjectableInstance(string $instanceType, bool $singleton): mixed
221+
{
222+
if ($singleton && method_exists($instanceType, 'getInstance')) {
223+
return $instanceType::getInstance();
207224
}
208-
return $instance;
225+
226+
return new $instanceType();
209227
}
210228

211229
/**

‎src/base/types/traits/Api/ApiTrait.php‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,17 @@ trait ApiTrait
4040
*/
4141
public function getApi()
4242
{
43-
$model = explode("\\", (string)($this->getModelNamespace() ?? ''));
44-
45-
return $model[count($model) - 1];
43+
$parts = $this->extractModelNamespaceParts();
44+
return $parts[count($parts) - 1];
4645
}
4746

4847
/**
4948
* @return mixed
5049
*/
5150
public function getDomain()
5251
{
53-
$model = explode("\\", $this->getModelNamespace() ?? '');
54-
return strlen($model[0]) || 1 === count($model) ? $model[0] : $model[1];
52+
$parts = $this->extractModelNamespaceParts();
53+
return strlen($parts[0]) || 1 === count($parts) ? $parts[0] : $parts[1];
5554
}
5655

5756
/**
@@ -77,29 +76,48 @@ protected function extractFields()
7776

7877
protected function hydrateFromRequest()
7978
{
80-
$class = new \ReflectionClass($this->getModelNamespace());
79+
$class = $this->newModelReflection();
8180
$this->model = $class->newInstance();
8281
$this->hydrateModelFromRequest($this->model, $this->data);
8382
}
8483

8584

8685
protected function hydrateBulkRequest()
8786
{
88-
$class = new \ReflectionClass($this->getModelNamespace());
87+
$class = $this->newModelReflection();
8988
$this->list = [];
9089
foreach ($this->data as $item) {
91-
if (is_array($item)) {
92-
if (count($this->list) < Config::getParam('api.block.limit', 1000)) {
93-
$model = $class->newInstance();
94-
$this->hydrateModelFromRequest($model, $item);
95-
$this->list[] = $model;
96-
} else {
97-
Logger::log(t('Max items per bulk insert raised'), LOG_WARNING, [count($this->data) . t('items')]);
98-
}
90+
if (!is_array($item)) {
91+
continue;
92+
}
93+
if ($this->hasBulkCapacity()) {
94+
$model = $class->newInstance();
95+
$this->hydrateModelFromRequest($model, $item);
96+
$this->list[] = $model;
97+
continue;
9998
}
99+
Logger::log(t('Max items per bulk insert raised'), LOG_WARNING, [count($this->data) . t('items')]);
100100
}
101101
}
102102

103+
/**
104+
* @return array<int, string>
105+
*/
106+
private function extractModelNamespaceParts(): array
107+
{
108+
return explode("\\", (string)($this->getModelNamespace() ?? ''));
109+
}
110+
111+
private function newModelReflection(): \ReflectionClass
112+
{
113+
return new \ReflectionClass((string)$this->getModelNamespace());
114+
}
115+
116+
private function hasBulkCapacity(): bool
117+
{
118+
return count($this->list) < Config::getParam('api.block.limit', 1000);
119+
}
120+
103121

104122
protected function saveBulk()
105123
{

0 commit comments

Comments
 (0)