From 85a9c6bb677a41f4fa47da9cc14f18107403fece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Fastr=C3=A9?= Date: Thu, 13 Mar 2025 14:18:54 +0100 Subject: [PATCH] Refactor export manager and normalize configuration handling Simplified formatter and aggregator logic by removing redundant fields and improving method checks (e.g., `hasAggregator`, `hasFilter`). Adjusted test cases to align with the updated structure and added tests for empty data normalization and denormalization. Improved code readability and ensured better handling of edge cases in export data processing. --- .../Export/ExportConfigNormalizer.php | 5 +- .../Export/ExportGenerator.php | 14 +- .../ChillMainBundle/Export/ExportManager.php | 10 ++ .../Export/ExportConfigNormalizerTest.php | 149 +++++++++++++++++- .../Tests/Export/ExportGeneratorTest.php | 8 +- 5 files changed, 167 insertions(+), 19 deletions(-) diff --git a/src/Bundle/ChillMainBundle/Export/ExportConfigNormalizer.php b/src/Bundle/ChillMainBundle/Export/ExportConfigNormalizer.php index a599ea71f..fb91f55b5 100644 --- a/src/Bundle/ChillMainBundle/Export/ExportConfigNormalizer.php +++ b/src/Bundle/ChillMainBundle/Export/ExportConfigNormalizer.php @@ -54,7 +54,6 @@ class ExportConfigNormalizer if ($filterData[FilterType::ENABLED_FIELD]) { $filtersSerialized[$alias]['form'] = $filter->normalizeFormData($filterData['form']); $filtersSerialized[$alias]['version'] = $filter->getNormalizationVersion(); - } } $serialized['filters'] = $filtersSerialized; @@ -72,7 +71,7 @@ class ExportConfigNormalizer $serialized['pick_formatter'] = $formData['pick_formatter']; $formatter = $this->exportManager->getFormatter($formData['pick_formatter']); - $serialized['formatter']['form'] = $formatter->normalizeFormData($formData['formatter']['form']); + $serialized['formatter']['form'] = $formatter->normalizeFormData($formData['formatter']); $serialized['formatter']['version'] = $formatter->getNormalizationVersion(); return $serialized; @@ -116,7 +115,7 @@ class ExportConfigNormalizer 'filters' => $filtersConfig, 'aggregators' => $aggregatorsConfig, 'pick_formatter' => $serializedData['pick_formatter'], - 'formatter' => ['form' => $formater->denormalizeFormData($serializedData['formatter']['form'], $serializedData['formatter']['version'])], + 'formatter' => $formater->denormalizeFormData($serializedData['formatter']['form'], $serializedData['formatter']['version']), 'centers' => array_filter(array_map(fn (int $id) => $this->centerRepository->find($id), $serializedData['centers']), fn ($item) => null !== $item), ]; } diff --git a/src/Bundle/ChillMainBundle/Export/ExportGenerator.php b/src/Bundle/ChillMainBundle/Export/ExportGenerator.php index bc3078cf2..f1a4d386b 100644 --- a/src/Bundle/ChillMainBundle/Export/ExportGenerator.php +++ b/src/Bundle/ChillMainBundle/Export/ExportGenerator.php @@ -81,10 +81,6 @@ final readonly class ExportGenerator $result = $export->getResult($query, $data['export'], $context); - if (!is_iterable($result)) { - throw new \UnexpectedValueException(sprintf('The result of the export should be an iterable, %s given', \gettype($result))); - } - $formatter = $this->exportManager->getFormatter($data['pick_formatter']); $filtersData = []; $aggregatorsData = []; @@ -101,7 +97,7 @@ final readonly class ExportGenerator if (method_exists($formatter, 'generate')) { return $formatter->generate( $result, - $data['formatter']['form'], + $data['formatter'], $exportAlias, $data['export'], $filtersData, @@ -115,7 +111,7 @@ final readonly class ExportGenerator $result, $data['formatter'], $exportAlias, - $data['export']['form'], + $data['export'], $filtersData, $aggregatorsData, ); @@ -194,7 +190,7 @@ final readonly class ExportGenerator } foreach ($data as $alias => $aggregatorData) { - if (true === $aggregatorData['enabled']) { + if ($this->exportManager->hasAggregator($alias) && true === $aggregatorData['enabled']) { yield $alias => $this->exportManager->getAggregator($alias); } } @@ -210,7 +206,7 @@ final readonly class ExportGenerator } foreach ($data as $alias => $filterData) { - if (true === $filterData['enabled']) { + if ($this->exportManager->hasFilter($alias) && true === $filterData['enabled']) { yield $alias => $this->exportManager->getFilter($alias); } } @@ -249,7 +245,7 @@ final readonly class ExportGenerator * build the array required for defining centers and circles in the initiate * queries of ExportElementsInterfaces. * - * @param list
+ * @param list
$centers */ private function buildCenterReachableScopes(array $centers) { diff --git a/src/Bundle/ChillMainBundle/Export/ExportManager.php b/src/Bundle/ChillMainBundle/Export/ExportManager.php index eb381a714..c723bba68 100644 --- a/src/Bundle/ChillMainBundle/Export/ExportManager.php +++ b/src/Bundle/ChillMainBundle/Export/ExportManager.php @@ -274,6 +274,16 @@ class ExportManager return $this->filters[$alias]; } + public function hasFilter(string $alias): bool + { + return array_key_exists($alias, $this->filters); + } + + public function hasAggregator(string $alias): bool + { + return array_key_exists($alias, $this->aggregators); + } + public function getAllFilters(): array { $filters = []; diff --git a/src/Bundle/ChillMainBundle/Tests/Export/ExportConfigNormalizerTest.php b/src/Bundle/ChillMainBundle/Tests/Export/ExportConfigNormalizerTest.php index 7315bf7fb..cf4518ccb 100644 --- a/src/Bundle/ChillMainBundle/Tests/Export/ExportConfigNormalizerTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Export/ExportConfigNormalizerTest.php @@ -77,9 +77,7 @@ class ExportConfigNormalizerTest extends TestCase 'aggregatorDisabled' => ['enabled' => false, 'form' => ['default' => '0']], ], 'pick_formatter' => 'xlsx', - 'formatter' => [ - 'form' => ['test' => '0'], - ], + 'formatter' => ['test' => '0'], ]; $expected = [ @@ -167,9 +165,150 @@ class ExportConfigNormalizerTest extends TestCase 'aggregatorDisabled' => ['enabled' => false, 'form' => ['default' => '0']], ], 'pick_formatter' => 'xlsx', - 'formatter' => [ - 'form' => ['test' => '0'], + 'formatter' => ['test' => '0'], + 'centers' => [$center], + ]; + + $exportConfigNormalizer = new ExportConfigNormalizer($exportManager->reveal(), $centerRepository->reveal()); + $actual = $exportConfigNormalizer->denormalizeConfig('export', $serialized, true); + + self::assertEquals($expected, $actual); + } + + public function testNormalizeConfigEmptyData(): void + { + $filterEnabled = $this->prophesize(FilterInterface::class); + $filterEnabled->normalizeFormData([])->shouldBeCalled()->willReturn([]); + $filterEnabled->getNormalizationVersion()->willReturn(1); + $filterDisabled = $this->prophesize(FilterInterface::class); + $filterDisabled->normalizeFormData([])->shouldNotBeCalled(); + + $aggregatorEnabled = $this->prophesize(AggregatorInterface::class); + $aggregatorEnabled->normalizeFormData([])->shouldBeCalled()->willReturn([]); + $aggregatorEnabled->getNormalizationVersion()->willReturn(1); + $aggregatorDisabled = $this->prophesize(AggregatorInterface::class); + $aggregatorDisabled->normalizeFormData([])->shouldNotBeCalled(); + + $export = $this->prophesize(ExportInterface::class); + $export->normalizeFormData([])->shouldBeCalled()->willReturn([]); + $export->getNormalizationVersion()->willReturn(1); + + $formatter = $this->prophesize(FormatterInterface::class); + $formatter->normalizeFormData([])->shouldBeCalled()->willReturn([]); + $formatter->getNormalizationVersion()->willReturn(1); + + $exportManager = $this->prophesize(ExportManager::class); + $exportManager->getFormatter('xlsx')->shouldBeCalled()->willReturn($formatter->reveal()); + $exportManager->getFilter('filterEnabled')->willReturn($filterEnabled->reveal()); + $exportManager->getFilter('filterDisabled')->willReturn($filterDisabled->reveal()); + $exportManager->getAggregator('aggregatorEnabled')->willReturn($aggregatorEnabled->reveal()); + $exportManager->getAggregator('aggregatorDisabled')->willReturn($aggregatorDisabled->reveal()); + $exportManager->getExport('export')->willReturn($export->reveal()); + + $center = $this->prophesize(Center::class); + $center->getId()->willReturn(10); + + $formData = [ + 'centers' => [$center->reveal()], + 'export' => [], + 'filters' => [ + 'filterEnabled' => ['enabled' => true, 'form' => []], + 'filterDisabled' => ['enabled' => false, 'form' => []], ], + 'aggregators' => [ + 'aggregatorEnabled' => ['enabled' => true, 'form' => []], + 'aggregatorDisabled' => ['enabled' => false, 'form' => []], + ], + 'pick_formatter' => 'xlsx', + 'formatter' => [], + ]; + + $expected = [ + 'export' => ['form' => [], 'version' => 1], + 'filters' => [ + 'filtersEnabled' => ['enabled' => true, 'form' => [], 'version' => 1], + 'filterDisabled' => ['enabled' => false], + ], + 'aggregators' => [ + 'aggregatorEnabled' => ['enabled' => true, 'form' => [], 'version' => 1], + 'aggregatorDisabled' => ['enabled' => false], + ], + 'pick_formatter' => 'xlsx', + 'formatter' => [ + 'form' => [], + 'version' => 1, + ], + 'centers' => [10], + ]; + + $exportConfigNormalizer = new ExportConfigNormalizer($exportManager->reveal(), $this->prophesize(CenterRepositoryInterface::class)->reveal()); + + $actual = $exportConfigNormalizer->normalizeConfig('export', $formData); + + self::assertEqualsCanonicalizing($expected, $actual); + } + + public function testDenormalizeConfigWithEmptyData(): void + { + $filterEnabled = $this->prophesize(FilterInterface::class); + $filterEnabled->denormalizeFormData([], 1)->shouldBeCalled()->willReturn([]); + $filterDisabled = $this->prophesize(FilterInterface::class); + $filterDisabled->denormalizeFormData(Argument::any(), Argument::type('int'))->shouldNotBeCalled(); + $filterDisabled->getFormDefaultData()->willReturn([]); + + $aggregatorEnabled = $this->prophesize(AggregatorInterface::class); + $aggregatorEnabled->denormalizeFormData([], 1)->shouldBeCalled()->willReturn([]); + $aggregatorDisabled = $this->prophesize(AggregatorInterface::class); + $aggregatorDisabled->denormalizeFormData(Argument::any(), Argument::type('int'))->shouldNotBeCalled(); + $aggregatorDisabled->getFormDefaultData()->willReturn([]); + + $export = $this->prophesize(ExportInterface::class); + $export->denormalizeFormData([], 1)->shouldBeCalled()->willReturn([]); + + $formatter = $this->prophesize(FormatterInterface::class); + $formatter->denormalizeFormData([], 1)->shouldBeCalled()->willReturn([]); + + $exportManager = $this->prophesize(ExportManager::class); + $exportManager->getFormatter('xlsx')->shouldBeCalled()->willReturn($formatter->reveal()); + $exportManager->getFilter('filterEnabled')->willReturn($filterEnabled->reveal()); + $exportManager->getFilter('filterDisabled')->willReturn($filterDisabled->reveal()); + $exportManager->getAggregator('aggregatorEnabled')->willReturn($aggregatorEnabled->reveal()); + $exportManager->getAggregator('aggregatorDisabled')->willReturn($aggregatorDisabled->reveal()); + $exportManager->getExport('export')->willReturn($export->reveal()); + + $centerRepository = $this->prophesize(CenterRepositoryInterface::class); + $centerRepository->find(10)->willReturn($center = new Center()); + + $serialized = [ + 'centers' => [10], + 'export' => ['form' => [], 'version' => 1], + 'filters' => [ + 'filterEnabled' => ['enabled' => true, 'form' => [], 'version' => 1], + 'filterDisabled' => ['enabled' => false], + ], + 'aggregators' => [ + 'aggregatorEnabled' => ['enabled' => true, 'form' => [], 'version' => 1], + 'aggregatorDisabled' => ['enabled' => false], + ], + 'pick_formatter' => 'xlsx', + 'formatter' => [ + 'form' => [], + 'version' => 1, + ], + ]; + + $expected = [ + 'export' => [], + 'filters' => [ + 'filterEnabled' => ['enabled' => true, 'form' => []], + 'filterDisabled' => ['enabled' => false, 'form' => []], + ], + 'aggregators' => [ + 'aggregatorEnabled' => ['enabled' => true, 'form' => []], + 'aggregatorDisabled' => ['enabled' => false, 'form' => []], + ], + 'pick_formatter' => 'xlsx', + 'formatter' => [], 'centers' => [$center], ]; diff --git a/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php b/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php index 21e7af344..3f4d51059 100644 --- a/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php +++ b/src/Bundle/ChillMainBundle/Tests/Export/ExportGeneratorTest.php @@ -53,7 +53,7 @@ class ExportGeneratorTest extends TestCase 'disabled_aggregator' => ['enabled' => false], ], 'pick_formatter' => 'xlsx', - 'formatter' => ['form' => $formatterData = ['key' => 'form4']], + 'formatter' => $formatterData = ['key' => 'form4'], 'centers' => [$centerA = new Center(), $centerB = new Center()], ]; $user = new User(); @@ -98,7 +98,11 @@ class ExportGeneratorTest extends TestCase $exportManager = $this->prophesize(ExportManager::class); $exportManager->getExport('dummy')->willReturn($export->reveal()); $exportManager->getFilter('dummy_filter')->willReturn($filter->reveal()); + $exportManager->hasFilter('dummy_filter')->willReturn(true); + $exportManager->hasFilter('disabled_filter')->willReturn(true); $exportManager->getAggregator('dummy_aggregator')->willReturn($aggregator->reveal()); + $exportManager->hasAggregator('dummy_aggregator')->willReturn(true); + $exportManager->hasAggregator('disabled_aggregator')->willReturn(true); $exportManager->getFormatter('xlsx')->willReturn($formatter->reveal()); $generator = new ExportGenerator($exportManager->reveal(), $exportConfigNormalizer->reveal(), new NullLogger()); @@ -118,7 +122,7 @@ class ExportGeneratorTest extends TestCase 'filters' => [], 'aggregators' => [], 'pick_formatter' => 'xlsx', - 'formatter' => ['form' => $formatterData = ['key' => 'form4']], + 'formatter' => $formatterData = ['key' => 'form4'], 'centers' => [$centerA = new Center(), $centerB = new Center()], ]; $user = new User();