Skip to content

Commit bdeae0f

Browse files
NattfarinnNattfarinn
committed
fix: Code review
1 parent 544fccd commit bdeae0f

File tree

1 file changed

+66
-82
lines changed

1 file changed

+66
-82
lines changed

‎src/bundle/Core/Command/VirtualFieldDuplicateFixCommand.php

+66-82
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
namespace Ibexa\Bundle\Core\Command;
1010

1111
use Doctrine\DBAL\Connection;
12-
use Exception;
1312
use Symfony\Component\Console\Command\Command;
1413
use Symfony\Component\Console\Input\InputInterface;
1514
use Symfony\Component\Console\Input\InputOption;
@@ -26,14 +25,17 @@ final class VirtualFieldDuplicateFixCommand extends Command
2625

2726
private const DEFAULT_SLEEP = 0;
2827

28+
protected static $defaultName = 'ibexa:content:remove-duplicate-fields';
29+
30+
protected static $defaultDescription = 'Removes duplicate fields created as a result of faulty IBX-5388 performance fix.';
31+
2932
/** @var \Doctrine\DBAL\Connection */
3033
private $connection;
3134

3235
public function __construct(
3336
Connection $connection
3437
) {
35-
parent::__construct('ibexa:content:remove-duplicate-fields');
36-
$this->setDescription('Removes duplicate fields created as a result of faulty IBX-5388 performance fix.');
38+
parent::__construct();
3739

3840
$this->connection = $connection;
3941
}
@@ -43,33 +45,26 @@ public function configure(): void
4345
$this->addOption(
4446
'batch-size',
4547
'b',
46-
InputOption::VALUE_OPTIONAL,
48+
InputOption::VALUE_REQUIRED,
4749
'Number of attributes affected per iteration',
4850
self::DEFAULT_BATCH_SIZE
4951
);
5052

5153
$this->addOption(
5254
'max-iterations',
5355
'i',
54-
InputOption::VALUE_OPTIONAL,
56+
InputOption::VALUE_REQUIRED,
5557
'Max iterations count (default or -1: unlimited)',
5658
self::MAX_ITERATIONS_UNLIMITED
5759
);
5860

5961
$this->addOption(
6062
'sleep',
6163
's',
62-
InputOption::VALUE_OPTIONAL,
64+
InputOption::VALUE_REQUIRED,
6365
'Wait between iterations, in milliseconds',
6466
self::DEFAULT_SLEEP
6567
);
66-
67-
$this->addOption(
68-
'force',
69-
'f',
70-
InputOption::VALUE_NONE,
71-
'Force operation (implies non-interactive mode)',
72-
);
7368
}
7469

7570
protected function execute(InputInterface $input, OutputInterface $output): int
@@ -78,11 +73,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
7873
$stopwatch = new Stopwatch(true);
7974
$stopwatch->start('total', 'command');
8075

81-
$force = $input->getOption('force');
82-
if ($force) {
83-
$input->setInteractive(false);
84-
}
85-
8676
$batchSize = (int)$input->getOption('batch-size');
8777
if ($batchSize === 0) {
8878
$style->warning('Batch size is set to 0. Nothing to do.');
@@ -99,67 +89,65 @@ protected function execute(InputInterface $input, OutputInterface $output): int
9989

10090
$sleep = (int)$input->getOption('sleep');
10191

102-
try {
103-
$totalCount = $this->getDuplicatedAttributeTotalCount($style, $stopwatch);
92+
$totalCount = $this->getDuplicatedAttributeTotalCount($style, $stopwatch);
10493

105-
if ($totalCount > 0) {
106-
$confirmation = $this->askForConfirmation($style);
107-
if (!$confirmation && !$force) {
108-
$style->info('Confirmation rejected. Terminating.');
94+
if ($totalCount === 0) {
95+
$style->success('Database is clean of attribute duplicates. Nothing to do.');
10996

110-
return Command::FAILURE;
111-
}
112-
} else {
113-
$style->success('Database is clean of attribute duplicates. Nothing to do.');
97+
return Command::SUCCESS;
98+
}
11499

115-
return Command::SUCCESS;
116-
}
100+
if ($input->isInteractive()) {
101+
$confirmation = $this->askForConfirmation($style);
102+
if (!$confirmation) {
103+
$style->info('Confirmation rejected. Terminating.');
117104

118-
$iteration = 1;
119-
$totalDeleted = 0;
120-
do {
121-
$deleted = 0;
122-
$stopwatch->start('iteration', 'sql');
123-
124-
$attributes = $this->getDuplicatedAttributesBatch($batchSize);
125-
foreach ($attributes as $attribute) {
126-
$attributeIds = $this->getDuplicatedAttributeIds($attribute);
127-
128-
$deleted += $this->deleteAttributes($attributeIds);
129-
$totalDeleted += $deleted;
130-
}
131-
132-
$style->info(
133-
sprintf(
134-
'Iteration %d: Removed %d duplicates (total removed this execution: %d). [Debug %s]',
135-
$iteration,
136-
$deleted,
137-
$totalDeleted,
138-
$stopwatch->stop('iteration')
139-
)
140-
);
141-
142-
if ($maxIterations !== self::MAX_ITERATIONS_UNLIMITED && ++$iteration > $maxIterations) {
143-
$style->warning('Max iterations count reached. Terminating.');
144-
145-
return self::FAILURE;
146-
}
147-
148-
// Wait, if needed, before moving to next iteration
149-
usleep($sleep * 1000);
150-
} while ($batchSize === count($attributes));
151-
152-
$style->success(sprintf(
153-
'Operation successful. Removed total of %d duplicates. [Debug %s]',
154-
$totalDeleted,
155-
$stopwatch->stop('total')
156-
));
157-
} catch (Exception $exception) {
158-
$style->error($exception->getMessage());
159-
160-
return Command::FAILURE;
105+
return Command::FAILURE;
106+
}
161107
}
162108

109+
$iteration = 1;
110+
$totalDeleted = 0;
111+
do {
112+
$deleted = 0;
113+
$stopwatch->start('iteration', 'sql');
114+
115+
$attributes = $this->getDuplicatedAttributesBatch($batchSize);
116+
foreach ($attributes as $attribute) {
117+
$attributeIds = $this->getDuplicatedAttributeIds($attribute);
118+
119+
$iterationDeleted = $this->deleteAttributes($attributeIds);
120+
121+
$deleted += $iterationDeleted;
122+
$totalDeleted += $iterationDeleted;
123+
}
124+
125+
$style->info(
126+
sprintf(
127+
'Iteration %d: Removed %d duplicate database rows (total removed this execution: %d). [Debug %s]',
128+
$iteration,
129+
$deleted,
130+
$totalDeleted,
131+
$stopwatch->stop('iteration')
132+
)
133+
);
134+
135+
if ($maxIterations !== self::MAX_ITERATIONS_UNLIMITED && ++$iteration > $maxIterations) {
136+
$style->warning('Max iterations count reached. Terminating.');
137+
138+
return self::SUCCESS;
139+
}
140+
141+
// Wait, if needed, before moving to next iteration
142+
usleep($sleep * 1000);
143+
} while ($batchSize === count($attributes));
144+
145+
$style->success(sprintf(
146+
'Operation successful. Removed total of %d duplicate database rows. [Debug %s]',
147+
$totalDeleted,
148+
$stopwatch->stop('total')
149+
));
150+
163151
return Command::SUCCESS;
164152
}
165153

@@ -229,22 +217,18 @@ private function getDuplicatedAttributeIds(array $attribute): array
229217
$query
230218
->select('id')
231219
->from('ezcontentobject_attribute')
232-
->where('version = :version')
220+
->andWhere('version = :version')
233221
->andWhere('contentclassattribute_id = :contentclassattribute_id')
234222
->andWhere('contentobject_id = :contentobject_id')
235223
->andWhere('language_id = :language_id')
236224
->orderBy('id', 'ASC')
237-
->setFirstResult(0);
225+
// Keep the original attribute row, the very first one
226+
->setFirstResult(1);
238227

239228
$query->setParameters($attribute);
240-
241229
$result = $query->execute()->fetchFirstColumn();
242-
$attributeIds = array_map('intval', $result);
243-
244-
// Keep the original attribute row, the very first one
245-
array_shift($attributeIds);
246230

247-
return $attributeIds;
231+
return array_map('intval', $result);
248232
}
249233

250234
private function askForConfirmation(SymfonyStyle $style): bool
@@ -265,7 +249,7 @@ private function deleteAttributes($ids): int
265249

266250
$query
267251
->delete('ezcontentobject_attribute')
268-
->where($query->expr()->in('id', $ids));
252+
->andWhere($query->expr()->in('id', $ids));
269253

270254
return (int)$query->execute();
271255
}

0 commit comments

Comments
 (0)