Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Do not render embedded resources #3

Closed

Conversation

TomHAnderson
Copy link
Contributor

Add a flag to the plugin for whether to render embedded resources or just show their embed links.

While it's more expensive to retrieve resources individually the option of excluding embedded resources, besides solving the recursive problem with HAL, gives the developer a different way to build APIs and clients. With this an API client which creates a local resource for every resource and embedded resource may lazy load those embedded resources.

This should be extended to set the render_embedded_resources flag on individual resources but as-is works for what I currently have plans for.

@TomHAnderson
Copy link
Contributor Author

I'd like to see some movement on this. Are the infinite resources actually a problem? Am I taking the wrong approach by using the ArraySerliazable hydrator instead of DoctrineEntity? Is the fix too simple?

My problem with DoctrineEntity is it's specific to each entity but maybe there's a way to code for this? I haven't put as much time into this question as I'd like.

}

if (isset($config['render_collections'])) {
$helper->setRenderCollections($config['render_collections']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method does not exist yet. :-)

I'm assuming the idea here would be such that if the flag is false, then the resource emitted for each item of a collection would only contain its hypermedia links? If so, how long would it take you to get that implemented and tested?

@TomHAnderson
Copy link
Contributor Author

It'll take me until, give or take, now. I've only run a cursory test so far.

@weierophinney
Copy link
Member

@TomHAnderson This is awesome -- works exactly like I'd expect!

weierophinney added a commit that referenced this pull request Nov 15, 2013
@TomHAnderson TomHAnderson deleted the render-embedded-resources branch January 3, 2014 04:54
@PoetikDragon
Copy link

Is there any way to select which resources to "render" and which ones to only show links for? Or is it all or nothing?

@TomHAnderson
Copy link
Contributor Author

All or nothing. If you can change it to be resource specific then go ahead.

@PoetikDragon
Copy link

Disabling all collections from rendering breaks apigility, so its kind of a problem that its not resource specific. See this:

zfcampus/zf-apigility#25

@weierophinney
Copy link
Member

@Poeticdragon Considering the seeing is not exposed from the admin ui, I'd argue it's a) advanced usage, and b) "buyer beware" at this stage. The point Tom is trying to make is that he either didn't have a need for making it resource specific, or couldn't find a way to do it. If you want that particular feature, provide some ideas on how it should work from a configuration perspective, and what the rendered resource would look like. Even better, see if you can work up a pull request implementing the feature.

@TomHAnderson
Copy link
Contributor Author

I too had a problem with infinite recursion and this was the solution. I have since avoided such design. a) I did take the time to try implementing this on a per-resource (now service) level and found no such pathways. b) I have a bridge I'd like to sell you.

This came up rather early in my Apigility work and for the time it worked well. I think the functionality has a place in Apigility, to be (re)implemented by the most interested party.

@PoetikDragon
Copy link

What is the technological constraint of moving those options out of $config['zf-hal']['renderer'] and putting them into the individual entities and collections under $config['zf-hal']['metadata_map']?

@TomHAnderson
Copy link
Contributor Author

That sounds like you don't want to pay for my bridge.

The issue is in the HAL plugin. Look there for renderEmbeddedEntities and maybe you can clean up the Entities / setRenderEmbeddedResources naming inconsistencies which have appeared.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants