Skip to content

Commit d7502dc

Browse files
tatevikg1tatevikg1
committed
After review 0
1 parent e91e5ad commit d7502dc

File tree

11 files changed

+205
-181
lines changed

11 files changed

+205
-181
lines changed

‎config/services/managers.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ services:
5151
PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrManager:
5252
autowire: true
5353
autoconfigure: true
54-
arguments:
55-
$dbPrefix: '%database_prefix%'
56-
$dynamicListTablePrefix: '%list_table_prefix%'
54+
55+
Doctrine\DBAL\Schema\AbstractSchemaManager:
56+
factory: ['@doctrine.dbal.default_connection', 'createSchemaManager']
5757

5858
PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrTablesManager:
5959
autowire: true

‎resources/translations/messages.en.xlf‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,10 @@ Thank you.</target>
734734
<source>Conflict: email and foreign key refer to different subscribers.</source>
735735
<target>__Conflict: email and foreign key refer to different subscribers.</target>
736736
</trans-unit>
737+
<trans-unit id="HHtgwG9" resname="Invalid email: %email%">
738+
<source>Invalid email: %email%</source>
739+
<target>__Invalid email: %email%</target>
740+
</trans-unit>
737741
</body>
738742
</file>
739743
</xliff>

‎src/Domain/Subscription/Model/SubscriberAttributeDefinition.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ public function setName(string $name): self
8585
public function setType(?AttributeTypeEnum $type): self
8686
{
8787
if ($type === null) {
88+
$this->type = null;
8889
return $this;
8990
}
9091

‎src/Domain/Subscription/Repository/DynamicListAttrRepository.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@ public function __construct(
2626
$this->fullTablePrefix = $dbPrefix . $dynamicListTablePrefix;
2727
}
2828

29+
public function transactional(callable $callback): mixed
30+
{
31+
$this->connection->beginTransaction();
32+
try {
33+
$result = $callback();
34+
$this->connection->commit();
35+
return $result;
36+
} catch (\Throwable $e) {
37+
$this->connection->rollBack();
38+
throw $e;
39+
}
40+
}
41+
2942
/**
3043
* @param array<int, array<string, mixed>> $rows
3144
* @return DynamicListAttrDto[]
@@ -39,6 +52,61 @@ private function denormalizeAll(array $rows): array
3952
);
4053
}
4154

55+
public function insertOne(string $listTable, DynamicListAttrDto $dto): DynamicListAttrDto
56+
{
57+
if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
58+
throw new InvalidArgumentException('Invalid list table');
59+
}
60+
$table = $this->fullTablePrefix . $listTable;
61+
$this->connection->insert($table, [
62+
'name' => $dto->name,
63+
'listorder' => $dto->listOrder ?? 0,
64+
]);
65+
$id = (int)$this->connection->lastInsertId();
66+
return new DynamicListAttrDto(id: $id, name: $dto->name, listOrder: $dto->listOrder ?? 0);
67+
}
68+
69+
/**
70+
* @param DynamicListAttrDto[] $dtos
71+
* @return DynamicListAttrDto[]
72+
*/
73+
public function insertMany(string $listTable, array $dtos): array
74+
{
75+
$result = [];
76+
foreach ($dtos as $dto) {
77+
$result[] = $this->insertOne($listTable, $dto);
78+
}
79+
return $result;
80+
}
81+
82+
public function updateById(string $listTable, int $id, array $updates): void
83+
{
84+
if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
85+
throw new InvalidArgumentException('Invalid list table');
86+
}
87+
$table = $this->fullTablePrefix . $listTable;
88+
$this->connection->update($table, $updates, ['id' => $id]);
89+
}
90+
91+
public function deleteById(string $listTable, int $id): void
92+
{
93+
if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
94+
throw new InvalidArgumentException('Invalid list table');
95+
}
96+
$table = $this->fullTablePrefix . $listTable;
97+
$this->connection->delete($table, ['id' => $id], ['integer']);
98+
}
99+
100+
public function existsById(string $listTable, int $id): bool
101+
{
102+
if (!preg_match('/^[A-Za-z0-9_]+$/', $listTable)) {
103+
throw new InvalidArgumentException('Invalid list table');
104+
}
105+
$table = $this->fullTablePrefix . $listTable;
106+
$stmt = $this->connection->executeQuery('SELECT COUNT(*) FROM ' . $table . ' WHERE id = :id', ['id' => $id]);
107+
return (bool)$stmt->fetchOne();
108+
}
109+
42110
/**
43111
* @return list<string>
44112
* @throws InvalidArgumentException

‎src/Domain/Subscription/Repository/SubscriberAttributeDefinitionRepository.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public function findOneByName(string $name): ?SubscriberAttributeDefinition
2020

2121
public function existsByTableName(string $tableName): bool
2222
{
23-
return $this->createQueryBuilder('s')
23+
return (bool) $this->createQueryBuilder('s')
2424
->select('COUNT(s.id)')
2525
->where('s.tableName IS NOT NULL')
2626
->andWhere('s.tableName = :tableName')

‎src/Domain/Subscription/Service/Manager/DynamicListAttrManager.php‎

Lines changed: 78 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,21 @@
44

55
namespace PhpList\Core\Domain\Subscription\Service\Manager;
66

7-
use Doctrine\DBAL\Connection;
8-
use Doctrine\DBAL\Exception;
9-
use Doctrine\DBAL\ParameterType;
107
use PhpList\Core\Domain\Subscription\Model\Dto\DynamicListAttrDto;
118
use PhpList\Core\Domain\Subscription\Repository\DynamicListAttrRepository;
129
use RuntimeException;
13-
use Throwable;
1410

1511
class DynamicListAttrManager
1612
{
17-
private string $prefix;
18-
19-
public function __construct(
20-
private readonly DynamicListAttrRepository $dynamicListAttrRepository,
21-
private readonly Connection $connection,
22-
string $dbPrefix = 'phplist_',
23-
string $dynamicListTablePrefix = 'listattr_',
24-
) {
25-
$this->prefix = $dbPrefix . $dynamicListTablePrefix;
13+
public function __construct(private readonly DynamicListAttrRepository $dynamicListAttrRepository,)
14+
{
2615
}
2716

2817
/**
2918
* Seed options into the just-created options table.
3019
*
3120
* @param string $listTable logical table (without global prefix)
3221
* @param DynamicListAttrDto[] $rawOptions
33-
* @throws Exception|Throwable
3422
*/
3523
public function insertOptions(string $listTable, array $rawOptions, array &$usedOrders = []): array
3624
{
@@ -39,8 +27,6 @@ public function insertOptions(string $listTable, array $rawOptions, array &$used
3927
return $result;
4028
}
4129

42-
$fullTable = $this->prefix . $listTable;
43-
4430
$options = [];
4531
$index = 0;
4632
foreach ($rawOptions as $opt) {
@@ -72,72 +58,93 @@ public function insertOptions(string $listTable, array $rawOptions, array &$used
7258
return $result;
7359
}
7460

75-
$this->connection->beginTransaction();
76-
try {
77-
return $this->createFromDtos($fullTable, $unique);
78-
} catch (Throwable $e) {
79-
$this->connection->rollBack();
80-
return [];
81-
}
61+
return $this->dynamicListAttrRepository->transactional(function () use ($listTable, $unique) {
62+
return $this->dynamicListAttrRepository->insertMany($listTable, $unique);
63+
});
8264
}
8365

8466
public function syncOptions(string $getTableName, array $options): array
8567
{
86-
$fullTable = $this->prefix . $getTableName;
8768
$result = [];
8869
$usedOrders = $this->getSetListOrders($options);
8970
[$incomingById, $incomingNew] = $this->splitOptionsSet($options, $usedOrders);
9071

91-
$this->connection->beginTransaction();
92-
try {
93-
[$currentById, $currentByName] = $this->splitCurrentSet($fullTable);
94-
95-
// 1) Updates for items with id
96-
$result = array_merge($result, $this->updateRowsById($incomingById, $currentById, $fullTable));
72+
return $this->dynamicListAttrRepository->transactional(function () use (
73+
$getTableName,
74+
$incomingById,
75+
$incomingNew,
76+
$usedOrders,
77+
&$result
78+
) {
79+
[$currentById, $currentByName] = $this->splitCurrentSet($getTableName);
80+
81+
// Track all lowercase names that should remain after sync (to avoid accidental pruning)
82+
$keptByLowerName = [];
83+
84+
// 1) Updates for items with id and inserts for id-missing-but-present ones
85+
$result = array_merge(
86+
$result,
87+
$this->updateRowsById(
88+
incomingById: $incomingById,
89+
currentById: $currentById,
90+
listTable: $getTableName
91+
)
92+
);
93+
foreach ($incomingById as $dto) {
94+
// Keep target names (case-insensitive)
95+
$keptByLowerName[mb_strtolower($dto->name)] = true;
96+
}
9797

98-
foreach ($incomingNew as $dto) {
99-
if (isset($currentByName[$dto->name])) {
100-
$this->connection->update(
101-
$fullTable,
102-
['name' => $dto->name, 'listorder' => $dto->listOrder],
103-
['id' => $dto->id],
98+
// Handle incoming items without id but matching an existing row by case-insensitive name
99+
foreach ($incomingNew as $key => $dto) {
100+
// $key is already lowercase (set in splitOptionsSet)
101+
if (isset($currentByName[$key])) {
102+
$existing = $currentByName[$key];
103+
$this->dynamicListAttrRepository->updateById(
104+
listTable: $getTableName,
105+
id: (int)$existing->id,
106+
updates: ['name' => $dto->name, 'listorder' => $dto->listOrder]
104107
);
105-
$result[] = $dto;
106-
unset($incomingNew[$dto->name]);
108+
$result[] = new DynamicListAttrDto(id: $existing->id, name: $dto->name, listOrder: $dto->listOrder);
109+
// Mark as kept and remove from incomingNew so it won't be re-inserted
110+
$keptByLowerName[$key] = true;
111+
unset($incomingNew[$key]);
107112
}
108113
}
109114

110-
// 2) Inserts for new items (no id)
111-
$result = array_merge($result, $this->insertOptions($getTableName, $incomingNew, $usedOrders));
115+
// 2) Inserts for truly new items (no id and no existing match)
116+
// Mark remaining new keys as kept, then insert them
117+
foreach (array_keys($incomingNew) as $lowerKey) {
118+
$keptByLowerName[$lowerKey] = true;
119+
}
120+
$result = array_merge(
121+
$result,
122+
$this->insertOptions(
123+
listTable: $getTableName,
124+
rawOptions: $incomingNew,
125+
usedOrders: $usedOrders
126+
)
127+
);
112128

113-
// 3) Prune: rows not present in input
114-
$missing = array_diff_key($currentByName, $incomingNew);
115-
foreach ($missing as $row) {
116-
// This row is not in input → consider removal
117-
if (!$this->optionHasReferences($getTableName, $row->id)) {
118-
$this->connection->delete($fullTable, ['id' => $row->id], ['integer']);
119-
} else {
120-
$result[] = $row;
129+
// 3) Prune: rows not present in the intended final set (case-insensitive)
130+
foreach ($currentByName as $lowerKey => $row) {
131+
if (!isset($keptByLowerName[$lowerKey])) {
132+
// This row is not in desired input → consider removal
133+
if (!$this->optionHasReferences($getTableName, (int)$row->id)) {
134+
$this->dynamicListAttrRepository->deleteById($getTableName, (int)$row->id);
135+
} else {
136+
$result[] = $row;
137+
}
121138
}
122139
}
123140

124-
$this->connection->commit();
125-
} catch (Throwable $e) {
126-
$this->connection->rollBack();
127-
throw $e;
128-
}
129-
130-
return $result;
141+
return $result;
142+
});
131143
}
132144

133145
private function optionHasReferences(string $listTable, int $id): bool
134146
{
135-
$fullTable = $this->prefix . $listTable;
136-
$stmt = $this->connection->executeQuery(
137-
'SELECT COUNT(*) FROM ' . $fullTable . ' WHERE id = :id',
138-
['id' => $id]
139-
);
140-
return (bool)$stmt->fetchOne();
147+
return $this->dynamicListAttrRepository->existsById($listTable, $id);
141148
}
142149

143150
private function getSetListOrders(array $options): array
@@ -185,12 +192,12 @@ private function splitOptionsSet(array $options, array &$usedOrders): array
185192
return [$incomingById, $incomingNew];
186193
}
187194

188-
private function splitCurrentSet(string $fullTable): array
195+
private function splitCurrentSet(string $listTable): array
189196
{
190197
$currentById = [];
191198
$currentByName = [];
192199

193-
$rows = $this->dynamicListAttrRepository->getAll($fullTable);
200+
$rows = $this->dynamicListAttrRepository->getAll($listTable);
194201
foreach ($rows as $listAttrDto) {
195202
$currentById[$listAttrDto->id] = $listAttrDto;
196203
$currentByName[mb_strtolower($listAttrDto->name)] = $listAttrDto;
@@ -199,29 +206,23 @@ private function splitCurrentSet(string $fullTable): array
199206
return [$currentById, $currentByName];
200207
}
201208

202-
private function updateRowsById(array $incomingById, array $currentById, string $fullTable): array
209+
private function updateRowsById(array $incomingById, array $currentById, string $listTable): array
203210
{
204211
$result = [];
205212

206-
$insertSql = 'INSERT INTO ' . $fullTable . ' (name, listorder) VALUES (:name, :listOrder)';
207-
$insertStmt = $this->connection->prepare($insertSql);
208-
209213
foreach ($incomingById as $dto) {
210214
if (!isset($currentById[$dto->id])) {
211-
$insertStmt->bindValue('name', $dto->name, ParameterType::STRING);
212-
$insertStmt->bindValue('listorder', $dto->listOrder, ParameterType::INTEGER);
213-
$insertStmt->executeStatement();
214-
215-
$result[] = new DynamicListAttrDto(
216-
id: (int) $this->connection->lastInsertId(),
217-
name: $dto->name,
218-
listOrder: $dto->listOrder
215+
// Unexpected: incoming has id but the current table does not — insert a new row
216+
$inserted = $this->dynamicListAttrRepository->insertOne(
217+
listTable: $listTable,
218+
dto: new DynamicListAttrDto(id: null, name: $dto->name, listOrder: $dto->listOrder)
219219
);
220+
$result[] = $inserted;
220221
} else {
221222
$cur = $currentById[$dto->id];
222223
$updates = [];
223224
if ($cur->name !== $dto->name) {
224-
$nameExists = $this->dynamicListAttrRepository->existsByName($fullTable, $dto->name);
225+
$nameExists = $this->dynamicListAttrRepository->existsByName($listTable, $dto->name);
225226
if ($nameExists) {
226227
throw new RuntimeException('Option name ' . $dto->name . ' already exists.');
227228
}
@@ -232,36 +233,12 @@ private function updateRowsById(array $incomingById, array $currentById, string
232233
}
233234

234235
if ($updates) {
235-
$this->connection->update($fullTable, $updates, ['id' => $dto->id]);
236+
$this->dynamicListAttrRepository->updateById($listTable, (int)$dto->id, $updates);
236237
}
237238
$result[] = $dto;
238239
}
239240
}
240241

241242
return $result;
242243
}
243-
244-
private function createFromDtos(string $fullTable, array $unique): array
245-
{
246-
$sql = 'INSERT INTO ' . $fullTable . ' (name, listorder) VALUES (:name, :listOrder)';
247-
$stmt = $this->connection->prepare($sql);
248-
249-
$result = [];
250-
foreach ($unique as $opt) {
251-
$stmt->bindValue('name', $opt->name, ParameterType::STRING);
252-
$stmt->bindValue('listOrder', $opt->listOrder, ParameterType::INTEGER);
253-
$stmt->executeStatement();
254-
255-
$inserted = new DynamicListAttrDto(
256-
id: (int) $this->connection->lastInsertId(),
257-
name: $opt->name,
258-
listOrder: $opt->listOrder
259-
);
260-
261-
$result[] = $inserted;
262-
}
263-
$this->connection->commit();
264-
265-
return $result;
266-
}
267244
}

0 commit comments

Comments
 (0)