Refactor attribute type system to use enums and manage dynamic list attributes table names

composer.json

@@ -77,7 +77,8 @@
         "symfony/lock": "^6.4",
         "webklex/php-imap": "^6.2",
         "ext-imap": "*",
-        "tatevikgr/rss-feed": "dev-main"
+        "tatevikgr/rss-feed": "dev-main",
+        "ext-pdo": "*"
     },
     "require-dev": {
         "phpunit/phpunit": "^9.5",

config/parameters.yml.dist

@@ -23,6 +23,8 @@ parameters:
     env(PHPLIST_DATABASE_PASSWORD): 'phplist'
     database_prefix: '%%env(DATABASE_PREFIX)%%'
     env(DATABASE_PREFIX): 'phplist_'
+    list_table_prefix: '%%env(LIST_TABLE_PREFIX)%%'
+    env(LIST_TABLE_PREFIX): 'listattr_'
 
     # Email configuration
     app.mailer_from: '%%env(MAILER_FROM)%%'

config/services/managers.yml

@@ -48,6 +48,21 @@ services:
     autowire: true
     autoconfigure: true
 
+  PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrManager:
+    autowire: true
+    autoconfigure: true
+    arguments:
+      $dbPrefix: '%database_prefix%'
+      $dynamicListTablePrefix: '%list_table_prefix%'
+
+  PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrTablesManager:
+    autowire: true
+    autoconfigure: true
+    public: true
+    arguments:
+      $dbPrefix: '%database_prefix%'
+      $dynamicListTablePrefix: '%list_table_prefix%'
+
   PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager:
     autowire: true
     autoconfigure: true

config/services/mappers.yml

@@ -4,10 +4,10 @@ services:
     autoconfigure: true
     public: false
 
-  PhpList\Core\Domain\Subscription\Service\CsvRowToDtoMapper:
+  PhpList\Core\Domain\Subscription\Service\ArrayToDtoMapper:
     autowire: true
     autoconfigure: true
 
-  PhpList\Core\Domain\Subscription\Service\CsvImporter:
+  PhpList\Core\Domain\Subscription\Service\CsvToDtoImporter:
     autowire: true
     autoconfigure: true

config/services/repositories.yml

@@ -70,7 +70,8 @@ services:
   PhpList\Core\Domain\Subscription\Repository\DynamicListAttrRepository:
     autowire: true
     arguments:
-      $prefix: '%database_prefix%'
+      $dbPrefix: '%database_prefix%'
+      $dynamicListTablePrefix: '%list_table_prefix%'
   PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository:
     parent: PhpList\Core\Domain\Common\Repository\AbstractRepository
     arguments:

src/Domain/Subscription/Exception/AttributeTypeChangeNotAllowed.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Exception;
+
+use RuntimeException;
+
+class AttributeTypeChangeNotAllowed extends RuntimeException
+{
+    public function __construct(string $oldType, string $newType)
+    {
+        parent::__construct(sprintf(
+            'attribute_definition.type_change_not_allowed:%s->%s',
+            $oldType,
+            $newType
+        ));
+    }
+}

src/Domain/Subscription/Model/AttributeTypeEnum.php

@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Model;
+
+use PhpList\Core\Domain\Subscription\Exception\AttributeTypeChangeNotAllowed;
+
+enum AttributeTypeEnum: string
+{
+    case Text = 'text';
+    case Number = 'number';
+    case Date = 'date';
+    case Select = 'select';
+    case Checkbox = 'checkbox';
+    case MultiSelect = 'multiselect';
+    case CheckboxGroup = 'checkboxgroup';
+    case Radio = 'radio';
+
+    public function equals(self $other): bool
+    {
+        return $this === $other;
+    }
+
+    public function isMultiValued(): bool
+    {
+        return match ($this) {
+            self::Select,
+            self::Checkbox,
+            self::MultiSelect,
+            self::Radio,
+            self::CheckboxGroup => true,
+            default => false,
+        };
+    }
+
+    public function canTransitionTo(self $toType): bool
+    {
+        if ($this === $toType) {
+            return true;
+        }
+
+        if ($this->isMultiValued() !== $toType->isMultiValued()) {
+            return false;
+        }
+
+        return true;
+    }
+
+    public function assertTransitionAllowed(self $toType): void
+    {
+        if (!$this->canTransitionTo($toType)) {
+            throw new AttributeTypeChangeNotAllowed($this->value, $toType->value);
+        }
+    }
+}

src/Domain/Subscription/Model/Dto/AttributeDefinitionDto.php

@@ -4,18 +4,26 @@
 
 namespace PhpList\Core\Domain\Subscription\Model\Dto;
 
+use InvalidArgumentException;
+use PhpList\Core\Domain\Subscription\Model\AttributeTypeEnum;
+
 class AttributeDefinitionDto
 {
     /**
      * @SuppressWarnings("BooleanArgumentFlag")
+     * @param DynamicListAttrDto[] $options
+     * @throws InvalidArgumentException
      */
     public function __construct(
         public readonly string $name,
-        public readonly ?string $type = null,
+        public readonly ?AttributeTypeEnum $type = null,
         public readonly ?int $listOrder = null,
         public readonly ?string $defaultValue = null,
         public readonly ?bool $required = false,
-        public readonly ?string $tableName = null,
+        public readonly array $options = [],
     ) {
+        if (trim($this->name) === '') {
+            throw new InvalidArgumentException('Name cannot be empty');
+        }
     }
 }

src/Domain/Subscription/Model/Dto/DynamicListAttrDto.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Model\Dto;
+
+use InvalidArgumentException;
+use Symfony\Component\Serializer\Annotation\SerializedName;
+
+class DynamicListAttrDto
+{
+    public readonly ?int $id;
+
+    public readonly string $name;
+
+    #[SerializedName('listorder')]
+    public readonly ?int $listOrder;
+
+    public function __construct(
+        ?int $id,
+        string $name,
+        ?int $listOrder = null
+    ) {
+        $trimmed = trim($name);
+        if ($trimmed === '') {
+            throw new InvalidArgumentException('Option name cannot be empty');
+        }
+
+        $this->id = $id;
+        $this->name = $trimmed;
+        $this->listOrder = $listOrder;
+    }
+}

src/Domain/Subscription/Model/SubscriberAttributeDefinition.php

@@ -48,9 +48,10 @@ public function getName(): string
         return $this->name;
     }
 
-    public function getType(): ?string
+    /** @SuppressWarnings(PHPMD.StaticAccess) */
+    public function getType(): ?AttributeTypeEnum
     {
-        return $this->type;
+        return $this->type === null ? null : AttributeTypeEnum::from($this->type);
     }
 
     public function getListOrder(): ?int
@@ -80,9 +81,19 @@ public function setName(string $name): self
         return $this;
     }
 
-    public function setType(?string $type): self
+    /** @SuppressWarnings(PHPMD.StaticAccess) */
+    public function setType(?AttributeTypeEnum $type): self
     {
-        $this->type = $type;
+        if ($type === null) {
+            return $this;
+        }
+
+        if ($this->type !== null) {
+            $currentType = AttributeTypeEnum::from($this->type);
+            $currentType->assertTransitionAllowed($type);
+        }
+        $this->type = $type->value;
+
         return $this;
     }
 

src/Domain/Subscription/Repository/DynamicListAttrRepository.php

@@ -6,14 +6,37 @@
 
 use Doctrine\DBAL\ArrayParameterType;
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Exception;
 use InvalidArgumentException;
+use PDO;
+use PhpList\Core\Domain\Subscription\Model\Dto\DynamicListAttrDto;
+use Symfony\Component\Serializer\Exception\ExceptionInterface;
+use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
 
 class DynamicListAttrRepository
 {
+    private string $fullTablePrefix;
+
     public function __construct(
         private readonly Connection $connection,
-        private readonly string $prefix = 'phplist_'
+        private readonly DenormalizerInterface $serializer,
+        string $dbPrefix = 'phplist_',
+        string $dynamicListTablePrefix = 'listattr_',
     ) {
+        $this->fullTablePrefix = $dbPrefix . $dynamicListTablePrefix;
+    }
+
+    /**
+     * @param array<int, array<string, mixed>> $rows
+     * @return DynamicListAttrDto[]
+     * @throws ExceptionInterface
+     */
+    private function denormalizeAll(array $rows): array
+    {
+        return array_map(
+            fn(array $row) => $this->serializer->denormalize($row, DynamicListAttrDto::class),
+            $rows
+        );
     }
 
     /**
@@ -30,7 +53,7 @@ public function fetchOptionNames(string $listTable, array $ids): array
             throw new InvalidArgumentException('Invalid list table');
         }
 
-        $table = $this->prefix . 'listattr_' . $listTable;
+        $table = $this->fullTablePrefix . $listTable;
 
         $queryBuilder = $this->connection->createQueryBuilder();
         $queryBuilder->select('name')
@@ -47,7 +70,7 @@ public function fetchSingleOptionName(string $listTable, int $id): ?string
             throw new InvalidArgumentException('Invalid list table');
         }
 
-        $table = $this->prefix . 'listattr_' . $listTable;
+        $table = $this->fullTablePrefix . $listTable;
 
         $queryBuilder = $this->connection->createQueryBuilder();
         $queryBuilder->select('name')
@@ -59,4 +82,39 @@ public function fetchSingleOptionName(string $listTable, int $id): ?string
 
         return $val === false ? null : (string) $val;
     }
+
+    public function existsByName(string $listTable, string $name): bool
+    {
+        if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
+            throw new InvalidArgumentException('Invalid list table');
+        }
+
+        $table = $this->fullTablePrefix . $listTable;
+
+        try {
+            $sql = 'SELECT 1 FROM ' . $table . ' WHERE LOWER(name) = LOWER(?) LIMIT 1';
+            $result = $this->connection->fetchOne($sql, [$name], [PDO::PARAM_STR]);
+
+            return $result !== false;
+        } catch (Exception $e) {
+            throw $e;
+        }
+    }
+
+    public function getAll(string $listTable): array
+    {
+        if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
+            throw new InvalidArgumentException('Invalid list table');
+        }
+
+        $table = $this->fullTablePrefix . $listTable;
+
+        $rows = $this->connection->createQueryBuilder()
+            ->select('id', 'name', 'listorder')
+            ->from($table)
+            ->executeQuery()
+            ->fetchAllAssociative();
+
+        return $this->denormalizeAll($rows);
+    }
 }

src/Domain/Subscription/Repository/SubscriberAttributeDefinitionRepository.php

@@ -17,4 +17,15 @@ public function findOneByName(string $name): ?SubscriberAttributeDefinition
     {
         return $this->findOneBy(['name' => $name]);
     }
+
+    public function existsByTableName(string $tableName): bool
+    {
+        return $this->createQueryBuilder('s')
+            ->select('COUNT(s.id)')
+            ->where('s.tableName IS NOT NULL')
+            ->andWhere('s.tableName = :tableName')
+            ->setParameter('tableName', $tableName)
+            ->getQuery()
+            ->getSingleScalarResult();
+    }
 }

src/Domain/Subscription/Service/CsvRowToDtoMapper.php → src/Domain/Subscription/Service/ArrayToDtoMapper.php

@@ -6,7 +6,7 @@
 
 use PhpList\Core\Domain\Subscription\Model\Dto\ImportSubscriberDto;
 
-class CsvRowToDtoMapper
+class ArrayToDtoMapper
 {
     private const FK_HEADER = 'foreignkey';
     private const KNOWN_HEADERS = [

src/Domain/Subscription/Service/CsvImporter.php → src/Domain/Subscription/Service/CsvToDtoImporter.php

@@ -11,10 +11,10 @@
 use Symfony\Contracts\Translation\TranslatorInterface;
 use Throwable;
 
-class CsvImporter
+class CsvToDtoImporter
 {
     public function __construct(
-        private readonly CsvRowToDtoMapper $rowMapper,
+        private readonly ArrayToDtoMapper $rowMapper,
         private readonly ValidatorInterface $validator,
         private readonly TranslatorInterface $translator,
     ) {
@@ -25,7 +25,7 @@ public function __construct(
      * @return array{valid: ImportSubscriberDto[], errors: array<int, array<string>>}
      * @throws CsvException
      */
-    public function import(string $csvFilePath): array
+    public function parseAndValidate(string $csvFilePath): array
     {
         $reader = Reader::createFromPath($csvFilePath, 'r');
         $reader->setHeaderOffset(0);