refactor(mapper): mapper improvements (#992)

Author: brendt

src/Tempest/Router/src/Mappers/PsrRequestToRequestMapper.php

@@ -43,10 +43,6 @@ public function map(mixed $from, mixed $to): array|object
             }
         }
 
-        $data = arr($data)
-            ->map(fn (mixed $value) => $value === '' ? null : $value)
-            ->toArray();
-
         $headersAsString = array_map(
             fn (array $items) => implode(',', $items),
             $from->getHeaders(),

src/Tempest/Router/src/Mappers/RequestToObjectMapper.php

@@ -6,8 +6,12 @@
 
 use Tempest\Mapper\Mapper;
 use Tempest\Router\Request;
+use Tempest\Validation\Exceptions\PropertyValidationException;
+use Tempest\Validation\Exceptions\ValidationException;
+use Tempest\Validation\Validator;
 
 use function Tempest\map;
+use function Tempest\reflect;
 
 final readonly class RequestToObjectMapper implements Mapper
 {
@@ -19,6 +23,30 @@ public function canMap(mixed $from, mixed $to): bool
     public function map(mixed $from, mixed $to): array|object
     {
         /** @var Request $from */
-        return map($from->body)->to($to);
+        $object = map($from->body)->to($to);
+
+        // We perform a new round of validation on the newly constructed object
+        // because we want to be sure that required uninitialized properties are also validated.
+        // This doesn't happen in the ArrayToObject mapper because we are more lenient there by design
+        // TODO: The better approach would be to have this RequestToObjectMapper be totally independent of ArrayToObjectMapper
+        $validator = new Validator();
+
+        $failingRules = [];
+
+        foreach (reflect($object)->getPublicProperties() as $property) {
+            $value = $property->isInitialized($object) ? $property->getValue($object) : null;
+
+            try {
+                $validator->validateProperty($property, $value);
+            } catch (PropertyValidationException $validationException) {
+                $failingRules[$property->getName()] = $validationException->failingRules;
+            }
+        }
+
+        if ($failingRules !== []) {
+            throw new ValidationException($object, $failingRules);
+        }
+
+        return $object;
     }
 }

tests/Fixtures/Modules/Posts/PostRequest.php

@@ -15,5 +15,5 @@ final class PostRequest implements Request
 
     public string $title;
 
-    public ?string $text;
+    public string $text;
 }

tests/Integration/Mapper/MapperTest.php

@@ -29,6 +29,7 @@
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithMapToCollisions;
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithMapToCollisionsJsonSerializable;
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithMultipleMapFrom;
+use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithNotNullInferenceA;
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithStrictOnClass;
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithStrictProperty;
 use Tests\Tempest\Integration\Mapper\Fixtures\ObjectWithStringProperty;

tests/Integration/Mapper/PsrRequestToRequestMapperTest.php

@@ -47,22 +47,6 @@ public function test_map_with(): void
         $this->assertEquals(['x-test' => 'test'], $request->headers);
     }
 
-    public function test_empty_strings_are_converted_to_null(): void
-    {
-        $mapper = new PsrRequestToRequestMapper();
-
-        /** @var PostRequest $request */
-        $request = $mapper->map(
-            from: $this->http->makePsrRequest(
-                uri: '/',
-                body: ['title' => 'a', 'text' => ''],
-            ),
-            to: PostRequest::class,
-        );
-
-        $this->assertNull($request->text);
-    }
-
     public function test_map_with_with_missing_data(): void
     {
         $this->expectException(MissingValuesException::class);

tests/Integration/Route/Fixtures/RequestObjectA.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Tests\Tempest\Integration\Route\Fixtures;
+
+final class RequestObjectA
+{
+    public RequestObjectB $b;
+}

tests/Integration/Route/Fixtures/RequestObjectB.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace Tests\Tempest\Integration\Route\Fixtures;
+
+final class RequestObjectB
+{
+    public string $name;
+}

tests/Integration/Route/RequestToObjectMapperTest.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace Tests\Tempest\Integration\Route;
+
+use Tempest\Http\Method;
+use Tempest\Router\GenericRequest;
+use Tempest\Validation\Exceptions\ValidationException;
+use Tempest\Validation\Rules\NotNull;
+use Tests\Tempest\Integration\FrameworkIntegrationTestCase;
+use Tests\Tempest\Integration\Route\Fixtures\RequestObjectA;
+use function Tempest\map;
+
+final class RequestToObjectMapperTest extends FrameworkIntegrationTestCase
+{
+    public function test_request(): void
+    {
+        $request = new GenericRequest(method: Method::POST, uri: '/', body: []);
+
+        try {
+            map($request)->to(RequestObjectA::class);
+        } catch (ValidationException $validationException) {
+            $this->assertInstanceOf(NotNull::class, $validationException->failingRules['b'][0]);
+        }
+    }
+}