DigitalOceanPHP / Client

DigitalOcean API v2 client for PHP
MIT License
710 stars 205 forks source link

Api\Image::getById() accepts only int, should accept int|string #297

Closed webignition closed 2 years ago

webignition commented 2 years ago

Api\Image::getById() is type-hinted to accept an int argument:

public function getById(int $id) {
    // ...
}

The DigitalOcean API docs for this feature state that the image_id parameter can be either int or string.

This isn't a significant problem for me as any images that I need to get are ones that I have created myself and which all have numeric-only identifiers. This is, however, a minor problem as I have to cast values to int before calling Api\Image::getById() to stop phpstan from being sad.

I'm not sure what the best fix would be here.

Ideally a union type could be used (public function getById(int|string $id)), however that is only possible with PHP 8.0+ and this library supports ^7.2.5 || ^8.0

One option would be to remove the type hint entirely and within the method check if the argument is either is_int() or is_string() and throw an exception if not?

/**
 * @param int|string $id
 *
 * @throws ExceptionInterface
 *
 * @return ImageEntity
 */
public function getById($id)
{
    if (!\is_int($id) && !\is_string($id)) {
        throw new InvalidArgumentException('$id must be an int or a string');
    }

    $image = $this->get(\sprintf('images/%d', $id));

    return new ImageEntity($image->image);
}

Another option (which is still perfectly valid) is to do nothing for now, wait until supporting only PHP 8.0+ and use a union type.

GrahamCampbell commented 2 years ago

Thanks, but the integer ids are fine. Their docs say both private and public images have integer ids. The slugs are just an alternative way to reference some public images. We're not gonna support that here. :)

webignition commented 2 years ago

Fair enough :)