Update behaviour for immutable and computed properties (#837)

* Strip computed properties from payload and update OpenAPI documentation for `computed` and `immutable` properties * code style: make boolean condition easier to read and understand * OpenAPI: Display `Computed` properties with the entity schema and `ComputedDefault` properties with the create schema * update phpunit snapshots * Bugfix: Initialise `$cookies` when creating a Request object from `Illuminate\Request` * Improvement: Utilizing the method-chaining nature of `Model.setAttribute` adds support for external model mutations and polymorphism. * PHPUnit: Update snapshots for `mongo` test suite (computed properties are defined for the entity in OpenAPI documentation) * Bugfix: Add `description` to Enum type to address invalid order of arguments * EloquentEntitySet::create - use the property source name for the attribute * `Property.IsComputed` no longer matches on ComputedDefault annotations. * CR Feedback: Update entity auto-detected key property annotation from `ComputedDefault` to `Computed` * Update tests * EPC-11157: validate guids property values * Remove "modern" PHP syntax * Prevent `Str::startsWith` being called with `null` on `Transaction:getAcceptedContentTypes` --------- Co-authored-by: sean.james <[email protected]>
flat3 · Sep 19, 2024 · b812db2 · b812db2
1 parent be527e5
commit b812db2
Show file tree

Hide file tree

Showing 158 changed files with 323 additions and 169 deletions.
diff --git a/src/Attributes/LodataEnum.php b/src/Attributes/LodataEnum.php
@@ -12,11 +12,20 @@
 #[Attribute(Attribute::TARGET_CLASS | Attribute::IS_REPEATABLE)]
 class LodataEnum extends LodataProperty
 {
-    protected string $enum;
-
-    public function __construct(string $name, string $enum, ?string $source = null)
-    {
-        parent::__construct($name, $source);
+    /** @var string */
+    protected $enum;
+
+    public function __construct(
+        string $name,
+        string $enum,
+        ?string $description = null,
+        ?string $source = null,
+        ?bool $nullable = true,
+        ?bool $immutable = false
+    ) {
+        parent::__construct($name, $description, $source);
+        $this->nullable = $nullable;
+        $this->immutable = $immutable;
         $this->enum = $enum;
     }
 
@@ -33,4 +42,4 @@ public function getType(): Type
 
         return Lodata::getEnumerationType($this->enum);
     }
-}
+}
diff --git a/src/ComplexType.php b/src/ComplexType.php
@@ -6,6 +6,8 @@
 
 use ArrayAccess;
 use Flat3\Lodata\Annotation\Core\V1\Computed;
+use Flat3\Lodata\Annotation\Core\V1\ComputedDefaultValue;
+use Flat3\Lodata\Annotation\Core\V1\Immutable;
 use Flat3\Lodata\Controller\Transaction;
 use Flat3\Lodata\Exception\Protocol\NotAcceptableException;
 use Flat3\Lodata\Helper\Constants;
@@ -243,9 +245,11 @@ public function getOpenAPISchema(): array
         return [
             'type' => Constants::oapiObject,
             'title' => $this->getName(),
-            'properties' => (object) $this->getDeclaredProperties()->map(function (DeclaredProperty $property) {
-                return $property->getOpenAPISchema();
-            })
+            'properties' => (object) $this->getProperties()
+                ->sliceByClass([DeclaredProperty::class, GeneratedProperty::class])
+                ->map(function (Property $property) {
+                    return $property->getOpenAPISchema();
+                })
         ];
     }
 
@@ -259,7 +263,8 @@ public function getOpenAPICreateSchema(): array
             'type' => Constants::oapiObject,
             'title' => __('lodata:::name (Create schema)', ['name' => $this->getName()]),
             'properties' => (object) $this->getDeclaredProperties()->filter(function (DeclaredProperty $property) {
-                return $property->getAnnotations()->sliceByClass([Computed::class])->isEmpty();
+                return $property->getAnnotations()->sliceByClass([Computed::class])->isEmpty()
+                    || $property->getAnnotations()->sliceByClass([ComputedDefaultValue::class])->hasEntries();
             })->map(function (DeclaredProperty $property) {
                 return $property->getOpenAPISchema();
             })
@@ -276,7 +281,7 @@ public function getOpenAPIUpdateSchema(): array
             'type' => Constants::oapiObject,
             'title' => __('lodata:::name (Update schema)', ['name' => $this->getName()]),
             'properties' => (object) $this->getDeclaredProperties()->filter(function (DeclaredProperty $property) {
-                return $property->getAnnotations()->sliceByClass([Computed::class])->isEmpty();
+                return $property->getAnnotations()->sliceByClass([Computed::class, Immutable::class])->isEmpty();
             })->map(function (DeclaredProperty $property) {
                 return $property->getOpenAPISchema();
             })
@@ -322,7 +327,7 @@ public function assertPropertyValues(PropertyValues $propertyValues): PropertyVa
     public function assertUpdateProperties(PropertyValues $propertyValues): PropertyValues
     {
         return $this->assertPropertyValues($propertyValues)->filter(function (PropertyValue $propertyValue) {
-            return !$propertyValue->getProperty()->isImmutable();
+            return !($propertyValue->getProperty()->isImmutable() || $propertyValue->getProperty()->isComputed());
         });
     }
 
@@ -353,7 +358,7 @@ public function assertCreateProperties(
                 continue;
             }
 
-            if ($declaredProperty->isNullable() || $declaredProperty->isComputed()) {
+            if ($declaredProperty->isNullable() || $declaredProperty->isComputed() || $declaredProperty->isComputedDefault()) {
                 continue;
             }
 
@@ -371,6 +376,8 @@ public function assertCreateProperties(
             $declaredProperty->assertAllowsValue(null);
         }
 
-        return $propertyValues;
+        return $propertyValues->filter(function (PropertyValue $propertyValue) {
+            return !$propertyValue->getProperty()->isComputed();
+        });
     }
 }
diff --git a/src/Controller/Request.php b/src/Controller/Request.php
@@ -63,6 +63,7 @@ public function __construct(IlluminateRequest $request)
     {
         $this->method = $request->getRealMethod();
         $this->headers = $request->headers;
+        $this->cookies = $request->cookies;
         $this->query = $request->query;
         $this->content = $request->content;
         $this->server = $request->server;

diff --git a/src/Controller/Transaction.php b/src/Controller/Transaction.php
@@ -642,7 +642,7 @@ public function getAcceptedContentTypes(): MediaTypes
     {
         $formatQueryOption = $this->getFormat()->getValue();
 
-        if (Str::startsWith($formatQueryOption, ['json', 'xml'])) {
+        if ($formatQueryOption && Str::startsWith($formatQueryOption, ['json', 'xml'])) {
             if (!in_array($formatQueryOption, ['json', 'xml'])) {
                 throw new BadRequestException(
                     'invalid_short_format',

diff --git a/src/Drivers/CSVEntityType.php b/src/Drivers/CSVEntityType.php
@@ -4,7 +4,7 @@
 
 namespace Flat3\Lodata\Drivers;
 
-use Flat3\Lodata\Annotation\Core\V1\ComputedDefaultValue;
+use Flat3\Lodata\Annotation\Core\V1\Computed;
 use Flat3\Lodata\DeclaredProperty;
 use Flat3\Lodata\EntityType;
 use Flat3\Lodata\Type;
@@ -18,6 +18,6 @@ class CSVEntityType extends EntityType
     public function __construct($identifier)
     {
         parent::__construct($identifier);
-        $this->setKey((new DeclaredProperty('offset', Type::int64()))->addAnnotation(new ComputedDefaultValue));
+        $this->setKey((new DeclaredProperty('offset', Type::int64()))->addAnnotation(new Computed));
     }
-}
+}
diff --git a/src/Drivers/EloquentEntitySet.php b/src/Drivers/EloquentEntitySet.php
@@ -7,7 +7,7 @@
 use Doctrine\DBAL\Schema\Column;
 use Exception;
 use Flat3\Lodata\Annotation\Capabilities\V1\DeepInsertSupport;
-use Flat3\Lodata\Annotation\Core\V1\ComputedDefaultValue;
+use Flat3\Lodata\Annotation\Core\V1\Computed;
 use Flat3\Lodata\Annotation\Core\V1\Description;
 use Flat3\Lodata\Attributes\LodataIdentifier;
 use Flat3\Lodata\Attributes\LodataProperty;
@@ -187,14 +187,12 @@ public function read(PropertyValue $key): Entity
      */
     protected function setModelAttributes(Model $model, PropertyValues $propertyValues): Model
     {
-        foreach ($propertyValues->getDeclaredPropertyValues() as $propertyValue) {
-            $model->setAttribute(
-                $this->getPropertySourceName($propertyValue->getProperty()),
-                $propertyValue->getPrimitive()->toMixed()
+        return $propertyValues->getDeclaredPropertyValues()->reduce(function(Model $model, PropertyValue $value) {
+            return $model->setAttribute(
+                $this->getPropertySourceName($value->getProperty()),
+                $value->getPrimitive()->toMixed(),
             );
-        }
-
-        return $model;
+        }, $model);
     }
 
     /**
@@ -229,7 +227,7 @@ public function create(PropertyValues $propertyValues): Entity
             foreach ($navigationProperty->getConstraints() as $constraint) {
                 $referencedProperty = $constraint->getReferencedProperty();
                 $model->setAttribute(
-                    $referencedProperty->getName(),
+                    $this->getPropertySourceName($referencedProperty),
                     $this->navigationSource->getParent()->getEntityId()->getPrimitive()->toMixed()
                 );
             }
@@ -677,7 +675,7 @@ public function columnToDeclaredProperty(Column $column): ?DeclaredProperty
 
         $defaultValue = $model->getAttributeValue($column->getName());
         if ($defaultValue) {
-            $property->addAnnotation(new ComputedDefaultValue);
+            $property->addAnnotation(new Computed);
             $property->setDefaultValue($defaultValue);
         }
 

diff --git a/src/EntityType.php b/src/EntityType.php
@@ -5,6 +5,7 @@
 namespace Flat3\Lodata;
 
 use Flat3\Lodata\Annotation\Core\V1\Computed;
+use Flat3\Lodata\Annotation\Core\V1\Immutable;
 use Flat3\Lodata\Controller\Transaction;
 use Flat3\Lodata\Exception\Internal\PathNotHandledException;
 use Flat3\Lodata\Facades\Lodata;
@@ -98,7 +99,8 @@ public function getOpenAPIUpdateSchema(): array
             'type' => Constants::oapiObject,
             'title' => __('lodata:::name (Update schema)', ['name' => $this->getName()]),
             'properties' => (object) $this->getDeclaredProperties()->filter(function (DeclaredProperty $property) {
-                return $property->getAnnotations()->sliceByClass([Computed::class])->isEmpty() && $property !== $this->getKey();
+                return $property->getAnnotations()->sliceByClass([Computed::class, Immutable::class])->isEmpty()
+                    && $property !== $this->getKey();
             })->map(function (DeclaredProperty $property) {
                 return $property->getOpenAPISchema();
             })

diff --git a/src/GeneratedProperty.php b/src/GeneratedProperty.php
@@ -13,6 +13,11 @@
  */
 abstract class GeneratedProperty extends Property
 {
+    public function __construct($name, ?Type $type = null)
+    {
+        parent::__construct($name, $type !== null ? $type : Type::untyped());
+    }
+
     /**
      * Generate the property value for this property on the provided entity
      * @param  ComplexValue  $value  Entity this property is generated on

diff --git a/src/Helper/ObjectArray.php b/src/Helper/ObjectArray.php
@@ -302,6 +302,19 @@ public function map(callable $callback)
         return array_map($callback, $this->array);
     }
 
+    /**
+     * @param  callable(mixed $initial, mixed $value, mixed $key): mixed  $callback
+     * @param $initial
+     * @return mixed|null
+     */
+    public function reduce(callable $callback, $initial = null)
+    {
+        foreach ($this->array as $key => $value) {
+            $initial = $callback($initial, $value, $key);
+        }
+        return $initial;
+    }
+
     /**
      * Sort the objects in the array
      * @param  callable  $callback

diff --git a/src/Primitive.php b/src/Primitive.php
@@ -57,6 +57,11 @@ public function __construct($value = null)
      * @return Primitive
      */
     abstract public function set($value);
+
+    public function allows($value): bool
+    {
+        return true;
+    }
 
     /**
      * Get the internal representation of the value

diff --git a/src/PrimitiveType.php b/src/PrimitiveType.php
@@ -99,4 +99,9 @@ public function getOpenAPISchema(): array
     {
         return $this->instance()->getOpenAPISchema();
     }
-}
+
+    public function allowsValue($value): bool
+    {
+        return $this->instance()->allows($value);
+    }
+}
diff --git a/src/Property.php b/src/Property.php
@@ -370,7 +370,11 @@ public function isSearchable(): bool
      */
     public function assertAllowsValue($value)
     {
-        if (!$this->isNullable() && $value === null) {
+        if ($value === null) {
+            if ($this->isNullable()) {
+                return;
+            }
+
             throw new BadRequestException(
                 'property_not_nullable',
                 sprintf("The property '%s' cannot be set to null", $this->getName())
@@ -383,6 +387,13 @@ public function assertAllowsValue($value)
                 sprintf("The value property '%s' exceeds the maximum length", $this->getName())
             );
         }
+
+        if (!$this->type->allowsValue($value)) {
+            throw new BadRequestException(
+                'property_value_invalid',
+                sprintf("The value property '%s' is not valid", $this->getName()),
+            );
+        }
     }
 
     /**
@@ -409,8 +420,16 @@ public function getOpenAPISchema(): array
     public function isComputed(): bool
     {
         return $this instanceof ComputedProperty ||
-            $this->hasAnnotation(new Computed, Boolean::true()) ||
-            $this->hasAnnotation(new ComputedDefaultValue, Boolean::true());
+            $this->hasAnnotation(new Computed, Boolean::true());
+    }
+
+    /**
+     * Determine whether this property is computed on create operations
+     * @return bool
+     */
+    public function isComputedDefault(): bool
+    {
+        return $this->hasAnnotation(new ComputedDefaultValue, Boolean::true());
     }
 
     /**

diff --git a/src/Type.php b/src/Type.php
@@ -229,4 +229,9 @@ public function is(string $class): bool
     {
         return is_a($this->factory, $class, true);
     }
+
+    public function allowsValue($value): bool
+    {
+        return true;
+    }
 }
diff --git a/src/Type/Guid.php b/src/Type/Guid.php
@@ -81,4 +81,9 @@ public function getOpenAPISchema(?Property $property = null): array
             'pattern' => '^'.Lexer::guid.'$',
         ]);
     }
+
+    public function allows($value): bool
+    {
+        return Lexer::patternCheck(Lexer::guid, (string) $value);
+    }
 }
diff --git a/tests/Drivers/WithNumericCollectionDriver.php b/tests/Drivers/WithNumericCollectionDriver.php
@@ -4,7 +4,7 @@
 
 namespace Flat3\Lodata\Tests\Drivers;
 
-use Flat3\Lodata\Annotation\Core\V1\ComputedDefaultValue;
+use Flat3\Lodata\Annotation\Core\V1\Computed;
 use Flat3\Lodata\DeclaredProperty;
 use Flat3\Lodata\Drivers\CollectionEntitySet;
 use Flat3\Lodata\EntityType;
@@ -22,7 +22,7 @@ protected function setUpDriver(): void
 
         $entityType = new EntityType('passenger');
         $entityType->setOpen();
-        $entityType->setKey((new DeclaredProperty('id', Type::int64()))->addAnnotation(new ComputedDefaultValue));
+        $entityType->setKey((new DeclaredProperty('id', Type::int64()))->addAnnotation(new Computed));
         $this->addPassengerProperties($entityType);
         $entityType->getDeclaredProperty('name')->setSearchable();
         $entitySet = new CollectionEntitySet($this->entitySet, $entityType);
@@ -42,4 +42,4 @@ protected function captureDriverState(): array
     {
         return array_values(Lodata::getEntitySet($this->entitySet)->getCollection()->toArray());
     }
-}
+}
diff --git a/tests/__snapshots__/Entity/CSVTest__test_generated_property__1.xml b/tests/__snapshots__/Entity/CSVTest__test_generated_property__1.xml
@@ -83,7 +83,7 @@
           <PropertyRef Name="offset"/>
         </Key>
         <Property Name="offset" Type="Edm.Int64" Nullable="false">
-          <Annotation Term="Org.OData.Core.V1.ComputedDefaultValue" Bool="true"/>
+          <Annotation Term="Org.OData.Core.V1.Computed" Bool="true"/>
         </Property>
         <Property Name="name" Type="Edm.String" Nullable="false" MaxLength="255"/>
         <Property Name="age" Type="Edm.Double" Nullable="true"/>

diff --git a/tests/__snapshots__/Entity/CSVTest__test_generated_property__3.json b/tests/__snapshots__/Entity/CSVTest__test_generated_property__3.json
@@ -120,7 +120,7 @@
         "offset": {
             "$Type": "Edm.Int64",
             "$Nullable": false,
-            "@Org.OData.Core.V1.ComputedDefaultValue": true
+            "@Org.OData.Core.V1.Computed": true
         },
         "name": {
             "$Type": "Edm.String",

diff --git a/tests/__snapshots__/Entity/CSVTest__test_generated_property__4.json b/tests/__snapshots__/Entity/CSVTest__test_generated_property__4.json
@@ -425,6 +425,13 @@
                             "type": "string",
                             "nullable": true
                         }
+                    },
+                    "cp": {
+                        "type": "integer",
+                        "format": "int32",
+                        "minimum": -2147483648,
+                        "maximum": 2147483647,
+                        "nullable": true
                     }
                 }
             },

diff --git a/tests/__snapshots__/Entity/EloquentTest__test_generated_property__4.json b/tests/__snapshots__/Entity/EloquentTest__test_generated_property__4.json
@@ -3972,6 +3972,13 @@
                             "type": "string",
                             "nullable": true
                         }
+                    },
+                    "cp": {
+                        "type": "integer",
+                        "format": "int32",
+                        "minimum": -2147483648,
+                        "maximum": 2147483647,
+                        "nullable": true
                     }
                 }
             },