Unexpected Behaviour When To-One Relationship Data Is NULL

[ginnj-gilmore]

Hello!

I'm working on a project that uses Yang to parse API responses we get from a JSONAPI server. It's worked wonderfully and I'm able to dig into the source code to understand it. I'm currently working with relationships and am running into an issue.

Given the following JSONAPI response:

{
  "data": {
    "type": "carts",
    "id": "d2a45f147d3508",
    "relationships": {
      "null-one": {
        "data": null
      },
      "has-one": {
        "data": {"type": "items", "id": "a0481682dabc68"}
      },
      "null-many": {
        "data": []
      },
      "has-many": {
        "data": [
          {"type": "items", "id": "b0ed0f339887c8"},
          {"type": "items", "id": "6bd5ec9c9321c3"}
        ]
      }
    }
  }
}

The null-one relationship returns {"data": null} which is JSONAPI spec but parsing it with Yang - as demonstrated by this example

$response = new \GuzzleHttp\Psr7\Response(200, [], $json);
$jsonapi = new \WoohooLabs\Yang\JsonApi\Response\JsonApiResponse($response);
$document = $jsonapi->document();

When running $document->toArray() I get the following results:

$results = [
    'jsonapi' => [
        'version' => '1.0',
    ],
    'data' => [
        'type' => 'carts',
        'id' => 'd2a45f147d3508',
        'relationships' => [
            'null-one' => [
                'data' => [], // array - expected null
            ],
            'has-one' => [
                'data' => [
                    'type' => 'items',
                    'id' => 'a0481682dabc68',
                ],
            ],
            'null-many' => [
                'data' => [],
            ],
            'has-many' => [
                'data' => [
                    [
                        'type' => 'items',
                        'id' => 'b0ed0f339887c8',
                    ],
                    [
                        'type' => 'items',
                        'id' => '6bd5ec9c9321c3',
                    ],
                ],
            ],
        ],
    ],
];

I would expect a relationship with its {"data": null} to return null instead of an empty array:

// truncated to relevant relationships
$expected = [
    'data' => [
        'type' => 'carts',
        'id' => 'd2a45f147d3508',
        'relationships' => [
            'null-one' => [
                'data' => null, // null
            ],
            'null-many' => [
                'data' => [],
            ],
        ],
    ],
];

This has been causing issues with our project and I would rather not make complicated test cases before passing the proper results through.

I've implemented a fix in WoohooLabs\Yang\JsonApi\Schema\Relationship to properly set its $isToOneRelationship to false when applicable which has the desired results.

I have not forked the project yet, only modified local composer.json source files for debugging. I'm running short on time so I'll fork and just use that in my composer.json for now.

Shall I make a pull request with my fixes once up? Or would you like to simply implement the following changes?

---

https://github.com/woohoolabs/yang/blob/master/src/JsonApi/Schema/Relationship.php#L44

if (self::isArrayKey($array, "data") === false) {
    $isToOneRelationship = array_key_exists("data", $array) && is_null($array["data"]);
    return self::createEmptyFromArray($name, $meta, $links, $resources, $isToOneRelationship);
}

---

https://github.com/woohoolabs/yang/blob/master/src/JsonApi/Schema/Relationship.php#L54

private static function createEmptyFromArray(
    string $name,
    array $meta,
    Links $links,
    ResourceObjects $resources,
    $isToOneRelationship = null
): Relationship {
    return new Relationship($name, $meta, $links, [], $resources, $isToOneRelationship);
}

[ginnj-gilmore]

I don't have PHPUnit install on my system right now so I can't run any tests

[ginnj-gilmore]

Sorry, another limitation we're encountering is PHP 7.1 specific syntax in Yang 1.3 onward. We're unable to run anything higher than PHP 7.0 on our servers so we're forced to use Yang 1.2 and below. I guess at this point it's less a matter of getting an updated Yang version and more me just sharing my fix.

We'll roll out with our own fork until we can get PHP 7.1 on our servers.

If someone can run tests on my fork, I'll be more than happy to make a pull request for it.

[kocsismate]

Hey,

Thank you for the report! I'll only have time to look at the issue tomorrow, but until then you might use Docker Compose to execute the tests. Just run docker-compose up in the project root and a PHP 7.1 container will fire up.

I'll answer you tomorrow with more details :)

[ginnj-gilmore]

I've been interested in Docker but haven't had a chance to play with it. I just took the time to install it via sudo yum install docker docker-compose but running docker-compose up is throwing an error. I unfortunately don't have time to troubleshoot this and/or learn Docker so I won't be able to run that.

It dawned on me that I could just composer require --dev phpunit/phpunit and then ./vendor/bin/phpunit which returned OK (227 tests, 273 assertions).

[ginnj-gilmore]

Another error on my part: seems I forgot to include the fix in WoohooLabs\Yang\JsonApi\Schema\Relationship::toArray()

https://github.com/woohoolabs/yang/blob/master/src/JsonApi/Schema/Relationship.php#L142

if (empty($this->resourceMap) === false) {
    $result["data"] = $this->isToOneRelationship ? reset($this->resourceMap) : $this->resourceMap;
}else{
    $result["data"] = $this->isToOneRelationship ? null : [];
}

I've pushed an update to my fork. It now yields the expected results.

[ginnj-gilmore]

Since I'm working off of the 1.2.0 tag, I've created a 1.2 branch which I'm able to reference in my composer.json with the following [truncated] config:

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/ginnj-gilmore/yang"
        }
    ],
    "require": {
        "woohoolabs/yang": "1.2.x-dev"
    }
}

[kocsismate]

Hey again!

I checked your commit in your repo, and I can say it is really nice a solution :) I also ran the tests, and they are all green! So I would be happy to merge your fix as soon as you create a PR.

Just one tiny little thing, I'd like you to change: instead of is_null($array["data"]), please use $array["data"] === null. It is faster and I like it more. :P

I've just created the 1.2 branch, so we can backport your fix and then you don't have to use your fork. :) Please create the PR for the master branch, and I'll cherry-pick your commit.