From 697317002e0502879b07afe13ca4fd335d9b7315 Mon Sep 17 00:00:00 2001 From: SilverFire - Dmitry Naumenko Date: Tue, 28 May 2019 12:33:23 +0300 Subject: [PATCH 1/4] Allow passing prepared objects instead of array definition --- src/SpecBaseObject.php | 7 ++++++- src/spec/Paths.php | 12 ++++++++++-- src/spec/Responses.php | 4 +++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/SpecBaseObject.php b/src/SpecBaseObject.php index 9a4e4a0c..f16a6de0 100644 --- a/src/SpecBaseObject.php +++ b/src/SpecBaseObject.php @@ -121,9 +121,14 @@ public function __construct(array $data) */ private function instantiate($type, $data) { - if (isset($data['$ref'])) { + if ($data instanceof $type) { + return $data; + } + + if (is_array($data) && isset($data['$ref'])) { return new Reference($data, $type); } + try { return new $type($data); } catch (\TypeError $e) { diff --git a/src/spec/Paths.php b/src/spec/Paths.php index dc3899ba..91ce343f 100644 --- a/src/spec/Paths.php +++ b/src/spec/Paths.php @@ -39,7 +39,7 @@ class Paths implements SpecObjectInterface, ArrayAccess, Countable, IteratorAggr /** * Create an object from spec data. - * @param array $data spec data read from YAML or JSON + * @param PathItem[]|array[] $data spec data read from YAML or JSON * @throws TypeErrorException in case invalid data is supplied. */ public function __construct(array $data) @@ -47,8 +47,16 @@ public function __construct(array $data) foreach ($data as $path => $object) { if ($object === null) { $this->_paths[$path] = null; - } else { + } elseif (is_array($object)) { $this->_paths[$path] = new PathItem($object); + } elseif ($object instanceof PathItem) { + $this->_paths[$path] = $object; + } else { + $givenType = gettype($object); + if ($givenType === 'object') { + $givenType = get_class($object); + } + throw new \TypeError(sprintf('Path MUST be either array or PathItem object, "%s" given', $givenType)); } } } diff --git a/src/spec/Responses.php b/src/spec/Responses.php index e9e044b7..a4eb795b 100644 --- a/src/spec/Responses.php +++ b/src/spec/Responses.php @@ -42,7 +42,9 @@ public function __construct(array $data) // From Spec: This field MUST be enclosed in quotation marks (for example, "200") for compatibility between JSON and YAML. $statusCode = (string) $statusCode; if (preg_match('~^(?:default|[1-5](?:[0-9][0-9]|XX))$~', $statusCode)) { - if (isset($response['$ref'])) { + if ($response instanceof Response || $response instanceof Reference) { + $this->_responses[$statusCode] = $response; + } elseif (isset($response['$ref'])) { $this->_responses[$statusCode] = new Reference($response, Response::class); } else { $this->_responses[$statusCode] = new Response($response); From 8e42370155143dd1b19bff3fd1f979a58ed8d989 Mon Sep 17 00:00:00 2001 From: SilverFire - Dmitry Naumenko Date: Thu, 27 Jun 2019 15:55:05 +0300 Subject: [PATCH 2/4] Added tests, enhanced PHPDocs --- src/spec/Paths.php | 2 +- src/spec/Responses.php | 15 ++++++++++---- tests/spec/PathTest.php | 45 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/spec/Paths.php b/src/spec/Paths.php index bac7c160..ae364954 100644 --- a/src/spec/Paths.php +++ b/src/spec/Paths.php @@ -60,7 +60,7 @@ public function __construct(array $data) if ($givenType === 'object') { $givenType = get_class($object); } - throw new \TypeError(sprintf('Path MUST be either array or PathItem object, "%s" given', $givenType)); + throw new TypeErrorException(sprintf('Path MUST be either array or PathItem object, "%s" given', $givenType)); } } } diff --git a/src/spec/Responses.php b/src/spec/Responses.php index 3815b231..81801059 100644 --- a/src/spec/Responses.php +++ b/src/spec/Responses.php @@ -10,6 +10,7 @@ use ArrayAccess; use ArrayIterator; use cebe\openapi\DocumentContextInterface; +use cebe\openapi\exceptions\TypeErrorException; use cebe\openapi\exceptions\UnresolvableReferenceException; use cebe\openapi\json\JsonPointer; use cebe\openapi\ReferenceContext; @@ -37,8 +38,8 @@ class Responses implements SpecObjectInterface, DocumentContextInterface, ArrayA /** * Create an object from spec data. - * @param array $data spec data read from YAML or JSON - * @throws \cebe\openapi\exceptions\TypeErrorException in case invalid data is supplied. + * @param array[]|Response[]|Reference[] $data spec data read from YAML or JSON + * @throws TypeErrorException in case invalid data is supplied. */ public function __construct(array $data) { @@ -48,10 +49,16 @@ public function __construct(array $data) if (preg_match('~^(?:default|[1-5](?:[0-9][0-9]|XX))$~', $statusCode)) { if ($response instanceof Response || $response instanceof Reference) { $this->_responses[$statusCode] = $response; - } elseif (isset($response['$ref'])) { + } elseif (is_array($response) && isset($response['$ref'])) { $this->_responses[$statusCode] = new Reference($response, Response::class); - } else { + } elseif (is_array($response)) { $this->_responses[$statusCode] = new Response($response); + } else { + $givenType = gettype($response); + if ($givenType === 'object') { + $givenType = get_class($response); + } + throw new TypeErrorException(sprintf('Response MUST be either an array, a Response or a Reference object, "%s" given', $givenType)); } } else { $this->_errors[] = "Responses: $statusCode is not a valid HTTP status code."; diff --git a/tests/spec/PathTest.php b/tests/spec/PathTest.php index 1e107b0b..a6e8e2c3 100644 --- a/tests/spec/PathTest.php +++ b/tests/spec/PathTest.php @@ -4,6 +4,8 @@ use cebe\openapi\spec\Operation; use cebe\openapi\spec\PathItem; use cebe\openapi\spec\Paths; +use cebe\openapi\spec\Response; +use cebe\openapi\spec\Responses; /** * @covers \cebe\openapi\spec\Paths @@ -63,6 +65,48 @@ public function testRead() } } + public function testCreateionFromObjects() + { + $paths = new Paths([ + '/pets' => new PathItem([ + 'get' => new Operation([ + 'responses' => new Responses([ + 200 => new Response(['description' => 'A list of pets.']), + 404 => ['description' => 'The pets list is gone 🙀'], + ]) + ]) + ]) + ]); + + $this->assertTrue($paths->hasPath('/pets')); + $this->assertInstanceOf(PathItem::class, $paths->getPath('/pets')); + $this->assertInstanceOf(PathItem::class, $paths['/pets']); + $this->assertInstanceOf(Operation::class, $paths->getPath('/pets')->get); + + $this->assertSame('A list of pets.', $paths->getPath('/pets')->get->responses->getResponse(200)->description); + $this->assertSame('The pets list is gone 🙀', $paths->getPath('/pets')->get->responses->getResponse(404)->description); + } + + public function badPathsConfigProvider() + { + yield [['/pets' => 'foo'], 'Path MUST be either array or PathItem object, "string" given']; + yield [['/pets' => 42], 'Path MUST be either array or PathItem object, "integer" given']; + yield [['/pets' => false], 'Path MUST be either array or PathItem object, "boolean" given']; + yield [['/pets' => new stdClass()], 'Path MUST be either array or PathItem object, "stdClass" given']; + // The last one can be supported in future, but now SpecBaseObjects::__construct() requires array explicitly + } + + /** + * @dataProvider badPathsConfigProvider + */ + public function testPathsCanNotBeCreatedFromBullshit($config, $expectedException) + { + $this->expectException(\cebe\openapi\exceptions\TypeErrorException::class); + $this->expectExceptionMessage($expectedException); + + new Paths($config); + } + public function testInvalidPath() { /** @var $paths Paths */ @@ -88,4 +132,5 @@ public function testInvalidPath() ], $paths->getErrors()); $this->assertFalse($result); } + } From 72ba6933dfc8b23bc216f37d1a48d7c3b637d201 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 28 Jun 2019 14:52:13 +0200 Subject: [PATCH 3/4] complete feature of passing objects to constructor completed test coverage --- src/SpecBaseObject.php | 2 +- src/spec/MediaType.php | 12 ++++++++- src/spec/Responses.php | 2 +- src/spec/Schema.php | 6 +++++ tests/spec/MediaTypeTest.php | 52 +++++++++++++++++++++++++++++++++++- tests/spec/ResponseTest.php | 31 +++++++++++++++++++++ tests/spec/SchemaTest.php | 47 ++++++++++++++++++++++++++++++++ 7 files changed, 148 insertions(+), 4 deletions(-) diff --git a/src/SpecBaseObject.php b/src/SpecBaseObject.php index b4944913..9227cc7a 100644 --- a/src/SpecBaseObject.php +++ b/src/SpecBaseObject.php @@ -99,7 +99,7 @@ public function __construct(array $data) } $this->_properties[$property] = []; foreach ($data[$property] as $key => $item) { - if ($type[1] === 'string') { + if ($type[1] === Type::STRING) { if (!is_string($item)) { $this->_errors[] = "property '$property' must be map, but entry '$key' is of type " . \gettype($item) . '.'; } diff --git a/src/spec/MediaType.php b/src/spec/MediaType.php index 2bb3d673..1e891050 100644 --- a/src/spec/MediaType.php +++ b/src/spec/MediaType.php @@ -50,7 +50,17 @@ public function __construct(array $data) if (!empty($encoding)) { foreach ($encoding as $property => $encodingData) { - $encoding[$property] = new Encoding($encodingData, $this->schema->properties[$property] ?? null); + if ($encodingData instanceof Encoding) { + $encoding[$property] = $encodingData; + } elseif (is_array($encodingData)) { + $encoding[$property] = new Encoding($encodingData, $this->schema->properties[$property] ?? null); + } else { + $givenType = gettype($encodingData); + if ($givenType === 'object') { + $givenType = get_class($encodingData); + } + throw new TypeErrorException(sprintf('Encoding MUST be either array or Encoding object, "%s" given', $givenType)); + } } $this->encoding = $encoding; } diff --git a/src/spec/Responses.php b/src/spec/Responses.php index 81801059..6c11eacf 100644 --- a/src/spec/Responses.php +++ b/src/spec/Responses.php @@ -38,7 +38,7 @@ class Responses implements SpecObjectInterface, DocumentContextInterface, ArrayA /** * Create an object from spec data. - * @param array[]|Response[]|Reference[] $data spec data read from YAML or JSON + * @param Response[]|Reference[]|array[] $data spec data read from YAML or JSON * @throws TypeErrorException in case invalid data is supplied. */ public function __construct(array $data) diff --git a/src/spec/Schema.php b/src/spec/Schema.php index 2c62a7ac..5488425a 100644 --- a/src/spec/Schema.php +++ b/src/spec/Schema.php @@ -121,6 +121,12 @@ public function __construct(array $data) $e ); } + } elseif (!($data['additionalProperties'] instanceof Schema || is_bool($data['additionalProperties']))) { + $givenType = gettype($data['additionalProperties']); + if ($givenType === 'object') { + $givenType = get_class($data['additionalProperties']); + } + throw new TypeErrorException(sprintf('Schema::$additionalProperties MUST be either array, boolean or a Schema object, "%s" given', $givenType)); } } parent::__construct($data); diff --git a/tests/spec/MediaTypeTest.php b/tests/spec/MediaTypeTest.php index e0c5d8d2..c2d38521 100644 --- a/tests/spec/MediaTypeTest.php +++ b/tests/spec/MediaTypeTest.php @@ -64,4 +64,54 @@ public function testRead() $this->assertEquals($expectedCat, $mediaType->examples['cat']->value); } -} \ No newline at end of file + + public function testCreateionFromObjects() + { + $mediaType = new MediaType([ + 'schema' => new \cebe\openapi\spec\Schema([ + 'type' => \cebe\openapi\spec\Type::OBJECT, + 'properties' => [ + 'id' => new \cebe\openapi\spec\Schema(['type' => 'string', 'format' => 'uuid']), + 'profileImage' => new \cebe\openapi\spec\Schema(['type' => 'string', 'format' => 'binary']), + ], + ]), + 'encoding' => [ + 'id' => [], + 'profileImage' => new \cebe\openapi\spec\Encoding([ + 'contentType' => 'image/png, image/jpeg', + 'headers' => [ + 'X-Rate-Limit-Limit' => new \cebe\openapi\spec\Header([ + 'description' => 'The number of allowed requests in the current period', + 'schema' => new \cebe\openapi\spec\Schema(['type' => 'integer']), + ]), + ], + ]), + ], + ]); + + // default value should be extracted + $this->assertEquals('text/plain', $mediaType->encoding['id']->contentType); + // object should be passed. + $this->assertInstanceOf(\cebe\openapi\spec\Encoding::class, $mediaType->encoding['profileImage']); + } + + public function badEncodingProvider() + { + yield [['encoding' => ['id' => 'foo']], 'Encoding MUST be either array or Encoding object, "string" given']; + yield [['encoding' => ['id' => 42]], 'Encoding MUST be either array or Encoding object, "integer" given']; + yield [['encoding' => ['id' => false]], 'Encoding MUST be either array or Encoding object, "boolean" given']; + yield [['encoding' => ['id' => new stdClass()]], 'Encoding MUST be either array or Encoding object, "stdClass" given']; + // The last one can be supported in future, but now SpecBaseObjects::__construct() requires array explicitly + } + + /** + * @dataProvider badEncodingProvider + */ + public function testPathsCanNotBeCreatedFromBullshit($config, $expectedException) + { + $this->expectException(\cebe\openapi\exceptions\TypeErrorException::class); + $this->expectExceptionMessage($expectedException); + + new MediaType($config); + } +} diff --git a/tests/spec/ResponseTest.php b/tests/spec/ResponseTest.php index 419eb9a5..d11b8e27 100644 --- a/tests/spec/ResponseTest.php +++ b/tests/spec/ResponseTest.php @@ -162,4 +162,35 @@ public function testResponseCodes() $this->assertFalse($result); } + + public function testCreateionFromObjects() + { + $responses = new Responses([ + 200 => new Response(['description' => 'A list of pets.']), + 404 => ['description' => 'The pets list is gone 🙀'], + ]); + + $this->assertSame('A list of pets.', $responses->getResponse(200)->description); + $this->assertSame('The pets list is gone 🙀', $responses->getResponse(404)->description); + } + + public function badResponseProvider() + { + yield [['200' => 'foo'], 'Response MUST be either an array, a Response or a Reference object, "string" given']; + yield [['200' => 42], 'Response MUST be either an array, a Response or a Reference object, "integer" given']; + yield [['200' => false], 'Response MUST be either an array, a Response or a Reference object, "boolean" given']; + yield [['200' => new stdClass()], 'Response MUST be either an array, a Response or a Reference object, "stdClass" given']; + // The last one can be supported in future, but now SpecBaseObjects::__construct() requires array explicitly + } + + /** + * @dataProvider badResponseProvider + */ + public function testPathsCanNotBeCreatedFromBullshit($config, $expectedException) + { + $this->expectException(\cebe\openapi\exceptions\TypeErrorException::class); + $this->expectExceptionMessage($expectedException); + + new Responses($config); + } } diff --git a/tests/spec/SchemaTest.php b/tests/spec/SchemaTest.php index fe125516..aa2d9a15 100644 --- a/tests/spec/SchemaTest.php +++ b/tests/spec/SchemaTest.php @@ -134,4 +134,51 @@ public function testDiscriminator() 'monster' => 'https://gigantic-server.com/schemas/Monster/schema.json', ], $schema->discriminator->mapping); } + + public function testCreateionFromObjects() + { + $schema = new Schema([ + 'allOf' => [ + new Schema(['type' => 'integer']), + new Schema(['type' => 'string']), + ], + 'additionalProperties' => new Schema([ + 'type' => 'object', + ]), + 'discriminator' => new Discriminator([ + 'mapping' => ['A' => 'B'], + ]), + ]); + + $this->assertSame('integer', $schema->allOf[0]->type); + $this->assertSame('string', $schema->allOf[1]->type); + $this->assertInstanceOf(Schema::class, $schema->additionalProperties); + $this->assertSame('object', $schema->additionalProperties->type); + $this->assertSame(['A' => 'B'], $schema->discriminator->mapping); + } + + + public function badSchemaProvider() + { + yield [['properties' => ['a' => 'foo']], 'Unable to instantiate cebe\openapi\spec\Schema Object with data \'foo\'']; + yield [['properties' => ['a' => 42]], 'Unable to instantiate cebe\openapi\spec\Schema Object with data \'42\'']; + yield [['properties' => ['a' => false]], 'Unable to instantiate cebe\openapi\spec\Schema Object with data \'\'']; + yield [['properties' => ['a' => new stdClass()]], "Unable to instantiate cebe\openapi\spec\Schema Object with data 'stdClass Object\n(\n)\n'"]; + + yield [['additionalProperties' => 'foo'], 'Schema::$additionalProperties MUST be either array, boolean or a Schema object, "string" given']; + yield [['additionalProperties' => 42], 'Schema::$additionalProperties MUST be either array, boolean or a Schema object, "integer" given']; + yield [['additionalProperties' => new stdClass()], 'Schema::$additionalProperties MUST be either array, boolean or a Schema object, "stdClass" given']; + // The last one can be supported in future, but now SpecBaseObjects::__construct() requires array explicitly + } + + /** + * @dataProvider badSchemaProvider + */ + public function testPathsCanNotBeCreatedFromBullshit($config, $expectedException) + { + $this->expectException(\cebe\openapi\exceptions\TypeErrorException::class); + $this->expectExceptionMessage($expectedException); + + new Schema($config); + } } From 526c5801ec4b362d101a67093247fe9d4625f8b8 Mon Sep 17 00:00:00 2001 From: Carsten Brandt Date: Fri, 28 Jun 2019 15:45:09 +0200 Subject: [PATCH 4/4] fix code style --- src/spec/PathItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spec/PathItem.php b/src/spec/PathItem.php index 08ee15e5..72120037 100644 --- a/src/spec/PathItem.php +++ b/src/spec/PathItem.php @@ -150,7 +150,7 @@ public function resolveReferences(ReferenceContext $context = null) $pathItem = $this->_ref->resolve($context); $this->_ref = null; // The properties of the referenced structure are merged with the local Path Item Object. - foreach(self::attributes() as $attribute => $type) { + foreach (self::attributes() as $attribute => $type) { if (!isset($pathItem->$attribute)) { continue; }