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

Normalization group not working in nested embedded entities #277

Closed
blaues0cke opened this issue Apr 7, 2017 · 13 comments
Closed

Normalization group not working in nested embedded entities #277

blaues0cke opened this issue Apr 7, 2017 · 13 comments

Comments

@blaues0cke
Copy link

blaues0cke commented Apr 7, 2017

I have the following entity structure: BrandMessage has Campaign has (CampaignSettings and RunningTime). So when I call the /api/brand-messages api the entities CampaignSettings and RunningTime are nested in the third level. Also, CampaignSettings and RunningTime are embedded entities.

When I call the /api/campaigns api, everything works as expected. runningTime and settings are present:

{
  "@context": "/app_dev.php/api/contexts/Campaign",
  "@id": "/app_dev.php/api/campaigns",
  "@type": "hydra:Collection",
  "hydra:member": [
    {
      "@id": "/app_dev.php/api/campaigns/e64a026e-1b88-11e7-8825-0242ac110002",
      "@type": "Campaign",
      "app": "/app_dev.php/api/apps/e64d3274-11e7-1b88-8825-0242ac110002",
      "runningTime": {
        "daysLeftTillEnd": 99,
        "daysLeftTillStart": -74,
        "end": "2017-07-16T02:31:13+00:00",
        "isFinished": false,
        "start": "2017-01-23T08:02:29+00:00"
      },
      "settings": {
        "participationLimit": 1,
        "receiptMandatory": true,
        "requiredProductCount": 5,
        "rewardInEuroCent": 100
      },
      "title": "...",
      "description": "..."
    },
   ...
}

But when I call the /api/brand-messages api, these properties (runningTime and settings) are just null:

{
  "@context": "/app_dev.php/api/contexts/BrandMessage",
  "@id": "/app_dev.php/api/brand-messages",
  "@type": "hydra:Collection",
  "hydra:member": [
    {
      "@id": "/app_dev.php/api/brand-messages/e64d3274-1b88-11e7-8825-0242ac110002",
      "@type": "BrandMessage",
      "backgroundImage": null,
      "campaign": {
        "@id": "/app_dev.php/api/campaigns/e64a3837-1b88-11e7-8825-0242ac110002",
        "@type": "Campaign",
        "app": "/app_dev.php/api/apps/e64d3274-11e7-1b88-8825-0242ac110002",
        "runningTime": null,
        "settings": null,
        "title": "...",
        "description": "..."
      },
      "colorPair": "secondary",
      "httpCallToActionLabel": "",
      "httpLink": "",
      "important": true,
      "type": "CAMPAIGN",
      "title": "..."
    },
    ...
  ],
  ...
}

I double checked all the serialisation groups and they even work in the third nesting level for title and description. Is there a default nesting limit for nested properties or something like that? Or am I facing a bug? :-)

@dunglas
Copy link
Member

dunglas commented Apr 7, 2017

There is no limit. The null value is probably a sign that it's an ORM related problem (the relations are probably not fetched).

@blaues0cke
Copy link
Author

blaues0cke commented Apr 7, 2017

Alright, thank you. Will dig in this tomorrow. A fast try using fetch="EAGER" on the campaign property in BrandMessage does not fix the issue. For embedded entities there is now fetch="EAGER" setting, so right now I can't remember any other "lazy loading" logic in there (especially for embedded entities) that could cause my issue.

@nik-lampe
Copy link

Might that be the same problem as in api-platform/core#1025 and api-platform/core#1043 ?

@soyuka
Copy link
Member

soyuka commented Apr 12, 2017

@blaues0cke can you try to avoid fetch="EAGER" and let our EagerLoadingExtension do the work?

I double checked all the serialisation groups and they even work in the third nesting level for title and description. Is there a default nesting limit for nested properties or something like that? Or am I facing a bug? :-)

I always though we need some kind of serializer debugger for groups, can be a headache.

Would you be able to reproduce this issue on a fork of api-platform/api-platform? As @dunglas said, this probably mean that relations have not been fetched by the query, and they're serialized as null. Usually when I have such issue, it is group-related.

@nik-lampe it seems like your issue is related to inheritance, I don't see any here.

@nik-lampe
Copy link

@soyuka That is true for the one issue, but in api-platform/core#1043 I'm experiencing the same issues without inheritance. So I guess maybe it's not about inheritance at all...

While debugging I found out, that the attributes for the entity aren't retrieved, because the propertyNameCollectionFactory receives the wrong resource class (as far as I understand it, maybe it is right, but then the logic is flawed somewhere else)

@soyuka
Copy link
Member

soyuka commented Apr 12, 2017

@nik-lampe you wrote lots of different issues / comments so it's hard to track ^^. I added a comment in your pull request, but if there are 2 different problems we need to carefully test both and fix both separately!

  1. This null issue (IMHO it's a serialization group issue, not related to how the ResourceClassResolver works (or maybe but I've a lot of embedded relations and no such issue)
  2. Then there is Wrong attributes serialized when embedding multiple different inherited entities core#1025. This is indeed a bug where the Parent entity is not aware of the Child metadata. Therefore, it can not serialize properties it doesn't know of. This is harder to fix, because the metadata is the one from Parent::class though it should be Parent::class + Child::class. The good question there is "how do we find every child that extends the parent?".
    An easy fix is might be to patch this class by retrieving the metadata for table inheritance.

@nik-lampe
Copy link

@soyuka yes, while applying this framework (which I really love btw) in my project I stumbled across multiple problems and was not aware, that they might all have the same or similar root cause. It was totally confusing for me as well. Sorry, about that :)

@blaues0cke
Copy link
Author

Thank you! I think the fastest way to figure out if my issue is related to the ones from @nik-lampe is to give the knows fixes/prs a try (api-platform/core#1046). I will do that afap.

@soyuka
Copy link
Member

soyuka commented Apr 12, 2017

@blaues0cke no, the best way to figure this out is to have a behat test case with your issue (or a fork of api-platform/api-platform with your entities + groups that reproduces the issue). The PR fix is not valid.

@Simperfit
Copy link
Contributor

I agree with @soyuka maybe we should add a failing behat test in order to fix it without breaking things

@blaues0cke
Copy link
Author

Can confirm that

api_platform:
    eager_loading:
        enabled: false

as explained in api-platform/core#1071 fixes this issue/is a workaround for this.

@dunglas
Copy link
Member

dunglas commented May 8, 2017

Thanks for the feedback @blaues0cke!

@soyuka
Copy link
Member

soyuka commented May 8, 2017

Maybe related to api-platform/core#1100

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

5 participants