Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[WIP] [Cache] Move cache ID namespacing to CacheNamespace class #214

Closed
wants to merge 7 commits into
from

Conversation

Projects
None yet
5 participants

CHH commented Nov 12, 2012

No description provided.

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DCOM-135

CHH commented Nov 12, 2012

This is a pull request to add feature #207

@Ocramius Ocramius commented on an outdated diff Nov 12, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+
+ /** @var int Current Namespace version */
+ protected $namespaceVersion;
+
+ /** @const Key to store the namespace's version */
+ const NAMESPACE_CACHE_KEY = "DoctrineCacheNamespaceVersion[%s]";
+
+ /**
+ * Constructor
+ *
+ * @param string $namespace
+ * @param Cache $cache
+ */
+ function __construct($namespace, Cache $cache)
+ {
+ $this->namespace = $namespace;

@Ocramius Ocramius commented on an outdated diff Nov 12, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+ */
+ protected $namespace;
+
+ /** @var int Current Namespace version */
+ protected $namespaceVersion;
+
+ /** @const Key to store the namespace's version */
+ const NAMESPACE_CACHE_KEY = "DoctrineCacheNamespaceVersion[%s]";
+
+ /**
+ * Constructor
+ *
+ * @param string $namespace
+ * @param Cache $cache
+ */
+ function __construct($namespace, Cache $cache)
@Ocramius

Ocramius Nov 12, 2012

Owner

Please add visibility explicitly to all methods

@Ocramius Ocramius commented on the diff Nov 12, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+ /**
+ * {@inheritdoc}
+ */
+ public function getStats()
+ {
+ return $this->cache->getStats();
+ }
+
+ /**
+ * Increments the namespace version, invalidating the namespace.
+ *
+ * @return void
+ */
+ public function incrementNamespaceVersion()
+ {
+ $version = $this->getNamespaceVersion();
@Ocramius

Ocramius Nov 12, 2012

Owner

Can squash 118~121

@Ocramius Ocramius and 1 other commented on an outdated diff Nov 12, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+ * @return string
+ */
+ protected function getNamespaceId($id)
+ {
+ return sprintf("%s[%s][%s]", $this->namespace, $id, $this->getNamespaceVersion());
+ }
+
+ /**
+ * Returns the cache key which contains the namespace's version
+ *
+ * @param string $namespace
+ * @return string
+ */
+ protected function getNamespaceCacheKey($namespace)
+ {
+ return sprintf(self::NAMESPACE_CACHE_KEY, $namespace);
@Ocramius

Ocramius Nov 12, 2012

Owner

I'd prefer static::NAMESPACE_CACHE_KEY here to allow overriding the namespace key. May be interesting to be able to set it (static or instance property).

@Ocramius Ocramius commented on the diff Nov 12, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+ * @param string $namespace
+ * @return string
+ */
+ protected function getNamespaceCacheKey($namespace)
+ {
+ return sprintf(self::NAMESPACE_CACHE_KEY, $namespace);
+ }
+
+ protected function getNamespaceVersion()
+ {
+ if (null !== $this->namespaceVersion) {
+ return $this->namespaceVersion;
+ }
+
+ $namespaceCacheKey = $this->getNamespaceCacheKey($this->namespace);
+ $namespaceVersion = $this->cache->fetch($namespaceCacheKey);
@Ocramius

Ocramius Nov 12, 2012

Owner

You are assuming here that $namespaceVersion is an integer, right? Maybe a cast is required.

@stof stof and 1 other commented on an outdated diff Nov 21, 2012

lib/Doctrine/Common/Cache/CacheNamespace.php
+class CacheNamespace implements Cache
+{
+ /** @var \Doctrine\Common\Cache\Cache */
+ protected $cache;
+
+ /**
+ * Prefix for all cache keys
+ * @var string
+ */
+ protected $namespace;
+
+ /** @var int Current Namespace version */
+ protected $namespaceVersion;
+
+ /** @const Key to store the namespace's version */
+ const NAMESPACE_CACHE_KEY = "DoctrineCacheNamespaceVersion[%s]";
@stof

stof Nov 21, 2012

Member

constant should be declared before properties

@FabioBatSilva FabioBatSilva and 1 other commented on an outdated diff Nov 21, 2012

tests/Doctrine/Tests/Common/Cache/CacheNamespaceTest.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace CHH\Silex\Test;
@FabioBatSilva

FabioBatSilva Nov 21, 2012

Owner

Wrong namespace

@CHH

CHH Nov 21, 2012

Fixed. Thanks!

CHH commented Jan 11, 2013

I'm closing the PR here and make another one for the new doctrine/cache repository.

@CHH CHH closed this Jan 11, 2013

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