From e17d3ba6b8c1f7f9f388407e8f2429f7c198959b Mon Sep 17 00:00:00 2001 From: neomerx Date: Wed, 30 Jan 2019 19:54:02 +0300 Subject: [PATCH] Close #82 --- .gitattributes | 6 +- src/Contracts/Schema/SchemaInterface.php | 8 +- src/Encoder/Encoder.php | 3 + src/Factories/Factory.php | 6 + src/Http/Headers/MediaType.php | 52 ++++++- src/I18n/format.php | 2 + src/Parser/Parser.php | 4 + .../ParseRelationshipLinksTrait.php | 6 +- src/Schema/BaseSchema.php | 4 +- src/Schema/Error.php | 3 + src/Schema/ErrorCollection.php | 109 +++++++------- src/Schema/Identifier.php | 133 ++++++++++++++++++ src/Schema/SchemaContainer.php | 3 +- tests/Data/Models/AuthorIdentity.php | 68 +-------- tests/Encoder/EncodeSimpleObjectsTest.php | 2 +- .../Issue154/CustomSchemaContainer.php | 10 ++ tests/Extensions/Issue81/CommentSchema.php | 2 +- tests/Extensions/Issue91/CategorySchema.php | 4 +- tests/Http/Headers/MediaTypeTest.php | 45 ++++++ 19 files changed, 342 insertions(+), 128 deletions(-) create mode 100644 src/Schema/Identifier.php diff --git a/.gitattributes b/.gitattributes index 2e2a53cf..6b20412e 100644 --- a/.gitattributes +++ b/.gitattributes @@ -7,7 +7,7 @@ /phpunit.xml export-ignore /.scrutinizer.yml export-ignore /.travis.yml export-ignore -/tests export-ignore +/perf export-ignore /sample export-ignore - -./sample/composer.json merge=ours +/spec export-ignore +/tests export-ignore diff --git a/src/Contracts/Schema/SchemaInterface.php b/src/Contracts/Schema/SchemaInterface.php index 2648bb24..98d47ab4 100644 --- a/src/Contracts/Schema/SchemaInterface.php +++ b/src/Contracts/Schema/SchemaInterface.php @@ -151,14 +151,18 @@ public function getResourceMeta($resource); /** * If `self` links should be added in relationships by default. * + * @param string $relationshipName + * * @return bool */ - public function isAddSelfLinkInRelationshipByDefault(): bool; + public function isAddSelfLinkInRelationshipByDefault(string $relationshipName): bool; /** * If `related` links should be added in relationships by default. * + * @param string $relationshipName + * * @return bool */ - public function isAddRelatedLinkInRelationshipByDefault(): bool; + public function isAddRelatedLinkInRelationshipByDefault(string $relationshipName): bool; } diff --git a/src/Encoder/Encoder.php b/src/Encoder/Encoder.php index 64a7f2c5..89898a0d 100644 --- a/src/Encoder/Encoder.php +++ b/src/Encoder/Encoder.php @@ -34,6 +34,8 @@ /** * @package Neomerx\JsonApi + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Encoder implements EncoderInterface { @@ -179,6 +181,7 @@ protected static function createFactory(): FactoryInterface * @return array * * @SuppressWarnings(PHPMD.ElseExpression) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ protected function encodeDataToArray($data): array { diff --git a/src/Factories/Factory.php b/src/Factories/Factory.php index 8291abfc..8dbf2c93 100644 --- a/src/Factories/Factory.php +++ b/src/Factories/Factory.php @@ -51,6 +51,9 @@ /** * @package Neomerx\JsonApi + * + * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Factory implements FactoryInterface { @@ -278,6 +281,9 @@ public function createLink(bool $isSubUrl, string $value, bool $hasMeta, $meta = /** * @inheritdoc + * + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) */ public function createRelationship( PositionInterface $position, diff --git a/src/Http/Headers/MediaType.php b/src/Http/Headers/MediaType.php index 73f8a323..95402ae9 100644 --- a/src/Http/Headers/MediaType.php +++ b/src/Http/Headers/MediaType.php @@ -121,7 +121,7 @@ public function matchesTo(MediaTypeInterface $mediaType): bool return $this->isTypeMatches($mediaType) && $this->isSubTypeMatches($mediaType) && - $this->isMediaParametersEqual($mediaType); + $this->isMediaParametersMatch($mediaType); } /** @@ -179,6 +179,42 @@ private function isSubTypeEquals(MediaTypeInterface $mediaType): bool return strcasecmp($this->getSubType(), $mediaType->getSubType()) === 0; } + /** + * @param MediaTypeInterface $mediaType + * + * @return bool + */ + private function isMediaParametersMatch(MediaTypeInterface $mediaType): bool + { + if ($this->bothMediaTypeParamsEmpty($mediaType) === true) { + return true; + } elseif ($this->bothMediaTypeParamsNotEmptyAndEqualInSize($mediaType)) { + // Type, subtype and param name should be compared case-insensitive + // https://tools.ietf.org/html/rfc7231#section-3.1.1.1 + $ourParameters = array_change_key_case($this->getParameters()); + $parametersToCompare = array_change_key_case($mediaType->getParameters()); + + // if at least one name are different they are not equal + if (empty(array_diff_key($ourParameters, $parametersToCompare)) === false) { + return false; + } + + // If we are here we have to compare values. Also some of the values should be compared case-insensitive + // according to https://tools.ietf.org/html/rfc7231#section-3.1.1.1 + // > 'Parameter values might or might not be case-sensitive, depending on + // the semantics of the parameter name.' + foreach ($ourParameters as $name => $value) { + if ($this->paramValuesMatch($name, $value, $parametersToCompare[$name]) === false) { + return false; + } + } + + return true; + } + + return false; + } + /** * @param MediaTypeInterface $mediaType * @@ -262,4 +298,18 @@ private function paramValuesEqual(string $name, string $value, string $valueToCo return $valuesEqual; } + + /** + * @param string $name + * @param string $value + * @param string $valueToCompare + * + * @return bool + */ + private function paramValuesMatch(string $name, string $value, string $valueToCompare): bool + { + $valuesEqual = $valueToCompare === '*' || $this->paramValuesEqual($name, $value, $valueToCompare); + + return $valuesEqual; + } } diff --git a/src/I18n/format.php b/src/I18n/format.php index b3a2ac86..80276800 100644 --- a/src/I18n/format.php +++ b/src/I18n/format.php @@ -25,6 +25,8 @@ * @see Messages::compose * * @return string + * + * @SuppressWarnings(PHPMD.StaticAccess) */ function format(string $message, ...$parameters): string { diff --git a/src/Parser/Parser.php b/src/Parser/Parser.php index f38f4b0f..dcfd9a71 100644 --- a/src/Parser/Parser.php +++ b/src/Parser/Parser.php @@ -35,6 +35,8 @@ /** * @package Neomerx\JsonApi + * + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class Parser implements ParserInterface { @@ -232,6 +234,8 @@ private function parseAsResource( * @param ResourceInterface $resource * * @return iterable + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function parseResource(ResourceInterface $resource): iterable { diff --git a/src/Parser/RelationshipData/ParseRelationshipLinksTrait.php b/src/Parser/RelationshipData/ParseRelationshipLinksTrait.php index 7aeb64b6..517a37e0 100644 --- a/src/Parser/RelationshipData/ParseRelationshipLinksTrait.php +++ b/src/Parser/RelationshipData/ParseRelationshipLinksTrait.php @@ -33,6 +33,8 @@ trait ParseRelationshipLinksTrait * @param array $description * * @return array + * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ private function parseRelationshipLinks( SchemaInterface $parentSchema, @@ -41,9 +43,9 @@ private function parseRelationshipLinks( array $description ): array { $addSelfLink = $description[SchemaInterface::RELATIONSHIP_LINKS_SELF] ?? - $parentSchema->isAddSelfLinkInRelationshipByDefault(); + $parentSchema->isAddSelfLinkInRelationshipByDefault($name); $addRelatedLink = $description[SchemaInterface::RELATIONSHIP_LINKS_RELATED] ?? - $parentSchema->isAddRelatedLinkInRelationshipByDefault(); + $parentSchema->isAddRelatedLinkInRelationshipByDefault($name); assert(is_bool($addSelfLink) === true || $addSelfLink instanceof LinkInterface); assert(is_bool($addRelatedLink) === true || $addRelatedLink instanceof LinkInterface); diff --git a/src/Schema/BaseSchema.php b/src/Schema/BaseSchema.php index 1812a184..4a506639 100644 --- a/src/Schema/BaseSchema.php +++ b/src/Schema/BaseSchema.php @@ -128,7 +128,7 @@ public function getResourceMeta($resource) /** * @inheritdoc */ - public function isAddSelfLinkInRelationshipByDefault(): bool + public function isAddSelfLinkInRelationshipByDefault(string $relationshipName): bool { return true; } @@ -136,7 +136,7 @@ public function isAddSelfLinkInRelationshipByDefault(): bool /** * @inheritdoc */ - public function isAddRelatedLinkInRelationshipByDefault(): bool + public function isAddRelatedLinkInRelationshipByDefault(string $relationshipName): bool { return true; } diff --git a/src/Schema/Error.php b/src/Schema/Error.php index a0e61089..56f471d7 100644 --- a/src/Schema/Error.php +++ b/src/Schema/Error.php @@ -90,6 +90,9 @@ class Error implements ErrorInterface * @param array|null $source * @param bool $hasMeta * @param mixed $meta + * + * @SuppressWarnings(PHPMD.BooleanArgumentFlag) + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function __construct( $idx = null, diff --git a/src/Schema/ErrorCollection.php b/src/Schema/ErrorCollection.php index ae174822..1a922400 100644 --- a/src/Schema/ErrorCollection.php +++ b/src/Schema/ErrorCollection.php @@ -31,6 +31,7 @@ * @package Neomerx\JsonApi * * @SuppressWarnings(PHPMD.TooManyPublicMethods) + * @SuppressWarnings(PHPMD.BooleanArgumentFlag) */ class ErrorCollection implements IteratorAggregate, ArrayAccess, Serializable, Countable { @@ -139,14 +140,14 @@ public function add(ErrorInterface $error): self * @return self */ public function addDataError( - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToData(); @@ -179,14 +180,14 @@ public function addDataError( * @return self */ public function addDataTypeError( - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToType(); @@ -219,14 +220,14 @@ public function addDataTypeError( * @return self */ public function addDataIdError( - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToId(); @@ -259,14 +260,14 @@ public function addDataIdError( * @return self */ public function addAttributesError( - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToAttributes(); @@ -298,17 +299,19 @@ public function addAttributesError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function addDataAttributeError( $name, - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToAttribute($name); @@ -341,14 +344,14 @@ public function addDataAttributeError( * @return self */ public function addRelationshipsError( - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToRelationships(); @@ -380,17 +383,19 @@ public function addRelationshipsError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function addRelationshipError( $name, - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToRelationship($name); @@ -422,17 +427,19 @@ public function addRelationshipError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function addRelationshipTypeError( $name, - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToRelationshipType($name); @@ -464,17 +471,19 @@ public function addRelationshipTypeError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function addRelationshipIdError( $name, - $title, - $detail = null, - $status = null, + string $title, + string $detail = null, + string $status = null, $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $pointer = $this->getPathToRelationshipId($name); @@ -506,6 +515,8 @@ public function addRelationshipIdError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ public function addQueryParameterError( string $name, @@ -515,8 +526,8 @@ public function addQueryParameterError( $idx = null, LinkInterface $aboutLink = null, ?iterable $typeLinks = null, - $code = null, - $hasMeta = false, + string $code = null, + bool $hasMeta = false, $meta = null ): self { $source = [ErrorInterface::SOURCE_PARAMETER => $name]; @@ -540,6 +551,8 @@ public function addQueryParameterError( * @param mixed $meta * * @return self + * + * @SuppressWarnings(PHPMD.ExcessiveParameterList) */ protected function addResourceError( string $title, @@ -550,7 +563,7 @@ protected function addResourceError( LinkInterface $aboutLink = null, ?iterable $typeLinks = null, string $code = null, - $hasMeta = false, + bool $hasMeta = false, $meta = null ): self { $source = [ErrorInterface::SOURCE_POINTER => $pointer]; diff --git a/src/Schema/Identifier.php b/src/Schema/Identifier.php new file mode 100644 index 00000000..2ac9764b --- /dev/null +++ b/src/Schema/Identifier.php @@ -0,0 +1,133 @@ +setId($index); + $this->setType($type); + + $this->hasMeta = $hasMeta; + $this->meta = $meta; + } + + /** + * @inheritdoc + */ + public function getId(): string + { + return $this->index; + } + + /** + * @param string $index + * + * @return self + */ + public function setId(string $index): self + { + $this->index = $index; + + return $this; + } + + /** + * @inheritdoc + */ + public function getType(): string + { + return $this->type; + } + + /** + * @param string $type + * + * @return self + */ + public function setType(string $type): self + { + $this->type = $type; + + return $this; + } + + /** + * @inheritdoc + */ + public function hasIdentifierMeta(): bool + { + return $this->hasMeta; + } + + /** + * @inheritdoc + */ + public function getIdentifierMeta() + { + return $this->meta; + } + + /** + * @param mixed $meta + * + * @return self + */ + public function setIdentifierMeta($meta): self + { + $this->meta = $meta; + $this->hasMeta = true; + + return $this; + } +} diff --git a/src/Schema/SchemaContainer.php b/src/Schema/SchemaContainer.php index ad3f26f3..7deec56c 100644 --- a/src/Schema/SchemaContainer.php +++ b/src/Schema/SchemaContainer.php @@ -85,6 +85,7 @@ public function __construct(FactoryInterface $factory, iterable $schemas) * * @SuppressWarnings(PHPMD.StaticAccess) * @SuppressWarnings(PHPMD.ElseExpression) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function register(string $type, $schema): void { @@ -160,7 +161,7 @@ public function hasSchema($resourceObject): bool * @SuppressWarnings(PHPMD.StaticAccess) * @SuppressWarnings(PHPMD.ElseExpression) */ - private function getSchemaByType(string $type): SchemaInterface + protected function getSchemaByType(string $type): SchemaInterface { if ($this->hasCreatedProvider($type) === true) { return $this->getCreatedProvider($type); diff --git a/tests/Data/Models/AuthorIdentity.php b/tests/Data/Models/AuthorIdentity.php index 1318b331..b03c04dd 100644 --- a/tests/Data/Models/AuthorIdentity.php +++ b/tests/Data/Models/AuthorIdentity.php @@ -18,80 +18,18 @@ * limitations under the License. */ -use Neomerx\JsonApi\Contracts\Schema\IdentifierInterface; +use Neomerx\JsonApi\Schema\Identifier; /** * @package Neomerx\Tests\JsonApi */ -class AuthorIdentity implements IdentifierInterface +class AuthorIdentity extends Identifier { - /** - * @var string - */ - private $index; - - /** - * @var bool - */ - private $hasMeta = false; - - /** - * @var mixed - */ - private $meta; - /** * @param string $index */ public function __construct(string $index) { - $this->index = $index; - } - - /** - * Get identifier's type. - * - * @return string - */ - public function getType(): string - { - return 'people'; - } - - /** - * @inheritdoc - */ - public function getId(): string - { - return $this->index; - } - - /** - * @inheritdoc - */ - public function hasIdentifierMeta(): bool - { - return $this->hasMeta; - } - - /** - * @inheritdoc - */ - public function getIdentifierMeta() - { - return $this->meta; - } - - /** - * @param mixed $meta - * - * @return AuthorIdentity - */ - public function setMeta($meta) - { - $this->meta = $meta; - $this->hasMeta = true; - - return $this; + parent::__construct($index, 'people'); } } diff --git a/tests/Encoder/EncodeSimpleObjectsTest.php b/tests/Encoder/EncodeSimpleObjectsTest.php index 4b9acfb3..48ee39a4 100644 --- a/tests/Encoder/EncodeSimpleObjectsTest.php +++ b/tests/Encoder/EncodeSimpleObjectsTest.php @@ -176,7 +176,7 @@ public function testEncodeIdentifier(): void { $encoder = Encoder::instance([]); - $identity = (new AuthorIdentity('123'))->setMeta('id meta'); + $identity = (new AuthorIdentity('123'))->setIdentifierMeta('id meta'); $actual = $encoder->encodeData($identity); $expected = <<setMeta($author->{Author::IDENTIFIER_META}); + $authorIdentity->setIdentifierMeta($author->{Author::IDENTIFIER_META}); } return $this->fixDescriptions( diff --git a/tests/Extensions/Issue91/CategorySchema.php b/tests/Extensions/Issue91/CategorySchema.php index 78343763..5070acee 100644 --- a/tests/Extensions/Issue91/CategorySchema.php +++ b/tests/Extensions/Issue91/CategorySchema.php @@ -68,7 +68,7 @@ public function getRelationships($resource): iterable /** * @inheritdoc */ - public function isAddSelfLinkInRelationshipByDefault(): bool + public function isAddSelfLinkInRelationshipByDefault(string $relationshipName): bool { return false; } @@ -76,7 +76,7 @@ public function isAddSelfLinkInRelationshipByDefault(): bool /** * @inheritdoc */ - public function isAddRelatedLinkInRelationshipByDefault(): bool + public function isAddRelatedLinkInRelationshipByDefault(string $relationshipName): bool { return false; } diff --git a/tests/Http/Headers/MediaTypeTest.php b/tests/Http/Headers/MediaTypeTest.php index 186158fd..3e591c65 100644 --- a/tests/Http/Headers/MediaTypeTest.php +++ b/tests/Http/Headers/MediaTypeTest.php @@ -98,6 +98,19 @@ public function testCompareMediaTypes2(): void self::assertTrue($type1->equalsTo($type3)); } + /** + * Test compare media types + * + * @return void + */ + public function testCompareMediaTypesWithoutParameters(): void + { + $type1 = new MediaType('text', 'html'); + $type2 = new MediaType('Text', 'HTML'); + + self::assertTrue($type1->equalsTo($type2)); + } + /** * Test compare media types * @@ -127,4 +140,36 @@ public function testMatchMediaTypesWithoutParameters(): void self::assertTrue($type1->matchesTo($type2)); } + + /** + * Test compare media types + * + * @return void + * + * @see https://github.com/neomerx/json-api/issues/221 + */ + public function testMatchMediaTypesWithoutParameters2(): void + { + // match by mask + + $type1 = new MediaType('multipart', 'form-data', ['boundary' => '*']); + $type2 = new MediaType('multipart', 'form-data', ['boundary' => '----WebKitFormBoundaryAAA']); + + self::assertTrue($type2->matchesTo($type1)); + self::assertFalse($type2->equalsTo($type1)); + + // cover some edge cases + + $type1 = new MediaType('multipart', 'form-data', ['name' => 'value1']); + $type2 = new MediaType('multipart', 'form-data', ['name' => 'value2']); + self::assertFalse($type2->matchesTo($type1)); + + $type1 = new MediaType('multipart', 'form-data', ['name1' => 'value']); + $type2 = new MediaType('multipart', 'form-data', ['name2' => 'value']); + self::assertFalse($type2->matchesTo($type1)); + + $type1 = new MediaType('multipart', 'form-data', ['name1' => 'value', 'name2' => 'value']); + $type2 = new MediaType('multipart', 'form-data', ['name2' => 'value']); + self::assertFalse($type2->matchesTo($type1)); + } }