From e7b62ce524028dc1b1c754a9ae0993a541aa5085 Mon Sep 17 00:00:00 2001 From: kasimi Date: Fri, 6 Oct 2017 12:44:54 +0200 Subject: [PATCH 1/3] Don't generate bogus warning message when importing empty yml file --- src/Files/Type/YmlFile.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Files/Type/YmlFile.php b/src/Files/Type/YmlFile.php index 4f4c2c6..6142a2f 100644 --- a/src/Files/Type/YmlFile.php +++ b/src/Files/Type/YmlFile.php @@ -27,7 +27,7 @@ public function __construct($debug, $filename, $rundir) try { - $content = Yaml::parse($this->fileData); + $content = (array) Yaml::parse($this->fileData); // Look for imports if (isset($content['imports']) && is_array($content['imports'])) From b453f68b3a16e00db7d4e49ae92590c638b669ea Mon Sep 17 00:00:00 2001 From: kasimi Date: Sat, 7 Oct 2017 13:05:45 +0200 Subject: [PATCH 2/3] Throw exception if yml file is empty --- src/Files/Type/YmlFile.php | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/Files/Type/YmlFile.php b/src/Files/Type/YmlFile.php index 6142a2f..43aca37 100644 --- a/src/Files/Type/YmlFile.php +++ b/src/Files/Type/YmlFile.php @@ -27,7 +27,12 @@ public function __construct($debug, $filename, $rundir) try { - $content = (array) Yaml::parse($this->fileData); + $content = Yaml::parse($this->fileData); + + if (!is_array($content)) + { + throw new ParseException("Empty file"); + } // Look for imports if (isset($content['imports']) && is_array($content['imports'])) @@ -40,9 +45,18 @@ public function __construct($debug, $filename, $rundir) { if (isset($import['resource'])) { - $importYmlFileName = $dirname . '/' . $import['resource']; - $importYmlFile = new YmlFile($debug, $importYmlFileName, $rundir); - $extraContent = $importYmlFile->getYaml(); + try + { + $importYmlFileName = $dirname . '/' . $import['resource']; + $importYmlFile = new YmlFile($debug, $importYmlFileName, $rundir); + $extraContent = $importYmlFile->getYaml(); + } + catch (FileLoadException $ex) + { + // The imported yml file will be loaded individually later. + // Let's avoid duplicate error messages here and continue with the current yml. + $extraContent = array(); + } // Imports are at the top of the yaml file, so these should be loaded first. // The values of the current yaml file will overwrite existing array values of the imports. From 741328fdc1a2b3486c825d18fd963f66f9a8f74a Mon Sep 17 00:00:00 2001 From: kasimi Date: Sat, 7 Oct 2017 13:06:09 +0200 Subject: [PATCH 3/3] Test valid/invalid/empty yml files --- tests/file_loader_test.php | 63 +++++++++++++++++++++++------- tests/testFiles/empty.yml | 0 tests/testFiles/empty_import.yml | 6 +++ tests/testFiles/invalid.yml | 2 + tests/testFiles/invalid_import.yml | 6 +++ tests/testFiles/valid.yml | 3 ++ 6 files changed, 65 insertions(+), 15 deletions(-) create mode 100644 tests/testFiles/empty.yml create mode 100644 tests/testFiles/empty_import.yml create mode 100644 tests/testFiles/invalid.yml create mode 100644 tests/testFiles/invalid_import.yml create mode 100644 tests/testFiles/valid.yml diff --git a/tests/file_loader_test.php b/tests/file_loader_test.php index 7269a2f..83033de 100644 --- a/tests/file_loader_test.php +++ b/tests/file_loader_test.php @@ -8,29 +8,62 @@ * */ +use Phpbb\Epv\Files\FileLoader; +use Phpbb\Epv\Files\Type\LangFile; +use Phpbb\Epv\Files\Type\MigrationFile; +use Phpbb\Epv\Files\Type\PHPFile; +use Phpbb\Epv\Files\Type\PHPFileInterface; +use Phpbb\Epv\Files\Type\YmlFile; +use Phpbb\Epv\Tests\Mock\Output; + class file_loader_test extends PHPUnit_Framework_TestCase { + + /** @var FileLoader */ + private static $loader; + public static function setUpBeforeClass() { require_once('./tests/Mock/Output.php'); + + static::$loader = new FileLoader(new Output(), false, 'tests/testFiles/', '.'); } - private function getLoader() + public function test_file_php() { + + $type = static::$loader->loadFile('tests/testFiles/test.txt.php'); + $typePhp = static::$loader->loadFile('tests/testFiles/test.php'); + $typeMigration = static::$loader->loadFile('tests/testFiles/migrations/test.php'); + + $this->assertTrue($type instanceof PHPFile); + $this->assertTrue($typePhp instanceof PHPFile); + $this->assertFalse($typePhp instanceof MigrationFile); + $this->assertFalse($typePhp instanceof LangFile); + $this->assertTrue($typeMigration instanceof PHPFileInterface); // It extends from the interface! + $this->assertTrue($typeMigration instanceof MigrationFile, 'type is migration file'); + } + + public function test_file_yml() { - return $file = new \Phpbb\Epv\Files\FileLoader(new \Phpbb\Epv\Tests\Mock\Output(), false, 'tests/testFiles/', '.'); + $validYml = static::$loader->loadFile('tests/testFiles/valid.yml'); + $invalidImportYml = static::$loader->loadFile('tests/testFiles/invalid_import.yml'); + $emptyImportYml = static::$loader->loadFile('tests/testFiles/empty_import.yml'); + + $this->assertTrue($validYml instanceof YmlFile); + $this->assertTrue($invalidImportYml instanceof YmlFile); + $this->assertTrue($emptyImportYml instanceof YmlFile); } - public function test_file_php() { - $file = $this->getLoader(); - - $type = $file->loadFile('tests/testFiles/test.txt.php'); - $typePhp = $file->loadFile('tests/testFiles/test.php'); - $typeMigration = $file->loadFile('tests/testFiles/migrations/test.php'); - - $this->assertTrue($type instanceof \Phpbb\Epv\Files\Type\PHPFile); - $this->assertTrue($typePhp instanceof \Phpbb\Epv\Files\Type\PHPFile); - $this->assertFalse($typePhp instanceof \Phpbb\Epv\Files\Type\MigrationFile); - $this->assertFalse($typePhp instanceof \Phpbb\Epv\Files\Type\LangFile); - $this->assertTrue($typeMigration instanceof \Phpbb\Epv\Files\Type\PHPFileInterface); // It extends from the interface! - $this->assertTrue($typeMigration instanceof \Phpbb\Epv\Files\Type\MigrationFile, 'type is migration file'); + public function test_file_invalid_yml() + { + $this->setExpectedException(Exception::class); + $invalidYml = static::$loader->loadFile('tests/testFiles/invalid.yml'); + $this->assertNull($invalidYml); + } + + public function test_file_empty_yml() + { + $this->setExpectedException(Exception::class); + $emptyYml = static::$loader->loadFile('tests/testFiles/empty.yml'); + $this->assertNull($emptyYml); } } diff --git a/tests/testFiles/empty.yml b/tests/testFiles/empty.yml new file mode 100644 index 0000000..e69de29 diff --git a/tests/testFiles/empty_import.yml b/tests/testFiles/empty_import.yml new file mode 100644 index 0000000..275232c --- /dev/null +++ b/tests/testFiles/empty_import.yml @@ -0,0 +1,6 @@ +imports: + - { resource: empty.yml } + +services: + some.service.name: + class: a\b\c diff --git a/tests/testFiles/invalid.yml b/tests/testFiles/invalid.yml new file mode 100644 index 0000000..a2f2665 --- /dev/null +++ b/tests/testFiles/invalid.yml @@ -0,0 +1,2 @@ +services: + some. diff --git a/tests/testFiles/invalid_import.yml b/tests/testFiles/invalid_import.yml new file mode 100644 index 0000000..c171877 --- /dev/null +++ b/tests/testFiles/invalid_import.yml @@ -0,0 +1,6 @@ +imports: + - { resource: invalid.yml } + +services: + some.service.name: + class: a\b\c diff --git a/tests/testFiles/valid.yml b/tests/testFiles/valid.yml new file mode 100644 index 0000000..36c176a --- /dev/null +++ b/tests/testFiles/valid.yml @@ -0,0 +1,3 @@ +services: + some.service.name: + class: a\b\c