diff options
| -rw-r--r-- | appinfo/application.php | 2 | ||||
| -rw-r--r-- | build.xml | 15 | ||||
| -rw-r--r-- | codeception.yml | 1 | ||||
| -rw-r--r-- | config/configexception.php | 28 | ||||
| -rw-r--r-- | config/configparser.php (renamed from service/configparser.php) | 92 | ||||
| -rw-r--r-- | phpdoc.xml | 2 | ||||
| -rw-r--r-- | service/configservice.php | 68 | ||||
| -rw-r--r-- | tests/api/GetConfigCest.php | 2 | ||||
| -rw-r--r-- | tests/unit/GalleryUnitTest.php | 10 | ||||
| -rw-r--r-- | tests/unit/config/ConfigParserTest.php | 211 | ||||
| -rw-r--r-- | tests/unit/service/ConfigServiceTest.php | 8 |
11 files changed, 335 insertions, 104 deletions
diff --git a/appinfo/application.php b/appinfo/application.php index a2859daf..c212a372 100644 --- a/appinfo/application.php +++ b/appinfo/application.php @@ -25,6 +25,7 @@ use OCP\IContainer; use OCP\AppFramework\App; use OCP\AppFramework\IAppContainer; +use OCA\Gallery\Config\ConfigParser; use OCA\Gallery\Controller\PageController; use OCA\Gallery\Controller\ConfigController; use OCA\Gallery\Controller\ConfigPublicController; @@ -39,7 +40,6 @@ use OCA\Gallery\Environment\Environment; use OCA\Gallery\Preview\Preview; use OCA\Gallery\Service\SearchFolderService; use OCA\Gallery\Service\ConfigService; -use OCA\Gallery\Service\ConfigParser; use OCA\Gallery\Service\SearchMediaService; use OCA\Gallery\Service\ThumbnailService; use OCA\Gallery\Service\PreviewService; diff --git a/build.xml b/build.xml deleted file mode 100644 index 85a0fb55..00000000 --- a/build.xml +++ /dev/null @@ -1,15 +0,0 @@ -<project name="owncloud-firstrunwizard" basedir="." default="test"> - - <!-- test - Tests if the code syntax is correct and executes phpunit tests --> - <target name="test"> - <apply executable="php" failonerror="true"> - <arg value="-l" /> - <fileset dir="."> - <include name="**/*.php" /> - <exclude name="**/l10n/**" /> - <exclude name="**/vendor/**" /> - </fileset> - </apply> - </target> - -</project> diff --git a/codeception.yml b/codeception.yml index b43d5ef3..cfa3519b 100644 --- a/codeception.yml +++ b/codeception.yml @@ -23,6 +23,7 @@ coverage: enabled: true include: - appinfo/*.php + - config/* - controller/* - environment/* - http/* diff --git a/config/configexception.php b/config/configexception.php new file mode 100644 index 00000000..d421f70f --- /dev/null +++ b/config/configexception.php @@ -0,0 +1,28 @@ +<?php +/** + * ownCloud - gallery + * + * This file is licensed under the Affero General Public License version 3 or + * later. See the COPYING file. + * + * @author Olivier Paroz <owncloud@interfasys.ch> + * + * @copyright Olivier Paroz 2015 + */ + +namespace OCA\Gallery\Config; + +/** + * Thrown when the configuration parser cannot parse a file + */ +class ConfigException extends \Exception { + + /** + * Constructor + * + * @param string $msg the message contained in the exception + */ + public function __construct($msg) { + parent::__construct($msg); + } +} diff --git a/service/configparser.php b/config/configparser.php index 81cd150c..2d81210e 100644 --- a/service/configparser.php +++ b/config/configparser.php @@ -10,9 +10,10 @@ * @copyright Olivier Paroz 2015 */ -namespace OCA\Gallery\Service; +namespace OCA\Gallery\Config; -use Symfony\Component\Yaml\Yaml; +use Symfony\Component\Yaml\Parser; +use Symfony\Component\Yaml\Exception\ParseException; use OCP\Files\Folder; use OCP\Files\File; @@ -20,15 +21,15 @@ use OCP\Files\File; /** * Parses configuration files * - * @package OCA\Gallery\Service + * @package OCA\Gallery\Config */ class ConfigParser { /** * Returns a parsed global configuration if one was found in the root folder * - * @param Folder $folder - * @param string $configName + * @param Folder $folder the current folder + * @param string $configName name of the configuration file * * @return null|array */ @@ -36,7 +37,7 @@ class ConfigParser { $featuresList = []; $parsedConfig = $this->parseConfig($folder, $configName); $key = 'features'; - if (array_key_exists('features', $parsedConfig)) { + if (array_key_exists($key, $parsedConfig)) { $featuresList = $parsedConfig[$key]; } @@ -46,31 +47,34 @@ class ConfigParser { /** * Returns a parsed configuration if one was found in the current folder * - * @param Folder $folder - * @param string $configName - * @param array $currentConfig - * @param array <string,bool> $configItems - * @param int $level + * @param Folder $folder the current folder + * @param string $configName name of the configuration file + * @param array $currentConfig the configuration collected so far + * @param array <string,bool> $completionStatus determines if we already have all we need for a + * config sub-section + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder * * @return array <null|array,array<string,bool>> + * @throws ConfigException */ - public function getFolderConfig($folder, $configName, $currentConfig, $configItems, $level) { + public function getFolderConfig($folder, $configName, $currentConfig, $completionStatus, $level + ) { $parsedConfig = $this->parseConfig($folder, $configName); - list($config, $configItems) = - $this->buildAlbumConfig($currentConfig, $parsedConfig, $configItems, $level); + list($config, $completionStatus) = + $this->buildAlbumConfig($currentConfig, $parsedConfig, $completionStatus, $level); - return [$config, $configItems]; + return [$config, $completionStatus]; } /** * Returns a parsed configuration * - * @param Folder $folder + * @param Folder $folder the current folder * @param string $configName * * @return array|string[] * - * @throws ServiceException + * @throws ConfigException */ private function parseConfig($folder, $configName) { try { @@ -78,15 +82,16 @@ class ConfigParser { $configFile = $folder->get($configName); $rawConfig = $configFile->getContent(); $saneConfig = $this->bomFixer($rawConfig); - $parsedConfig = Yaml::parse($saneConfig); + $yaml = new Parser(); + $parsedConfig = $yaml->parse($saneConfig); + //\OC::$server->getLogger()->debug("rawConfig : {path}", ['path' => $rawConfig]); - } catch (\Exception $exception) { - $errorMessage = "Problem while parsing the configuration file"; - throw new ServiceException($errorMessage); + return $parsedConfig; + } catch (\Exception $exception) { + $errorMessage = "Problem while reading or parsing the configuration file"; + throw new ConfigException($errorMessage); } - - return $parsedConfig; } /** @@ -110,35 +115,35 @@ class ConfigParser { /** * Returns either the local config or one merged with a config containing sorting information * - * @param array $currentConfig - * @param array $parsedConfig - * @param array <string,bool> $configItems - * @param int $level + * @param array $currentConfig the configuration collected so far + * @param array $parsedConfig the configuration collected in the current folder + * @param array <string,bool> $completionStatus determines if we already have all we need for a + * config sub-section + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder * - * @return array<null|array,array<string,bool>> + * @return array <null|array,array<string,bool>> */ - private function buildAlbumConfig($currentConfig, $parsedConfig, $configItems, $level) { - foreach ($configItems as $key => $complete) { + private function buildAlbumConfig($currentConfig, $parsedConfig, $completionStatus, $level) { + foreach ($completionStatus as $key => $complete) { if (!$this->isConfigItemComplete($key, $parsedConfig, $complete)) { $parsedConfigItem = $parsedConfig[$key]; if ($this->isConfigUsable($parsedConfigItem, $level)) { list($configItem, $itemComplete) = $this->addConfigItem($key, $parsedConfigItem, $level); $currentConfig = array_merge($currentConfig, $configItem); - $configItems[$key] = $itemComplete; + $completionStatus[$key] = $itemComplete; } - } } - return [$currentConfig, $configItems]; + return [$currentConfig, $completionStatus]; } /** * Determines if we already have everything we need for this configuration sub-section * - * @param string $key - * @param array $parsedConfig + * @param string $key the configuration sub-section identifier + * @param array $parsedConfig the configuration for that sub-section * @param bool $complete * * @return bool @@ -150,8 +155,12 @@ class ConfigParser { /** * Determines if we can use this configuration sub-section * - * @param array $parsedConfigItem - * @param int $level + * It's possible in two cases: + * * the configuration was collected from the currently opened folder + * * the configuration was collected in a parent folder and is inheritable + * + * @param array $parsedConfigItem the configuration for a sub-section + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder * * @return bool */ @@ -164,9 +173,9 @@ class ConfigParser { /** * Adds a config sub-section to the global config * - * @param string $key - * @param array $parsedConfigItem - * @param int $level + * @param string $key the configuration sub-section identifier + * @param array $parsedConfigItem the configuration for a sub-section + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder * * @return array<null|array<string,string>,bool> */ @@ -186,7 +195,7 @@ class ConfigParser { /** * Determines if we can use a configuration sub-section found in parent folders * - * @param array $parsedConfigItem + * @param array $parsedConfigItem the configuration for a sub-section * * @return bool */ @@ -201,7 +210,6 @@ class ConfigParser { } return $inherit; - } } @@ -19,11 +19,13 @@ </transformations> <files> <directory>appinfo</directory> + <directory>config</directory> <directory>controller</directory> <directory>environment</directory> <directory>http</directory> <directory>middleware</directory> <directory>preview</directory> <directory>service</directory> + <directory>utility</directory> </files> </phpdoc> diff --git a/service/configservice.php b/service/configservice.php index d7b6d376..3a881899 100644 --- a/service/configservice.php +++ b/service/configservice.php @@ -15,6 +15,8 @@ namespace OCA\Gallery\Service; use OCP\Files\Folder; use OCP\ILogger; +use OCA\Gallery\Config\ConfigParser; +use OCA\Gallery\Config\ConfigException; use OCA\Gallery\Environment\Environment; /** @@ -38,7 +40,7 @@ class ConfigService extends FilesService { /** * @var array <string,bool> */ - private $configItems = ['information' => false, 'sorting' => false]; + private $completionStatus = ['information' => false, 'sorting' => false]; /** * @var ConfigParser */ @@ -76,7 +78,7 @@ class ConfigService extends FilesService { try { $featuresList = $this->configParser->getFeaturesList($rootFolder, $this->configName); - } catch (ServiceException $exception) { + } catch (ConfigException $exception) { $featuresList = $this->buildErrorMessage($exception, $rootFolder); } } @@ -92,9 +94,9 @@ class ConfigService extends FilesService { * * permissions * * ID * - * @param Folder $folderNode - * @param string $folderPathFromRoot - * @param array $features + * @param Folder $folderNode the current folder + * @param string $folderPathFromRoot path from the current folder to the virtual root + * @param array $features the list of features retrieved fro the configuration file * * @return array|null * @throws ForbiddenServiceException @@ -121,7 +123,7 @@ class ConfigService extends FilesService { /** * Determines if we have a configuration file to work with * - * @param Folder $rootFolder + * @param Folder $rootFolder the virtual root folder * * @return bool */ @@ -135,11 +137,11 @@ class ConfigService extends FilesService { * Goes through all the parent folders until either we're told the album is private or we've * reached the root folder * - * @param Folder $folder - * @param string $privacyChecker - * @param string $configName - * @param int $level - * @param array $config + * @param Folder $folder the current folder + * @param string $privacyChecker name of the file which blacklists folders + * @param string $configName name of the configuration file + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder + * @param array $config the configuration collected so far * * @return array<null|array,bool> */ @@ -169,20 +171,20 @@ class ConfigService extends FilesService { * Returns a parsed configuration if one was found in the current folder or generates an error * message to send back * - * @param Folder $folder - * @param string $configName - * @param array $config - * @param int $level + * @param Folder $folder the current folder + * @param string $configName name of the configuration file + * @param array $config the configuration collected so far + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder * * @return array */ private function buildFolderConfig($folder, $configName, $config, $level) { try { - list($config, $configItems) = $this->configParser->getFolderConfig( - $folder, $configName, $config, $this->configItems, $level + list($config, $completionStatus) = $this->configParser->getFolderConfig( + $folder, $configName, $config, $this->completionStatus, $level ); - $this->configItems = $configItems; - } catch (ServiceException $exception) { + $this->completionStatus = $completionStatus; + } catch (ConfigException $exception) { $config = $this->buildErrorMessage($exception, $folder); } @@ -192,24 +194,24 @@ class ConfigService extends FilesService { /** * Builds the error message to send back when there is an error * - * @fixme Missing translation and should not contain HTML + * @fixme Missing translation * - * @param ServiceException $exception - * @param Folder $folder + * @param ConfigException $exception + * @param Folder $folder the current folder * * @return array<array<string,string>,bool> */ private function buildErrorMessage($exception, $folder) { $configPath = $this->environment->getPathFromVirtualRoot($folder); - $errorMessage = $exception->getMessage() . "</br></br>Config location: /$configPath"; + $errorMessage = $exception->getMessage() . ". Config location: /$configPath"; $this->logger->error($errorMessage); $config = ['error' => ['message' => $errorMessage]]; - $configItems = $this->configItems; - foreach ($configItems as $key) { - $configItems[$key] = true; + $completionStatus = $this->completionStatus; + foreach ($completionStatus as $key) { + $completionStatus[$key] = true; } - $this->configItems = $configItems; + $this->completionStatus = $completionStatus; return [$config]; } @@ -242,13 +244,13 @@ class ConfigService extends FilesService { /** * Looks for an album configuration in the parent folder * - * We will look up to the real root folder, not the virtual root of a shared folder + * We will look up to the virtual root of a shared folder, for privacy reasons * - * @param Folder $folder - * @param string $privacyChecker - * @param string $configName - * @param int $level - * @param array $config + * @param Folder $folder the current folder + * @param string $privacyChecker name of the file which blacklists folders + * @param string $configName name of the configuration file + * @param int $level the starting level is 0 and we add 1 each time we visit a parent folder + * @param array $config the configuration collected so far * * @return array<null|array,bool> */ diff --git a/tests/api/GetConfigCest.php b/tests/api/GetConfigCest.php index 8769281c..329386f9 100644 --- a/tests/api/GetConfigCest.php +++ b/tests/api/GetConfigCest.php @@ -121,7 +121,7 @@ class GetConfigCest { 'features' => [ [ 'error' => [ - 'message' => 'Problem while parsing the configuration file</br></br>Config location: /' + 'message' => 'Problem while reading or parsing the configuration file. Config location: /' ] ] diff --git a/tests/unit/GalleryUnitTest.php b/tests/unit/GalleryUnitTest.php index 6595a1f4..b506e3f5 100644 --- a/tests/unit/GalleryUnitTest.php +++ b/tests/unit/GalleryUnitTest.php @@ -147,8 +147,6 @@ abstract class GalleryUnitTest extends \Test\TestCase { ->willReturn($filename); $file->method('getMimeType') ->willReturn('image/jpeg'); - - return $file; } private function mockSvgFileMethods($file) { @@ -159,8 +157,6 @@ abstract class GalleryUnitTest extends \Test\TestCase { ->willReturn($filename); $file->method('getMimeType') ->willReturn('image/svg+xml'); - - return $file; } private function mockAnimatedGifFileMethods($file) { @@ -173,9 +169,7 @@ abstract class GalleryUnitTest extends \Test\TestCase { ->willReturn('image/gif'); $file->method('fopen') ->with('rb') - ->willReturn(fopen(__DIR__ . '/../_data/' . $filename, 'rb')); - - return $file; + ->willReturn(fopen(__DIR__ . '/../_data/' . $filename, 'rb'));; } private function mockNoMediaFileMethods($file) { @@ -186,8 +180,6 @@ abstract class GalleryUnitTest extends \Test\TestCase { ->willReturn($filename); $file->method('getMimeType') ->willReturn('image/jpeg'); - - return $file; } protected function mockBadFile() { diff --git a/tests/unit/config/ConfigParserTest.php b/tests/unit/config/ConfigParserTest.php new file mode 100644 index 00000000..4362efba --- /dev/null +++ b/tests/unit/config/ConfigParserTest.php @@ -0,0 +1,211 @@ +<?php +/** + * ownCloud - gallery + * + * This file is licensed under the Affero General Public License version 3 or + * later. See the COPYING file. + * + * @author Olivier Paroz <owncloud@interfasys.ch> + * + * @copyright Olivier Paroz 2015 + */ + +namespace OCA\Gallery\Config; + +use Symfony\Component\Yaml\Parser; +use Symfony\Component\Yaml\Dumper; +use Symfony\Component\Yaml\Exception\ParseException; + +/** + * Class ConfigParserTest + * + * @package OCA\Gallery\Config + */ +class ConfigParserTest extends \Test\GalleryUnitTest { + + /** @var string */ + protected $configName = 'gallery.cnf'; + /** @var ConfigParser */ + protected $configParser; + + /** + * Test set up + */ + protected function setUp() { + parent::setUp(); + + $this->configParser = new ConfigParser(); + } + + public function providesGetFeaturesListData() { + $emptyConfig = []; + $noFeatures = [ + 'information' => [ + 'description_link' => 'readme.md' + ] + ]; + $emptyFeatures = [ + 'features' => [] + ]; + $featureList = [ + 'external_shares' => "no", + 'native_svg' => "yes", + ]; + $features = [ + 'features' => $featureList + ]; + + return [ + [$emptyConfig, []], + [$noFeatures, []], + [$emptyFeatures, []], + [$features, $featureList], + ]; + } + + /** + * @dataProvider providesGetFeaturesListData + * + * @param $config + * @param $expectedResult + */ + public function testGetFeaturesList($config, $expectedResult) { + $folder = $this->mockFolderWithConfig($config); + + $response = $this->configParser->getFeaturesList($folder, $this->configName); + + $this->assertEquals($expectedResult, $response); + } + + /** + * @expectedException \OCA\Gallery\Config\ConfigException + */ + public function testGetFeaturesListWithBrokenConfig() { + $folder = $this->mockFolder('home::user', 121212, []); + $folder->method('get') + ->with($this->configName) + ->willThrowException(new \Exception('Computer says no')); + + $this->configParser->getFeaturesList($folder, $this->configName); + } + + public function providesGetFolderConfigData() { + $emptyConfig = []; + $description = 'My cute description'; + $copyright = 'Copyright 2004-2016 interfaSys sàrl'; + $infoList = [ + 'description_link' => $description, + 'copyright_link' => $copyright, + 'inherit' => 'yes' + ]; + $information = [ + 'information' => $infoList + ]; + + $sortingList = [ + 'type' => 'name', + 'order' => 'des', + 'inherit' => 'yes' + ]; + $sorting = [ + 'sorting' => $sortingList + ]; + $standardRootConfig = array_merge($information, $sorting); + $standardLevel = 0; + $rootLevel = 1; + $nothingCompleted = ['information' => false, 'sorting' => false]; + $sortingCompleted = ['information' => false, 'sorting' => true]; + $infoCompleted = ['information' => true, 'sorting' => false]; + $allCompleted = ['information' => true, 'sorting' => true]; + + // One config in the current folder only + $currentConfigOnlyResult = $standardRootConfig; + $currentConfigOnlyResult['information']['level'] = $standardLevel; + $currentConfigOnlyResult['sorting']['level'] = $standardLevel; + + // Sorting with missing type + $brokenSortingConfig = [ + 'sorting' => [ + 'order' => 'des', + 'inherit' => 'no' + ] + ]; + + // Sorting with different type + $dateSortingConfig = [ + 'sorting' => [ + 'type' => 'date', + 'order' => 'des', + ] + ]; + + $dateSortingConfigResult = array_merge($standardRootConfig, $dateSortingConfig); + $dateSortingConfigResult['information']['level'] = $rootLevel; + + $infoConfig = [ + 'information' => [ + 'description_link' => 'Local conf', + 'copyright_link' => '2015 me', + ] + ]; + + // Full information is inherited from root + $infoConfigResult = array_merge($standardRootConfig, $infoConfig); + $infoConfigResult['sorting']['level'] = $rootLevel; + + return [ + [ + $emptyConfig, $nothingCompleted, $standardRootConfig, $standardLevel, + [$currentConfigOnlyResult, $allCompleted] + ], + [ + $emptyConfig, $nothingCompleted, $brokenSortingConfig, + $standardLevel, [$emptyConfig, $nothingCompleted] + ], + [ + $dateSortingConfig, $sortingCompleted, $standardRootConfig, $rootLevel, + [$dateSortingConfigResult, $allCompleted] + ], + [ + $infoConfig, $infoCompleted, $standardRootConfig, $rootLevel, + [$infoConfigResult, $allCompleted] + ], + + ]; + } + + /** + * @dataProvider providesGetFolderConfigData + * + * @param $currentConfig + * @param $configItems + * @param $newConfig + * @param $level + * @param $expectedResult + */ + public function testGetFolderConfig( + $currentConfig, $configItems, $newConfig, $level, $expectedResult + ) { + $folder = $this->mockFolderWithConfig($newConfig); + + $response = $this->configParser->getFolderConfig( + $folder, $this->configName, $currentConfig, $configItems, $level + ); + + $this->assertEquals($expectedResult, $response); + } + + private function mockFolderWithConfig($config) { + $file = $this->mockFile(212121); + $yaml = new Dumper(); + $file->method('getContent') + ->willReturn($yaml->dump($config)); + $folder = $this->mockFolder('home::user', 121212, [$file]); + $folder->method('get') + ->with($this->configName) + ->willReturn($file); + + return $folder; + } + +} diff --git a/tests/unit/service/ConfigServiceTest.php b/tests/unit/service/ConfigServiceTest.php index f4b4d5aa..2b27a2fc 100644 --- a/tests/unit/service/ConfigServiceTest.php +++ b/tests/unit/service/ConfigServiceTest.php @@ -12,6 +12,8 @@ namespace OCA\Gallery\Service; +use OCA\Gallery\Config\ConfigParser; +use OCA\Gallery\Config\ConfigException; /** * Class ConfigServiceTest @@ -31,7 +33,7 @@ class ConfigServiceTest extends \Test\GalleryUnitTest { public function setUp() { parent::setUp(); - $this->configParser = $this->getMockBuilder('\OCA\Gallery\Service\ConfigParser') + $this->configParser = $this->getMockBuilder('\OCA\Gallery\Config\ConfigParser') ->disableOriginalConstructor() ->getMock(); $this->service = new ConfigService ( @@ -51,9 +53,9 @@ class ConfigServiceTest extends \Test\GalleryUnitTest { $configItems = ['information' => false, 'sorting' => false]; // Default in the class $level = 0; $configPath = 'Some/folder'; - $exception = new ServiceException('Boom'); + $exception = new ConfigException('Boom'); $result = - [[['error' => ['message' => 'Boom' . "</br></br>Config location: /$configPath"]]]]; + [[['error' => ['message' => 'Boom' . ". Config location: /$configPath"]]]]; $this->mockGetPathFromVirtualRoot($folder, $configPath); $this->mockGetFolderConfigWithBrokenSetup( |