fix handling of delimiter in actual csv data; added test to prevent h…

…eader and row column count mismatch Signed-off-by: Vinzenz Rosenkranz <[email protected]>
DH-Center-Tuebingen · Dec 13, 2024 · fe27d16 · fe27d16
1 parent 8c31020
commit fe27d16
Show file tree

Hide file tree

Showing 9 changed files with 115 additions and 71 deletions.
diff --git a/app/Exceptions/CsvColumnMismatchException.php b/app/Exceptions/CsvColumnMismatchException.php
@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+
+class CsvColumnMismatchException extends Exception {
+    public $headerLine;
+    public $dataLine;
+    public $headerColumns;
+    public $dataColumns;
+
+    public function __construct(string $headerLine, string $dataLine, int $headerColumns, int $dataColumns) {
+        $this->headerLine = $headerLine;
+        $this->dataLine = $dataLine;
+        $this->headerColumns = $headerColumns;
+        $this->dataColumns = $dataColumns;
+    }
+}
diff --git a/app/Exceptions/Structs/AttributeImportExceptionStruct.php b/app/Exceptions/Structs/AttributeImportExceptionStruct.php
@@ -5,7 +5,6 @@
 use JsonSerializable;
 
 class AttributeImportExceptionStruct implements JsonSerializable {
-
     public $type;
     public $columnIndex;
     public $columnName;

diff --git a/app/File/Csv.php b/app/File/Csv.php
@@ -2,11 +2,13 @@
 
 namespace App\File;
 
-class Csv extends Parser {
+use App\Exceptions\CsvColumnMismatchException;
 
+class Csv extends Parser {
     private string $delimiter;
     private bool $hasHeaderRow;
     private array $headers = [];
+    private int $headerCount = 0;
     private string $encoding = 'UTF-8';
     private int $rows = 0;
 
@@ -16,21 +18,20 @@ public function __construct(bool $hasHeaderRow = false, string $delimiter = ",",
         $this->encoding = $encoding;
     }
 
-    public function getHeaders() {
+    public function getHeaders(): array {
         return $this->headers;
     }
 
-
-
     /**
      * Parses a CSV file.
-     * 
+     *
      * @param resource $fileHandle - The file handle to the CSV file
+     * @param callable $$rowCallback -
      */
-    public function parse($fileHandle, $rowCallback) {
+    public function parse($fileHandle, callable $rowCallback): void {
         $rowIndex = 0;
         $this->rows = 0;
-        // We don't use the fgetcsvfunction to change the file encoding to UTF-8.
+        // We don't use the fgetcsv function to change the file encoding to UTF-8.
         while(($row = fgets($fileHandle)) !== false) {
             $this->rows++;
             if($this->hasHeaderRow && $rowIndex === 0) {
@@ -47,14 +48,14 @@ public function parse($fileHandle, $rowCallback) {
 
     /**
      * Gets the headers from the CSV file.
-     * 
+     *
      * If there is no header (hasHeaderRow = false), it will return an array of numbers (1, 2, 3, ...)
      * and the array will not be changed.
-     * 
-     * @param array $lines - The lines of the CSV file
+     *
+     * @param resource $lines - The lines of the CSV file
      * @return array $headers - The headers of the CSV file
      */
-    public function parseHeaders($fileHandle) {
+    public function parseHeaders($fileHandle): array {
         $headers = null;
 
         $row = fgets($fileHandle);
@@ -69,15 +70,17 @@ public function parseHeaders($fileHandle) {
         } else {
             $headers = $this->generateNumberHeaders($row);
         }
+        $this->headerCount = count($headers);
 
         rewind($fileHandle);
         $this->headers = $headers;
         return $headers;
     }
 
-    private function generateNumberHeaders($row) {
+    private function generateNumberHeaders(string $row): array {
         $headers = [];
-        $parts = explode($this->delimiter, $row);
+        // $parts = explode($this->delimiter, $row);
+        $parts = $this->fromCsvLine($row);
         for($i = 1; $i <= count($parts); $i++) {
             $headers[] = "#$i";
         }
@@ -87,28 +90,33 @@ private function generateNumberHeaders($row) {
 
     /**
      * Gets a single row from a CSV line string.
-     * 
+     *
      * @param string $line - The line from the CSV file
      * @return array $row - Associative array using the headers as keys and the values from the line as values
      */
-    private function parseRow($line) {
+    private function parseRow(string $line): array {
         if(empty($this->headers)) {
             throw new \Exception("Headers are not set. Run parseHeaders first.");
         }
 
         $row = [];
 
-        $arr = $this->fromCsvLine($line);
-        for($i = 0; $i < count($arr); $i++) {
-            $row[$this->headers[$i]] = trim($arr[$i]);
+        $cols = $this->fromCsvLine($line);
+        $colCount = count($cols);
+        if($this->headerCount != $colCount) {
+            $headerRow = implode($this->delimiter, $this->headers);
+            $dataRow = implode($this->delimiter, $cols);
+            throw new CsvColumnMismatchException($headerRow, $dataRow, $this->headerCount, $colCount);
+        }
+        for($i = 0; $i < $colCount; $i++) {
+            $row[$this->headers[$i]] = trim($cols[$i]);
         }
         return $row;
     }
 
-
     /**
      * Converts the data to UTF-8 if it is not already.
-     * 
+     *
      * @param array|string $data - The data to convert
      * @return array|string $data - The converted data
      */
@@ -129,24 +137,23 @@ private function isUtf8(): bool {
         return $this->encoding == 'UTF-8';
     }
 
-
-    public function getRows() {
+    public function getRows(): int {
         return $this->rows;
     }
 
-    public function getDataRows() {
+    public function getDataRows(): int {
         return $this->rows - ($this->hasHeaderRow ? 1 : 0);
     }
 
     /**
      * Helper function to get the CSV from a line.
      * Always using the correct delimiter.
      */
-    private function fromCsvLine($line) {
+    private function fromCsvLine(string $line): array {
         return str_getcsv($line, separator: $this->delimiter);
     }
 
-    private function verifyColumnExists($column) {
+    private function verifyColumnExists(string $column): void {
         if(!in_array($column, $this->headers)) {
             throw new \Exception("Column '$column' does not exist.");
         }

diff --git a/app/File/Parser.php b/app/File/Parser.php
@@ -3,5 +3,5 @@
 namespace App\File;
 
 abstract class Parser {
-    abstract public function parse(string $filePath, callable $rowCallback);
+    abstract public function parse($filePath, callable $rowCallback);
 }
diff --git a/app/Import/EntityImporter.php b/app/Import/EntityImporter.php
@@ -2,6 +2,7 @@
 
 namespace App\Import;
 
+use App\Exceptions\CsvColumnMismatchException;
 use App\File\Csv;
 use App\Entity;
 use App\EntityType;
@@ -40,7 +41,7 @@ public function __construct($metadata, $data) {
         $this->entityTypeId = $data['entity_type_id'];
         $this->attributesMap = $data['attributes'];
 
-        // The parent column is optional, therefore we only set it 
+        // The parent column is optional, therefore we only set it
         // to another value than null, if there is valid data set.
         if(array_key_exists('parent_column', $data)) {
             $parentColumn = trim($data['parent_column']);
@@ -103,14 +104,23 @@ public function validateImportData($filepath) {
             return $this->resolver;
         }
 
-        $csvTable->parse($handle, function ($row, $index) use (&$result, &$status) {
-            $namesValid = $this->validateName($row, $index);
-            if($namesValid) {
-                // The location depends on the name column. If the name is not correct, we can't check the location.
-                $this->validateLocation($row, $index);
-            }
-            $this->validateAttributesInRow($row, $index);
-        });
+        try {
+            $csvTable->parse($handle, function ($row, $index) {
+                $namesValid = $this->validateName($row, $index);
+                if($namesValid) {
+                    // The location depends on the name column. If the name is not correct, we can't check the location.
+                    $this->validateLocation($row, $index);
+                }
+                $this->validateAttributesInRow($row, $index);
+            });
+        } catch(CsvColumnMismatchException $csvMismatchException) {
+            return $this->resolver->conflict(__("entity-importer.csv-column-mismatch", [
+                "data" => $csvMismatchException->dataLine,
+                "data_count" => $csvMismatchException->dataColumns,
+                "header_data" => $csvMismatchException->headerLine,
+                "header_count" => $csvMismatchException->headerColumns,
+            ]));
+        }
 
         if($csvTable->getDataRows() == 0) {
             $this->resolver->conflict(__("entity-importer.empty"));

diff --git a/app/Import/ImportResolution.php b/app/Import/ImportResolution.php
@@ -2,15 +2,13 @@
 
 namespace App\Import;
 
-
 enum ImportResolutionType {
     case CREATE;
     case UPDATE;
     case CONFLICT;
 }
 
 class ImportResolution {
-
     private array $status;
     private array $errors = [];
 

diff --git a/lang/de/entity-importer.php b/lang/de/entity-importer.php
@@ -6,6 +6,7 @@
     'file-not-found' => 'Datei konnte nicht gelesen werden.',
     'missing-data' => 'Benötigte Spalte fehlt: :column',
     'invalid-data' => 'Ungültige Daten: [:column] => :value',
+    'csv-column-mismatch' => 'Die Anzahl der Spalten in der aktuellen Zeile \':data\' (:data_count Spalten) stimmt nicht mit den Spalten der Kopfzeile \':header_data\' (:header_count Spalten) überein.',
     'name-column-does-not-exist' => 'Die Spalte für den Namen existiert nicht: :column',
     'parent-column-does-non-exist' => 'Die Spalte für die Eltern-Entität existiert nicht: :column',
     'parent-entity-does-not-exist' => 'Die Eltern-Entität existiert nicht: :entity',

diff --git a/lang/en/entity-importer.php b/lang/en/entity-importer.php
@@ -5,6 +5,7 @@
     'file-not-found' => 'File could not be read.',
     'missing-data' => 'Required column is missing: :column',
     'invalid-data' => 'Invalid data: [:column] => :value',
+    'csv-column-mismatch' => 'Column count of current row \':data\' (:data_count columns) does not match column count in header row \':header_data\' (:header_count columns).',
     'attribute-could-not-be-imported' => 'Attribute could not be imported: :attribute',
     'attribute-id-does-not-exist' => 'The attribute id does not exist: :attributes',
     'attribute-column-does-not-exist' => 'The attribute columns do not exist: :columns',