mezzio / mezzio-hal

Hypertext Application Language implementation for PHP and PSR-7
https://docs.mezzio.dev/mezzio-hal/
BSD 3-Clause "New" or "Revised" License
19 stars 21 forks source link

HalResource::validateElementName() with numbered array #7

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

While trying to create a resource from a numbered array an exception is thrown due to empty($name) check on zero index on this line.

Code to reproduce the issue

$array = [
    ['foo' => 'bar'],
];
$resource->embed('foobar', $resourceGenerator->fromArray($array));

Expected results

The resource should be generated just fine.

Actual results

Exception is thrown: $name provided to Zend\Expressive\Hal\HalResource cannot be empty


Originally posted by @grizzm0 at https://github.com/zendframework/zend-expressive-hal/issues/42

weierophinney commented 4 years ago

I've confirmed this with a similar structure if an array looks like the following, and fed to fromArray();

Array
(
    [0] => Array
        (
            [id] => 1
            [name] => Bank #1
            [phone] => 555-555-5555
            [zone_id] => 18
        )

    [1] => Array
        (
            [id] => 2
            [name] => Bank #2
            [phone] => 555-555-5555
            [zone_id] => 18
        )

    [2] => Array
        (
            [id] => 7
            [name] => Test Company
            [phone] => 555-555-5555
            [zone_id] => 18
        )
)

Seems the array_walk() in Zend\Expressive\Hal\HalResource::__construct() doesn't like multi-dimensional array. It is calling validateElementName() on the keys of the sub.


Originally posted by @adamculp at https://github.com/zendframework/zend-expressive-hal/issues/42#issuecomment-491599579

weierophinney commented 4 years ago

I've been able to create a reproduce case finally, from what @grizzm0 originally wrote:

use Psr\Container\ContainerInterface;
use Psr\Http\Message\ServerRequestInterface;
use Zend\Expressive\Hal\HalResource;
use Zend\Expressive\Hal\LinkGenerator;
use Zend\Expressive\Hal\Metadata\MetadataMap;
use Zend\Expressive\Hal\ResourceGenerator;

$generator = new ResourceGenerator(
    new MetadataMap(),
    new class implements ContainerInterface {
        public function has($name) : bool
        {
            return false;
        }

        public function get($name)
        {
            return new $name();
        }
    },
    new LinkGenerator(new class implements LinkGenerator\UrlGeneratorInterface {
        public function generate(ServerRequestInterface $request, string $routeName, array $routeParams = [], array $queryParams = []) : string
        {
            return 'https://not-a-url.localdomain/foo/bar';
        }
    })
);

$resource = new HalResource();
$array = [
    ['foo' => 'bar'],
];

$resource->embed('foobar', $generator->fromArray($array));

I can start debugging from here.


Originally posted by @weierophinney at https://github.com/zendframework/zend-expressive-hal/issues/42#issuecomment-491995834

weierophinney commented 4 years ago

ResourceGenerator::fromArray(), and, by extension, the HalResource constructor, is designed solely to create a resource based on an associative array. If you want the generated resource to represent a collection, you need to pass an associative array with a key pointing to an array, where every item in the array is already a HalResource instance (HalResource::isResourceCollection() tests for instances of HalResource, returning false if any item is not one).

Based on the examples provided, you have arrays of associative arrays, and you want to embed these as a collection in another resource. The way to do it with current code is as follows:

$collection = array_map(function ($item) use ($generator) {
    return $generator->fromArray($item);
}, $array);
$resource = $resource->embed('foobar', $collection);

I've just made those changes to the reproduce case I posted earlier, and it works perfectly.

So, either we (a) need more documentation, or (b) need a new method in the ResourceGenerator.

If we were to go route (b), I'd argue for a collectionFromArray() method that returns an array of HalResource items from a nested array of associative arrays. If we go route (a), it's a cookbook chapter.

Do those approaches work for either of you? If not, can you provide a full use case demonstrating how you're trying to use the functionality and what you expect to happen, please?


Originally posted by @weierophinney at https://github.com/zendframework/zend-expressive-hal/issues/42#issuecomment-492001245

weierophinney commented 4 years ago

I would say by the very needs, that many times a common use case is where an associative array is required. This would facilitate the return of multiple records from a database, for instance, where an associative array is a proper way to utilize the results.

So, at a minimum, documentation with how to pass an associative array in (as shown in @weierophinney example above). However, the fromArray() method should also be able to handle both an associative array as well as a single level array for one record.


Originally posted by @adamculp at https://github.com/zendframework/zend-expressive-hal/issues/42#issuecomment-492008051

weierophinney commented 4 years ago

@adamculp and I had a skype discussion, and the typical use case here is getting an array of records back from a data source.

The problem is that, even when using metadata, we have no way to know how to map an associative array to a resource for purposes of generating links, which means that, at best, you end up with vanilla JSON objects.

When it comes to HalResource, it requires that the data passed to it is an associative array, as it's building a resource (i.e., an object). If you pass an array of data to the constructor's $embedded argument, that array needs string keys (this is the association type), and an array of HalResource instances (as anything embedded needs to be a resource as well).

The assumption I'm seeing in this issue, then, is that using the generator should allow you to bypass those restrictions. But it doesn't.

ResourceGenerator::fromArray() exists essentially to allow you to create a bare resource, optionally with a self relational link (by passing the optional $uri argument). In the examples in this thread, it's no different than calling new HalResource($data), and, as such, it has the same restrictions.

So, how do you create a HalResource representing a collection of records returned from the database, or embed that collection in an existing resource?

You have several options:

What it comes down to is:

I'll make a note to write up documentation to make this more clear, as well as explain why the limitations exist.


Originally posted by @weierophinney at https://github.com/zendframework/zend-expressive-hal/issues/42#issuecomment-492254045