Skip to content

Commit

Permalink
Add better coercion with deprecations
Browse files Browse the repository at this point in the history 
Emit a deprecation notice if a suspect value is being coerced
for a known type. This will be escalated to an exception in v2.
  • Loading branch information
@iquito
iquito committed Jun 11, 2022
1 parent 810d47f commit 79f11e7
Show file tree
Hide file tree
Showing 9 changed files with 633 additions and 114 deletions.
4 changes: 3 additions & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
"php": ">=8.0",
"symfony/console": "^5.0|^6.0",
"symfony/finder": "^5.0|^6.0",
"squirrelphp/queries": "^1.2"
"squirrelphp/debug": "^2.0",
"squirrelphp/queries": "^1.2",
"squirrelphp/types": "^0.5.2"
},
"require-dev": {
"bamarni/composer-bin-plugin": "^1.3",
Expand Down
10 changes: 10 additions & 0 deletions phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,21 @@ parameters:
count: 1
path: src/Builder/SelectIterator.php

-
message: "#^Match expression does not handle remaining value\\: mixed$#"
count: 1
path: src/MultiRepositoryReadOnly.php

-
message: "#^Offset 'where' on array\\{repositories\\: array, tables\\?\\: array, where\\: array, lock\\?\\: bool\\} in isset\\(\\) always exists and is not nullable\\.$#"
count: 1
path: src/MultiRepositoryReadOnly.php

-
message: "#^Match expression does not handle remaining value\\: mixed$#"
count: 1
path: src/RepositoryReadOnly.php

-
message: "#^Offset 'limit' on array\\{fields\\?\\: array\\<string\\>, field\\?\\: string, where\\?\\: array\\<int\\|string, mixed\\>, order\\?\\: array\\<int\\|string, string\\>, offset\\?\\: int, lock\\?\\: bool\\} in isset\\(\\) does not exist\\.$#"
count: 1
Expand Down
46 changes: 28 additions & 18 deletions src/MultiRepositoryReadOnly.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
use Squirrel\Queries\DBException;
use Squirrel\Queries\DBInterface;
use Squirrel\Queries\Exception\DBInvalidOptionException;
use Squirrel\Types\Coerce;
use Squirrel\Types\Coerceable;

/**
* If more than one table needs to be selected or updated at once this class
* If more than one table needs to be selected at once this class
* combines the knowledge of multiple Repository classes to create
* a query which is simple and secure
*/
Expand Down Expand Up @@ -790,34 +792,42 @@ private function processSelectResult(
$selectTypes[$key] = 'string';
}
} elseif ($selectTypes[$key] === 'uint' || $selectTypes[$key] === 'ufloat') {
// Non-numeric values are kept as string
if (
!\is_numeric($value)
|| \strval(\floatval($value)) !== $value
) {
// Non-coerceable values are kept as string
if (!Coerceable::toFloat($value)) {
$selectTypes[$key] = 'string';
} elseif (
// Numeric values where we lose some information when converting to integer are kept as a string
$selectTypes[$key] === 'uint'
&& \strval(\intval($value)) !== $value
&& !Coerceable::toInt($value)
) {
$selectTypes[$key] = 'string';
} else { // No information loss detected, use int or float type
$selectTypes[$key] = \substr($selectTypes[$key], 1);
}
}

$entry[$key] = match ($selectTypes[$key]) {
'int' => \intval($value),
'bool' => \boolval($value),
'float' => \floatval($value),
'string' => \strval($value),
default => throw Debug::createException(
DBInvalidOptionException::class,
'Unknown casting "' . $selectTypes[$key] . '" for object variable ' . Debug::sanitizeData($key),
ignoreClasses: [MultiRepositoryReadOnlyInterface::class, BuilderInterface::class],
),
};
try {
$entry[$key] = match ($selectTypes[$key]) {
'int' => Coerce::toInt($value),
'bool' => Coerce::toBool($value),
'float' => Coerce::toFloat($value),
'string' => Coerce::toString($value),
default => throw Debug::createException(
DBInvalidOptionException::class,
'Unknown casting "' . $selectTypes[$key] . '" for object variable ' . Debug::sanitizeData($key),
ignoreClasses: [MultiRepositoryReadOnlyInterface::class, BuilderInterface::class],
),
};
} catch (\TypeError $e) {
\trigger_error('Wrong type for ' . $key . ': ' . $e->getMessage(), E_USER_DEPRECATED);

$entry[$key] = match ($selectTypes[$key]) {
'int' => \intval($value),
'bool' => \boolval($value),
'float' => \floatval($value),
'string' => \strval($value),
};
}
}

return $entry;
Expand Down
4 changes: 2 additions & 2 deletions src/MultiRepositoryWriteable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
use Squirrel\Queries\DBException;

/**
* QueryHandler functionality: If more than one table needs to be selected or updated
* at once QueryHandler combines the knowledge of multiple Repository classes to create
* If more than one table needs to be selected or updated at once this class
* combines the knowledge of multiple Repository classes to create
* a query which is simple and secure
*/
class MultiRepositoryWriteable extends MultiRepositoryReadOnly implements MultiRepositoryWriteableInterface
Expand Down
99 changes: 52 additions & 47 deletions src/RepositoryReadOnly.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Squirrel\Queries\DBInterface;
use Squirrel\Queries\Exception\DBInvalidOptionException;
use Squirrel\Queries\LargeObject;
use Squirrel\Types\Coerce;

/**
* Repository functionality: Get data from one table
Expand Down Expand Up @@ -327,12 +328,7 @@ private function prepareSelectQueryForLowerLayer(array $query): array
return $query;
}

/**
* @param mixed $shouldBeBoolean
* @param string $settingName
* @return bool
*/
private function booleanSettingValidation($shouldBeBoolean, string $settingName): bool
private function booleanSettingValidation(mixed $shouldBeBoolean, string $settingName): bool
{
// Make sure the setting is a boolean or at least an integer which can be clearly interpreted as boolean
if (
Expand Down Expand Up @@ -498,13 +494,11 @@ protected function preprocessWhere(array $where): array
/**
* Cast an object variable (array or scalar) to the correct type for a SQL query
*
* @param mixed $value
* @param null|string $fieldName
* @return int|float|string|LargeObject|array<int|string,mixed>|null
*
* @throws DBInvalidOptionException
*/
protected function castTableVariable($value, ?string $fieldName = null)
protected function castTableVariable(mixed $value, ?string $fieldName = null): int|float|string|LargeObject|array|null
{
// Array - go through elements and cast them
if (\is_array($value)) {
Expand All @@ -521,14 +515,9 @@ protected function castTableVariable($value, ?string $fieldName = null)
/**
* Cast an object variable (only single scalar) to the correct type for a SQL query
*
* @param mixed $value
* @param string|null $fieldName
* @param bool $isChange
* @return int|float|string|LargeObject|null
*
* @throws DBInvalidOptionException
*/
protected function castOneTableVariable($value, ?string $fieldName = null, bool $isChange = false)
protected function castOneTableVariable(mixed $value, ?string $fieldName = null, bool $isChange = false): int|float|string|LargeObject|null
{
// Only scalar values and null are allowed
if (!\is_null($value) && !\is_scalar($value)) {
Expand All @@ -547,7 +536,7 @@ protected function castOneTableVariable($value, ?string $fieldName = null, bool
// user is on his own
if (!isset($fieldName)) {
if (\is_bool($value)) {
$value = \intval($value);
$value = $value === true ? 1 : 0;
}

/**
Expand Down Expand Up @@ -580,16 +569,31 @@ protected function castOneTableVariable($value, ?string $fieldName = null, bool
return $value;
}

// We know the field type - only basic types allowed
switch ($this->objectTypes[$fieldName]) {
case 'int':
return \intval($value);
case 'bool':
return \intval(\boolval($value));
case 'float':
return \floatval($value);
case 'string':
return \strval($value);
try {
// We know the field type - only basic types allowed
switch ($this->objectTypes[$fieldName]) {
case 'int':
return Coerce::toInt($value);
case 'bool':
return Coerce::toBool($value) === true ? 1 : 0;
case 'float':
return Coerce::toFloat($value);
case 'string':
return Coerce::toString($value);
}
} catch (\TypeError $e) {
\trigger_error('Wrong type for ' . $fieldName . ' in query: ' . $e->getMessage(), E_USER_DEPRECATED);

switch ($this->objectTypes[$fieldName]) {
case 'int':
return \intval($value);
case 'bool':
return \boolval($value) === true ? 1 : 0;
case 'float':
return \floatval($value);
case 'string':
return \strval($value);
}
}

// Blob = binary large object
Expand Down Expand Up @@ -723,35 +727,36 @@ protected function preprocessOrder(array $orderOptions): array
/**
* Cast an object variable to the correct type for use in an object
*
* @param mixed $value
* @param string $fieldName
* @return bool|int|float|string|null
*
* @throws DBInvalidOptionException
*/
protected function castObjVariable($value, string $fieldName)
protected function castObjVariable(mixed $value, string $fieldName): bool|int|float|string|null
{
// Field is null and can be null according to config
if (\is_null($value) && $this->objectTypesNullable[$fieldName] === true) {
return $value;
}

switch ($this->objectTypes[$fieldName]) {
case 'int':
return \intval($value);
case 'bool':
return \boolval($value);
case 'float':
return \floatval($value);
case 'string':
case 'blob':
return \strval($value);
try {
return match ($this->objectTypes[$fieldName]) {
'int' => Coerce::toInt($value),
'bool' => Coerce::toBool($value),
'float' => Coerce::toFloat($value),
'string', 'blob' => Coerce::toString($value),
default => throw Debug::createException(
DBInvalidOptionException::class,
'Unknown casting for object variable: ' . Debug::sanitizeData($fieldName),
ignoreClasses: [RepositoryReadOnlyInterface::class, BuilderInterface::class],
),
};
} catch (\TypeError $e) {
\trigger_error('Wrong type for ' . $fieldName . ' in result: ' . $e->getMessage(), E_USER_DEPRECATED);

return match ($this->objectTypes[$fieldName]) {
'int' => \intval($value),
'bool' => \boolval($value),
'float' => \floatval($value),
'string', 'blob' => \strval($value),
};
}

throw Debug::createException(
DBInvalidOptionException::class,
'Unknown casting for object variable: ' . Debug::sanitizeData($fieldName),
ignoreClasses: [RepositoryReadOnlyInterface::class, BuilderInterface::class],
);
}
}
38 changes: 38 additions & 0 deletions tests/ErrorHandlerTrait.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

namespace Squirrel\Entities\Tests;

trait ErrorHandlerTrait
{
protected mixed $phpunitErrorHandler = null;
protected array $listOfDeprecations = [];

protected function allowDeprecations(): void
{
$this->listOfDeprecations = [];

$this->phpunitErrorHandler = \set_error_handler(function (int $severity, string $message, string $filepath, int $line): bool {
if ($severity === E_USER_DEPRECATED) {
$this->listOfDeprecations[] = $message;
} else {
return ($this->phpunitErrorHandler)($severity, $message, $filepath, $line);
}

return true;
});
}

protected function getDeprecationList(): array
{
return $this->listOfDeprecations;
}

protected function tearDown(): void
{
if ($this->phpunitErrorHandler !== null) {
\restore_error_handler();
}

parent::tearDown();
}
}
Loading

0 comments on commit 79f11e7

Please sign in to comment.