From aed10ab625e371253e46a15eae3e4bcc850c01c3 Mon Sep 17 00:00:00 2001 From: Theodore Brown Date: Sun, 6 Oct 2024 17:51:37 -0500 Subject: [PATCH] Remove deprecated methods --- src/Entities.php | 91 ++++------------------------------------ src/Helpers.php | 31 +------------- src/QueryOptions.php | 33 --------------- test/EntitiesDbTest.php | 8 ++-- test/HelpersTest.php | 86 ++++++++++++++++++------------------- test/src/LegacyUsers.php | 4 -- test/src/ModernUsers.php | 4 -- test/src/Users.php | 19 ++------- 8 files changed, 58 insertions(+), 218 deletions(-) diff --git a/src/Entities.php b/src/Entities.php index 1ed79d7..1a5f550 100644 --- a/src/Entities.php +++ b/src/Entities.php @@ -6,15 +6,8 @@ use PeachySQL\PeachySql; use PeachySQL\QueryBuilder\SqlParams; -use PeachySQL\SqlException; use Teapot\{HttpException, StatusCode}; -/** - * @psalm-type PropArray = array{ - * col?: string, nullGroup?: bool, notDefault?: bool, alias?: string, type?: string, - * timeZone?: \DateTimeZone|null, getValue?: callable|null, dependsOn?: list, output?: bool - * } - */ abstract class Entities { /** @@ -33,11 +26,7 @@ abstract protected function getMap(): array; public function __construct(PeachySql $db) { $this->db = $db; - /** @psalm-suppress DeprecatedMethod */ - $legacyPropMap = $this->getPropMap(); - /** @var array $rawPropMap */ - $rawPropMap = array_replace_recursive(Helpers::selectMapToPropMap($this->getSelectMap()), $legacyPropMap); - $bcProps = Helpers::rawPropMapToProps($rawPropMap); + $bcProps = Helpers::selectMapToPropMap($this->getSelectMap()); $selectProps = $this->getSelectProps(); foreach ($selectProps as $prop) { @@ -107,24 +96,6 @@ protected function getDefaultSort(): array return [$this->idField => 'asc']; } - /** - * Specify a friendly error message for constraint violations (when inserting/updating rows) - * @deprecated - */ - protected function getDuplicateError(): string - { - return ''; - } - - /** - * Specify a friendly error message for constraint violations (when attempting to delete rows) - * @deprecated - */ - protected function getConstraintError(): string - { - return ''; - } - /** * Can be used to return a separate property map for filtering/sorting (but not inserting/updating) */ @@ -133,17 +104,6 @@ protected function getSelectMap(): array return $this->getMap(); } - /** - * Merge additional property information with getSelectMap(). - * Look at the Prop class constructor to see supported options. - * @deprecated Use getSelectProps() instead. - * @return array - */ - protected function getPropMap(): array - { - return []; - } - /** * Merge additional property information with getSelectMap(). * Look at the Prop class constructor to see supported options. @@ -201,18 +161,7 @@ public function deleteByIds(array $ids): int return 0; } - try { - return $this->db->deleteFrom($this->getTableName(), [$this->idColumn => $ids]); - } catch (SqlException $e) { - /** @psalm-suppress DeprecatedMethod */ - $constraintError = $this->getConstraintError(); - - if ($constraintError !== '' && $e->getSqlState() === '23000') { - throw new HttpException($constraintError, StatusCode::CONFLICT, $e); - } else { - throw $e; - } - } + return $this->db->deleteFrom($this->getTableName(), [$this->idColumn => $ids]); } public function updateById(int|string $id, array $data): int @@ -220,11 +169,7 @@ public function updateById(int|string $id, array $data): int $row = Helpers::allPropertiesToColumns($this->map, $this->processValues($data, [$id])); $row = $this->processRow($row, [$id]); - try { - return $this->db->updateRows($this->getTableName(), $row, [$this->idColumn => $id]); - } catch (SqlException $e) { - throw $this->properException($e); - } + return $this->db->updateRows($this->getTableName(), $row, [$this->idColumn => $id]); } /** @@ -240,11 +185,7 @@ public function patchByIds(array $ids, array $mergePatch): int $colVals = self::propertiesToColumns($this->map, $this->processValues($mergePatch, $ids)); $colVals = $this->processRow($colVals, $ids); - try { - return $this->db->updateRows($this->getTableName(), $colVals, [$this->idColumn => $ids]); - } catch (SqlException $e) { - throw $this->properException($e); - } + return $this->db->updateRows($this->getTableName(), $colVals, [$this->idColumn => $ids]); } /** @@ -280,27 +221,11 @@ public function addEntities(array $entities): array } } - try { - $ids = $this->db->insertRows($this->getTableName(), $rows, $this->getIdentityIncrement())->ids; - foreach ($existingIds as $offset => $id) { - array_splice($ids, $offset, 0, [$id]); - } - return $ids; - } catch (SqlException $e) { - throw $this->properException($e); - } - } - - private function properException(SqlException $e): \Exception - { - /** @psalm-suppress DeprecatedMethod */ - $duplicateError = $this->getDuplicateError(); - - if ($duplicateError !== '' && $e->getSqlState() === '23000') { - return new HttpException($duplicateError, StatusCode::CONFLICT, $e); - } else { - return $e; + $ids = $this->db->insertRows($this->getTableName(), $rows, $this->getIdentityIncrement())->ids; + foreach ($existingIds as $offset => $id) { + array_splice($ids, $offset, 0, [$id]); } + return $ids; } /** diff --git a/src/Helpers.php b/src/Helpers.php index 7bcce89..a5c6a6f 100644 --- a/src/Helpers.php +++ b/src/Helpers.php @@ -8,12 +8,11 @@ /** * @internal - * @psalm-import-type PropArray from Entities */ class Helpers { /** - * @return array + * @return array */ public static function selectMapToPropMap(array $map, string $context = ''): array { @@ -32,7 +31,7 @@ public static function selectMapToPropMap(array $map, string $context = ''): arr if (is_array($val)) { $propMap = array_merge($propMap, self::selectMapToPropMap($val, $newKey)); } else { - $propMap[$newKey] = ['col' => $val]; + $propMap[$newKey] = new Prop($newKey, $val); } } @@ -236,32 +235,6 @@ public static function getFieldPropMap(array $fields, array $propMap): array return $fieldProps; } - /** - * @param array $map - * @return array - */ - public static function rawPropMapToProps(array $map): array - { - $props = []; - - foreach ($map as $name => $options) { - $props[$name] = new Prop( - $name, - $options['col'] ?? '', - $options['nullGroup'] ?? false, - !($options['notDefault'] ?? false), - $options['alias'] ?? '', - $options['type'] ?? null, - $options['timeZone'] ?? false, - $options['getValue'] ?? null, - $options['dependsOn'] ?? [], - $options['output'] ?? true - ); - } - - return $props; - } - /** * @param array $props * @return array diff --git a/src/QueryOptions.php b/src/QueryOptions.php index b00e41c..e4d3af1 100644 --- a/src/QueryOptions.php +++ b/src/QueryOptions.php @@ -33,39 +33,6 @@ public function __construct(array $filter, array $originalFilter, array $sort, a $this->fieldProps = $fieldProps; } - /** - * @deprecated Use readonly property instead - */ - public function getFilter(): array - { - return $this->filter; - } - - /** - * @deprecated Use readonly property instead - */ - public function getOriginalFilter(): array - { - return $this->originalFilter; - } - - /** - * @deprecated Use readonly property instead - */ - public function getSort(): array - { - return $this->sort; - } - - /** - * @deprecated Use readonly property instead - * @return Prop[] - */ - public function getFieldProps(): array - { - return $this->fieldProps; - } - public function getColumns(): string { $columns = ''; diff --git a/test/EntitiesDbTest.php b/test/EntitiesDbTest.php index cd8afe4..696c024 100644 --- a/test/EntitiesDbTest.php +++ b/test/EntitiesDbTest.php @@ -209,7 +209,7 @@ public function testEmptyQueries(Entities $entities): void /** * @dataProvider entitiesProvider */ - public function testGetDuplicateError(Entities $entities): void + public function testDuplicateError(Entities $entities): void { $users = [ [ @@ -228,14 +228,14 @@ public function testGetDuplicateError(Entities $entities): void $entities->addEntities($users); throw new \Exception('Failed to throw duplicate name exception'); } catch (\Exception $e) { - $this->assertSame('A user with this name already exists', $e->getMessage()); + $this->assertStringContainsStringIgnoringCase(' duplicate ', $e->getMessage()); } } /** * @dataProvider dbProvider */ - public function testGetConstraintError(PeachySql $db): void + public function testConstraintError(PeachySql $db): void { $user = [ 'name' => 'Some Name', @@ -251,7 +251,7 @@ public function testGetConstraintError(PeachySql $db): void $entities->deleteByIds([$id]); throw new \Exception('Failed to throw constraint violation exception'); } catch (\Exception $e) { - $this->assertSame('Failed to delete user: it still has things referencing it', $e->getMessage()); + $this->assertStringContainsStringIgnoringCase(' constraint ', $e->getMessage()); } } diff --git a/test/HelpersTest.php b/test/HelpersTest.php index a0a547f..6bc79f3 100644 --- a/test/HelpersTest.php +++ b/test/HelpersTest.php @@ -28,15 +28,15 @@ class HelpersTest extends TestCase public function testSelectMapToPropMap(): void { $expected = [ - 'name' => ['col' => 'UserName'], - 'client.id' => ['col' => 'ClientID'], - 'client.name' => ['col' => 'ClientName'], - 'client.isDisabled' => ['col' => 'isDisabled'], - 'group.type.id' => ['col' => 'GroupTypeID'], - 'group.type.name' => ['col' => 'GroupName'], + 'name' => new Prop('name', 'UserName'), + 'client.id' => new Prop('client.id', 'ClientID'), + 'client.name' => new Prop('client.name', 'ClientName'), + 'client.isDisabled' => new Prop('client.isDisabled', 'isDisabled'), + 'group.type.id' => new Prop('group.type.id', 'GroupTypeID'), + 'group.type.name' => new Prop('group.type.name', 'GroupName'), ]; - $this->assertSame($expected, Helpers::selectMapToPropMap($this->propertyMap)); + $this->assertEquals($expected, Helpers::selectMapToPropMap($this->propertyMap)); } public function testPropMapToSelectMap(): void @@ -59,13 +59,13 @@ public function testMapRows(): void /** @psalm-suppress MixedInferredReturnType, MixedReturnStatement */ $usernameMapper = fn(array $row): string => ($row['UserID'] === 1) ? 'testUser' : $row['UserName']; - $rawPropMap = [ - 'id' => ['col' => 'UserID'], - 'username' => ['col' => 'a.UserName', 'getValue' => $usernameMapper, 'dependsOn' => ['id']], - 'client.id' => ['col' => 'a.ClientID'], - 'client.isDisabled' => ['col' => 'c.isDisabled', 'alias' => 'Disabled', 'type' => 'bool'], - 'client.type.id' => ['col' => 'TypeID', 'nullGroup' => true], - 'dateCreated' => ['col' => 'DateCreatedUTC', 'timeZone' => new \DateTimeZone('UTC')], + $props = [ + new Prop('id', 'UserID'), + new Prop('username', 'a.UserName', getValue: $usernameMapper, dependsOn: ['id']), + new Prop('client.id', 'a.ClientID'), + new Prop('client.isDisabled', 'c.isDisabled', alias: 'Disabled', type: 'bool'), + new Prop('client.type.id', 'TypeID', nullGroup: true), + new Prop('dateCreated', 'DateCreatedUTC', timeZone: new \DateTimeZone('UTC')), ]; // test mapping all fields with nullable client.type group @@ -81,7 +81,6 @@ public function testMapRows(): void ['id' => 1, 'username' => 'testUser', 'client' => ['id' => null, 'isDisabled' => false, 'type' => null], 'dateCreated' => null], ]; - $props = Helpers::rawPropMapToProps($rawPropMap); $propMap = Helpers::propListToPropMap($props); $this->assertSame($expected, Helpers::mapRows($generator(), $propMap)); @@ -114,8 +113,7 @@ public function testMapRows(): void ['id' => 1, 'username' => 'testUser', 'client' => null, 'dateCreated' => null], ]; - $rawPropMap['client.id']['nullGroup'] = true; - $props = Helpers::rawPropMapToProps($rawPropMap); + $props[2] = new Prop('client.id', 'a.ClientID', nullGroup: true); $propMap = Helpers::propListToPropMap($props); $fieldProps = Helpers::getFieldPropMap(['id', 'username', 'client.isDisabled', 'dateCreated'], $propMap); $this->assertSame($expected, Helpers::mapRows($generator(), $fieldProps)); @@ -139,17 +137,16 @@ public function testMapRows(): void public function testGetFieldPropMap(): void { - $rawPropMap = [ - 'username' => ['col' => 'UserName', 'notDefault' => true], - 'client.id' => ['col' => 'ClientID', 'nullGroup' => true], - 'client.name' => ['col' => 'Company', 'alias' => 'ClientName'], - 'client.isDisabled' => ['col' => 'isDisabled'], - 'group.type.id' => ['col' => 'GroupTypeID'], - 'group.type.name' => ['col' => 'GroupName'], - 'groupName' => ['col' => 'DiffGroupName'], + $props = [ + new Prop('username', 'UserName', isDefault: false), + new Prop('client.id', 'ClientID', nullGroup: true), + new Prop('client.name', 'Company', alias: 'ClientName'), + new Prop('client.isDisabled', 'isDisabled'), + new Prop('group.type.id', 'GroupTypeID'), + new Prop('group.type.name', 'GroupName'), + new Prop('groupName', 'DiffGroupName'), ]; - $props = Helpers::rawPropMapToProps($rawPropMap); $propMap = Helpers::propListToPropMap($props); $expected = $propMap; unset($expected['username']); @@ -181,9 +178,12 @@ public function testGetFieldPropMap(): void // test selection when dependant field is in a nullable group $valueGetter = fn(array $_row): string => ''; - $rawPropMap['groupName']['dependsOn'] = ['client.name']; - $rawPropMap['groupName']['getValue'] = $valueGetter; - $props = Helpers::rawPropMapToProps($rawPropMap); + $props[6] = new Prop( + name: 'groupName', + col: 'DiffGroupName', + getValue: $valueGetter, + dependsOn: ['client.name'], + ); $propMap = Helpers::propListToPropMap($props); $expected = ['groupName', 'client.name']; @@ -208,14 +208,13 @@ public function testGetFieldPropMap(): void // test dependent fields marked as not default or excluded from output $dependencies = ['username', 'client.secret']; - $rawPropMap = [ - 'username' => ['col' => 'UserName', 'notDefault' => true], - 'client.id' => ['col' => 'ClientID', 'nullGroup' => true, 'getValue' => $valueGetter, 'dependsOn' => $dependencies], - 'client.name' => ['col' => 'Company', 'alias' => 'ClientName'], - 'client.secret' => ['col' => 'Secret', 'output' => false], + $props = [ + new Prop('username', 'UserName', isDefault: false), + new Prop('client.id', 'ClientID', nullGroup: true, getValue: $valueGetter, dependsOn: $dependencies), + new Prop('client.name', 'Company', alias: 'ClientName'), + new Prop('client.secret', 'Secret', output: false), ]; - $props = Helpers::rawPropMapToProps($rawPropMap); $propMap = Helpers::propListToPropMap($props); // don't select dependent username field since client.id isn't output anyway @@ -258,18 +257,15 @@ public function testPropListToPropMap(): void public function testPropMapToAliasMap(): void { - $utc = new \DateTimeZone('UTC'); - - $rawPropMap = [ - 'id' => ['col' => 'UserID'], - 'username' => ['col' => 'a.UserName'], - 'client.isDisabled' => ['col' => 'c.isDisabled', 'alias' => 'isClientDisabled', 'type' => 'bool'], - 'dateCreated' => ['col' => 'DateCreatedUTC', 'timeZone' => $utc], - 'computed' => ['getValue' => fn(array $_): bool => true], - 'computed2' => ['alias' => 'aliased', 'getValue' => fn(array $_): bool => false], + $props = [ + new Prop('id', 'UserID'), + new Prop('username', 'a.UserName'), + new Prop('client.isDisabled', 'c.isDisabled', alias: 'isClientDisabled', type: 'bool'), + new Prop('dateCreated', 'DateCreatedUTC', timeZone: new \DateTimeZone('UTC')), + new Prop('computed', getValue: fn(array $_): bool => true), + new Prop('computed2', alias: 'aliased', getValue: fn(array $_): bool => false), ]; - $props = Helpers::rawPropMapToProps($rawPropMap); $propMap = Helpers::propListToPropMap($props); $expected = [ diff --git a/test/src/LegacyUsers.php b/test/src/LegacyUsers.php index 1800bc8..800a749 100644 --- a/test/src/LegacyUsers.php +++ b/test/src/LegacyUsers.php @@ -7,10 +7,6 @@ use theodorejb\Phaster\{Entities, QueryOptions}; use PeachySQL\QueryBuilder\SqlParams; -/** - * @psalm-import-type PropArray from Entities - * https://github.com/vimeo/psalm/issues/8645 - */ class LegacyUsers extends Entities { protected function getTableName(): string diff --git a/test/src/ModernUsers.php b/test/src/ModernUsers.php index 6889330..07258bc 100644 --- a/test/src/ModernUsers.php +++ b/test/src/ModernUsers.php @@ -6,10 +6,6 @@ use theodorejb\Phaster\{Entities, Prop, QueryOptions}; -/** - * @psalm-import-type PropArray from Entities - * https://github.com/vimeo/psalm/issues/8645 - */ class ModernUsers extends Entities { protected function getTableName(): string diff --git a/test/src/Users.php b/test/src/Users.php index 5819f67..679b7a7 100644 --- a/test/src/Users.php +++ b/test/src/Users.php @@ -5,11 +5,8 @@ namespace theodorejb\Phaster\Test\src; use theodorejb\Phaster\Entities; +use theodorejb\Phaster\Prop; -/** - * @psalm-import-type PropArray from Entities - * https://github.com/vimeo/psalm/issues/8645 - */ class Users extends Entities { protected function getMap(): array @@ -23,10 +20,10 @@ protected function getMap(): array ]; } - protected function getPropMap(): array + protected function getSelectProps(): array { return [ - 'isDisabled' => ['type' => 'bool'], + new Prop('isDisabled', 'isDisabled', type: 'bool'), ]; } @@ -36,14 +33,4 @@ protected function getDefaultValues(): array 'isDisabled' => false, ]; } - - protected function getDuplicateError(): string - { - return 'A user with this name already exists'; - } - - protected function getConstraintError(): string - { - return 'Failed to delete user: it still has things referencing it'; - } }