Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Implement service-based Resource (cache) validation#15738

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
mpdude wants to merge6 commits intosymfony:2.8fromwebfactory:resource-checkers
Closed

Implement service-based Resource (cache) validation#15738

mpdude wants to merge6 commits intosymfony:2.8fromwebfactory:resource-checkers

Conversation

mpdude
Copy link
Contributor

QA
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?yes
Tests pass?yes
Fixed tickets#7230,#15692,#7782
LicenseMIT
Doc PRsymfony/symfony-docs#5136

Overview

Currently, any metadata passed toConfigCache (namely implementations ofResourceInterface) is serialized to disk. When theConfigCache is validated, the metadata is unserialized and queried throughResourceInterface::isFresh() to determine whether the cache is fresh. That way,ResourceInterface implementations cannot interact with services, for example a database connection.

This PR introduces the new concept ofResourceCheckers. Services implementingResourceCheckerInterface can be tagged asconfig_cache.resource_checker with an optional priority.

Clients that wish to useConfigCache can then obtain an instance from theconfig_cache_factory service (which implementsConfigCacheFactoryInterface). The factory will take care of injecting resource checkers into theConfigCache instance so that they can be used for cache validation.

Checking cache metadata is easy forResourceCheckers:

  • First, theResourceCheckerInterface::supports() implementation is passed the metadata object in question. If the checker cannot handle the type of resource passed,supports() should returnfalse.
  • Otherwise, theResourceCheckerInterface::isFresh() method will be called and given the resource as well as the timestamp at which the cache was initialized. If that method returnsfalse, the cache is considered stale. If it returnstrue, the resource is considered unchanged and willnot be passed to any additional checkers.

BC and migration path

This PR does not (intend to) break BC but it comes with deprecations. The main reason is thatResourceInterface contains anisFresh() method that does not make sense in the general case of resources.

Thus,ResourceInterface::isFresh() is marked as deprecated and should be removed in Symfony 3.0. Resource implementations that can (or wish to) be validated in that simple manner can implement theSelfCheckingResourceInterface sub-interface that still contains (and will keep) theisFresh() method. The change should be as simple as changing theextends list.

Apart from that,ResourceInterface will be kept as the base interface for resource implementations. It is used in several@api interfaces and thus cannot easily be substituted.

For the Symfony 2.x series, aBCResourceInterfaceChecker will be kept that performs validation throughResourceInterface::isFresh() but will trigger a deprecation warning. The remedy is to either implement a custom ResourceChecker with a priority higher than -1000; or to switch to the aforementionedSelfCheckingResourceInterface which is used at a priority of -990 (without deprecation warning).

TheConfigCache andConfigCacheFactory classes can be used as previously but do not feature checker-based cache validation.

Outlook and closing remarks:

This PR supersedes#7230,#15692 and works at least in parts towards the goal of#7176.

TheResourceCheckerInterface,...ConfigCache and...ConfigCacheFactory no longer need to be aware of thedebug flag. The different validation rules applied previously are now just a matter ofResourceChecker configuration (i. e. "no checkers" inprod).

It might be possible to remove thedebug flag from Symfony'sRouter and/orTranslator classes in the future as well because it was only passed on to theConfigCache there.


public function isFresh(ResourceInterface $metadata, $timestamp)
{
trigger_error('Resource checking through ResourceInterface::isFresh() is deprecated since 2.8 and will be removed in 3.0', E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

missing@ (which would make the testsuite fail btw)

@fabpot
Copy link
Member

The UPGRADE-2.8.md and 3.0 file should be updated with instruction on how to upgrade.

@mpdude
Copy link
ContributorAuthor

I added entries in the UPGRADING files and hope that's what you expect.

Currently, any metadata passed to ConfigCache (namely implementations of ResourceInterface) is serialized to disk. When the ConfigCache is validated, the metadata is unserialized and queried through ResourceInterface::isFresh() to determine whether the caceh is fresh. That way, ResourceInterface implementations cannot interact with services, for example a database connection.This PR introduces the new concept of ResourceCheckers. Services implementing ResourceCheckerInterface can be tagged as "config_cache.resource_checker" with an optional priority.Clients that wish to use ConfigCache can then obtain an instance from the config_cache_factory service (which implements ConfigCacheFactoryInterface). The factory will take care of injecting resource checkers into the ConfigCache instance so that they can be used for cache validation.Checking cache metadata is easy for ResourceCheckers:- First, the ResourceCheckerInterface::supports() implementation is passed the metadata object in question. If the checker cannot handle the type of resource passed, support() should return false.- Otherwise, the ResourceCheckerInterface::isFresh() method will be called and given the resource as well as the timestamp at which the cache was initialized. If that method returns false, the cache is considered stale. If it returns true, the resource is considered valid and will *not* be passed to any additional checkers.BC and migration path:This PR does not (intend to) break BC but it comes with deprecations. The main reason is that ResourceInterface contains an isFresh() method that does not make sense in the general case of resources.Thus, ResourceInterface::isFresh() is marked as deprecated and shall be removed in Symfony 3.0. Resource implementations that can (or wish to) be validated in that simple manner can implement the SelfCheckingResourceInterface sub-interface that still contains (and will keep) the isFresh() method. The change should be as simple as changing the "extends" list.Apart from that, ResourceInterface will be kept as the base interface for resource implementations. Note that ResourceInterface is used in several @api interfaces and thus cannot easily be substituted.For the Symfony 2.x series, a BCResourceInterfaceChecker will be kept that performs validation through ResourceInterface::isFresh() but will trigger a deprecation warning. The remedy is to either implement a custom ResourceChecker with a priority higher than -1000; or to switch to the aforementioned SelfCheckingResourceInterface which is used at a priority of -990 (without deprecation warning).The ConfigCache and ConfigCacheFactory classes can be used as previously but do not feature checker-based cache validation.Outlook and closing remarks:This PR supersedes#7230,#15692 and works at least in parts towards the goal of#7176.The ResourceCheckerInterface, ...ConfigCache and ...ConfigCacheFactory no longer need to be aware of the "debug" flag. The different validation rules applied previously are now just a matter of ResourceChecker configuration (i. e. "no checkers" in prod). It might be possible to remove the "debug" flag from Symfony's Router and/or Translator classes in the future as well because it was only passed on to the ConfigCache there.
$this->fresh = $isFresh;
}

public function __toString() {
Copy link
Contributor

Choose a reason for hiding this comment

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

{ should be in next line.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Hey FabBot, are you sleeping :)?

krsort($resourceCheckers);
$resourceCheckers = call_user_func_array('array_merge', $resourceCheckers);

$container->getDefinition('config_cache_factory')->replaceArgument(0, $resourceCheckers);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this befindDefinition()?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Well, that service is defined and not an alias. Do you think it should be one?

@mpdude
Copy link
ContributorAuthor

@fabpot Anything else you'd like to see changed? Once this is merged I'd like to continue discussing#15719 and#7796, both would need to be addressed in 2.8/3.0 because of deprecations/removals.

@mpdude
Copy link
ContributorAuthor

Build failure due to transient test on Windows, added to the list at#15617 (comment)

@mpdude
Copy link
ContributorAuthor

ping@fabpot

@fabpot
Copy link
Member

👍

@fabpot
Copy link
Member

ping @symfony/deciders

@dunglas
Copy link
Member

👍

@fabpot
Copy link
Member

Thank you@mpdude.

@fabpotfabpot reopened thisSep 25, 2015
@fabpotfabpot closed thisSep 25, 2015
@mpdude
Copy link
ContributorAuthor

Thank you@fabpot. Now let's get#15719 resolved (a quick win IMO) and we have a simple and clean ResourceInterface in 3.0.

@stof
Copy link
Member

This PR is breaking tests

@mpdude
Copy link
ContributorAuthor

Where?

@stof
Copy link
Member

Ah, I understand the issue: this change is not yet merged into master, andhttp-kernel 2.8 allows config at both 2.8 ad 3.0.
@fabpot can you merge branches to fix the build ?

@fabpot
Copy link
Member

done

@@ -104,34 +104,6 @@ public function testTransWithCachingWithInvalidLocale()
$translator->trans('foo');
}

public function testLoadResourcesWithCaching()
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason why this test fails ?

$translator->setConfigCacheFactory(new \Symfony\Component\Config\ResourceCheckerConfigCacheFactory([new \Symfony\Component\Config\Resource\SelfCheckingResourceChecker(),new \Symfony\Component\Config\Resource\BCResourceInterfaceChecker()]));

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Don't know, but maybe you want to use the default ConfigCacheFactory in that case?

Copy link
Contributor

Choose a reason for hiding this comment

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

I usedResourceCheckerConfigCacheFactory, the problems occurs when we want to work with the debug mode, is there a way to pass debug mode forResourceCheckerConfigCacheFactory like theold one ?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

7 participants
@mpdude@fabpot@dunglas@stof@stloyd@aitboudad@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp