Simplify a little bit the ExcelLoader code (#2103)

* Simplify a little bit the `ExcelLoader` code

Core review changes

* Fixing bug in `ExcelExtractor` skipping first row when no headers needed

* Core review changes

Author: stloyd

src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelExtractor.php

@@ -52,10 +52,6 @@ public function extract(FlowContext $context) : \Generator
         // Offset must be a positive number
         $offset = $this->offset ?? 1;
 
-        if (!$this->withHeader) {
-            $offset++;
-        }
-
         foreach ($context->streams()->list($this->path, $this->filter()) as $stream) {
             foreach ($this->extractRows($stream, $headers, $offset) as $row) {
                 // Ensure $row is an array before passing to array_to_rows

src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/ExcelLoader.php

@@ -74,21 +74,12 @@ public function load(Rows $rows, FlowContext $context) : void
             timeFormat: $this->timeFormat,
         );
 
-        $streams = $context->streams();
+        $stream = $context->streams()->writeTo($this->path, $rows->partitions()->toArray());
 
-        if ($rows->partitions()->count()) {
-            $stream = $streams->writeTo($this->path, $rows->partitions()->toArray());
-        } else {
-            $stream = $streams->writeTo($this->path);
-        }
-
-        $filePath = $stream->path()->path();
         $manager = $this->getWorkbookManager();
-        $manager->open($filePath);
-
-        $rowIndex = 0;
+        $manager->open($stream->path()->path());
 
-        foreach ($rows as $row) {
+        foreach ($rows as $rowIndex => $row) {
             $sheetName = $this->resolveSheetName($row);
 
             $rowForExcel = $this->sheetNameEntryName !== null && $row->has($this->sheetNameEntryName)
@@ -103,8 +94,6 @@ public function load(Rows $rows, FlowContext $context) : void
             $values = $normalizer->normalize($rowForExcel);
             $styles = $this->resolveCellStyles($rowForExcel, $rowIndex, $sheetName);
             $manager->writeRow($sheetName, $values, $styles);
-
-            $rowIndex++;
         }
     }
 
@@ -243,9 +232,7 @@ private function resolveWriterType() : ExcelWriter
             return $this->writerType;
         }
 
-        $extension = $this->path->extension();
-
-        return match (\strtolower((string) $extension)) {
+        return match ((string) $this->path->extension()) {
             'ods' => ExcelWriter::ODS,
             default => ExcelWriter::XLSX,
         };

src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/RowsNormalizer/ExcelRowsNormalizer.php

@@ -4,6 +4,7 @@
 
 namespace Flow\ETL\Adapter\Excel\RowsNormalizer;
 
+use Flow\ETL\Exception\InvalidArgumentException;
 use Flow\ETL\Row;
 use Flow\ETL\Row\Entry;
 use Flow\ETL\Row\Entry\{BooleanEntry,
@@ -66,68 +67,37 @@ public function normalize(Row $row) : array
      */
     private function normalizeEntry(Entry $entry) : bool|float|int|string|null
     {
-        $value = match ($entry::class) {
+        return match ($entry::class) {
             BooleanEntry::class,
             IntegerEntry::class,
             FloatEntry::class,
             StringEntry::class => $entry->value(),
             DateTimeEntry::class => $entry->value()?->format($this->dateTimeFormat),
             DateEntry::class => $entry->value()?->format($this->dateFormat),
             TimeEntry::class => $entry->value()?->format($this->timeFormat),
-            UuidEntry::class => $entry->toString(),
             EnumEntry::class => $this->normalizeEnumEntry($entry),
             JsonEntry::class,
             ListEntry::class,
             MapEntry::class,
             StructureEntry::class => $this->normalizeToJson($entry->value()),
+            UuidEntry::class,
             XMLEntry::class,
             XMLElementEntry::class,
             HTMLEntry::class,
             HTMLElementEntry::class => $entry->toString(),
-            default => $entry->toString(),
+            default => throw new InvalidArgumentException('Unknown entry type: ' . $entry::class),
         };
-
-        if ($value === null) {
-            return null;
-        }
-
-        if (\is_bool($value)) {
-            return $value;
-        }
-
-        if (\is_int($value)) {
-            return $value;
-        }
-
-        if (\is_float($value)) {
-            return $value;
-        }
-
-        if (\is_string($value)) {
-            return $value;
-        }
-
-        return $entry->toString();
-    }
-
-    private function normalizeEnum(\UnitEnum $enum) : string
-    {
-        if ($enum instanceof \BackedEnum) {
-            return (string) $enum->value;
-        }
-
-        return $enum->name;
     }
 
     private function normalizeEnumEntry(EnumEntry $entry) : ?string
     {
         $value = $entry->value();
 
-        if ($value === null) {
-            return null;
+        if ($value instanceof \BackedEnum) {
+            return (string) $value->value;
         }
 
-        return $this->normalizeEnum($value);
+        return $value?->name;
     }
 
     private function normalizeToJson(mixed $value) : ?string

src/adapter/etl-adapter-excel/src/Flow/ETL/Adapter/Excel/WorkbookManager.php

@@ -77,15 +77,18 @@ public function open(string $filePath) : void
      */
     public function writeHeader(string $sheetName, array $headers, ?Style $style = null) : void
     {
-        $sheet = $this->getOrCreateSheet($sheetName);
         $writer = $this->writers[$this->currentFilePath];
 
-        $row = $style !== null
-            ? OpenSpoutRow::fromValuesWithStyle($headers, $style)
-            : OpenSpoutRow::fromValues($headers);
+        if ($sheetName !== $writer->getCurrentSheet()->getName()) {
+            $sheet = $this->getOrCreateSheet($sheetName);
+            $writer->setCurrentSheet($sheet);
+        }
 
-        $writer->setCurrentSheet($sheet);
-        $writer->addRow($row);
+        $writer->addRow(
+            $style !== null
+                ? OpenSpoutRow::fromValuesWithStyle($headers, $style)
+                : OpenSpoutRow::fromValues($headers)
+        );
 
         $key = $this->sheetKey($sheetName);
         $this->headersWritten[$key] = true;
@@ -97,21 +100,19 @@ public function writeHeader(string $sheetName, array $headers, ?Style $style = n
      */
     public function writeRow(string $sheetName, array $values, ?array $styles = null) : void
     {
-        $sheet = $this->getOrCreateSheet($sheetName);
         $writer = $this->writers[$this->currentFilePath];
 
-        if ($styles !== null) {
-            $filteredStyles = \array_filter($styles, static fn (?Style $style) : bool => $style !== null);
-
-            $row = \count($filteredStyles) > 0
-                ? OpenSpoutRow::fromValuesWithStyles($values, $filteredStyles)
-                : OpenSpoutRow::fromValues($values);
-        } else {
-            $row = OpenSpoutRow::fromValues($values);
+        if ($sheetName !== $writer->getCurrentSheet()->getName()) {
+            $sheet = $this->getOrCreateSheet($sheetName);
+            $writer->setCurrentSheet($sheet);
         }
 
-        $writer->setCurrentSheet($sheet);
-        $writer->addRow($row);
+        $writer->addRow(
+            OpenSpoutRow::fromValuesWithStyles(
+                $values,
+                $styles ? \array_filter($styles, static fn (?Style $style) : bool => $style !== null) : []
+            )
+        );
     }
 
     private function countSheetsForCurrentFile() : int

src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelExtractorTest.php

@@ -113,7 +113,7 @@ public function test_extract_excel_file_with_offset_without_header(string $fixtu
             ->fetch()
             ->toArray();
 
-        self::assertCount(6, $rows);
+        self::assertCount(7, $rows);
 
         foreach ($rows as $row) {
             self::assertSame(['e00', 'e01', 'e02'], \array_keys($row));
@@ -166,7 +166,7 @@ public function test_extract_excel_file_without_header(string $fixtureName) : vo
             ->fetch()
             ->toArray();
 
-        self::assertCount(10, $rows);
+        self::assertCount(11, $rows);
 
         foreach ($rows as $row) {
             self::assertSame(['e00', 'e01', 'e02'], \array_keys($row));

src/adapter/etl-adapter-excel/tests/Flow/ETL/Adapter/Excel/Tests/Integration/ExcelLoaderTest.php

@@ -386,6 +386,7 @@ public function test_round_trip_without_header(ExcelWriter $writer, string $exte
 
         self::assertEquals(
             [
+                ['e00' => 1, 'e01' => 'Alice'],
                 ['e00' => 2, 'e01' => 'Bob'],
             ],
             $rows