Fixed bug in detecting array value type when first element was null

flow-php · Sep 27, 2024 · 51bd525 · 51bd525
1 parent cb98b18
commit 51bd525
Show file tree

Hide file tree

Showing 6 changed files with 163 additions and 14 deletions.
diff --git a/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JsonTest.php b/src/adapter/etl-adapter-json/tests/Flow/ETL/Adapter/JSON/Tests/Integration/JsonTest.php
@@ -17,9 +17,15 @@ final class JsonTest extends TestCase
 {
     public function test_json_loader() : void
     {
+        $path = __DIR__ . '/var/test_json_loader.json';
+
+        if (\file_exists($path)) {
+            \unlink($path);
+        }
+
         df()
             ->read(new FakeExtractor(100))
-            ->write(to_json($path = __DIR__ . '/var/test_json_loader.json'))
+            ->write(to_json($path))
             ->run();
 
         self::assertEquals(
@@ -55,16 +61,21 @@ public function test_json_loader_loading_empty_string() : void
 
     public function test_json_loader_overwrite_mode() : void
     {
+        $path = __DIR__ . '/var/test_json_loader.json';
+
+        if (\file_exists($path)) {
+            \unlink($path);
+        }
 
         df()
             ->read(new FakeExtractor(100))
-            ->write(to_json($path = __DIR__ . '/var/test_json_loader.json'))
+            ->write(to_json($path))
             ->run();
 
         df()
             ->read(new FakeExtractor(100))
             ->mode(overwrite())
-            ->write(to_json($path = __DIR__ . '/var/test_json_loader.json'))
+            ->write(to_json($path))
             ->run();
 
         $content = \file_get_contents($path);

diff --git a/src/core/etl/src/Flow/ETL/PHP/Type/ArrayContentDetector.php b/src/core/etl/src/Flow/ETL/PHP/Type/ArrayContentDetector.php
@@ -4,9 +4,9 @@
 
 namespace Flow\ETL\PHP\Type;
 
-use function Flow\ETL\DSL\{type_array, type_null};
+use function Flow\ETL\DSL\{type_array, type_null, type_string};
 use Flow\ETL\Exception\InvalidArgumentException;
-use Flow\ETL\PHP\Type\Native\ScalarType;
+use Flow\ETL\PHP\Type\Native\{NullType, ScalarType};
 
 final class ArrayContentDetector
 {
@@ -20,7 +20,7 @@ final class ArrayContentDetector
 
     private readonly int $uniqueValuesTypeCount;
 
-    public function __construct(Types $uniqueKeysType, Types $uniqueValuesType, bool $isList = false)
+    public function __construct(Types $uniqueKeysType, private readonly Types $uniqueValuesType, bool $isList = false)
     {
         $this->firstKeyType = $uniqueKeysType->first();
         $this->firstValueType = $uniqueValuesType->first();
@@ -63,4 +63,33 @@ public function isStructure() : bool
             && 1 === $this->uniqueKeysTypeCount
             && $this->firstKeyType()?->isString();
     }
+
+    public function valueType() : Type
+    {
+        $type = null;
+
+        foreach ($this->uniqueValuesType->all() as $nextType) {
+            if (null === $type) {
+                $type = $nextType;
+
+                continue;
+            }
+
+            if ($type instanceof NullType) {
+                $type = $nextType->makeNullable(true);
+
+                continue;
+            }
+
+            if ($nextType instanceof NullType) {
+                $type = $type->makeNullable(true);
+            }
+        }
+
+        if ($type === null) {
+            return type_string(true);
+        }
+
+        return $type;
+    }
 }
diff --git a/src/core/etl/src/Flow/ETL/PHP/Type/TypeDetector.php b/src/core/etl/src/Flow/ETL/PHP/Type/TypeDetector.php
@@ -21,7 +21,7 @@
 use Flow\ETL\PHP\Type\Logical\List\ListElement;
 use Flow\ETL\PHP\Type\Logical\Structure\StructureElement;
 use Flow\ETL\PHP\Type\Logical\{ListType, StructureType};
-use Flow\ETL\PHP\Type\Native\{ArrayType, EnumType, ScalarType};
+use Flow\ETL\PHP\Type\Native\{ArrayType, EnumType};
 
 final class TypeDetector
 {
@@ -62,17 +62,17 @@ public function detectType(mixed $value) : Type
                 \array_is_list($value)
             );
 
-            /** @var Type $firstValue */
-            $firstValue = $detector->firstValueType();
-            /** @var ScalarType $firstKeyType */
-            $firstKeyType = $detector->firstKeyType();
-
             if ($detector->isList()) {
-                return new ListType(ListElement::fromType($firstValue->makeNullable($valueTypes->has(type_null()))));
+                return new ListType(ListElement::fromType($detector->valueType()->makeNullable($valueTypes->has(type_null()))));
             }
 
             if ($detector->isMap()) {
-                return type_map($firstKeyType, $firstValue->makeNullable($valueTypes->has(type_null())));
+                /**
+                 * @psalm-suppress PossiblyNullArgument
+                 *
+                 * @phpstan-ignore-next-line
+                 */
+                return type_map($detector->firstKeyType(), $detector->valueType()->makeNullable($valueTypes->has(type_null())));
             }
 
             if ($detector->isStructure()) {

diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetector/Fixtures/github_user_event.json b/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetector/Fixtures/github_user_event.json
@@ -0,0 +1,33 @@
+{
+  "id": "42298808514",
+  "type": "CreateEvent",
+  "actor": {
+    "id": 1921950,
+    "login": "norberttech",
+    "display_login": "norberttech",
+    "gravatar_id": "",
+    "url": "https:\/\/api.github.com\/users\/norberttech",
+    "avatar_url": "https:\/\/avatars.githubusercontent.com\/u\/1921950?"
+  },
+  "repo": {
+    "id": 863548915,
+    "name": "flow-php\/symfony-http-foundation-bridge",
+    "url": "https:\/\/api.github.com\/repos\/flow-php\/symfony-http-foundation-bridge"
+  },
+  "payload": {
+    "ref": null,
+    "ref_type": "repository",
+    "master_branch": "main",
+    "description": "Flow PHP Symfony Http Foundation Bridge",
+    "pusher_type": "user"
+  },
+  "public": true,
+  "created_at": "2024-09-26T13:39:50Z",
+  "org": {
+    "id": 73495297,
+    "login": "flow-php",
+    "gravatar_id": "",
+    "url": "https:\/\/api.github.com\/orgs\/flow-php",
+    "avatar_url": "https:\/\/avatars.githubusercontent.com\/u\/73495297?"
+  }
+}
diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetector/StructuresTypeDetectorTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetector/StructuresTypeDetectorTest.php
@@ -0,0 +1,54 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flow\ETL\Tests\Unit\PHP\Type\TypeDetector;
+
+use function Flow\ETL\DSL\{structure_element, type_boolean, type_int, type_map, type_string, type_structure};
+use Flow\ETL\PHP\Type\TypeDetector;
+use PHPUnit\Framework\TestCase;
+
+final class StructuresTypeDetectorTest extends TestCase
+{
+    public function test_detecting_structures_with_nested_arrays() : void
+    {
+        $typeDetector = new TypeDetector();
+
+        $structure = \json_decode(\file_get_contents(__DIR__ . '/Fixtures/github_user_event.json'), true, 512, JSON_THROW_ON_ERROR);
+        $type = $typeDetector->detectType($structure);
+
+        self::assertEquals(
+            type_structure([
+                structure_element('id', type_string()),
+                structure_element('type', type_string()),
+                structure_element('actor', type_structure([
+                    structure_element('id', type_int()),
+                    structure_element('login', type_string()),
+                    structure_element('display_login', type_string()),
+                    structure_element('gravatar_id', type_string()),
+                    structure_element('url', type_string()),
+                    structure_element('avatar_url', type_string()),
+                ])),
+                structure_element('repo', type_structure([
+                    structure_element('id', type_int()),
+                    structure_element('name', type_string()),
+                    structure_element('url', type_string()),
+                ])),
+                structure_element('payload', type_map(
+                    key_type: type_string(),
+                    value_type: type_string(true)
+                )),
+                structure_element('public', type_boolean()),
+                structure_element('created_at', type_string()),
+                structure_element('org', type_structure([
+                    structure_element('id', type_int()),
+                    structure_element('login', type_string()),
+                    structure_element('gravatar_id', type_string()),
+                    structure_element('url', type_string()),
+                    structure_element('avatar_url', type_string()),
+                ])),
+            ]),
+            $type,
+        );
+    }
+}
diff --git a/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetectorTest.php b/src/core/etl/tests/Flow/ETL/Tests/Unit/PHP/Type/TypeDetectorTest.php
@@ -120,6 +120,28 @@ public static function provide_logical_types_data() : \Generator
             'list<structure{id: integer, name: string}>',
         ];
 
+        yield 'nullable list of integers' => [
+            [
+                null,
+                1,
+                2,
+                null,
+                3,
+            ],
+            ListType::class,
+            'list<?integer>',
+        ];
+
+        yield 'nullable map of string to int' => [
+            [
+                'one' => null,
+                'two' => null,
+                'three' => 3,
+            ],
+            MapType::class,
+            'map<string, ?integer>',
+        ];
+
         yield 'map with string key, of maps string with string' => [
             [
                 'one' => [