From 58cd520e323d149cb5eae75c10c072ea586b06f1 Mon Sep 17 00:00:00 2001
From: Alexander Kurilo <alex@kurilo.me>
Date: Thu, 26 Jun 2014 19:46:45 +0300
Subject: [PATCH 1/4] Fix attempt of traversing bool in FileLockRegion
---
lib/Doctrine/ORM/Cache/Region/FileLockRegion.php | 9 +++++++--
.../Tools/Export/AbstractClassMetadataExporterTest.php | 6 ++++--
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
index 7e20d5f73..5994e40cf 100644
--- a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
+++ b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
@@ -201,8 +201,13 @@ class FileLockRegion implements ConcurrentRegion
*/
public function evictAll()
{
- foreach (glob(sprintf("%s/*.%s" , $this->directory, self::LOCK_EXTENSION)) as $filename) {
- @unlink($filename);
+ // The check below is necessary because on some platforms glob returns false
+ // when nothing matched (even though no errors occurred)
+ $filenames = glob(sprintf("%s/*.%s" , $this->directory, self::LOCK_EXTENSION));
+ if (is_array($filenames)) {
+ foreach ($filenames as $filename) {
+ @unlink($filename);
+ }
}
return $this->region->evictAll();
diff --git a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php
index 3ad2c88f1..7c3c33c6a 100644
--- a/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php
+++ b/tests/Doctrine/Tests/ORM/Tools/Export/AbstractClassMetadataExporterTest.php
@@ -393,8 +393,10 @@ abstract class AbstractClassMetadataExporterTest extends \Doctrine\Tests\OrmTest
return unlink($path);
} else if (is_dir($path)) {
$files = glob(rtrim($path,'/').'/*');
- foreach ($files as $file){
- $this->_deleteDirectory($file);
+ if (is_array($files)) {
+ foreach ($files as $file){
+ $this->_deleteDirectory($file);
+ }
}
return rmdir($path);
}
From aca719be4123623d8cbcd1892f367f633b4ec2c4 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Thu, 15 Jan 2015 00:36:26 +0100
Subject: [PATCH 2/4] #1072 DDC-3191 - adding test for failing `glob()`
operations on the `FileLockRegion`
---
.../Tests/ORM/Cache/FileLockRegionTest.php | 60 ++++++++++++++++---
1 file changed, 51 insertions(+), 9 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
index e699bb39f..51fb9e34d 100644
--- a/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
+++ b/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
@@ -27,17 +27,12 @@ class FileLockRegionTest extends AbstractRegionTest
*/
protected $directory;
+ /**
+ * {@inheritDoc}
+ */
public function tearDown()
{
- if ( ! is_dir($this->directory)) {
- return;
- }
-
- foreach (new RecursiveIteratorIterator(new RecursiveDirectoryIterator($this->directory), RecursiveIteratorIterator::CHILD_FIRST) as $file) {
- $file->isFile()
- ? @unlink($file->getRealPath())
- : @rmdir($file->getRealPath());
- }
+ $this->cleanTestDirectory($this->directory);
}
/**
@@ -247,4 +242,51 @@ class FileLockRegionTest extends AbstractRegionTest
$this->assertNotNull($this->region->get($key));
$this->assertFileNotExists($file);
}
+
+ /**
+ * @group 1072
+ * @group DDC-3191
+ *
+ * Note that this test will only fail on some OSs. Attempts to write this test using `open_basedir` failed
+ * due to the inability to restore `open_basedir` on cleanup
+ */
+ public function testHandlesScanErrorsGracefullyOnEvictAll()
+ {
+ $baseDir = sys_get_temp_dir() . '/doctrine_lock_'. uniqid();
+ $subDir = $baseDir . '/foo/bar';
+ $region = new FileLockRegion(new DefaultRegion('concurren_region_test', $this->cache), $subDir, 60);
+
+ $this->cleanTestDirectory($baseDir);
+
+ rmdir($baseDir);
+
+ $region->evictAll();
+
+ $this->assertFalse(is_dir($baseDir));
+ }
+
+ /**
+ * @param string|null $path directory to clean
+ */
+ private function cleanTestDirectory($path)
+ {
+ $path = $path ?: $this->directory;
+
+ if ( ! is_dir($path)) {
+ return;
+ }
+
+ $directoryIterator = new RecursiveIteratorIterator(
+ new RecursiveDirectoryIterator($path),
+ RecursiveIteratorIterator::CHILD_FIRST
+ );
+
+ foreach ($directoryIterator as $file) {
+ if ($file->isFile()) {
+ @unlink($file->getRealPath());
+ } else {
+ @rmdir($file->getRealPath());
+ }
+ }
+ }
}
\ No newline at end of file
From e2acd74cb4d894843845f1f412a93beb3747efa2 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Thu, 15 Jan 2015 00:37:41 +0100
Subject: [PATCH 3/4] #1072 DDC-3191 - minor performance optimization
---
lib/Doctrine/ORM/Cache/Region/FileLockRegion.php | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
index 5994e40cf..083329c55 100644
--- a/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
+++ b/lib/Doctrine/ORM/Cache/Region/FileLockRegion.php
@@ -204,7 +204,8 @@ class FileLockRegion implements ConcurrentRegion
// The check below is necessary because on some platforms glob returns false
// when nothing matched (even though no errors occurred)
$filenames = glob(sprintf("%s/*.%s" , $this->directory, self::LOCK_EXTENSION));
- if (is_array($filenames)) {
+
+ if ($filenames) {
foreach ($filenames as $filename) {
@unlink($filename);
}
From 8b223c5c83ee3d557c2c9554411b7698f7255b43 Mon Sep 17 00:00:00 2001
From: Marco Pivetta <ocramius@gmail.com>
Date: Thu, 15 Jan 2015 00:54:25 +0100
Subject: [PATCH 4/4] #1072 DDC-3191 - test cleanup: reflection is better than
mixing up I/O operations and global state
---
.../Tests/ORM/Cache/FileLockRegionTest.php | 21 +++++++------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
index 51fb9e34d..6fdef6fea 100644
--- a/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
+++ b/tests/Doctrine/Tests/ORM/Cache/FileLockRegionTest.php
@@ -246,23 +246,18 @@ class FileLockRegionTest extends AbstractRegionTest
/**
* @group 1072
* @group DDC-3191
- *
- * Note that this test will only fail on some OSs. Attempts to write this test using `open_basedir` failed
- * due to the inability to restore `open_basedir` on cleanup
*/
public function testHandlesScanErrorsGracefullyOnEvictAll()
{
- $baseDir = sys_get_temp_dir() . '/doctrine_lock_'. uniqid();
- $subDir = $baseDir . '/foo/bar';
- $region = new FileLockRegion(new DefaultRegion('concurren_region_test', $this->cache), $subDir, 60);
+ $region = $this->createRegion();
+ $reflectionDirectory = new \ReflectionProperty($region, 'directory');
- $this->cleanTestDirectory($baseDir);
+ $reflectionDirectory->setAccessible(true);
+ $reflectionDirectory->setValue($region, str_repeat('a', 10000));
- rmdir($baseDir);
-
- $region->evictAll();
-
- $this->assertFalse(is_dir($baseDir));
+ set_error_handler(function () {}, E_WARNING);
+ $this->assertTrue($region->evictAll());
+ restore_error_handler();
}
/**
@@ -289,4 +284,4 @@ class FileLockRegionTest extends AbstractRegionTest
}
}
}
-}
\ No newline at end of file
+}