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

Collection of Doctrine Single table inheritance entities #1071

Closed
desmax opened this issue Apr 20, 2017 · 20 comments
Closed

Collection of Doctrine Single table inheritance entities #1071

desmax opened this issue Apr 20, 2017 · 20 comments

Comments

@desmax
Copy link
Contributor

desmax commented Apr 20, 2017

I have an object, that has collection containing different entities, that use Single inheritance.
Problem is that only base fields are fetched, entity specific are null. As I understand its happening because Doctrine partial query is used, but how can I force load all fields for given collection?

@soyuka
Copy link
Member

soyuka commented Apr 20, 2017

#1025 can you try the 2.0 branch?

@desmax
Copy link
Contributor Author

desmax commented Apr 20, 2017

@soyuka I am sorry for duplicate. And I am on master already.

@soyuka
Copy link
Member

soyuka commented Apr 20, 2017

Hmm this patch hasn't been merged back to master yet :|.

@soyuka
Copy link
Member

soyuka commented Apr 20, 2017

@desmax done through #1072 can you update?

@desmax
Copy link
Contributor Author

desmax commented Apr 20, 2017

@soyuka I don't seem to see any difference...

@bwegrzyn
Copy link
Contributor

@desmax Can you try to disabled eager loading and try again?

api_platform:
    eager_loading:
        enabled: false

Also, can you provide a sample of your entities?

@soyuka
Copy link
Member

soyuka commented Apr 20, 2017

Even better disable it on the resource:

/*
 * @ApiResource(attributes={"force_eager"=false})
 */

@bwegrzyn
Copy link
Contributor

@soyuka Interestingly... that doesn't disabled eager loading on a resource for me. I have to force it through config.yml. That might be a separate issue.

@soyuka
Copy link
Member

soyuka commented Apr 20, 2017

This is the condition:

            if (false === $forceEager && ClassMetadataInfo::FETCH_EAGER !== $mapping['fetch']) {

Anyway, I need to refactor this fetchEager thing #1035

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

@bwegrzyn disabling eager loading via config does help.
On resource object I have one to many relation to base single inherited master class, marked as abstract.
All non abstract entities extend base class, some of them have additional fields, that are not present in base class. Base class is marked with @ApiResource() annotation.

oneToMany:
  history:
     targetEntity: AppBundle\Entity\HistoryAction\HistoryAction
     mappedBy: vacancy
AppBundle\Entity\HistoryAction\HistoryAction:
    type: entity

    inheritanceType: SINGLE_TABLE

    discriminatorColumn:
        name: type
        type: string

    discriminatorMap:
        created:              Created
        ...

    cache:
        usage: NONSTRICT_READ_WRITE

    id:
        id:
            type: integer
            id: true
            generator:
                strategy: AUTO

    gedmo:
        translation:
            locale: language

    fields:
        created:
            type: datetime
            gedmo:
                timestampable:
                    on: create

    manyToOne:
        user:
            targetEntity: AppBundle\Entity\User
            inversedBy: history
            joinColumn:
                nullable: false

        vacancy:
            targetEntity: AppBundle\Entity\Vacancy
            inversedBy: history
            joinColumn:
                nullable: false

AppBundle\Entity\HistoryAction\Created:
    type: entity

    fields:
        initialFields:
            type: array

Another thing that bugging me is that array typed initialFields is not serialized unless I mark sub entity as ApiResource (field value is fetched and not null). That makes no sense to mark it as ApiResource, for now I force that value to serialize via custom Normalizer.

@bwegrzyn
Copy link
Contributor

You must mark each entity with @ApiResource, even if they extend a base class that is marked with @ApiResource. Otherwise, the platform doesn't know about it and may generate the wrong query to fetch the data.

This is probably why it works when eager loading is disabled. Without eager loading, the API Platform will load the main resource, and then issue a separate query for each relation. These queries are standard Doctrine find queries and they load the entire object. When eager loading is enabled, it generates just one query and fetches everything it requires at once (including all relations). If child resources are not marked with @ApiResource, then it won't fetch it's properties in the query it generates because it fetches only the properties it needs through object partials.

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

@bwegrzyn

Without eager loading, the API Platform will load the main resource, and then issue a separate query for each relation

yes, that is the case. Still, even if I mark all sub classes as Resource - it does not load what it has to.(eager ON)

When I mark all sub classes as Resource, fields from them are still not serialized.
But when I mark base class and only one subclass - that subclass is serialized correctly, wtf.

Plus obviously, my generated documentation is a bit messy and misleading.

@bwegrzyn
Copy link
Contributor

Hmm, I'm not sure. This should have been fixed by #1063.

Maybe you can contribute a failing test case?

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

@bwegrzyn I got the problem with serialization. your fix $context['resource_class'] = $resourceClass; is implemented in JsonLd Item normalizer and for that entity I use custom normalizer that extends AbstractItemNormalizer (because JsonLd normalizer is final, damn!)
Maybe we could move your fix one level higher?

p.s. and I don't have to mark sub classes as ApiResource

@soyuka
Copy link
Member

soyuka commented Apr 21, 2017

@desmax Why do you need a custom normalizer? I'd try not to extend our AbstractNormalizer but instead inject the JsonLd\Serializer\ItemNormalizer and call it when I'm done doing custom things.
I don't think we can move that fix one level higher :| (not sure).

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

@soyuka I need to translate one field into user's language. Ok I will try injecting.

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

Injecting turned out a lot cleaner, thanks. Though I still have issue with eager loading not fetching subclass specific fields. Will try to debug now.

@soyuka
Copy link
Member

soyuka commented Apr 21, 2017

Might be related to #1069

@desmax
Copy link
Contributor Author

desmax commented Apr 21, 2017

@soyuka how about this? #1073

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

3 participants