Merge pull request #215 from moufmouf/fix/computedcolumns

Adding a @readonly annotation

Author: moufmouf

doc/annotations.md

@@ -179,6 +179,28 @@ This annotation can be put on a column comment to alter the visibility of the "i
 For instance, if you put the `@ProtectedOneToMany` on the "country_id" column of a "users" table,
 then in the `Country` bean, the `getUsers()` method will be protected.
 
+The @ReadOnly annotation
+-----------------------------------------------------
+<small>(Available in TDBM 5.2+)</small>
+
+Columns with the "@ReadOnly" annotation cannot be written at all by TDBM.
+
+Add this annotation to any column that is generated/computed in your database.
+
+For instance:
+
+```sql
+CREATE TABLE `products` (
+    `id` INT UNSIGNED AUTO_INCREMENT NOT NULL,
+    `data` JSON NOT NULL,
+    `names_virtual` VARCHAR(20) GENERATED ALWAYS AS (`data` ->> '$.name') NOT NULL COMMENT '@ReadOnly', 
+    PRIMARY KEY (`id`)
+)
+```
+
+Note: TDBM is based in Doctrine DBAL and Doctrine DBAL offers no way of knowing which columns are computed. So each time
+you have a generated column in your data model, you will need to put the `@ReadOnly` annotation explicitly. 
+
 The @Json annotations
 ---------------------
 

phpstan.neon

@@ -23,6 +23,9 @@ parameters:
      -
         message: '#TheCodingMachine\\TDBM\\Schema\\LockFileSchemaManager::__construct\(\) does not call parent constructor from Doctrine\\DBAL\\Schema\\AbstractSchemaManager.#'
         path: src/Schema/LockFileSchemaManager.php
+     -
+        message: '#is "array". Please provide a more specific .* annotation#'
+        path: src/Test/Dao/Bean/Generated/PlayerBaseBean.php
     #reportUnmatchedIgnoredErrors: false
 includes:
     - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon

src/Utils/AbstractBeanPropertyDescriptor.php

@@ -4,6 +4,7 @@
 namespace TheCodingMachine\TDBM\Utils;
 
 use Doctrine\DBAL\Schema\Table;
+use TheCodingMachine\TDBM\Utils\Annotation\ReadOnly;
 use Zend\Code\Generator\DocBlock\Tag\ParamTag;
 use Zend\Code\Generator\MethodGenerator;
 
@@ -149,7 +150,7 @@ public function getTable(): Table
     /**
      * Returns the PHP code for getters and setters.
      *
-     * @return MethodGenerator[]
+     * @return (MethodGenerator|null)[]
      */
     abstract public function getGetterSetterCode(): array;
 
@@ -180,4 +181,12 @@ abstract public function getCloneRule(): ?string;
      * @return bool
      */
     abstract public function isTypeHintable() : bool;
+
+    /**
+     * Returns true if the property is tagged with the "ReadOnly" annotation.
+     * ReadOnly annotations should be used on generated/computed database columns.
+     *
+     * @return bool
+     */
+    abstract public function isReadOnly(): bool;
 }

src/Utils/Annotation/AnnotationParser.php

@@ -43,6 +43,7 @@ public static function buildWithDefaultAnnotations(array $additionalAnnotations)
             'ProtectedGetter' => ProtectedGetter::class,
             'ProtectedSetter' => ProtectedSetter::class,
             'ProtectedOneToMany' => ProtectedOneToMany::class,
+            'ReadOnly' => ReadOnly::class,
             'JsonKey' => JsonKey::class,
             'JsonIgnore' => JsonIgnore::class,
             'JsonInclude' => JsonInclude::class,

src/Utils/Annotation/ReadOnly.php

@@ -0,0 +1,15 @@
+<?php
+namespace TheCodingMachine\TDBM\Utils\Annotation;
+
+/**
+ * Declares a column as "read-only".
+ * Read-only columns cannot be set neither in a setter nor in a constructor argument.
+ * They are very useful on generated/computed columns.
+ *
+ * This annotation can only be used in a database column comment.
+ *
+ * @Annotation
+ */
+final class ReadOnly
+{
+}

src/Utils/BeanDescriptor.php

@@ -214,8 +214,8 @@ public function getBeanPropertyDescriptors(): array
      */
     public function getConstructorProperties(): array
     {
-        $constructorProperties = array_filter($this->beanPropertyDescriptors, function (AbstractBeanPropertyDescriptor $property) {
-            return !$property instanceof InheritanceReferencePropertyDescriptor && $property->isCompulsory();
+        $constructorProperties = array_filter($this->beanPropertyDescriptors, static function (AbstractBeanPropertyDescriptor $property) {
+            return !$property instanceof InheritanceReferencePropertyDescriptor && $property->isCompulsory() && !$property->isReadOnly();
         });
 
         return $constructorProperties;

src/Utils/ObjectBeanPropertyDescriptor.php

@@ -148,7 +148,7 @@ public function isPrimaryKey(): bool
     /**
      * Returns the PHP code for getters and setters.
      *
-     * @return MethodGenerator[]
+     * @return (MethodGenerator|null)[]
      */
     public function getGetterSetterCode(): array
     {
@@ -177,17 +177,21 @@ public function getGetterSetterCode(): array
             $getter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
         }
 
-        $setter = new MethodGenerator($setterName);
-        $setter->setDocBlock(new DocBlockGenerator('The setter for the ' . $referencedBeanName . ' object bound to this object via the ' . implode(' and ', $this->foreignKey->getUnquotedLocalColumns()) . ' column.'));
+        if (!$this->isReadOnly()) {
+            $setter = new MethodGenerator($setterName);
+            $setter->setDocBlock(new DocBlockGenerator('The setter for the ' . $referencedBeanName . ' object bound to this object via the ' . implode(' and ', $this->foreignKey->getUnquotedLocalColumns()) . ' column.'));
 
-        $setter->setParameter(new ParameterGenerator('object', ($isNullable ? '?' : '') . $this->beanNamespace . '\\' . $referencedBeanName));
+            $setter->setParameter(new ParameterGenerator('object', ($isNullable ? '?' : '') . $this->beanNamespace . '\\' . $referencedBeanName));
 
-        $setter->setReturnType('void');
+            $setter->setReturnType('void');
 
-        $setter->setBody('$this->setRef(' . var_export($tdbmFk->getCacheKey(), true) . ', $object, ' . var_export($tableName, true) . ');');
+            $setter->setBody('$this->setRef(' . var_export($tdbmFk->getCacheKey(), true) . ', $object, ' . var_export($tableName, true) . ');');
 
-        if ($this->isSetterProtected()) {
-            $setter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
+            if ($this->isSetterProtected()) {
+                $setter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
+            }
+        } else {
+            $setter = null;
         }
 
         return [$getter, $setter];
@@ -313,6 +317,11 @@ private function isSetterProtected(): bool
         return $this->findAnnotation(Annotation\ProtectedSetter::class) !== null;
     }
 
+    public function isReadOnly(): bool
+    {
+        return $this->findAnnotation(Annotation\ReadOnly::class) !== null;
+    }
+
     /**
      * @param string $type
      * @return null|object

src/Utils/ScalarBeanPropertyDescriptor.php

@@ -207,7 +207,7 @@ public function isPrimaryKey(): bool
     /**
      * Returns the PHP code for getters and setters.
      *
-     * @return MethodGenerator[]
+     * @return (MethodGenerator|null)[]
      */
     public function getGetterSetterCode(): array
     {
@@ -260,26 +260,30 @@ public function getGetterSetterCode(): array
             $getter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
         }
 
-        $setter = new MethodGenerator($columnSetterName);
-        $setterDocBlock = new DocBlockGenerator(sprintf('The setter for the "%s" column.', $this->column->getName()));
-        $setterDocBlock->setTag(new ParamTag($variableName, $types))->setWordWrap(false);
-        $setter->setDocBlock($setterDocBlock);
+        if (!$this->isReadOnly()) {
+            $setter = new MethodGenerator($columnSetterName);
+            $setterDocBlock = new DocBlockGenerator(sprintf('The setter for the "%s" column.', $this->column->getName()));
+            $setterDocBlock->setTag(new ParamTag($variableName, $types))->setWordWrap(false);
+            $setter->setDocBlock($setterDocBlock);
 
-        $parameter = new ParameterGenerator($variableName, $paramType);
-        $setter->setParameter($parameter);
-        $setter->setReturnType('void');
+            $parameter = new ParameterGenerator($variableName, $paramType);
+            $setter->setParameter($parameter);
+            $setter->setReturnType('void');
 
-        $setter->setBody(sprintf(
-            '%s
+            $setter->setBody(sprintf(
+                '%s
 $this->set(%s, $%s, %s);',
-            $resourceTypeCheck,
-            var_export($this->column->getName(), true),
-            $variableName,
-            var_export($this->table->getName(), true)
-        ));
-
-        if ($this->isSetterProtected()) {
-            $setter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
+                $resourceTypeCheck,
+                var_export($this->column->getName(), true),
+                $variableName,
+                var_export($this->table->getName(), true)
+            ));
+
+            if ($this->isSetterProtected()) {
+                $setter->setVisibility(AbstractMemberGenerator::VISIBILITY_PROTECTED);
+            }
+        } else {
+            $setter = null;
         }
 
         return [$getter, $setter];
@@ -415,11 +419,12 @@ private function isSetterProtected(): bool
         return $this->findAnnotation(Annotation\ProtectedSetter::class) !== null;
     }
 
-    /**
-     * @param string $type
-     * @return null|object
-     */
-    private function findAnnotation(string $type)
+    public function isReadOnly(): bool
+    {
+        return $this->findAnnotation(Annotation\ReadOnly::class) !== null;
+    }
+
+    private function findAnnotation(string $type): ?object
     {
         return $this->getAnnotations()->findAnnotation($type);
     }

tests/TDBMAbstractServiceTest.php

@@ -43,6 +43,7 @@
 use TheCodingMachine\TDBM\Utils\Annotation\AddInterface;
 use TheCodingMachine\TDBM\Utils\DefaultNamingStrategy;
 use TheCodingMachine\TDBM\Utils\PathFinder\PathFinder;
+use function stripos;
 
 abstract class TDBMAbstractServiceTest extends TestCase
 {
@@ -415,6 +416,19 @@ private static function initSchema(Connection $connection): void
             $connection->exec($sqlStmt);
         }
 
+        // Let's generate computed columns
+        if ($connection->getDatabasePlatform() instanceof MySqlPlatform && !self::isMariaDb($connection)) {
+            $connection->exec('CREATE TABLE `players` (
+               `id` INT UNSIGNED AUTO_INCREMENT NOT NULL,
+               `player_and_games` JSON NOT NULL,
+               `names_virtual` VARCHAR(20) GENERATED ALWAYS AS (`player_and_games` ->> \'$.name\') NOT NULL COMMENT \'@ReadOnly\',
+               `animal_id` INT COMMENT \'@ReadOnly\',
+               PRIMARY KEY (`id`),
+               FOREIGN KEY (animal_id) REFERENCES animal(id)
+            );
+            ');
+        }
+
         self::insert($connection, 'country', [
             'label' => 'France',
         ]);
@@ -757,4 +771,13 @@ protected static function delete(Connection $connection, string $tableName, arra
         }
         $connection->delete($connection->quoteIdentifier($tableName), $quotedData);
     }
+
+    protected static function isMariaDb(Connection $connection): bool
+    {
+        if (!$connection->getDatabasePlatform() instanceof MySqlPlatform) {
+            return false;
+        }
+        $version = $connection->fetchColumn('SELECT VERSION()');
+        return stripos($version, 'maria') !== false;
+    }
 }

tests/TDBMDaoGeneratorTest.php

@@ -66,6 +66,7 @@
 use TheCodingMachine\TDBM\Test\Dao\Bean\InheritedObjectBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\NodeBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\PersonBean;
+use TheCodingMachine\TDBM\Test\Dao\Bean\PlayerBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\RefNoPrimKeyBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\RoleBean;
 use TheCodingMachine\TDBM\Test\Dao\Bean\StateBean;
@@ -82,6 +83,7 @@
 use TheCodingMachine\TDBM\Test\Dao\InheritedObjectDao;
 use TheCodingMachine\TDBM\Test\Dao\NodeDao;
 use TheCodingMachine\TDBM\Test\Dao\PersonDao;
+use TheCodingMachine\TDBM\Test\Dao\PlayerDao;
 use TheCodingMachine\TDBM\Test\Dao\RefNoPrimKeyDao;
 use TheCodingMachine\TDBM\Test\Dao\RoleDao;
 use TheCodingMachine\TDBM\Test\Dao\StateDao;
@@ -90,6 +92,7 @@
 use TheCodingMachine\TDBM\Utils\PathFinder\PathFinder;
 use TheCodingMachine\TDBM\Utils\TDBMDaoGenerator;
 use Symfony\Component\Process\Process;
+use function get_class;
 
 class TDBMDaoGeneratorTest extends TDBMAbstractServiceTest
 {
@@ -2277,4 +2280,52 @@ public function testFindFromRawSQLOnInheritance(): void
         $this->assertNotNull($objects->first());
         $this->assertEquals(6, $objects->count());
     }
+
+    public function testGeneratedColumnsAreNotPartOfTheConstructor(): void
+    {
+        if (!$this->tdbmService->getConnection()->getDatabasePlatform() instanceof MySqlPlatform || self::isMariaDb($this->tdbmService->getConnection())) {
+            $this->markTestSkipped('ReadOnly column is only tested with MySQL');
+        }
+
+        $dao = new PlayerDao($this->tdbmService);
+
+        $player = new PlayerBean([
+            'id' => 1,
+            'name' => 'Sally',
+            'games_played' =>
+                [
+                    'Battlefield' =>
+                        [
+                            'weapon' => 'sniper rifle',
+                            'rank' => 'Sergeant V',
+                            'level' => 20,
+                        ],
+                    'Crazy Tennis' =>
+                        [
+                            'won' => 4,
+                            'lost' => 1,
+                        ],
+                    'Puzzler' =>
+                        [
+                            'time' => 7,
+                        ],
+                ],
+        ]);
+
+        $dao->save($player);
+
+        $this->assertTrue(true);
+    }
+
+    public function testCanReadVirtualColumn(): void
+    {
+        if (!$this->tdbmService->getConnection()->getDatabasePlatform() instanceof MySqlPlatform || self::isMariaDb($this->tdbmService->getConnection())) {
+            $this->markTestSkipped('ReadOnly column is only tested with MySQL');
+        }
+
+        $dao = new PlayerDao($this->tdbmService);
+
+        $player = $dao->getById(1);
+        $this->assertSame('Sally', $player->getNamesVirtual());
+    }
 }