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

support of partial entities #67

Closed
jmfeurprier opened this issue Dec 7, 2017 · 8 comments
Closed

support of partial entities #67

jmfeurprier opened this issue Dec 7, 2017 · 8 comments
Labels

Comments

@jmfeurprier
Copy link
Contributor

jmfeurprier commented Dec 7, 2017

Hi,

In an attempt to optimize a CMS using the mapado REST client SDK, I tried implementing HTTP-based normalization/denormalization groups on the API I'm using, i.e. :

GET /articles/1234

would return a complete entity like this:

{
  	"id": 1234,
  	"body": "<p>Article body</p>",
  	"template": "article",
  	"section": 2345,
  	"title": "Article title",
  	"sponsor": null,
  	"authors": [
  		456,
  		567
  	],
  	"tags": [
  		567,
  		678
  	],
  	"status": "PUBLISHED",
  	"createdAt": "2017-09-21T15:28:10-04:00",
  	"modifiedAt": "2017-10-23T15:40:04-04:00"
  }

While

GET /articles/1234?_groups[]=bare

would return only a partial entity like this:

{
  	"id": 1234,
  	"title": "Article title"
  }

Unfortunately, the REST client calls every setter on the entity, and for values which were not returned by the API, attempts to set NULL values with setters which don't allow them, causing fatal errors.

A possible "fix" / solution would be to skip setter calls for which the corresponding property was not provided in the API response.

Does it make sense?

@jdeniau
Copy link
Member

jdeniau commented Dec 8, 2017

Totally ! I think it's an easy pick for a PR if you'd like

@jmfeurprier
Copy link
Contributor Author

jmfeurprier commented Dec 8, 2017

Hi,

I investigated what was causing the fatal error, and it is more related to proxy generation and caching mixed together than setters:

If I first fetch a partial entity, the entity will be created, using only retrieved values, and will then be stored in cache for later retrieval (using its unique identifier).

If after this, I try to fetch this entity in a "complete" way, its values will be retrieved from cache, and this is when the setters will be called with NULL values because of the way the proxified entities (relations) are handled.

At this point, I thought of another way to make it work: I introduced a special query parameter "_noCache" to explicitely tell the Entity Repository not to cache the retrieved entity. It works, and now performance is great, but this is not what I first had in mind.

This is what the EntityRepository::saveToCache() method looks like now:

    protected function saveToCache($key, $value, array $queryParams = [])
    {
        if (!empty($queryParams['_noCache'])) {
            return;
        }

        // original code here
    }

Call to this method from methods find() and __call() now provide the $queryParams variable.

What's your opinion on this ?

@jdeniau
Copy link
Member

jdeniau commented Dec 9, 2017

Well it may work but as I read it it seems like really custom logic you have to pass in your method from your app and that's not really what we want (the purpose is to be really abstract and powerful).

I'll have to see what we made at mapado because we use partial entities intensively so I think we have made this work, but it may be is custom logic.

An idea would be to change the cache key depending on the request Uri but it may be hard to implement.

@zwit maybe you have remember what we have made?

@jmfeurprier
Copy link
Contributor Author

I'm not satisfied either by the implementation I pasted above. Any idea is welcome.

@jdeniau jdeniau removed the easy pick label Dec 15, 2017
@jdeniau
Copy link
Member

jdeniau commented Dec 15, 2017

I investigate about this, but I don't really know how it append.

All call to fetchFromCache keys contains the query including the parameters

So if you make two call like /foo/1?_groups[]=bar and /foo/1, you should have two differents HTTP call and two different cache keys and values.

The proxy is then only made when deserializing the JSON in Serializer if you have a relation.

If you use the Symfony bundle, you may have more informations in the debug toolbar.

Can you provide more information like your entities, the sdk configuration part and the call you make (and ideally the output of the rest-client-sdk part in the debug toolbar) to investigate ?

@jdeniau
Copy link
Member

jdeniau commented Jan 30, 2019

@jmfeurprier is the issue still opened ?

@jmfeurprier
Copy link
Contributor Author

Hi,

I switched to other projects which don't use the REST SDK client, so we're not in need any more for fixes (we ended overriding some parts of the tool to suit our customer very specific needs).

Feel free to close the issue.

@jdeniau jdeniau closed this as completed Jan 31, 2019
@jdeniau
Copy link
Member

jdeniau commented Jan 31, 2019

OK thanks.
feel free to re-open the issue if needed

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

No branches or pull requests

2 participants