Refactor SpreadsheetListFormatter to avoid race condition

improve label caching, enhance type safety, and streamline header generation and value handling.
This commit is contained in:
Julien Fastré 2025-06-27 16:08:43 +02:00
parent d4704dc2ef
commit 95ccfff868
Signed by: julienfastre
GPG Key ID: BDE2190974723FCB
2 changed files with 42 additions and 55 deletions

View File

@ -2154,11 +2154,6 @@ parameters:
count: 1
path: src/Bundle/ChillMainBundle/Export/Formatter/SpreadSheetFormatter.php
-
message: "#^Instanceof between string and DateTimeInterface will always evaluate to false\\.$#"
count: 1
path: src/Bundle/ChillMainBundle/Export/Formatter/SpreadsheetListFormatter.php
-
message: "#^PHPDoc tag @var for property Chill\\\\MainBundle\\\\Export\\\\Helper\\\\ExportAddressHelper\\:\\:\\$unitNamesKeysCache contains unresolvable type\\.$#"
count: 1

View File

@ -24,6 +24,7 @@ use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Contracts\Translation\TranslatableInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
/**
@ -33,21 +34,6 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
{
use ExportManagerAwareTrait;
protected $exportAlias;
protected $exportData;
protected $formatterData;
/**
* This variable cache the labels internally.
*
* @var string[]
*/
protected $labelsCache;
protected $result;
public function __construct(private readonly TranslatorInterface $translator) {}
/**
@ -109,29 +95,25 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
public function generate($result, $formatterData, string $exportAlias, array $exportData, array $filtersData, array $aggregatorsData, ExportGenerationContext $context): FormattedExportGeneration
{
$this->result = $result;
$this->exportAlias = $exportAlias;
$this->exportData = $exportData;
$this->formatterData = $formatterData;
$spreadsheet = new Spreadsheet();
$worksheet = $spreadsheet->getActiveSheet();
$cacheLabels = $this->prepareCacheLabels($result, $exportAlias, $exportData);
$this->prepareHeaders($worksheet);
$this->prepareHeaders($cacheLabels, $worksheet, $result, $formatterData, $exportAlias, $exportData);
$i = 1;
foreach ($result as $row) {
if (true === $this->formatterData['numerotation']) {
if (true === $formatterData['numerotation']) {
$worksheet->setCellValue('A'.($i + 1), (string) $i);
}
$a = $this->formatterData['numerotation'] ? 'B' : 'A';
$a = $formatterData['numerotation'] ? 'B' : 'A';
foreach ($row as $key => $value) {
$row = $a.($i + 1);
$formattedValue = $this->getLabel($key, $value);
$formattedValue = $this->getLabel($cacheLabels, $key, $value, $result, $exportAlias, $exportData);
if ($formattedValue instanceof \DateTimeInterface) {
$worksheet->setCellValue($row, Date::PHPToExcel($formattedValue));
@ -145,6 +127,8 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
->getNumberFormat()
->setFormatCode(NumberFormat::FORMAT_DATE_DATETIME);
}
} elseif ($formattedValue instanceof TranslatableInterface) {
$worksheet->setCellValue($row, $formattedValue->trans($this->translator));
} else {
$worksheet->setCellValue($row, $formattedValue);
}
@ -154,7 +138,7 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
++$i;
}
switch ($this->formatterData['format']) {
switch ($formatterData['format']) {
case 'ods':
$writer = \PhpOffice\PhpSpreadsheet\IOFactory::createWriter($spreadsheet, 'Ods');
$contentType = 'application/vnd.oasis.opendocument.spreadsheet';
@ -177,7 +161,7 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
default:
// this should not happen
// throw an exception to ensure that the error is catched
throw new \OutOfBoundsException('The format '.$this->formatterData['format'].' is not supported');
throw new \OutOfBoundsException('The format '.$formatterData['format'].' is not supported');
}
$tempfile = \tempnam(\sys_get_temp_dir(), '');
@ -230,34 +214,34 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
/**
* Give the label corresponding to the given key and value.
*
* @param string $key
* @param string $value
*
* @return string
* @return string|\DateTimeInterface|int|null|float|TranslatableInterface
*
* @throws \LogicException if the label is not found
*/
protected function getLabel($key, $value)
private function getLabel(array $labelsCache, $key, $value, array $result, string $exportAlias, array $exportData)
{
if (null === $this->labelsCache) {
$this->prepareCacheLabels();
if (!\array_key_exists($key, $labelsCache)) {
throw new \OutOfBoundsException(sprintf('The key "%s" is not present in the list of keys handled by this query. Check your `getKeys` and `getLabels` methods. Available keys are %s.', $key, \implode(', ', \array_keys($labelsCache))));
}
if (!\array_key_exists($key, $this->labelsCache)) {
throw new \OutOfBoundsException(sprintf('The key "%s" is not present in the list of keys handled by this query. Check your `getKeys` and `getLabels` methods. Available keys are %s.', $key, \implode(', ', \array_keys($this->labelsCache))));
}
return $this->labelsCache[$key]($value);
return $labelsCache[$key]($value);
}
/**
* Prepare the label cache which will be used by getLabel. This function
* should be called only once in the generation lifecycle.
* Prepare the label cache which will be used by getLabel.
*
* @param array $result
* @param string $exportAlias
* @param array $exportData
*
* @return array The labels cache
*/
protected function prepareCacheLabels()
private function prepareCacheLabels(array $result, string $exportAlias, array $exportData): array
{
$export = $this->getExportManager()->getExport($this->exportAlias);
$keys = $export->getQueryKeys($this->exportData);
$labelsCache = [];
$export = $this->getExportManager()->getExport($exportAlias);
$keys = $export->getQueryKeys($exportData);
foreach ($keys as $key) {
// get an array with all values for this key if possible
@ -267,29 +251,37 @@ class SpreadsheetListFormatter implements FormatterInterface, ExportManagerAware
}
return $v[$key];
}, $this->result);
// store the label in the labelsCache property
$this->labelsCache[$key] = $export->getLabels($key, $values, $this->exportData);
}, $result);
// store the label in the labelsCache
$labelsCache[$key] = $export->getLabels($key, $values, $exportData);
}
return $labelsCache;
}
/**
* add the headers to the csv file.
*
* @param Worksheet $worksheet
* @param array $result
* @param array $formatterData
* @param string $exportAlias
* @param array $exportData
*/
protected function prepareHeaders(Worksheet $worksheet)
protected function prepareHeaders(array $labelsCache, Worksheet $worksheet, array $result, array $formatterData, string $exportAlias, array $exportData)
{
$keys = $this->getExportManager()->getExport($this->exportAlias)->getQueryKeys($this->exportData);
$keys = $this->getExportManager()->getExport($exportAlias)->getQueryKeys($exportData);
// we want to keep the order of the first row. So we will iterate on the first row of the results
$first_row = \count($this->result) > 0 ? $this->result[0] : [];
$first_row = \count($result) > 0 ? $result[0] : [];
$header_line = [];
if (true === $this->formatterData['numerotation']) {
if (true === $formatterData['numerotation']) {
$header_line[] = $this->translator->trans('Number');
}
foreach ($first_row as $key => $value) {
$header_line[] = $this->translator->trans(
$this->getLabel($key, '_header')
$this->getLabel($labelsCache, $key, '_header', $result, $exportAlias, $exportData)
);
}