search

laminas-api-tools / api-tools-hal

Laminas Module providing Hypermedia Application Language assets and rendering
https://api-tools.getlaminas.org/documentation
BSD 3-Clause "New" or "Revised" License
6 stars 12 forks source link

Different output from onRenderEntity and onRenderCollection when adding additional Entities in Events #21

Open weierophinney opened 4 years ago

weierophinney commented 4 years ago

As stated by Matus Zeman @matuszeman here https://groups.google.com/a/zend.com/forum/#!topic/apigility-users/zck43pReklw there seems to be a bug in the Hal helper when rendering single entities.

Quoted from the google group: "I believe there is a bug in Hal helper (https://github.com/zfcampus/zf-hal/blob/master/src/Plugin/Hal.php#L1140) where convertEntityToArray() method caches converted entities so when you set new entities or links into entity using 'renderEntity' event convertEntityToArray() returns original entity anyway ignoring everything - https://github.com/zfcampus/zf-hal/blob/master/src/Plugin/Hal.php#L495"

Code for single Entity (http://192.168.33.101/events/2182)

    public function onRenderEntity($e)
    {
        $halEntity = $e->getParam('entity');
        $version = $this->isEntityOfInterest($halEntity->entity, 'Events', 1);

        if (false !== $version) {
            if($halEntity->entity instanceof EventsEntity) {
                $repository = $this->sm->get('Umid\V1\Rest\Unternehmen\UnternehmenResource');
                $item = $repository->fetch($halEntity->entity['events_location_Id']);
                $halEntity->entity['unternehmen'] = $item;
            }
            $this->injectEventsEntityLinks($halEntity, $version);
            return;
        }
    }

Dump of $halEntity

object(ZF\Hal\Entity)[637]
  protected 'id' => string '2182' (length=4)
  protected 'links' => 
    object(ZF\Hal\Link\LinkCollection)[635]
      protected 'links' => 
        array (size=1)
          'self' => 
            object(ZF\Hal\Link\Link)[636]
              protected 'props' => 
                array (size=0)
                  empty
              protected 'relation' => string 'self' (length=4)
              protected 'route' => string 'umid.rest.events' (length=16)
              protected 'routeOptions' => 
                array (size=0)
                  empty
              protected 'routeParams' => 
                array (size=1)
                  'events_id' => string '2182' (length=4)
              protected 'url' => null
  protected 'entity' => 
    object(Umid\V1\Rest\Events\EventsEntity)[502]
      public 'Id' => string '2182' (length=4)
      ...
      public 'unternehmen' => 
        object(Umid\V1\Rest\Unternehmen\UnternehmenEntity)[696]
          public 'Id' => string '12608' (length=5)
          public 'land_Id' => string '1' (length=1)
          public 'bundesland_Id' => string '0' (length=1)
          ...

Output - embedded entity is missing

{
    Id: "2182"
    events_location_Id: "12608"
    location_Id: null
    unternehmen_Id: "12608"
    sehenswuerdigkeit_Id: "0"
    veranstalter_Id: "12608"
    ...
    -_links: {
        -self: {
            href: "http:\/\/192.168.33.101\/events\/2182"
        }
    }
}

Code for Collection (http://192.168.33.101/events)

    public function onRenderCollectionEntity($e)
    {
        $entity = $e->getParam('entity');
        $version = $this->isEntityOfInterest($entity, 'Events', 1);

        if (false !== $version) {
            $halEntity = new HalEntity($entity, $entity['Id']);
            if($halEntity instanceof EventsEntity) {
                $repository = $this->sm->get('Umid\V1\Rest\Unternehmen\UnternehmenResource');
                $item = $repository->fetch($halEntity['events_location_Id']);
                $halEntity['unternehmen'] = $item;
            }
            $this->injectEventsEntityLinks($halEntity, $version);
            $e->setParam('entity', $halEntity);
            return;
        }
    }

Dump of $halEntity

object(ZF\Hal\Entity)[685]
  protected 'id' => string '2182' (length=4)
  protected 'links' => 
    object(ZF\Hal\Link\LinkCollection)[687]
      protected 'links' => 
        array (size=1)
          'self' => 
            object(ZF\Hal\Link\Link)[690]
              protected 'props' => 
                array (size=0)
                  empty
              protected 'relation' => string 'self' (length=4)
              protected 'route' => string 'umid.rest.events' (length=16)
              protected 'routeOptions' => 
                array (size=0)
                  empty
              protected 'routeParams' => 
                array (size=1)
                  'events_id' => string '2182' (length=4)
              protected 'url' => null
  protected 'entity' => 
    object(Umid\V1\Rest\Events\EventsEntity)[667]
      public 'Id' => string '2182' (length=4)
      ...
      public 'unternehmen' => 
        object(Umid\V1\Rest\Unternehmen\UnternehmenEntity)[716]
          public 'Id' => string '12608' (length=5)
          public 'land_Id' => string '1' (length=1)
          public 'bundesland_Id' => string '0' (length=1)
          ...

Output - with embedded entity

{
    Id: "2182"
    events_location_Id: "12608"
    location_Id: null
    unternehmen_Id: "12608"
    sehenswuerdigkeit_Id: "0"
    veranstalter_Id: "12608"
    ...
    -_embedded: {
        -unternehmen: {
            Id: "12608"
            land_Id: "1"
            bundesland_Id: "0"
            landkreis_Id: "0"
            region_Id: "0"
            ...
            -_links: {
                -self: {
                    href: "http:\/\/192.168.33.101\/unternehmen\/12608"
                }
            }
        }
    }
    -_links: {
        -self: {
            href: "http:\/\/192.168.33.101\/events\/2182"
        }
    }
}

Originally posted by @Alanin at https://github.com/zfcampus/zf-hal/issues/53

weierophinney commented 4 years ago

Desired behaviour from my point of view would be:

renderEntity and renderCollection.entity have same behaviour and event params

Currently

renderEntity event:

Set links/embedded entities are NOT rendered in HAL response due to described behaviour in description of this issue.

renderCollection.entity :

Set links/embedded entities are rendered in HAL response.

Desired

Make both events consistent in terms of params and behaviour. This would avoid confusion and remove unnecessary code e.g. https://gist.github.com/matuszeman/b797d9c70a4ce9e0974a#file-gistfile1-php-L27

renderEntity event:

Set links/embedded entities on 'entity' are rendered in HAL response.

renderCollection.entity :

Set links/embedded entities on 'entity' are rendered in HAL response.

Additional ideas

HalEntity holds extracted array of domain entity only and domain entity identifier e.g. class name or entity itself (not used for further modification of an result though). Add 'embedded' property into HalEntity as we have 'links' now. Method on HalEntity::getEmbedded() returns embedded collection which could be used to add new embedded entities.

renderEntity event:

renderCollection.entity :

Originally posted by @matuszeman at https://github.com/zfcampus/zf-hal/issues/53#issuecomment-52769379

weierophinney commented 4 years ago

@matuszeman Actually, your suggestion to unify the parameters for the events is not really possible. The reason renderCollection.entity does not have a ZF\Hal\Entity instance is because, at the time we trigger, we do not yet know if the value is such an entity, or if it is a value object/array composed by the collection. One reason for the event is actually to allow developers to tie into the logic and create ZF\Hal\Entity instances themselves. If you look at the lines following the triggering of the event, we create a ZF\Hal\Entity if the entity isn't one yet.

Regarding the original issue posted by @Alanin however: the problem is that we're keying the serializedEntities property on objects! This won't work unless we use SplObjectHash; I'm going to create a fix for that if I can figure out a test first...

Originally posted by @weierophinney at https://github.com/zfcampus/zf-hal/issues/53#issuecomment-68792330

weierophinney commented 4 years ago

Okay, interesting: we're using an SplObjectStorage instance for the serializedProperties, so that's not actually the issue.

Originally posted by @weierophinney at https://github.com/zfcampus/zf-hal/issues/53#issuecomment-68794276

weierophinney commented 4 years ago

@Alanin Can you please provide a unit test for this? Based on your writeup, I'm unable to build a failing unit test at this point.

Originally posted by @weierophinney at https://github.com/zfcampus/zf-hal/issues/53#issuecomment-68794585