Remove Field::getTypeObject() method (#1072)

atk4 · Oct 24, 2022 · 7c09563 · 7c09563
1 parent 0c6bce5
commit 7c09563
Show file tree

Hide file tree

Showing 13 changed files with 41 additions and 53 deletions.
diff --git a/docs/model.rst b/docs/model.rst
@@ -284,7 +284,7 @@ calculated by your callback method right after individual record is loaded by th
     }, 'type' => 'float']);
 
 .. important:: always use argument `$m` instead of `$this` inside your callbacks. If model is to be
-   `clone`d, the code relying on `$this` would reference original model, but the code using
+   cloned, the code relying on `$this` would reference original model, but the code using
    `$m` will properly address the model which triggered the callback.
 
 This can also be useful for calculating relative times::

diff --git a/docs/persistence/sql/expressions.rst b/docs/persistence/sql/expressions.rst
@@ -69,7 +69,7 @@ Parameters
 
 Because some values are un-safe to use in the query and can contain dangerous
 values they are kept outside of the SQL query string and are using
-`PDO's bindParam <http://php.net/manual/en/pdostatement.bindparam.php>`_
+`PDO's bindValue <https://www.php.net/manual/en/pdostatement.bindvalue.php>`_
 instead. DSQL can consist of multiple objects and each object may have
 some parameters. During `rendering`_ those parameters are joined together to
 produce one complete query.

diff --git a/docs/quickstart.rst b/docs/quickstart.rst
@@ -147,7 +147,7 @@ following categories:
 Persistence
 ^^^^^^^^^^^
 
-When you create instance of a model (`new Model`) you need to specify
+When you create instance of a model (`new Model()`) you need to specify
 :php:class:`Persistence` as a parameter. If you don't you can still use
 the model, but it won't be able to :php:meth:`Model::load()` or
 :php:meth:`Model::save()` data.

diff --git a/src/Field.php b/src/Field.php
@@ -35,6 +35,10 @@ class Field implements Expressionable
     public function __construct(array $defaults = [])
     {
         $this->setDefaults($defaults);
+
+        if (!(new \ReflectionProperty($this, 'type'))->isInitialized($this)) {
+            $this->type = 'string';
+        }
     }
 
     /**
@@ -56,18 +60,16 @@ public function setDefaults(array $properties, bool $passively = false): self
     {
         $this->_setDefaults($properties, $passively);
 
-        $this->getTypeObject(); // assert type exists
-
-        return $this;
-    }
+        // assert type exists
+        if (isset($properties['type'])) {
+            if ($this->type === 'array') { // remove in v4.1
+                throw new Exception('Atk4 "array" type is no longer supported, originally, it serialized value to JSON, to keep this behaviour, use "json" type');
+            }
 
-    public function getTypeObject(): Type
-    {
-        if ($this->type === 'array') { // remove in 2022-mar
-            throw new Exception('Atk4 "array" type is no longer supported, originally, it serialized value to JSON, to keep this behaviour, use "json" type');
+            Type::getType($this->type);
         }
 
-        return Type::getType($this->type ?? 'string');
+        return $this;
     }
 
     /**
@@ -133,16 +135,13 @@ public function __construct()
      */
     public function normalize($value)
     {
-        $this->getTypeObject(); // assert type exists
-
         try {
             if ($this->issetOwner() && $this->getOwner()->hook(Model::HOOK_NORMALIZE, [$this, $value]) === false) {
                 return $value;
             }
 
             if (is_string($value)) {
                 switch ($this->type) {
-                    case null:
                     case 'string':
                         $value = trim(preg_replace('~\r?\n|\r|\s~', ' ', $value)); // remove all line-ends and trim
 
@@ -178,18 +177,13 @@ public function normalize($value)
                 }
             } elseif ($value !== null) {
                 switch ($this->type) {
-                    case null:
                     case 'string':
                     case 'text':
                     case 'integer':
                     case 'float':
                     case 'atk4_money':
                         if (is_bool($value)) {
-                            if ($this->type === 'boolean') {
-                                $value = $value ? '1' : '0';
-                            } else {
-                                throw new Exception('Must not be boolean type');
-                            }
+                            throw new Exception('Must not be boolean type');
                         } elseif (is_int($value)) {
                             $value = (string) $value;
                         } elseif (is_float($value)) {
@@ -217,7 +211,6 @@ public function normalize($value)
             }
 
             switch ($this->type) {
-                case null:
                 case 'string':
                 case 'text':
                     if ($this->required && !$value) {

diff --git a/src/Model/FieldPropertiesTrait.php b/src/Model/FieldPropertiesTrait.php
@@ -14,7 +14,7 @@ trait FieldPropertiesTrait
     public bool $neverSave = false;
 
     /** DBAL type registered in \Doctrine\DBAL\Types\Type. */
-    public ?string $type = null;
+    public string $type;
     /** Nullable field can be null, otherwise the value must be set, even if it is an empty value. */
     public bool $nullable = true;
     /** Required field must have non-empty value. A null value is considered empty too. */

diff --git a/src/Persistence.php b/src/Persistence.php
@@ -11,6 +11,7 @@
 use Atk4\Core\HookTrait;
 use Atk4\Core\NameTrait;
 use Doctrine\DBAL\Platforms;
+use Doctrine\DBAL\Types\Type;
 
 abstract class Persistence
 {
@@ -408,7 +409,7 @@ protected function _typecastSaveField(Field $field, $value)
 
         // native DBAL DT types have no microseconds support
         if (in_array($field->type, ['datetime', 'date', 'time'], true)
-            && str_starts_with(get_class($field->getTypeObject()), 'Doctrine\DBAL\Types\\')) {
+            && str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')) {
             if ($value === '') {
                 return null;
             } elseif (!$value instanceof \DateTimeInterface) {
@@ -426,7 +427,7 @@ protected function _typecastSaveField(Field $field, $value)
             return $value;
         }
 
-        $res = $field->getTypeObject()->convertToDatabaseValue($value, $this->getDatabasePlatform());
+        $res = Type::getType($field->type)->convertToDatabaseValue($value, $this->getDatabasePlatform());
         if (is_resource($res) && get_resource_type($res) === 'stream') {
             $res = stream_get_contents($res);
         }
@@ -451,7 +452,7 @@ protected function _typecastLoadField(Field $field, $value)
 
         // native DBAL DT types have no microseconds support
         if (in_array($field->type, ['datetime', 'date', 'time'], true)
-            && str_starts_with(get_class($field->getTypeObject()), 'Doctrine\DBAL\Types\\')) {
+            && str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')) {
             $format = ['date' => 'Y-m-d', 'datetime' => 'Y-m-d H:i:s', 'time' => 'H:i:s'][$field->type];
             if (str_contains($value, '.')) { // time possibly with microseconds, otherwise invalid format
                 $format = preg_replace('~(?<=H:i:s)(?![. ]*u)~', '.u', $format);
@@ -473,7 +474,7 @@ protected function _typecastLoadField(Field $field, $value)
             return $value;
         }
 
-        $res = $field->getTypeObject()->convertToPHPValue($value, $this->getDatabasePlatform());
+        $res = Type::getType($field->type)->convertToPHPValue($value, $this->getDatabasePlatform());
         if (is_resource($res) && get_resource_type($res) === 'stream') {
             $res = stream_get_contents($res);
         }

diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php
@@ -181,13 +181,6 @@ public function add(Model $model, array $defaults = []): void
             $model->table = 'data';
         }
 
-        if ($model->idField) {
-            $f = $model->getField($model->idField);
-            if ($f->type === null) {
-                $f->type = 'integer';
-            }
-        }
-
         if (!is_object($model->table)) {
             $this->seedData($model);
         }

diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php
@@ -629,7 +629,7 @@ public function typecastSaveField(Field $field, $value)
     {
         $value = parent::typecastSaveField($field, $value);
 
-        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->getTypeObject())) {
+        if ($value !== null && $this->binaryTypeIsEncodeNeeded($field->type)) {
             $value = $this->binaryTypeValueEncode($value);
         }
 
@@ -640,7 +640,7 @@ public function typecastLoadField(Field $field, $value)
     {
         $value = parent::typecastLoadField($field, $value);
 
-        if ($value !== null && $this->binaryTypeIsDecodeNeeded($field->getTypeObject(), $value)) {
+        if ($value !== null && $this->binaryTypeIsDecodeNeeded($field->type, $value)) {
             $value = $this->binaryTypeValueDecode($value);
         }
 

diff --git a/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php b/src/Persistence/Sql/BinaryTypeCompatibilityTypecastTrait.php
@@ -49,7 +49,7 @@ private function binaryTypeValueDecode(string $value): string
         return $res;
     }
 
-    private function binaryTypeIsEncodeNeeded(Type $type): bool
+    private function binaryTypeIsEncodeNeeded(string $type): bool
     {
         // binary values for PostgreSQL and MSSQL databases are stored natively, but we need
         // to encode first to hold the binary type info for PDO parameter type binding
@@ -59,7 +59,7 @@ private function binaryTypeIsEncodeNeeded(Type $type): bool
             || $platform instanceof SQLServerPlatform
             || $platform instanceof OraclePlatform
         ) {
-            if (in_array($type->getName(), ['binary', 'blob'], true)) {
+            if (in_array($type, ['binary', 'blob'], true)) {
                 return true;
             }
         }
@@ -70,7 +70,7 @@ private function binaryTypeIsEncodeNeeded(Type $type): bool
     /**
      * @param scalar $value
      */
-    private function binaryTypeIsDecodeNeeded(Type $type, $value): bool
+    private function binaryTypeIsDecodeNeeded(string $type, $value): bool
     {
         if ($this->binaryTypeIsEncodeNeeded($type)) {
             // always decode for Oracle platform to assert the value is always encoded,

diff --git a/src/Persistence/Sql/Oracle/Query.php b/src/Persistence/Sql/Oracle/Query.php
@@ -48,8 +48,8 @@ protected function _subrenderCondition(array $row): string
         }
 
         if (count($row) >= 2 && $field instanceof Field
-            && in_array($field->getTypeObject()->getName(), ['text', 'blob'], true)) {
-            if ($field->getTypeObject()->getName() === 'text') {
+            && in_array($field->type, ['text', 'blob'], true)) {
+            if ($field->type === 'text') {
                 $field = $this->expr('LOWER([])', [$field]);
                 $value = $this->expr('LOWER([])', [$value]);
             }

diff --git a/src/Reference/HasOne.php b/src/Reference/HasOne.php
@@ -25,7 +25,8 @@ protected function init(): void
             $this->ourField = $this->link;
         }
 
-        if ($this->type === null) { // different default value than in Model\FieldPropertiesTrait
+        // for references use "integer" as a default type
+        if (!(new \ReflectionProperty($this, 'type'))->isInitialized($this)) {
             $this->type = 'integer';
         }
 
@@ -40,7 +41,7 @@ protected function init(): void
             foreach ($fieldPropsRefl as $fieldPropRefl) {
                 $v = $this->{$fieldPropRefl->getName()};
                 $vDefault = \PHP_MAJOR_VERSION < 8
-                    ? $fieldPropRefl->getDeclaringClass()->getDefaultProperties()[$fieldPropRefl->getName()]
+                    ? ($fieldPropRefl->getDeclaringClass()->getDefaultProperties()[$fieldPropRefl->getName()] ?? null)
                     : (null ?? $fieldPropRefl->getDefaultValue()); // @phpstan-ignore-line for PHP 7.x
                 if ($v !== $vDefault) {
                     $fieldSeed[$fieldPropRefl->getName()] = $v;

diff --git a/src/Schema/Migrator.php b/src/Schema/Migrator.php
@@ -228,31 +228,31 @@ protected function stripDatabaseFromTableName(string $tableName): string
      */
     public function field(string $fieldName, array $options = []): self
     {
-        if (($options['type'] ?? null) === null) {
-            $options['type'] = 'string';
-        } elseif ($options['type'] === 'time' && $this->getDatabasePlatform() instanceof OraclePlatform) {
-            $options['type'] = 'string';
+        $type = $options['type'] ?? 'string';
+        unset($options['type']);
+        if ($type === 'time' && $this->getDatabasePlatform() instanceof OraclePlatform) {
+            $type = 'string';
         }
 
         $refType = $options['ref_type'] ?? self::REF_TYPE_NONE;
         unset($options['ref_type']);
 
-        $column = $this->table->addColumn($this->getDatabasePlatform()->quoteSingleIdentifier($fieldName), $options['type']);
+        $column = $this->table->addColumn($this->getDatabasePlatform()->quoteSingleIdentifier($fieldName), $type);
 
         if (($options['nullable'] ?? true) && $refType !== self::REF_TYPE_PRIMARY) {
             $column->setNotnull(false);
         }
 
-        if ($column->getType()->getName() === 'integer' && $refType !== self::REF_TYPE_NONE) {
+        if ($type === 'integer' && $refType !== self::REF_TYPE_NONE) {
             $column->setUnsigned(true);
         }
 
         // TODO remove, hack for createForeignKey so ID columns are unsigned
-        if ($column->getType()->getName() === 'integer' && str_ends_with($fieldName, '_id')) {
+        if ($type === 'integer' && str_ends_with($fieldName, '_id')) {
             $column->setUnsigned(true);
         }
 
-        if (in_array($column->getType()->getName(), ['string', 'text'], true)) {
+        if (in_array($type, ['string', 'text'], true)) {
             if ($this->getDatabasePlatform() instanceof SqlitePlatform) {
                 $column->setPlatformOption('collation', 'NOCASE');
             }
@@ -303,7 +303,7 @@ public function setModel(Model $model): Model
             }
 
             $options = [
-                'type' => $refype !== self::REF_TYPE_NONE && $persistField->type === null ? 'integer' : $persistField->type,
+                'type' => $persistField->type,
                 'ref_type' => $refype,
                 'nullable' => ($field->nullable && !$field->required) || ($persistField->nullable && !$persistField->required),
             ];

diff --git a/tests/ReferenceSqlTest.php b/tests/ReferenceSqlTest.php
@@ -319,7 +319,7 @@ public function testAggregateHasMany(): void
         static::assertSame('atk4_money', $i->getField('total_vat')->type);
 
         // type was not set and is not inherited
-        static::assertNull($i->getField('total_net')->type);
+        static::assertSame('string', $i->getField('total_net')->type);
 
         static::assertSame(40.0, (float) $i->get('total_net'));
         static::assertSame(9.2, $i->get('total_vat'));