From 216b1f234f6bcee2e1029ed963ebe531415e2fa2 Mon Sep 17 00:00:00 2001 From: Alex Goodwin Date: Mon, 1 Jun 2020 14:01:46 +1000 Subject: [PATCH 1/5] Simplify string-input checking --- .../Associations/AssociationStubBase.php | 3 +- .../ObjectMap/AssociationStubBaseTest.php | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/Models/ObjectMap/Entities/Associations/AssociationStubBase.php b/src/Models/ObjectMap/Entities/Associations/AssociationStubBase.php index 8ea3fec2..b204efb5 100644 --- a/src/Models/ObjectMap/Entities/Associations/AssociationStubBase.php +++ b/src/Models/ObjectMap/Entities/Associations/AssociationStubBase.php @@ -319,7 +319,8 @@ abstract public function morphicType(): string; */ protected function checkStringInput($input): bool { - if (null === $input || !is_string($input) || empty($input)) { + // null inputs are caught by the is_string check + if (!is_string($input) || empty($input)) { return false; } return true; diff --git a/tests/Orchestra/Unit/Models/ObjectMap/AssociationStubBaseTest.php b/tests/Orchestra/Unit/Models/ObjectMap/AssociationStubBaseTest.php index a16af8e8..68ed5032 100644 --- a/tests/Orchestra/Unit/Models/ObjectMap/AssociationStubBaseTest.php +++ b/tests/Orchestra/Unit/Models/ObjectMap/AssociationStubBaseTest.php @@ -47,4 +47,35 @@ public function testNotCompatibleOnDifferentMorphTypes() $this->assertFalse($foo->isCompatible($bar)); $this->assertFalse($bar->isCompatible($foo)); } + + public function stringInputProvider(): array + { + $result = []; + $result[] = [null, false]; + $result[] = [new \stdClass, false]; + $result[] = ['', false]; + $result[] = ['hiver', true]; + + return $result; + } + + /** + * @dataProvider stringInputProvider + * + * @param $input + * @param bool $expected + * @throws \ReflectionException + */ + public function testCheckStringInput($input, bool $expected) + { + $type = AssociationStubRelationType::NULL_ONE(); + $foo = new AssociationStubMonomorphic('', '', [], $type); + + $reflec = new \ReflectionClass($foo); + $method = $reflec->getMethod('checkStringInput'); + $method->setAccessible(true); + + $actual = $method->invokeArgs($foo, [$input]); + $this->assertEquals($expected, $actual); + } } From 78832c266b339117b101c04adf95085bfc068039 Mon Sep 17 00:00:00 2001 From: Alex Goodwin Date: Mon, 1 Jun 2020 14:08:29 +1000 Subject: [PATCH 2/5] Fill mutation hole --- .../Unit/Serialisers/SerialiserLowLevelWritersTest.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/Orchestra/Unit/Serialisers/SerialiserLowLevelWritersTest.php b/tests/Orchestra/Unit/Serialisers/SerialiserLowLevelWritersTest.php index 95ba107a..e597485b 100644 --- a/tests/Orchestra/Unit/Serialisers/SerialiserLowLevelWritersTest.php +++ b/tests/Orchestra/Unit/Serialisers/SerialiserLowLevelWritersTest.php @@ -19,6 +19,7 @@ use POData\Providers\Metadata\ResourcePropertyKind; use POData\Providers\Metadata\ResourceType; use POData\Providers\Metadata\ResourceTypeKind; +use POData\Providers\Metadata\Type\Boolean; use POData\Providers\Metadata\Type\DateTime; use POData\Providers\Metadata\Type\StringType; @@ -37,9 +38,9 @@ public function testUTF8StringNotMangled() public function testDateWithNonDateIType() { $date = Carbon::create(2019, 1, 1, 0, 0, 0); - $type = new StringType(); + $type = new Boolean(); - $expected = '2019-01-01 00:00:00'; + $expected = 'false'; $actual = SerialiserLowLevelWriters::primitiveToString($type, $date); From 824fe5485c1a06fe6b6a4afdec04b51ed3e7fa5e Mon Sep 17 00:00:00 2001 From: Alex Goodwin Date: Mon, 1 Jun 2020 14:34:04 +1000 Subject: [PATCH 3/5] Simplify write-output checking --- src/Query/LaravelWriteQuery.php | 5 +- .../Unit/Query/LaravelWriteQueryTest.php | 48 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/Query/LaravelWriteQuery.php b/src/Query/LaravelWriteQuery.php index 29da6186..77102bdf 100644 --- a/src/Query/LaravelWriteQuery.php +++ b/src/Query/LaravelWriteQuery.php @@ -60,8 +60,11 @@ protected function createUpdateDeleteProcessInput(array $data, array $paramList, protected function createUpdateDeleteProcessOutput(JsonResponse $result) { $outData = $result->getData(true); + $keys = array_keys($outData); + $expKeys = ['id', 'status', 'errors']; + $isGood = $expKeys === (array_intersect($expKeys, $keys)); - if (!(key_exists('id', $outData) && key_exists('status', $outData) && key_exists('errors', $outData))) { + if (!$isGood) { throw ODataException::createInternalServerError( 'Controller response array missing at least one of id, status and/or errors fields.' ); diff --git a/tests/Orchestra/Unit/Query/LaravelWriteQueryTest.php b/tests/Orchestra/Unit/Query/LaravelWriteQueryTest.php index 68e576ae..063144b7 100644 --- a/tests/Orchestra/Unit/Query/LaravelWriteQueryTest.php +++ b/tests/Orchestra/Unit/Query/LaravelWriteQueryTest.php @@ -11,7 +11,9 @@ use AlgoWeb\PODataLaravel\Orchestra\Tests\Models\OrchestraTestModel; use AlgoWeb\PODataLaravel\Orchestra\Tests\TestCase; use AlgoWeb\PODataLaravel\Query\LaravelWriteQuery; +use Illuminate\Http\JsonResponse; use Mockery as m; +use POData\Common\ODataException; use POData\Providers\Metadata\ResourceSet; use POData\UriProcessor\ResourcePathProcessor\SegmentParser\KeyDescriptor; @@ -68,4 +70,50 @@ public function testDefaultShouldUpdateIsFalseTopLevel() $foo->updateResource($resourceSet, $model, $keyDesc, $data); } + + public function processOutputProvider(): array + { + $result = []; + $result[] = [ [], false]; + $result[] = [ ['id'], false]; + $result[] = [ ['status'], false]; + $result[] = [ ['errors'], false]; + $result[] = [ ['id', 'status'], false]; + $result[] = [ ['id', 'errors'], false]; + $result[] = [ ['status', 'errors'], false]; + $result[] = [ ['id', 'status', 'errors'], true]; + + return $result; + } + + /** + * @dataProvider processOutputProvider + * + * @param array $keys + * @param bool $pass + * @throws \ReflectionException + */ + public function testCreateUpdateDeleteProcessOutput(array $keys, bool $pass) + { + $query = new LaravelWriteQuery(); + + $reflec = new \ReflectionClass($query); + $method = $reflec->getMethod('createUpdateDeleteProcessOutput'); + $method->setAccessible(true); + + if (!$pass) { + $this->expectException(ODataException::class); + $this->expectExceptionMessage('Controller response array missing at least one of id, status and/or errors fields.'); + } + + $data = array_flip($keys); + $response = m::mock(JsonResponse::class)->makePartial(); + $response->shouldReceive('getData')->withArgs(['true'])->andReturn($data); + + $method->invokeArgs($query, [$response]); + + if ($pass) { + $this->assertTrue(true); + } + } } From 3654654d2fe9f40154aa870f2be23226066f20fd Mon Sep 17 00:00:00 2001 From: Alex Goodwin Date: Mon, 1 Jun 2020 14:48:45 +1000 Subject: [PATCH 4/5] Remove redundant varname check --- src/Query/LaravelWriteQuery.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Query/LaravelWriteQuery.php b/src/Query/LaravelWriteQuery.php index 77102bdf..0a69be00 100644 --- a/src/Query/LaravelWriteQuery.php +++ b/src/Query/LaravelWriteQuery.php @@ -36,7 +36,7 @@ protected function createUpdateDeleteProcessInput(array $data, array $paramList, $varType = isset($spec['type']) ? $spec['type'] : null; $varName = $spec['name']; if (null == $varType && null !== $sourceEntityInstance) { - $parms[] = ('id' == $varName) ? $sourceEntityInstance->getKey() : $sourceEntityInstance->{$varName}; + $parms[] = $sourceEntityInstance->{$varName}; continue; } // TODO: Give this smarts and actively pick up instantiation details From ae294399ce61eecebd678fc65e37335b2416ef5b Mon Sep 17 00:00:00 2001 From: Alex Goodwin Date: Mon, 1 Jun 2020 15:39:16 +1000 Subject: [PATCH 5/5] Make defaults explicit in EntityField --- src/Models/MetadataTrait.php | 2 -- src/Models/ObjectMap/Entities/EntityField.php | 12 ++++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Models/MetadataTrait.php b/src/Models/MetadataTrait.php index c6c3a5a4..1d8cbfcf 100644 --- a/src/Models/MetadataTrait.php +++ b/src/Models/MetadataTrait.php @@ -329,8 +329,6 @@ public function extractGubbins() $nuField = new EntityField(); $nuField->setName($name); $nuField->setIsNullable($field['nullable']); - $nuField->setReadOnly(false); - $nuField->setCreateOnly(false); $nuField->setDefaultValue($field['default']); $nuField->setIsKeyField($this->getKeyName() == $name); $nuField->setFieldType(EntityFieldType::PRIMITIVE()); diff --git a/src/Models/ObjectMap/Entities/EntityField.php b/src/Models/ObjectMap/Entities/EntityField.php index fbefb94d..d537ae31 100644 --- a/src/Models/ObjectMap/Entities/EntityField.php +++ b/src/Models/ObjectMap/Entities/EntityField.php @@ -7,6 +7,10 @@ use POData\Providers\Metadata\Type\EdmPrimitiveType; use POData\Providers\Metadata\Type\TypeCode; +/** + * Class EntityField + * @package AlgoWeb\PODataLaravel\Models\ObjectMap\Entities + */ class EntityField { /** @@ -22,7 +26,7 @@ class EntityField /** * @var bool */ - private $isNullable; + private $isNullable = false; /** * @var mixed @@ -32,17 +36,17 @@ class EntityField /** * @var bool */ - private $readOnly; + private $readOnly = false; /** * @var bool */ - private $createOnly; + private $createOnly = false; /** * @var bool */ - private $isKeyField; + private $isKeyField = false; /** * @var EntityFieldPrimitiveType