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

IBX-85: Fixed Content cache containing invalid "uri" for file field types #3096

Merged
merged 7 commits into from
Apr 27, 2021

Conversation

Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Apr 7, 2021

Question Answer
JIRA issue IBX-85
Bug/Improvement yes
New feature no
Target version 7.5
BC breaks no
Tests pass yes
Doc needed no

This fixes the issue of Content sharing field with Site-Access aware/reliant external data.

The root of the issue is that "uri" for BinaryFileField is calculated right after it is acquired from the database, in SPI persistence layer. This causes cache to contain this calculated "uri" value in all SiteAccess' that share the same cache key.

Solution involves:

  1. Extending the eZ\Publish\Core\MVC\Symfony\FieldType\BinaryBase\ContentDownloadUrlGenerator with additional methods that would allow recovering of the route and parameters used for the "uri" calculation. A new eZ\Publish\SPI\FieldType\BinaryBase\RouteAwarePathGenerator is introduced to mark their presence.
  2. Making eZ\Publish\Core\FieldType\BinaryBase\BinaryBaseStorage::getFieldData, which is called internally by eZ\Publish\Core\Persistence\Legacy\Content\Handler::load, add "route" and "route_parameters" to external data (if RouteAwarePathGenerator is used). They are stored into cache alongside the calculated "uri" for BC compatibility.
  3. Have classes descending from eZ\Publish\Core\FieldType\BinaryBase\Type use newly introduced regenerateUri method that recalculates the "uri" from cached "route" and "route_parameters", if they are present.

How to reproduce the issue

  1. Add a new Media File
    image

  2. Either manually clear cache and visit admin file content page, or trigger any admin edit actions that result in cache being regenerated.

  3. Observe the URI generated for content.fields in template.
    (vendor/ezsystems/ezplatform-admin-ui/src/bundle/Resources/views/themes/admin/content/location_view.html.twig)
    image
    image

  4. Visit the content location. Observe that the URL in template for content.fields is the same as the one visible in admin. That's the bug. (eZ/Bundle/EzPublishCoreBundle/Resources/views/default/content/full.html.twig)
    image
    image

Note

Our content field templates do regenerate the URI during rendering (eZ/Bundle/EzPublishCoreBundle/Resources/views/content_fields.html.twig).
image

TODO:

  • Implement feature / fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@Steveb-p Steveb-p force-pushed the ibx-85-fix-content-field-cache branch 2 times, most recently from 2ecd194 to da9fd3d Compare April 8, 2021 09:25
@Steveb-p Steveb-p force-pushed the ibx-85-fix-content-field-cache branch from da9fd3d to 0a40640 Compare April 8, 2021 09:42
@Steveb-p Steveb-p marked this pull request as ready for review April 8, 2021 09:59
@Steveb-p Steveb-p requested review from a team, adamwojs and alongosz April 8, 2021 09:59
@Steveb-p Steveb-p requested a review from ViniTou April 26, 2021 08:29
Copy link

@tomaszszopinski tomaszszopinski left a comment

Choose a reason for hiding this comment

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

QA approved on eZSystems on 2.5.18 with patch.

@lserwatka lserwatka merged commit 177cdeb into 7.5 Apr 27, 2021
@lserwatka lserwatka deleted the ibx-85-fix-content-field-cache branch April 27, 2021 13:04
@lserwatka
Copy link
Member

Could you merge it up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants