Skip to content

Conversation

@soyuka
Copy link
Member

@soyuka soyuka commented Aug 1, 2017

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1290
License MIT
Doc PR api-platform/docs#233

The following will bypass readable and readableLink:

@ApiProperty(attributes={"fetchEager": true})

ping @bendavies could you give this a try?

@bendavies
Copy link
Contributor

bendavies commented Aug 1, 2017

@soyuka thanks for the quick work!
There is a bug now, however. your new if statement for $readable will exit early, where as the old one checks all conditions. This causes all relations to eager join.

@soyuka
Copy link
Member Author

soyuka commented Aug 1, 2017

Hmm yes indeed updated ^^.

@bendavies
Copy link
Contributor

Perfect, works for me!

@bendavies
Copy link
Contributor

bendavies commented Aug 1, 2017

Obviously the docs should be updated, but also clarified, as at the moment the eager loading page (https://api-platform.com/docs/core/performance) doesn't really say that eager loading is linked to serialization.


if (false === $propertyMetadata->isReadableLink() || false === $propertyMetadata->isReadable()) {
continue;
if (false === $propertyMetadata->getAttribute('fetchEager', false)) {
Copy link
Member

Choose a reason for hiding this comment

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

You can merge this condition with the previous one!

Copy link
Member Author

Choose a reason for hiding this comment

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

not really :|. I tried things in my head but it doesn't seem to end up well because it's not a condition to break but rather a condition to not break when we should have.

Copy link
Contributor

Choose a reason for hiding this comment

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

plus this is far more readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

okay got it I think:

if ((false === $propertyMetadata->isReadableLink() || false === $propertyMetadata->isReadable()) && false === $propertyMetadata->getAttribute('fetchEager', false)) {
    continue;
}

@bendavies
Copy link
Contributor

good to merge @soyuka ?

@Simperfit
Copy link
Contributor

Needs rebase but good to merge.

The following will bypass `readable` and `readableLink`:

```
@ApiProperty(attributes={"fetchEager": true})
```
@soyuka soyuka merged commit 9ae308f into api-platform:master Aug 8, 2017
@soyuka soyuka deleted the fix/1290 branch January 29, 2018 09:38
hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
Allow to fetchEager a non-serializable property fix api-platform#1290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants