From c7cb269834933173596899f20e4f6e429ed7d436 Mon Sep 17 00:00:00 2001 From: Mark Scherer Date: Tue, 2 Jan 2024 14:49:16 +0100 Subject: [PATCH] Complete baking of enums. (#972) * Complete baking of enums. * Update src/Command/EnumCommand.php Co-authored-by: othercorey * Fix up index/view for enums. * Fix up index/view for enums. * Switch key/value, use CamelCase * Stricter validation * Update src/Command/ModelCommand.php Co-authored-by: Mark Story * Cleanup * Cleanup * Add more tests. --------- Co-authored-by: othercorey Co-authored-by: Mark Story --- src/Command/EnumCommand.php | 86 +++++++++++++- src/Command/ModelCommand.php | 108 +++++++++++++++++- src/Utility/Model/EnumParser.php | 45 ++++++++ src/View/Helper/BakeHelper.php | 4 + templates/bake/Model/enum.twig | 4 + templates/bake/Template/index.twig | 4 +- templates/bake/Template/view.twig | 13 +++ tests/TestCase/Command/EnumCommandTest.php | 32 ++++++ tests/TestCase/Command/ModelCommandTest.php | 29 +++++ .../TestCase/Utility/Model/EnumParserTest.php | 47 ++++++++ .../Model/testBakeEnumBackedIntWithCases.php | 25 ++++ .../Model/testBakeEnumBackedWithCases.php | 25 ++++ .../Model/testBakeTableWithEnumConfig.php | 25 ++++ .../App/Model/Enum/BakeUserNullableGender.php | 25 ++++ .../App/Model/Enum/BakeUserStatus.php | 6 +- 15 files changed, 468 insertions(+), 10 deletions(-) create mode 100644 src/Utility/Model/EnumParser.php create mode 100644 tests/TestCase/Utility/Model/EnumParserTest.php create mode 100644 tests/comparisons/Model/testBakeEnumBackedIntWithCases.php create mode 100644 tests/comparisons/Model/testBakeEnumBackedWithCases.php create mode 100644 tests/comparisons/Model/testBakeTableWithEnumConfig.php create mode 100644 tests/test_app/App/Model/Enum/BakeUserNullableGender.php diff --git a/src/Command/EnumCommand.php b/src/Command/EnumCommand.php index 2118450e..c5ff42bc 100644 --- a/src/Command/EnumCommand.php +++ b/src/Command/EnumCommand.php @@ -16,8 +16,12 @@ */ namespace Bake\Command; +use Bake\Utility\Model\EnumParser; use Cake\Console\Arguments; +use Cake\Console\ConsoleIo; use Cake\Console\ConsoleOptionParser; +use Cake\Utility\Inflector; +use InvalidArgumentException; /** * Enum code generator. @@ -64,8 +68,20 @@ public function template(): string */ public function templateData(Arguments $arguments): array { + $cases = EnumParser::parseCases($arguments->getArgument('cases'), (bool)$arguments->getOption('int')); + $isOfTypeInt = $this->isOfTypeInt($cases); + $backingType = $isOfTypeInt ? 'int' : 'string'; + if ($arguments->getOption('int')) { + if ($cases && !$isOfTypeInt) { + throw new InvalidArgumentException('Cases do not match requested `int` backing type.'); + } + + $backingType = 'int'; + } + $data = parent::templateData($arguments); - $data['backingType'] = $arguments->getOption('int') ? 'int' : 'string'; + $data['backingType'] = $backingType; + $data['cases'] = $this->formatCases($cases); return $data; } @@ -82,12 +98,76 @@ public function buildOptionParser(ConsoleOptionParser $parser): ConsoleOptionPar $parser->setDescription( 'Bake backed enums for use in models.' - )->addOption('int', [ - 'help' => 'Using backed enums with int instead of string as return type', + )->addArgument('name', [ + 'help' => 'Name of the enum to bake. You can use Plugin.name to bake plugin enums.', + 'required' => true, + ])->addArgument('cases', [ + 'help' => 'List of either `one,two` for string or `foo:0,bar:1` for int type.', + ])->addOption('int', [ + 'help' => 'Using backed enums with int instead of string as return type.', 'boolean' => true, 'short' => 'i', ]); return $parser; } + + /** + * @param array $definition + * @return bool + */ + protected function isOfTypeInt(array $definition): bool + { + if (!$definition) { + return false; + } + + foreach ($definition as $value) { + if (!is_int($value)) { + return false; + } + } + + return true; + } + + /** + * @param array $cases + * @return array + */ + protected function formatCases(array $cases): array + { + $formatted = []; + foreach ($cases as $case => $value) { + $case = Inflector::camelize(Inflector::underscore($case)); + if (is_string($value)) { + $value = '\'' . $value . '\''; + } + $formatted[] = 'case ' . $case . ' = ' . $value . ';'; + } + + return $formatted; + } + + /** + * Generate a class stub + * + * @param string $name The class name + * @param \Cake\Console\Arguments $args The console arguments + * @param \Cake\Console\ConsoleIo $io The console io + * @return void + */ + protected function bake(string $name, Arguments $args, ConsoleIo $io): void + { + parent::bake($name, $args, $io); + + $path = $this->getPath($args); + $filename = $path . $name . '.php'; + + // Work around composer caching that classes/files do not exist. + // Check for the file as it might not exist in tests. + if (file_exists($filename)) { + require_once $filename; + } + } } diff --git a/src/Command/ModelCommand.php b/src/Command/ModelCommand.php index c8fd2bae..afcf486d 100644 --- a/src/Command/ModelCommand.php +++ b/src/Command/ModelCommand.php @@ -17,6 +17,7 @@ namespace Bake\Command; use Bake\CodeGen\FileBuilder; +use Bake\Utility\Model\EnumParser; use Bake\Utility\TableScanner; use Cake\Console\Arguments; use Cake\Console\ConsoleIo; @@ -28,9 +29,12 @@ use Cake\Database\Schema\CachedCollection; use Cake\Database\Schema\TableSchema; use Cake\Database\Schema\TableSchemaInterface; +use Cake\Database\Type\EnumType; +use Cake\Database\TypeFactory; use Cake\Datasource\ConnectionManager; use Cake\ORM\Table; use Cake\Utility\Inflector; +use ReflectionEnum; use function Cake\Core\pluginSplit; /** @@ -111,6 +115,8 @@ public function bake(string $name, Arguments $args, ConsoleIo $io): void $tableObject = $this->getTableObject($name, $table); $this->validateNames($tableObject->getSchema(), $io); $data = $this->getTableContext($tableObject, $table, $name, $args, $io); + + $this->bakeEnums($tableObject, $data, $args, $io); $this->bakeTable($tableObject, $data, $args, $io); $this->bakeEntity($tableObject, $data, $args, $io); $this->bakeFixture($tableObject->getAlias(), $tableObject->getTable(), $args, $io); @@ -168,6 +174,7 @@ public function getTableContext( $behaviors = $this->getBehaviors($tableObject); $connection = $this->connection; $hidden = $this->getHiddenFields($tableObject, $args); + $enumSchema = $this->getEnumDefinitions($tableObject->getSchema()); return compact( 'associations', @@ -181,7 +188,8 @@ public function getTableContext( 'rulesChecker', 'behaviors', 'connection', - 'hidden' + 'hidden', + 'enumSchema', ); } @@ -1118,7 +1126,7 @@ public function getCounterCache(Table $model): array * Bake an entity class. * * @param \Cake\ORM\Table $model Model name or object - * @param array $data An array to use to generate the Table + * @param array $data An array to use to generate the Table * @param \Cake\Console\Arguments $args CLI Arguments * @param \Cake\Console\ConsoleIo $io CLI io * @return void @@ -1170,7 +1178,7 @@ public function bakeEntity(Table $model, array $data, Arguments $args, ConsoleIo * Bake a table class. * * @param \Cake\ORM\Table $model Model name or object - * @param array $data An array to use to generate the Table + * @param array $data An array to use to generate the Table * @param \Cake\Console\Arguments $args CLI Arguments * @param \Cake\Console\ConsoleIo $io CLI Arguments * @return void @@ -1435,6 +1443,12 @@ protected function possibleEnumFields(TableSchemaInterface $schema): array foreach ($schema->columns() as $column) { $columnSchema = $schema->getColumn($column); + if (str_starts_with($columnSchema['type'], 'enum-')) { + $fields[] = $column; + + continue; + } + if (!in_array($columnSchema['type'], ['string', 'integer', 'tinyinteger', 'smallinteger'], true)) { continue; } @@ -1444,4 +1458,92 @@ protected function possibleEnumFields(TableSchemaInterface $schema): array return $fields; } + + /** + * @param \Cake\Database\Schema\TableSchemaInterface $schema + * @return array + */ + protected function getEnumDefinitions(TableSchemaInterface $schema): array + { + $enums = []; + + foreach ($schema->columns() as $column) { + $columnSchema = $schema->getColumn($column); + if ( + !in_array($columnSchema['type'], ['string', 'integer', 'tinyinteger', 'smallinteger'], true) + && !str_starts_with($columnSchema['type'], 'enum-') + ) { + continue; + } + + if (empty($columnSchema['comment']) || strpos($columnSchema['comment'], '[enum]') === false) { + continue; + } + + $enumsDefinitionString = trim(mb_substr($columnSchema['comment'], strpos($columnSchema['comment'], '[enum]') + 6)); + $isInt = in_array($columnSchema['type'], ['integer', 'tinyinteger', 'smallinteger'], true); + if (str_starts_with($columnSchema['type'], 'enum-')) { + $dbType = TypeFactory::build($columnSchema['type']); + if ($dbType instanceof EnumType) { + $class = $dbType->getEnumClassName(); + $reflectionEnum = new ReflectionEnum($class); + $backingType = (string)$reflectionEnum->getBackingType(); + if ($backingType === 'int') { + $isInt = true; + } + } + } + $enumsDefinition = EnumParser::parseCases($enumsDefinitionString, $isInt); + if (!$enumsDefinition) { + continue; + } + + $enums[$column] = [ + 'type' => $isInt ? 'int' : 'string', + 'cases' => $enumsDefinition, + ]; + } + + return $enums; + } + + /** + * @param \Cake\ORM\Table $model + * @param array $data + * @param \Cake\Console\Arguments $args + * @param \Cake\Console\ConsoleIo $io + * @return void + */ + protected function bakeEnums(Table $model, array $data, Arguments $args, ConsoleIo $io): void + { + $enums = $data['enumSchema']; + if (!$enums) { + return; + } + + $entity = $this->_entityName($model->getAlias()); + + foreach ($enums as $column => $data) { + $enumCommand = new EnumCommand(); + + $name = $entity . Inflector::camelize($column); + if ($this->plugin) { + $name = $this->plugin . '.' . $name; + } + + $enumCases = $data['cases']; + + $cases = []; + foreach ($enumCases as $k => $v) { + $cases[] = $k . ':' . $v; + } + + $args = new Arguments( + [$name, implode(',', $cases)], + ['int' => $data['type'] === 'int'] + $args->getOptions(), + ['name', 'cases'] + ); + $enumCommand->execute($args, $io); + } + } } diff --git a/src/Utility/Model/EnumParser.php b/src/Utility/Model/EnumParser.php new file mode 100644 index 00000000..df3ed5dc --- /dev/null +++ b/src/Utility/Model/EnumParser.php @@ -0,0 +1,45 @@ + + */ + public static function parseCases(?string $casesString, bool $int): array + { + if ($casesString === null || $casesString === '') { + return []; + } + + $enumCases = explode(',', $casesString); + + $definition = []; + foreach ($enumCases as $k => $enumCase) { + $case = $value = trim($enumCase); + if (str_contains($case, ':')) { + $value = trim(mb_substr($case, strpos($case, ':') + 1)); + $case = mb_substr($case, 0, strpos($case, ':')); + } elseif ($int) { + $value = $k; + } + + if (!preg_match('/^[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*$/', $case)) { + throw new InvalidArgumentException(sprintf('`%s` is not a valid enum case', $case)); + } + if (is_string($value) && str_contains($value, '\'')) { + throw new InvalidArgumentException(sprintf('`%s` value cannot contain `\'` character', $case)); + } + + $definition[$case] = $int ? (int)$value : $value; + } + + return $definition; + } +} diff --git a/src/View/Helper/BakeHelper.php b/src/View/Helper/BakeHelper.php index 95572064..52e11dd6 100644 --- a/src/View/Helper/BakeHelper.php +++ b/src/View/Helper/BakeHelper.php @@ -233,6 +233,9 @@ public function getViewFieldsData(array $fields, SchemaInterface $schema, array if (isset($associationFields[$field])) { return 'string'; } + if ($type && str_starts_with($type, 'enum-')) { + return 'enum'; + } $numberTypes = ['decimal', 'biginteger', 'integer', 'float', 'smallinteger', 'tinyinteger']; if (in_array($type, $numberTypes, true)) { return 'number'; @@ -258,6 +261,7 @@ public function getViewFieldsData(array $fields, SchemaInterface $schema, array 'number' => [], 'string' => [], 'boolean' => [], + 'enum' => [], 'date' => [], 'text' => [], ]; diff --git a/templates/bake/Model/enum.twig b/templates/bake/Model/enum.twig index fe229b11..83f48243 100644 --- a/templates/bake/Model/enum.twig +++ b/templates/bake/Model/enum.twig @@ -24,6 +24,10 @@ {{ DocBlock.classDescription(name, 'Enum', [])|raw }} enum {{ name }}: {{ backingType }} implements EnumLabelInterface { +{% if cases %} + {{ Bake.concat('\n ', cases) }} + +{% endif %} /** * @return string */ diff --git a/templates/bake/Template/index.twig b/templates/bake/Template/index.twig index 8651c8f2..0c630067 100644 --- a/templates/bake/Template/index.twig +++ b/templates/bake/Template/index.twig @@ -49,7 +49,9 @@ {% endif %} {% if isKey is not same as(true) %} {% set columnData = Bake.columnData(field, schema) %} -{% if columnData.type not in ['integer', 'float', 'decimal', 'biginteger', 'smallinteger', 'tinyinteger'] %} +{% if columnData.type starts with 'enum-' %} + {{ field }} === null ? '' : h(${{ singularVar }}->{{ field }}->label()) ?> +{% elseif columnData.type not in ['integer', 'float', 'decimal', 'biginteger', 'smallinteger', 'tinyinteger'] %} {{ field }}) ?> {% elseif columnData.null %} {{ field }} === null ? '' : $this->Number->format(${{ singularVar }}->{{ field }}) ?> diff --git a/templates/bake/Template/view.twig b/templates/bake/Template/view.twig index 6f8f2552..ae63d0e2 100644 --- a/templates/bake/Template/view.twig +++ b/templates/bake/Template/view.twig @@ -76,6 +76,19 @@ {% endfor %} {% endif %} +{% if groupedFields.enum %} +{% for field in groupedFields.enum %} + + +{% set columnData = Bake.columnData(field, schema) %} +{% if columnData.null %} + {{ field }} === null ? '' : h(${{ singularVar }}->{{ field }}->label()) ?> +{% else %} + {{ field }}->label()) ?> +{% endif %} + +{% endfor %} +{% endif %} {% if groupedFields.date %} {% for field in groupedFields.date %} diff --git a/tests/TestCase/Command/EnumCommandTest.php b/tests/TestCase/Command/EnumCommandTest.php index 678aaf40..bc34dcb1 100644 --- a/tests/TestCase/Command/EnumCommandTest.php +++ b/tests/TestCase/Command/EnumCommandTest.php @@ -68,4 +68,36 @@ public function testBakeEnumBackedInt() $result = file_get_contents($this->generatedFile); $this->assertSameAsFile(__FUNCTION__ . '.php', $result); } + + /** + * test baking an enum with string return type and cases + * + * @return void + */ + public function testBakeEnumBackedWithCases() + { + $this->generatedFile = APP . 'Model/Enum/FooBar.php'; + $this->exec('bake enum FooBar foo,bar:b,bar_baz', ['y']); + + $this->assertExitCode(CommandInterface::CODE_SUCCESS); + $this->assertFileExists($this->generatedFile); + $result = file_get_contents($this->generatedFile); + $this->assertSameAsFile(__FUNCTION__ . '.php', $result); + } + + /** + * test baking an enum with string return type and cases + * + * @return void + */ + public function testBakeEnumBackedIntWithCases() + { + $this->generatedFile = APP . 'Model/Enum/FooBar.php'; + $this->exec('bake enum FooBar foo,bar,bar_baz:9 -i', ['y']); + + $this->assertExitCode(CommandInterface::CODE_SUCCESS); + $this->assertFileExists($this->generatedFile); + $result = file_get_contents($this->generatedFile); + $this->assertSameAsFile(__FUNCTION__ . '.php', $result); + } } diff --git a/tests/TestCase/Command/ModelCommandTest.php b/tests/TestCase/Command/ModelCommandTest.php index 49ec8f45..731cbd2a 100644 --- a/tests/TestCase/Command/ModelCommandTest.php +++ b/tests/TestCase/Command/ModelCommandTest.php @@ -2111,6 +2111,35 @@ public function testBakeTableWithEnum(): void $this->assertStringContainsString('$this->getSchema()->setColumnType(\'status\', \Cake\Database\Type\EnumType::from(\Bake\Test\App\Model\Enum\BakeUserStatus::class));', $result); } + /** + * test generation with enum config in column comment + * + * @return void + */ + public function testBakeTableWithEnumConfig(): void + { + $this->generatedFile = APP . 'Model/Table/BakeUsersTable.php'; + + $bakeUsers = $this->getTableLocator()->get('BakeUsers'); + $attributes = [ + 'type' => 'string', + 'null' => true, + 'comment' => '[enum]male,female,diverse', + ]; + $bakeUsers->setSchema($bakeUsers->getSchema()->addColumn('nullable_gender', $attributes)); + + $this->exec('bake model --no-validation --no-test --no-fixture --no-entity BakeUsers', ['y']); + + $this->assertExitCode(CommandInterface::CODE_SUCCESS); + $this->assertFileExists($this->generatedFile); + $result = file_get_contents($this->generatedFile); + $this->assertStringContainsString('$this->getSchema()->setColumnType(\'nullable_gender\', \Cake\Database\Type\EnumType::from(\Bake\Test\App\Model\Enum\BakeUserNullableGender::class));', $result); + + $generatedEnumFile = APP . 'Model/Enum/BakeUserNullableGender.php'; + $result = file_get_contents($generatedEnumFile); + $this->assertSameAsFile(__FUNCTION__ . '.php', $result); + } + /** * test generation with counter cache * diff --git a/tests/TestCase/Utility/Model/EnumParserTest.php b/tests/TestCase/Utility/Model/EnumParserTest.php new file mode 100644 index 00000000..bff2c71c --- /dev/null +++ b/tests/TestCase/Utility/Model/EnumParserTest.php @@ -0,0 +1,47 @@ +assertSame([], $cases); + + $cases = EnumParser::parseCases('foo, bar', false); + $this->assertSame(['foo' => 'foo', 'bar' => 'bar'], $cases); + + $cases = EnumParser::parseCases('foo:f, bar:b', false); + $this->assertSame(['foo' => 'f', 'bar' => 'b'], $cases); + + $cases = EnumParser::parseCases('foo:0, bar:1', true); + $this->assertSame(['foo' => 0, 'bar' => 1], $cases); + + $cases = EnumParser::parseCases('foo, bar', true); + $this->assertSame(['foo' => 0, 'bar' => 1], $cases); + } +} diff --git a/tests/comparisons/Model/testBakeEnumBackedIntWithCases.php b/tests/comparisons/Model/testBakeEnumBackedIntWithCases.php new file mode 100644 index 00000000..fcd88e07 --- /dev/null +++ b/tests/comparisons/Model/testBakeEnumBackedIntWithCases.php @@ -0,0 +1,25 @@ +name)); + } +} diff --git a/tests/comparisons/Model/testBakeEnumBackedWithCases.php b/tests/comparisons/Model/testBakeEnumBackedWithCases.php new file mode 100644 index 00000000..403e8fda --- /dev/null +++ b/tests/comparisons/Model/testBakeEnumBackedWithCases.php @@ -0,0 +1,25 @@ +name)); + } +} diff --git a/tests/comparisons/Model/testBakeTableWithEnumConfig.php b/tests/comparisons/Model/testBakeTableWithEnumConfig.php new file mode 100644 index 00000000..1287d1ac --- /dev/null +++ b/tests/comparisons/Model/testBakeTableWithEnumConfig.php @@ -0,0 +1,25 @@ +name)); + } +} diff --git a/tests/test_app/App/Model/Enum/BakeUserNullableGender.php b/tests/test_app/App/Model/Enum/BakeUserNullableGender.php new file mode 100644 index 00000000..1287d1ac --- /dev/null +++ b/tests/test_app/App/Model/Enum/BakeUserNullableGender.php @@ -0,0 +1,25 @@ +name)); + } +} diff --git a/tests/test_app/App/Model/Enum/BakeUserStatus.php b/tests/test_app/App/Model/Enum/BakeUserStatus.php index be156019..5dcf0ec7 100644 --- a/tests/test_app/App/Model/Enum/BakeUserStatus.php +++ b/tests/test_app/App/Model/Enum/BakeUserStatus.php @@ -19,14 +19,14 @@ enum BakeUserStatus: int implements EnumLabelInterface { - case INACTIVE = 0; - case ACTIVE = 1; + case Inactive = 0; + case Active = 1; /** * @return string */ public function label(): string { - return Inflector::humanize(mb_strtolower($this->name)); + return Inflector::humanize(Inflector::underscore($this->name)); } }