Refactored and validated alias inheritance in schemas

The changes in various classes improve reliability and readability of the code. In Schema.php, a refactored constructor and stricter filename validation ensures correct file handling. Enhanced match and validation handling in ValidatorSchema and SchemaDataPrep improves alias processing. Furthermore, explicit casting in Utils.php's mergeConfigs function ensures reliable array handling.
JBZoo · Apr 5, 2024 · 894090f · 894090f
1 parent 9dccfa1
commit 894090f
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 75 deletions.
diff --git a/src/Schema.php b/src/Schema.php
@@ -48,7 +48,6 @@ public function __construct(null|array|string $csvSchemaFilenameOrArray = null)
             && \file_exists($csvSchemaFilenameOrArray)
         ) {
             $this->filename = $csvSchemaFilenameOrArray;
-            $data = new Data();
             $fileExtension = \pathinfo($csvSchemaFilenameOrArray, \PATHINFO_EXTENSION);
 
             if ($fileExtension === 'yml' || $fileExtension === 'yaml') {

diff --git a/src/SchemaDataPrep.php b/src/SchemaDataPrep.php
@@ -105,35 +105,12 @@ public static function validateAlias(string $alias): void
             throw new \InvalidArgumentException('Empty alias');
         }
 
-        if (\preg_match(self::getAliasRegex(), $alias) === 0) {
+        $regex = self::getAliasRegex();
+        if ($regex !== '' && \preg_match($regex, $alias) === 0) {
             throw new \InvalidArgumentException("Invalid alias: \"{$alias}\"");
         }
     }
 
-    private function parseAliasParts(string $inherit): array
-    {
-        $alias = null;
-        $keyword = null;
-        $columnName = null;
-        $rules = null;
-
-        $parts = \explode('/', $inherit);
-        if (\count($parts) === 2) {
-            [$alias, $keyword] = $parts;
-        } elseif (\count($parts) === 3) {
-            [$alias, $keyword, $columnName] = $parts;
-        } elseif (\count($parts) === 4) {
-            [$alias, $keyword, $columnName, $rules] = $parts;
-        }
-
-        return [
-            'alias'   => $alias,
-            'keyword' => $keyword,
-            'column'  => $columnName,
-            'rules'   => $rules,
-        ];
-    }
-
     /**
      * @return Schema[]
      */
@@ -142,6 +119,8 @@ private function prepareAliases(AbstractData $data): array
         $includes = [];
 
         foreach ($data->getArray('includes') as $alias => $includedPathOrArray) {
+            $alias = (string)$alias;
+
             self::validateAlias($alias);
 
             if (\is_array($includedPathOrArray)) {
@@ -171,8 +150,8 @@ private function buildFilenamePattern(): string
     {
         $inherit = $this->data->findString('filename_pattern.inherit');
 
-        if (\str_ends_with($inherit, '/filename_pattern')) {
-            $inheritParts = $this->parseAliasParts($inherit);
+        if ($inherit !== '') {
+            $inheritParts = self::parseAliasParts($inherit);
             $parent = $this->getParentSchema($inheritParts['alias']);
             return $parent->getData()->get('filename_pattern');
         }
@@ -185,13 +164,13 @@ private function buildByKey(string $key = 'structural_rules'): array
         $inherit = $this->data->findString("{$key}.inherit");
 
         $parentConfig = [];
-        if (\preg_match('/' . self::ALIAS_REGEX . '\/' . $key . '$/i', $inherit) === 1) {
-            $inheritParts = $this->parseAliasParts($inherit);
+        if ($inherit !== '') {
+            $inheritParts = self::parseAliasParts($inherit);
             $parent = $this->getParentSchema($inheritParts['alias']);
             $parentConfig = $parent->getData()->getArray($key);
         }
 
-        $result = Utils::mergeConfigs(self::DEFAULTS[$key], $parentConfig, $this->data->getArray($key));
+        $result = Utils::mergeConfigs((array)self::DEFAULTS[$key], $parentConfig, $this->data->getArray($key));
         unset($result['inherit']);
 
         return $result;
@@ -206,8 +185,8 @@ private function buildColumns(): array
             $columnInherit = $columnData->getString('inherit');
 
             $parentConfig = [];
-            if (\preg_match('/' . self::ALIAS_REGEX . '\/columns\/[^\/]+$/i', $columnInherit) === 1) {
-                $inheritParts = $this->parseAliasParts($columnInherit);
+            if ($columnInherit !== '') {
+                $inheritParts = self::parseAliasParts($columnInherit);
                 $parent = $this->getParentSchema($inheritParts['alias']);
                 $parentColumn = $parent->getColumn($inheritParts['column']);
                 if ($parentColumn === null) {
@@ -254,8 +233,8 @@ private function buildRules(array $rules, string $typeOfRules): array
         $inherit = $rules['inherit'] ?? '';
 
         $parentConfig = [];
-        if (\preg_match('/' . self::ALIAS_REGEX . '\/columns\/[^\/]+\/' . $typeOfRules . '$/i', $inherit) === 1) {
-            $inheritParts = $this->parseAliasParts($inherit);
+        if ($inherit !== '') {
+            $inheritParts = self::parseAliasParts($inherit);
             $parent = $this->getParentSchema($inheritParts['alias']);
             $parentColumn = $parent->getColumn($inheritParts['column']);
             if ($parentColumn === null) {
@@ -265,9 +244,21 @@ private function buildRules(array $rules, string $typeOfRules): array
             $parentConfig = $parentColumn->getData()->getArray($typeOfRules);
         }
 
-        $actualRules = Utils::mergeConfigs(self::DEFAULTS[$typeOfRules], $parentConfig, $rules);
+        $actualRules = Utils::mergeConfigs((array)self::DEFAULTS[$typeOfRules], $parentConfig, $rules);
         unset($actualRules['inherit']);
 
         return $actualRules;
     }
+
+    private static function parseAliasParts(string $inherit): array
+    {
+        $parts = \explode('/', $inherit);
+        self::validateAlias($parts[0]);
+
+        if (\count($parts) === 1) {
+            return ['alias' => $parts[0]];
+        }
+
+        return ['alias' => $parts[0], 'column' => $parts[1]];
+    }
 }
diff --git a/src/Utils.php b/src/Utils.php
@@ -430,6 +430,9 @@ public static function fixArgv(array $originalArgs): array
         return $newArgumens;
     }
 
+    /**
+     * @param array<string, null|array|bool|string>|int[]|string[] ...$configs
+     */
     public static function mergeConfigs(array ...$configs): array
     {
         $merged = (array)\array_shift($configs); // Start with the first array

diff --git a/tests/SchemaInheritTest.php b/tests/SchemaInheritTest.php
@@ -118,7 +118,7 @@ public function testOverideFilenamePattern(): void
                 'parent' => ['filename_pattern' => '/.*/i'],
             ],
             'filename_pattern' => [
-                'inherit' => 'parent/filename_pattern',
+                'inherit' => 'parent',
             ],
         ]);
 
@@ -141,7 +141,7 @@ public function testOverideCsvFull(): void
                     ],
                 ],
             ],
-            'csv' => ['inherit' => 'parent/csv'],
+            'csv' => ['inherit' => 'parent'],
         ]);
 
         isSame([
@@ -170,7 +170,7 @@ public function testOverideCsvPartial(): void
                 ],
             ],
             'csv' => [
-                'inherit'  => 'parent/csv',
+                'inherit'  => 'parent',
                 'encoding' => 'utf-32',
             ],
         ]);
@@ -199,7 +199,7 @@ public function testOverideStructuralRulesFull(): void
                 ],
             ],
             'structural_rules' => [
-                'inherit' => 'parent/structural_rules',
+                'inherit' => 'parent',
             ],
         ]);
 
@@ -223,7 +223,7 @@ public function testOverideStructuralRulesPartial1(): void
                 ],
             ],
             'structural_rules' => [
-                'inherit'             => 'parent/structural_rules',
+                'inherit'             => 'parent',
                 'allow_extra_columns' => true,
             ],
         ]);
@@ -240,7 +240,7 @@ public function testOverideStructuralRulesPartial2(): void
         $schema = new Schema([
             'includes'         => ['parent' => ['structural_rules' => []]],
             'structural_rules' => [
-                'inherit'             => 'parent/structural_rules',
+                'inherit'             => 'parent',
                 'allow_extra_columns' => true,
             ],
         ]);
@@ -275,13 +275,13 @@ public function testOverideColumnFull(): void
         $schema = new Schema([
             'includes' => ['parent' => ['columns' => [$parentColum0, $parentColum1]]],
             'columns'  => [
-                ['inherit' => 'parent/columns/0'],
-                ['inherit' => 'parent/columns/1'],
-                ['inherit' => 'parent/columns/0:'],
-                ['inherit' => 'parent/columns/1:'],
-                ['inherit' => 'parent/columns/Name'],
-                ['inherit' => 'parent/columns/0:Name'],
-                ['inherit' => 'parent/columns/1:Name'],
+                ['inherit' => 'parent/0'],
+                ['inherit' => 'parent/1'],
+                ['inherit' => 'parent/0:'],
+                ['inherit' => 'parent/1:'],
+                ['inherit' => 'parent/Name'],
+                ['inherit' => 'parent/0:Name'],
+                ['inherit' => 'parent/1:Name'],
             ],
         ]);
 
@@ -315,7 +315,7 @@ public function testOverideColumnPartial(): void
             'includes' => ['parent' => ['columns' => [$parentColum]]],
             'columns'  => [
                 [
-                    'inherit' => 'parent/columns/Name',
+                    'inherit' => 'parent/Name',
                     'name'    => 'Child name',
                     'rules'   => [
                         'is_int'       => true,
@@ -366,7 +366,7 @@ public function testOverideColumnRulesFull(): void
             'columns'  => [
                 [
                     'name'  => 'Child name',
-                    'rules' => ['inherit' => 'parent/columns/0:/rules'],
+                    'rules' => ['inherit' => 'parent/0:'],
                 ],
             ],
         ]);
@@ -410,7 +410,7 @@ public function testOverideColumnRulesPartial(): void
                 [
                     'name'  => 'Child name',
                     'rules' => [
-                        'inherit'      => 'parent/columns/0:/rules',
+                        'inherit'      => 'parent/0:',
                         'allow_values' => ['d', 'c'],
                         'length_max'   => 100,
                     ],
@@ -456,7 +456,7 @@ public function testOverideColumnAggregateRulesFull(): void
             'columns'  => [
                 [
                     'name'            => 'Child name',
-                    'aggregate_rules' => ['inherit' => 'parent/columns/0:/aggregate_rules'],
+                    'aggregate_rules' => ['inherit' => 'parent/0:'],
                 ],
             ],
         ]);
@@ -498,7 +498,7 @@ public function testOverideColumnAggregateRulesPartial(): void
                 [
                     'name'            => 'Child name',
                     'aggregate_rules' => [
-                        'inherit' => 'parent/columns/0:/aggregate_rules',
+                        'inherit' => 'parent/0:',
                         'sum_max' => 4200,
                         'sum_min' => 1,
                     ],

diff --git a/tests/schemas/inherit/child-of-child.yml b/tests/schemas/inherit/child-of-child.yml
@@ -24,17 +24,17 @@ filename_pattern: /child-of-child-\d.csv$/i
 
 
 csv:
-  inherit: 'parent-1_0/csv'
-  delimiter: 'dd'
-  quote_char: 'qq'
-  enclosure: 'ee'
+  inherit: parent-1_0
+  delimiter: dd
+  quote_char: qq
+  enclosure: ee
   encoding: utf-32
   bom: false
 
 
 structural_rules:
-  inherit: 'parent-1_0/structural_rules'
+  inherit: parent-1_0
   allow_extra_columns: false
 
 columns:
-  - inherit: 'parent-1_0/columns/Second Column'
+  - inherit: parent-1_0/Second Column
diff --git a/tests/schemas/inherit/child.yml b/tests/schemas/inherit/child.yml
@@ -21,56 +21,56 @@ includes:
 
 
 filename_pattern:
-  inherit: 'parent/filename_pattern'
+  inherit: parent
 
 
 csv:
-  inherit: 'parent/csv'
+  inherit: parent
   header: true
 
 
 structural_rules:
-  inherit: 'parent/structural_rules'
+  inherit: parent
   strict_column_order: true
 
 
 columns:
   # 0
-  - inherit: 'parent/columns/Name'
+  - inherit: parent/Name
 
   # 1
-  - inherit: 'parent/columns/Name'
+  - inherit: parent/Name
     name: Overridden name by column name
 
   # 2
-  - inherit: 'parent/columns/0:'
+  - inherit: 'parent/0:'
     name: Overridden name by column index
 
   # 3
-  - inherit: 'parent/columns/0:Name'
+  - inherit: parent/0:Name
     name: Overridden name by column index and column name
 
   # 4
-  - inherit: 'parent/columns/0:Name'
+  - inherit: parent/0:Name
     name: Overridden name by column index and column name + added rules
     rules:
       length_min: 1
 
   # 5
-  - inherit: 'parent/columns/0:Name'
+  - inherit: parent/0:Name
     name: Overridden name by column index and column name + added aggregate rules
     aggregate_rules:
       nth_num: [ 10, 0.05 ]
 
   # 6
   - name: Overridden only rules
     rules:
-      inherit: 'parent/columns/0:Name/rules'
+      inherit: parent/0:Name
 
   # 7
   - name: Overridden only aggregation rules
     aggregate_rules:
-      inherit: 'parent/columns/0:Name/aggregate_rules'
+      inherit: parent/0:Name
 
   # 8
-  - inherit: 'parent/columns/Second Column'
+  - inherit: parent/Second Column
diff --git a/tests/schemas/inherit/parent.yml b/tests/schemas/inherit/parent.yml
@@ -19,9 +19,9 @@ filename_pattern: /parent-\d.csv$/i
 
 csv:
   header: false
-  delimiter: 'd'
-  quote_char: 'q'
-  enclosure: 'e'
+  delimiter: d
+  quote_char: q
+  enclosure: e
   encoding: utf-16
   bom: true