{"action":"opened","issue":{"url":"https://api.github.com/repos/woohoolabs/yin/issues/30","repository_url":"https://api.github.com/repos/woohoolabs/yin","labels_url":"https://api.github.com/repos/woohoolabs/yin/issues/30/labels{/name}","comments_url":"https://api.github.com/repos/woohoolabs/yin/issues/30/comments","events_url":"https://api.github.com/repos/woohoolabs/yin/issues/30/events","html_url":"https://github.com/woohoolabs/yin/issues/30","id":168879085,"number":30,"title":"ResourceIdentifier::fromArray returning null not being handles gracefully","user":{"login":"emil-nasso","id":1119706,"avatar_url":"https://avatars.githubusercontent.com/u/1119706?v=3","gravatar_id":"","url":"https://api.github.com/users/emil-nasso","html_url":"https://github.com/emil-nasso","followers_url":"https://api.github.com/users/emil-nasso/followers","following_url":"https://api.github.com/users/emil-nasso/following{/other_user}","gists_url":"https://api.github.com/users/emil-nasso/gists{/gist_id}","starred_url":"https://api.github.com/users/emil-nasso/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/emil-nasso/subscriptions","organizations_url":"https://api.github.com/users/emil-nasso/orgs","repos_url":"https://api.github.com/users/emil-nasso/repos","events_url":"https://api.github.com/users/emil-nasso/events{/privacy}","received_events_url":"https://api.github.com/users/emil-nasso/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2016-08-02T13:00:26Z","updated_at":"2016-08-02T13:00:26Z","closed_at":null,"body":"If you need to fetch a relationship from a request, you would normally do it like this:\r\n\r\n```php\r\n$resource = $jsonApi->getRequest()->getResourceToOneRelationship('spaceship');\r\n```\r\n\r\nThis returns a `ToOneRelationShip`-object, if the relationship is found, or `null` if it is not.\r\n\r\nThe `getResourceToOneRelationship` method in the `Request` class looks like this\r\n\r\n```php\r\npublic function getResourceToOneRelationship($relationship)\r\n    {\r\n        $data = $this->getResource();\r\n        if (isset($data[\"relationships\"][$relationship][\"data\"]) === false) {\r\n            return null;\r\n        }\r\n        return new ToOneRelationship(ResourceIdentifier::fromArray($data[\"relationships\"][$relationship][\"data\"]));\r\n    }\r\n```\r\n\r\nThe last line calls the `fromArray` method. The problem is that the `fromArray` method does this check:\r\n\r\n```php\r\nif (isset($array[\"type\"]) === false || isset($array[\"id\"]) === false) {\r\n            return null;\r\n        }\r\n```\r\n\r\nSo an invalid relationship returns null, but the `ToOneRelationship` object gets created anyway. I think it would be clearer if `getResourceToOneRelationship` returned null in this case.\r\n\r\nTo catch the case where the input is malformed, you would have to write code like this with the current code:\r\n\r\n```php\r\n $resource = $jsonApi->getRequest()->getResourceToOneRelationship('spaceship');\r\n        if (isset($resource)) {\r\n           $resourceIdentifier = $resource->getResourceIdentifier();\r\n            if ( isset($resourceIdentifier) ) {\r\n                //get the id from the resourceIdentifier\r\n            }\r\n       }\r\n```\r\n\r\nIf you don't, you will very likely get an Fatal Error in php.\r\n\r\nI would propose that `getResourceToOneRelationship` was changed to make sure that `ResourceIdentifier:fromArray` considers the data valid, something like this:\r\n\r\n```php\r\npublic function getResourceToOneRelationship($relationship)\r\n    {\r\n        $data = $this->getResource();\r\n\r\n        if (isset($data[\"relationships\"][$relationship][\"data\"]) === false) {\r\n            return null;\r\n        }\r\n\r\n        $resourceIdentifier = ResourceIdentifier::fromArray($data[\"relationships\"][$relationship][\"data\"]);\r\n\r\n        if (isset($resourceIdentifier) === false) {\r\n            return null;\r\n        }\r\n\r\n        return new ToOneRelationship($resourceIdentifier);\r\n    }\r\n```\r\n\r\nWith this in place, `getResourceToOneRelationship` would return null if the relationship didn't exist, or one of the fields `type` or `id` was missing.\r\n\r\nThe `getResourceToManyRelationship` would need to be fixed too. I'm not 100% sure on how though. Skip non valid relationships? Return null from the method if one of the identifiers are invalid? Rise some kind of exception? Exceptions could perhaps be used in the `getResourceToOneRelationship` case too? Some kind of malformed input exception.\r\n\r\nI didn't consider this fully thought through, so i didn't create a PR. What do you think about all this?"}}
{"actor":{"display_login":"emil-nasso"}}