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

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

Open
weierophinney opened this issue Dec 31, 2019 · 4 comments

Comments

@weierophinney
Copy link
Contributor

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 zfcampus/zf-hal#53

@weierophinney
Copy link
Contributor Author

Desired behaviour from my point of view would be:

renderEntity and renderCollection.entity have same behaviour and event params

Currently

renderEntity event:

  • entity - ZF\Hal\Entity - with 'entity' property representing my domain entity.

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

renderCollection.entity :

  • collection - ZF\Hal\Collection
  • entity - my domain entity/resource
  • resource (I think this is left here for BC)

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:

  • halEntity - ZF\Hal\Entity
  • entity - my domain entity

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

renderCollection.entity :

  • collection - ZF\Hal\Collection
  • halEntity - ZF\Hal\Entity
  • entity - my domain 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:

  • halEntity - ZF\Hal\Entity - adding extra embedded entities would be done using similar fashion as with links currently.

renderCollection.entity :

  • halCollection - ZF\Hal\Collection
  • halEntity - ZF\Hal\Entity

Originally posted by @matuszeman at zfcampus/zf-hal#53 (comment)

@weierophinney
Copy link
Contributor Author

@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 zfcampus/zf-hal#53 (comment)

@weierophinney
Copy link
Contributor Author

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


Originally posted by @weierophinney at zfcampus/zf-hal#53 (comment)

@weierophinney
Copy link
Contributor Author

@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 zfcampus/zf-hal#53 (comment)

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