Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTP\Response::__get() assumed HTTP\Response->data will always be an object though HTTP\Response->data is typed as mixed #30

Open
apotek opened this issue Jan 9, 2024 · 0 comments
Assignees

Comments

@apotek
Copy link
Contributor

apotek commented Jan 9, 2024

Description

The class variable data on the HTTP\Response class is type-hinted as follows:

    /**
     * Response data.
     *
     * @var mixed
     */
    public $data;

This is correct, since the data is set as follows:

        $contents = $response->getBody()->getContents();
        return $contents ? json_decode($contents, null, 512, JSON_THROW_ON_ERROR) : null;

Meaning that the value of data could be any valid return value from json_decode which means it could be an array, an object, true, false, null. And of course if there are no contents, the class variable data remains undefined.

PHP Documentation on json_decode Return Values

Returns the value encoded in json as an appropriate PHP type. Unquoted values true, false and null are returned as true, false and null respectively. null is returned if the json cannot be decoded or if the encoded data is deeper than the nesting limit.

If a caller uses the magic __get() method and the response data is a boolean, or an array, there will be a fatal error in that method, since it assumes that the returned data was decoded into a JSON object.

Proposed resolutions

Either:

  1. Increase flexibility: implement __get($name) such that the method checks if data is an array or object and then returns (if array) the value of the array at $name or (if object) the $name property of the object, or undefined if neither of those are the case.

Or

  1. Increase strictness (if possible): Since this is not a general purpose library, we can instead perhaps research if the Orange Dam API in fact always returns objects. If that is the case, and it is a part of the API specification (ie, it can be trusted), we can re-type data from mixed to object and leave the __get() method as it is.

Acceptance Criteria

Callers should be able to call HTTP\Response->{$property_name} without worrying about the response data type.

@apotek apotek self-assigned this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant