[Config] RFC - Simplify and redesign ResourceInterface #7176

mpdude · 2013-02-25T12:23:49Z

Currently, Config/ResourceInterface and its implementations serve a single purpose - that is, checking whether data from a ConfigCache is still valid. For that, the cache is associated with a set of Resources which can later be checked for freshness. ConfigCache seems to be the only client of ResourceInterface right now.

The problem with the interface is that it somewhat assumes that resources are filesystem objects. Just as Loaders (Config/Loader/LoaderInterface) can load about anything, the same should be true for Resources.

For example, it is no problem to have a loader pull some information (think of translations) from a database. At a later time, you might need to check if the generated message catalogue is still up to date.

Implementing ResourceInterface in this case is difficult - __toString() and getResource() hardly make sense for "translations from a database", and at least to getResource() is the Loader's job. Anyway, both methods are not used by ConfigCache as the current sole client.

Freshness check based on timestamp

Checking isFresh($timestamp) - which means "is the Resource still the same as it was at $timestamp" - might also be implemented differently for the said database.

The $timestamp need not be a timestamp but just an opaque identifier (think ETag). Even more, as Resource implementations being serialized carry resource type-specific data anyway (a filename, for example), the entire way of validating them should be encapsulated in the Resource. That is, a Resource could upon creation get or keep what it needs for validation.
For example, a FileResource would grab the mtime() of the underlying file, keep it along with the filename (in its serialized form) and later on (when unserialized) check the mtime() again, comparing the two values.

A more simple, self-validating Resource interface

All this would lead to a ResourceInterface like

namespace Symfony\Component\Config\Resource;

interface ResourceInterface
{
    /**
     * Returns true if the resource has been updated since it has been created.
     * @return Boolean true if the resource haseen updated, false otherwise
     */
    public function hasChanged();
}

plus the requirement the implementors are (un)serializable without side effects.

Resource checks using services

I also made a suggestion how Resource validation could be shifted from the Resource implementation to some kind of "voter" or "checker" class which allows for using services (a database connection, a loader implementation) during the check which is not possible if the check has to be within the unserialized resource.

That would even leave Resource as a marker interface and have the "voter" implement a single "hasChanged(Resource $r)" method.

To sum up

I think simplifying the ResourceInterface would be a good thing to do because

it adds conceptual clarity
yields a segregated interface
allows for new Resource types that can reflect what loaders can already do

Shifting the freshness check from Resource implementations to some kind of "voter"/"checker" additionally makes it possible to use services for validation, being a solution for at least one issue.

Opinions please. Will do the work if others agree :-).

Update: The current WIP is #7230.

The text was updated successfully, but these errors were encountered:

apfelbox · 2014-06-22T22:41:56Z

I like it 👍

mpdude · 2015-04-10T14:27:59Z

@stof What do you think? This is an old issue and not entirely up to date, but maybe something worth discussing for 3.0?

stof · 2015-08-01T18:22:13Z

@mpdude yes, it would be great to solve this issue. We would have to re-evaluate #7230

mpdude · 2015-09-04T13:26:58Z

@stof What is the right way to change or deprecate an interface for 3.0? I don't see how this could be done with deprecation error messages.

apfelbox · 2015-09-04T13:28:30Z

@mpdude I think adding the PHPDoc and a global trigger_error in the head of the file.

…pdude) This PR was squashed before being merged into the 2.8 branch (closes #15738). Discussion ---------- Implement service-based Resource (cache) validation | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #7230, #15692, #7782 | License | MIT | Doc PR | symfony/symfony-docs#5136 ### Overview 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 cache 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, `supports()` 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 unchanged 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 should 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. It 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. Commits ------- 20d3722 Implement service-based Resource (cache) validation

This PR was merged into the 2.8 branch. Discussion ---------- Deprecate ResourceInterface::getResource() | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | n/a The return value of this method does not make sense if you do not exactly know about the type of resource at hand. For example, it may be [an array](https://github.com/symfony/symfony/blob/b49fa129bdb3c0aa970a006b16dd1ca63a9d7ebd/src/Symfony/Component/HttpKernel/Config/EnvParametersResource.php#L57) or a [file path](https://github.com/symfony/symfony/blob/87800ae47e64429f2544b798575d1cc1d4e5464a/src/Symfony/Component/Config/Resource/FileResource.php#L51). As all usages of getResource() within Symfony are in tests of particular Resource implementations anyway, deprecating and later removing this method helps us with simplifying the ResourceInterface (#7176). Commits ------- 87c0c7d Deprecate ResourceInterface::getResource()

The return value of this method does not make sense if you do not exactly know about the type of resource at hand. For example, it may be [an array](https://github.com/symfony/symfony/blob/b49fa129bdb3c0aa970a006b16dd1ca63a9d7ebd/src/Symfony/Component/HttpKernel/Config/EnvParametersResource.php#L57) or a [file path](https://github.com/symfony/symfony/blob/87800ae47e64429f2544b798575d1cc1d4e5464a/src/Symfony/Component/Config/Resource/FileResource.php#L51). As all usages of getResource() within Symfony are in tests of particular Resource implementations anyway, deprecating and later removing this interface helps us with simplifying the ResourceInterface (symfony/symfony#7176).

This was referenced Feb 25, 2013

[Config] additions from ResourceWatcher #5744

Closed

[Routing][Config] Ability to utilize database resources in router resources/config cache #5159

Closed

mpdude mentioned this issue Mar 7, 2013

[Config] Allow for service-based Resource (cache) validation #7230

Closed

5 tasks

mpdude mentioned this issue Apr 9, 2014

Refactored resource loading logic out of the resources kriswallsmith/assetic#576

Closed

stof added the Config label Sep 5, 2014

stof mentioned this issue Sep 5, 2014

[Translator] Do not use cache if debug is on #11862

Closed

bnw mentioned this issue Sep 4, 2015

Introduce MetadataValidators to test arbitrary metadata for freshness. #15692

Closed

This was referenced Sep 7, 2015

Deprecate ResourceInterface::getResource() #15719

Merged

Implement service-based Resource (cache) validation #15738

Closed

fabpot closed this as completed Oct 5, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Config] RFC - Simplify and redesign ResourceInterface #7176

[Config] RFC - Simplify and redesign ResourceInterface #7176

mpdude commented Feb 25, 2013

apfelbox commented Jun 22, 2014

mpdude commented Apr 10, 2015

stof commented Aug 1, 2015

mpdude commented Sep 4, 2015

apfelbox commented Sep 4, 2015

[Config] RFC - Simplify and redesign ResourceInterface #7176

[Config] RFC - Simplify and redesign ResourceInterface #7176

Comments

mpdude commented Feb 25, 2013

apfelbox commented Jun 22, 2014

mpdude commented Apr 10, 2015

stof commented Aug 1, 2015

mpdude commented Sep 4, 2015

apfelbox commented Sep 4, 2015