Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Make getNamespacedId protected instead of private #196

Closed
wants to merge 24 commits into from

8 participants

@jnonon

We want to extend the cache class provider MemcacheCache.php in order to set a custom namespace. However, because getNamespacedId is a private method, basically a complete rewrite and code duplication of the class is needed.
Is there any reason for this method to be private instead of protected?
Thanks in advace,

--JN

beberlei and others added some commits
@beberlei beberlei Release 2.2.0-RC5 0548a9c
@beberlei beberlei Bump dev version to 2.2.1 0281ab9
@beberlei beberlei Fix version number and pointer to build-commons. 195445d
@beberlei beberlei Bump dev version to 2.2.1 acba5b4
@beberlei beberlei Release 2.2.0 84838bb
@FabioBatSilva FabioBatSilva Fix DDC-1660 c4f463d
@FabioBatSilva FabioBatSilva ignore target if is not valid and ignores is true 962df0d
@FabioBatSilva FabioBatSilva test property and method c1e4dd3
@FabioBatSilva FabioBatSilva assert class exists 6eebb92
@FabioBatSilva FabioBatSilva workaround : turn off target validation on SimpleAnnotationReader db42e45
@beberlei beberlei Merge branch 'DDC-1660' into 2.2 c9089a6
@beberlei beberlei Release 2.2.1 6c7f9cf
@beberlei beberlei Bump dev version to 2.2.2 4a1d5e6
@cowboy129 cowboy129 fix memcache expire bug
memcache will expire immediately if expire argument is 0
5cda579
@cowboy129 cowboy129 Add test case for memcache expire bug.
After save cache, sleep 1 second and check if cache hits.
ef7891b
@beberlei beberlei Merge branch 'DCOM-98' into 2.2 8f977f5
@beberlei beberlei Release 2.2.2 14eb4d6
@beberlei beberlei Bump dev version to 2.2.3 1e0aa60
@stof stof Added the support of namespace alias in getManagerForClass 0e073d6
@beberlei beberlei Merge remote-tracking branch 'origin/2.2' into 2.2 b846809
@till till Merge AnnotationReader list 7494531
@beberlei beberlei Bump dev version to 2.2.4 d14fd32
@beberlei beberlei Release 2.2.3 9e13fac
@jnonon jnonon Update lib/Doctrine/Common/Cache/CacheProvider.php
Devs,

Is there any reason to have the method getNamespacedId as private instead of protected? 
Right now, if we need to make a modification to the namespace configuration, is basically a complete rewrite of the extended class in order to get it done.
Maybe I am missing something...
b71b6b8
@doctrinebot
Collaborator

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-119

@stof
Collaborator

Please use the master branch as base of your patch, not the 2.2 branch

@jnonon

Will do. Thanks.

@jnonon jnonon closed this
@aequasi

Where did this go?

@aequasi aequasi referenced this pull request in doctrine/cache
Closed

Allowing more extension of the Cache Provider #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 16, 2012
  1. @beberlei

    Release 2.2.0-RC5

    beberlei authored
  2. @beberlei

    Bump dev version to 2.2.1

    beberlei authored
Commits on Jan 29, 2012
  1. @beberlei
  2. @beberlei

    Bump dev version to 2.2.1

    beberlei authored
  3. @beberlei

    Release 2.2.0

    beberlei authored
Commits on Mar 3, 2012
  1. @FabioBatSilva @beberlei

    Fix DDC-1660

    FabioBatSilva authored beberlei committed
  2. @FabioBatSilva @beberlei

    ignore target if is not valid and ignores is true

    FabioBatSilva authored beberlei committed
  3. @FabioBatSilva @beberlei

    test property and method

    FabioBatSilva authored beberlei committed
  4. @FabioBatSilva @beberlei

    assert class exists

    FabioBatSilva authored beberlei committed
  5. @FabioBatSilva @beberlei
  6. @beberlei
  7. @beberlei

    Release 2.2.1

    beberlei authored
  8. @beberlei

    Bump dev version to 2.2.2

    beberlei authored
Commits on Mar 30, 2012
  1. @cowboy129 @beberlei

    fix memcache expire bug

    cowboy129 authored beberlei committed
    memcache will expire immediately if expire argument is 0
  2. @cowboy129 @beberlei

    Add test case for memcache expire bug.

    cowboy129 authored beberlei committed
    After save cache, sleep 1 second and check if cache hits.
  3. @beberlei
Commits on Apr 13, 2012
  1. @beberlei

    Release 2.2.2

    beberlei authored
  2. @beberlei

    Bump dev version to 2.2.3

    beberlei authored
Commits on Jul 29, 2012
  1. @stof @beberlei
  2. @beberlei
Commits on Aug 29, 2012
  1. @till @beberlei

    Merge AnnotationReader list

    till authored beberlei committed
  2. @beberlei

    Bump dev version to 2.2.4

    beberlei authored
  3. @beberlei

    Release 2.2.3

    beberlei authored
Commits on Oct 2, 2012
  1. @jnonon

    Update lib/Doctrine/Common/Cache/CacheProvider.php

    jnonon authored
    Devs,
    
    Is there any reason to have the method getNamespacedId as private instead of protected? 
    Right now, if we need to make a modification to the namespace configuration, is basically a complete rewrite of the extended class in order to get it done.
    Maybe I am missing something...
This page is out of date. Refresh to see the latest.
View
2  lib/Doctrine/Common/Annotations/AnnotationReader.php
@@ -68,6 +68,8 @@
'codeCoverageIgnore' => true, 'codeCoverageIgnoreStart' => true, 'codeCoverageIgnoreEnd' => true,
'Required' => true, 'Attribute' => true, 'Attributes' => true,
'Target' => true, 'SuppressWarnings' => true,
+ 'ingroup' => true, 'code' => true, 'endcode' => true,
+ 'package_version' => true,
);
/**
View
3  lib/Doctrine/Common/Annotations/SimpleAnnotationReader.php
@@ -68,7 +68,6 @@ public function addNamespace($namespace)
*/
public function getClassAnnotations(\ReflectionClass $class)
{
- $this->parser->setTarget(Target::TARGET_CLASS);
return $this->parser->parse($class->getDocComment(), 'class '.$class->getName());
}
@@ -81,7 +80,6 @@ public function getClassAnnotations(\ReflectionClass $class)
*/
public function getMethodAnnotations(\ReflectionMethod $method)
{
- $this->parser->setTarget(Target::TARGET_METHOD);
return $this->parser->parse($method->getDocComment(), 'method '.$method->getDeclaringClass()->name.'::'.$method->getName().'()');
}
@@ -94,7 +92,6 @@ public function getMethodAnnotations(\ReflectionMethod $method)
*/
public function getPropertyAnnotations(\ReflectionProperty $property)
{
- $this->parser->setTarget(Target::TARGET_PROPERTY);
return $this->parser->parse($property->getDocComment(), 'property '.$property->getDeclaringClass()->name.'::$'.$property->getName());
}
View
2  lib/Doctrine/Common/Cache/CacheProvider.php
@@ -129,7 +129,7 @@ public function deleteAll()
* @param string $id The id to namespace
* @return string $id The namespaced id
*/
- private function getNamespacedId($id)
+ protected function getNamespacedId($id)
{
$namespaceCacheKey = sprintf(self::DOCTRINE_NAMESPACE_CACHEKEY, $this->namespace);
$namespaceVersion = ($this->doContains($namespaceCacheKey)) ? $this->doFetch($namespaceCacheKey) : 1;
View
5 lib/Doctrine/Common/Cache/MemcacheCache.php
@@ -82,6 +82,9 @@ protected function doContains($id)
*/
protected function doSave($id, $data, $lifeTime = 0)
{
+ if ($lifeTime > 30 * 24 * 3600) {
+ $lifeTime = time() + $lifeTime;
+ }
return $this->memcache->set($id, $data, 0, (int) $lifeTime);
}
@@ -115,4 +118,4 @@ protected function doGetStats()
Cache::STATS_MEMORY_AVAILIABLE => $stats['limit_maxbytes'],
);
}
-}
+}
View
5 lib/Doctrine/Common/Cache/MemcachedCache.php
@@ -82,6 +82,9 @@ protected function doContains($id)
*/
protected function doSave($id, $data, $lifeTime = 0)
{
+ if ($lifeTime > 30 * 24 * 3600) {
+ $lifeTime = time() + $lifeTime;
+ }
return $this->memcached->set($id, $data, (int) $lifeTime);
}
@@ -118,4 +121,4 @@ protected function doGetStats()
Cache::STATS_MEMORY_AVAILIABLE => $stats['limit_maxbytes'],
);
}
-}
+}
View
6 lib/Doctrine/Common/Persistence/AbstractManagerRegistry.php
@@ -155,6 +155,12 @@ public function getManager($name = null)
*/
public function getManagerForClass($class)
{
+ // Check for namespace alias
+ if (strpos($class, ':') !== false) {
+ list($namespaceAlias, $simpleClassName) = explode(':', $class);
+ $class = $this->getAliasNamespace($namespaceAlias) . '\\' . $simpleClassName;
+ }
+
$proxyClass = new \ReflectionClass($class);
if ($proxyClass->implementsInterface($this->proxyInterfaceName)) {
$class = $proxyClass->getParentClass()->getName();
View
2  lib/Doctrine/Common/Version.php
@@ -36,7 +36,7 @@ class Version
/**
* Current Doctrine Version
*/
- const VERSION = '2.2.0-DEV';
+ const VERSION = '2.2.4';
/**
* Compares a Doctrine version with the current one.
View
15 tests/Doctrine/Tests/Common/Annotations/AbstractReaderTest.php
@@ -311,6 +311,21 @@ public function testInvalidAnnotationUsageButIgnoredClass()
$this->assertEquals(2, count($annots));
}
+ /**
+ * @group DDC-1660
+ * @group regression
+ */
+ public function testInvalidAnnotationButIgnored()
+ {
+ $reader = $this->getReader();
+ $class = new \ReflectionClass('Doctrine\Tests\Common\Annotations\Fixtures\ClassDDC1660');
+
+ $this->assertTrue(class_exists('Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Version'));
+ $this->assertCount(0, $reader->getClassAnnotations($class));
+ $this->assertCount(0, $reader->getMethodAnnotations($class->getMethod('bar')));
+ $this->assertCount(0, $reader->getPropertyAnnotations($class->getProperty('foo')));
+ }
+
abstract protected function getReader();
}
View
11 tests/Doctrine/Tests/Common/Annotations/Fixtures/Annotation/Version.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures\Annotation;
+
+/**
+ * @Annotation
+ * @Target("PROPERTY")
+ */
+final class Version
+{
+}
View
30 tests/Doctrine/Tests/Common/Annotations/Fixtures/ClassDDC1660.php
@@ -0,0 +1,30 @@
+<?php
+
+namespace Doctrine\Tests\Common\Annotations\Fixtures;
+
+/**
+ * @since 2.0
+ * @version $Id: SomeEntityClass.php 509 2012-02-03 09:38:48Z mf $
+ */
+class ClassDDC1660
+{
+
+ /**
+ * @var string
+ * @since 2.0
+ * @version 1
+ */
+ public $foo;
+
+ /**
+ * @param string
+ * @return string
+ * @since 2.0
+ * @version 1
+ */
+ public function bar($param)
+ {
+ return null;
+ }
+
+}
View
54 tests/Doctrine/Tests/Common/Annotations/SimpleAnnotationReaderTest.php
@@ -25,6 +25,42 @@ public function testImportDetectsNonExistentAnnotation()
}
/**
+ * Contrary to the behavior of the default annotation reader, we do just ignore
+ * these in the simple annotation reader (so, no expected exception here).
+ */
+ public function testClassWithInvalidAnnotationTargetAtClassDocBlock()
+ {
+ parent::testClassWithInvalidAnnotationTargetAtClassDocBlock();
+ }
+
+ /**
+ * Contrary to the behavior of the default annotation reader, we do just ignore
+ * these in the simple annotation reader (so, no expected exception here).
+ */
+ public function testClassWithInvalidAnnotationTargetAtPropertyDocBlock()
+ {
+ parent::testClassWithInvalidAnnotationTargetAtPropertyDocBlock();
+ }
+
+ /**
+ * Contrary to the behavior of the default annotation reader, we do just ignore
+ * these in the simple annotation reader (so, no expected exception here).
+ */
+ public function testClassWithInvalidNestedAnnotationTargetAtPropertyDocBlock()
+ {
+ parent::testClassWithInvalidNestedAnnotationTargetAtPropertyDocBlock();
+ }
+
+ /**
+ * Contrary to the behavior of the default annotation reader, we do just ignore
+ * these in the simple annotation reader (so, no expected exception here).
+ */
+ public function testClassWithInvalidAnnotationTargetAtMethodDocBlock()
+ {
+ parent::testClassWithInvalidAnnotationTargetAtMethodDocBlock();
+ }
+
+ /**
* @expectedException Doctrine\Common\Annotations\AnnotationException
*/
public function testInvalidAnnotationUsageButIgnoredClass()
@@ -32,11 +68,29 @@ public function testInvalidAnnotationUsageButIgnoredClass()
parent::testInvalidAnnotationUsageButIgnoredClass();
}
+ /**
+ * @group DDC-1660
+ * @group regression
+ *
+ * Contrary to the behavior of the default annotation reader, @version is not ignored
+ */
+ public function testInvalidAnnotationButIgnored()
+ {
+ $reader = $this->getReader();
+ $class = new \ReflectionClass('Doctrine\Tests\Common\Annotations\Fixtures\ClassDDC1660');
+
+ $this->assertTrue(class_exists('Doctrine\Tests\Common\Annotations\Fixtures\Annotation\Version'));
+ $this->assertCount(1, $reader->getClassAnnotations($class));
+ $this->assertCount(1, $reader->getMethodAnnotations($class->getMethod('bar')));
+ $this->assertCount(1, $reader->getPropertyAnnotations($class->getProperty('foo')));
+ }
+
protected function getReader()
{
$reader = new SimpleAnnotationReader();
$reader->addNamespace(__NAMESPACE__);
$reader->addNamespace(__NAMESPACE__ . '\Fixtures');
+ $reader->addNamespace(__NAMESPACE__ . '\Fixtures\Annotation');
return $reader;
}
View
8 tests/Doctrine/Tests/Common/Cache/MemcacheCacheTest.php
@@ -21,10 +21,18 @@ public function setUp()
}
}
+ public function testNoExpire() {
+ $cache = $this->_getCacheDriver();
+ $cache->save('noexpire', 'value', 0);
+ sleep(1);
+ $this->assertTrue($cache->contains('noexpire'), 'Memcache provider should support no-expire');
+ }
+
protected function _getCacheDriver()
{
$driver = new MemcacheCache();
$driver->setMemcache($this->_memcache);
return $driver;
}
+
}
View
7 tests/Doctrine/Tests/Common/Cache/MemcachedCacheTest.php
@@ -24,6 +24,13 @@ public function setUp()
}
}
+ public function testNoExpire() {
+ $cache = $this->_getCacheDriver();
+ $cache->save('noexpire', 'value', 0);
+ sleep(1);
+ $this->assertTrue($cache->contains('noexpire'), 'Memcache provider should support no-expire');
+ }
+
protected function _getCacheDriver()
{
$driver = new MemcachedCache();
Something went wrong with that request. Please try again.