Fix version comparison with minor and patch level requirements
If an app requires a specific minor or path level server version, the version_compare prevented the installation as only the major version had been compared and that checks obviously returns `false`. Now the full version is used for comparison, making it possible to release apps for a specific minor or patch level version of Nextcloud. Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
parent
ecc3bc64aa
commit
ac939e8fd4
|
@ -27,6 +27,7 @@
|
|||
namespace OC\App\AppStore\Fetcher;
|
||||
|
||||
use OC\App\AppStore\Version\VersionParser;
|
||||
use OC\App\CompareVersion;
|
||||
use OC\Files\AppData\Factory;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OCP\Http\Client\IClientService;
|
||||
|
@ -34,17 +35,23 @@ use OCP\IConfig;
|
|||
use OCP\ILogger;
|
||||
|
||||
class AppFetcher extends Fetcher {
|
||||
|
||||
/** @var CompareVersion */
|
||||
private $compareVersion;
|
||||
|
||||
/**
|
||||
* @param Factory $appDataFactory
|
||||
* @param IClientService $clientService
|
||||
* @param ITimeFactory $timeFactory
|
||||
* @param IConfig $config
|
||||
* @param CompareVersion $compareVersion
|
||||
* @param ILogger $logger
|
||||
*/
|
||||
public function __construct(Factory $appDataFactory,
|
||||
IClientService $clientService,
|
||||
ITimeFactory $timeFactory,
|
||||
IConfig $config,
|
||||
CompareVersion $compareVersion,
|
||||
ILogger $logger) {
|
||||
parent::__construct(
|
||||
$appDataFactory,
|
||||
|
@ -56,6 +63,7 @@ class AppFetcher extends Fetcher {
|
|||
|
||||
$this->fileName = 'apps.json';
|
||||
$this->setEndpoint();
|
||||
$this->compareVersion = $compareVersion;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -70,8 +78,6 @@ class AppFetcher extends Fetcher {
|
|||
/** @var mixed[] $response */
|
||||
$response = parent::fetch($ETag, $content);
|
||||
|
||||
$ncVersion = $this->getVersion();
|
||||
$ncMajorVersion = explode('.', $ncVersion)[0];
|
||||
foreach($response['data'] as $dataKey => $app) {
|
||||
$releases = [];
|
||||
|
||||
|
@ -83,12 +89,13 @@ class AppFetcher extends Fetcher {
|
|||
// Exclude all versions not compatible with the current version
|
||||
$versionParser = new VersionParser();
|
||||
$version = $versionParser->getVersion($release['rawPlatformVersionSpec']);
|
||||
if (
|
||||
// Major version is bigger or equals to the minimum version of the app
|
||||
version_compare($ncMajorVersion, $version->getMinimumVersion(), '>=')
|
||||
// Major version is smaller or equals to the maximum version of the app
|
||||
&& version_compare($ncMajorVersion, $version->getMaximumVersion(), '<=')
|
||||
) {
|
||||
$ncVersion = $this->getVersion();
|
||||
$min = $version->getMinimumVersion();
|
||||
$max = $version->getMaximumVersion();
|
||||
$minFulfilled = $this->compareVersion->isCompatible($ncVersion, $min, '>=');
|
||||
$maxFulfilled = $max !== '' &&
|
||||
$this->compareVersion->isCompatible($ncVersion, $max, '<=');
|
||||
if ($minFulfilled && $maxFulfilled) {
|
||||
$releases[] = $release;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,97 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
*
|
||||
* @author 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as
|
||||
* published by the Free Software Foundation, either version 3 of the
|
||||
* License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*
|
||||
*/
|
||||
|
||||
namespace OC\App;
|
||||
|
||||
use InvalidArgumentException;
|
||||
|
||||
class CompareVersion {
|
||||
|
||||
const REGEX_MAJOR = '/^\d+$/';
|
||||
const REGEX_MAJOR_MINOR = '/^\d+.\d+$/';
|
||||
const REGEX_MAJOR_MINOR_PATCH = '/^\d+.\d+.\d+$/';
|
||||
const REGEX_SERVER = '/^\d+.\d+.\d+(.\d+)?$/';
|
||||
|
||||
/**
|
||||
* Checks if the given server version fulfills the given (app) version requirements.
|
||||
*
|
||||
* Version requirements can be 'major.minor.patch', 'major.minor' or just 'major',
|
||||
* so '13.0.1', '13.0' and '13' are valid.
|
||||
*
|
||||
* @param string $actual version as major.minor.patch notation
|
||||
* @param string $required version where major is requried and minor and patch are optional
|
||||
* @param string $comparator passed to `version_compare`
|
||||
* @return bool whether the requirement is fulfilled
|
||||
* @throws InvalidArgumentException if versions specified in an invalid format
|
||||
*/
|
||||
public function isCompatible(string $actual, string $required,
|
||||
string $comparator = '>='): bool {
|
||||
|
||||
if (!preg_match(self::REGEX_SERVER, $actual)) {
|
||||
throw new InvalidArgumentException('server version is invalid');
|
||||
}
|
||||
|
||||
if (preg_match(self::REGEX_MAJOR, $required) === 1) {
|
||||
return $this->compareMajor($actual, $required, $comparator);
|
||||
} else if (preg_match(self::REGEX_MAJOR_MINOR, $required) === 1) {
|
||||
return $this->compareMajorMinor($actual, $required, $comparator);
|
||||
} else if (preg_match(self::REGEX_MAJOR_MINOR_PATCH, $required) === 1) {
|
||||
return $this->compareMajorMinorPatch($actual, $required, $comparator);
|
||||
} else {
|
||||
throw new InvalidArgumentException('required version is invalid');
|
||||
}
|
||||
}
|
||||
|
||||
private function compareMajor(string $actual, string $required,
|
||||
string $comparator) {
|
||||
$actualMajor = explode('.', $actual)[0];
|
||||
$requiredMajor = explode('.', $required)[0];
|
||||
|
||||
return version_compare($actualMajor, $requiredMajor, $comparator);
|
||||
}
|
||||
|
||||
private function compareMajorMinor(string $actual, string $required,
|
||||
string $comparator) {
|
||||
$actualMajor = explode('.', $actual)[0];
|
||||
$actualMinor = explode('.', $actual)[1];
|
||||
$requiredMajor = explode('.', $required)[0];
|
||||
$requiredMinor = explode('.', $required)[1];
|
||||
|
||||
return version_compare("$actualMajor.$actualMinor",
|
||||
"$requiredMajor.$requiredMinor", $comparator);
|
||||
}
|
||||
|
||||
private function compareMajorMinorPatch($actual, $required, $comparator) {
|
||||
$actualMajor = explode('.', $actual)[0];
|
||||
$actualMinor = explode('.', $actual)[1];
|
||||
$actualPatch = explode('.', $actual)[2];
|
||||
$requiredMajor = explode('.', $required)[0];
|
||||
$requiredMinor = explode('.', $required)[1];
|
||||
$requiredPatch = explode('.', $required)[2];
|
||||
|
||||
return version_compare("$actualMajor.$actualMinor.$actualPatch",
|
||||
"$requiredMajor.$requiredMinor.$requiredPatch", $comparator);
|
||||
}
|
||||
|
||||
}
|
|
@ -22,6 +22,7 @@
|
|||
namespace Test\App\AppStore\Fetcher;
|
||||
|
||||
use OC\App\AppStore\Fetcher\AppFetcher;
|
||||
use OC\App\CompareVersion;
|
||||
use OC\Files\AppData\Factory;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OCP\Files\IAppData;
|
||||
|
@ -33,18 +34,21 @@ use OCP\Http\Client\IClientService;
|
|||
use OCP\Http\Client\IResponse;
|
||||
use OCP\IConfig;
|
||||
use OCP\ILogger;
|
||||
use PHPUnit_Framework_MockObject_MockObject;
|
||||
use Test\TestCase;
|
||||
|
||||
class AppFetcherTest extends TestCase {
|
||||
/** @var IAppData|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var IAppData|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $appData;
|
||||
/** @var IClientService|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var IClientService|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $clientService;
|
||||
/** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var ITimeFactory|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $timeFactory;
|
||||
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var IConfig|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $config;
|
||||
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
|
||||
/** @var CompareVersion|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $compareVersion;
|
||||
/** @var ILogger|PHPUnit_Framework_MockObject_MockObject */
|
||||
protected $logger;
|
||||
/** @var AppFetcher */
|
||||
protected $fetcher;
|
||||
|
@ -57,7 +61,7 @@ EOD;
|
|||
public function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
/** @var Factory|\PHPUnit_Framework_MockObject_MockObject $factory */
|
||||
/** @var Factory|PHPUnit_Framework_MockObject_MockObject $factory */
|
||||
$factory = $this->createMock(Factory::class);
|
||||
$this->appData = $this->createMock(IAppData::class);
|
||||
$factory->expects($this->once())
|
||||
|
@ -67,6 +71,7 @@ EOD;
|
|||
$this->clientService = $this->createMock(IClientService::class);
|
||||
$this->timeFactory = $this->createMock(ITimeFactory::class);
|
||||
$this->config = $this->createMock(IConfig::class);
|
||||
$this->compareVersion = new CompareVersion();
|
||||
$this->logger = $this->createMock(ILogger::class);
|
||||
|
||||
$this->config
|
||||
|
@ -79,6 +84,7 @@ EOD;
|
|||
$this->clientService,
|
||||
$this->timeFactory,
|
||||
$this->config,
|
||||
$this->compareVersion,
|
||||
$this->logger
|
||||
);
|
||||
}
|
||||
|
|
|
@ -0,0 +1,91 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
*
|
||||
* @author 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
|
||||
*
|
||||
* @license GNU AGPL version 3 or any later version
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as
|
||||
* published by the Free Software Foundation, either version 3 of the
|
||||
* License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*
|
||||
*/
|
||||
|
||||
namespace Test\App;
|
||||
|
||||
use InvalidArgumentException;
|
||||
use OC\App\CompareVersion;
|
||||
use Test\TestCase;
|
||||
|
||||
class CompareVersionTest extends TestCase {
|
||||
|
||||
/** @var CompareVersion */
|
||||
private $compare;
|
||||
|
||||
protected function setUp() {
|
||||
parent::setUp();
|
||||
|
||||
$this->compare = new CompareVersion();
|
||||
}
|
||||
|
||||
public function comparisonData() {
|
||||
return [
|
||||
// Compatible versions
|
||||
['13.0.0.3', '13.0.0', '>=', true],
|
||||
['13.0.0.3', '13.0.0', '<=', true],
|
||||
['13.0.0', '13.0.0', '>=', true],
|
||||
['13.0.0', '13.0', '<=', true],
|
||||
['13.0.0', '13', '>=', true],
|
||||
['13.0.1', '13', '>=', true],
|
||||
['13.0.1', '13', '<=', true],
|
||||
// Incompatible major versions
|
||||
['13.0.0.3', '13.0.0', '<', false],
|
||||
['12.0.0', '13.0.0', '>=', false],
|
||||
['12.0.0', '13.0', '>=', false],
|
||||
['12.0.0', '13', '>=', false],
|
||||
// Incompatible minor and patch versions
|
||||
['13.0.0', '13.0.1', '>=', false],
|
||||
['13.0.0', '13.1', '>=', false],
|
||||
// Compatible minor and patch versions
|
||||
['13.0.1', '13.0.0', '>=', true],
|
||||
['13.2.0', '13.1', '>=', true],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider comparisonData
|
||||
*/
|
||||
public function testComparison(string $actualVersion, string $requiredVersion,
|
||||
string $comparator, bool $expected) {
|
||||
$isCompatible = $this->compare->isCompatible($actualVersion, $requiredVersion,
|
||||
$comparator);
|
||||
|
||||
$this->assertEquals($expected, $isCompatible);
|
||||
}
|
||||
|
||||
public function testInvalidServerVersion() {
|
||||
$actualVersion = '13';
|
||||
$this->expectException(InvalidArgumentException::class);
|
||||
|
||||
$this->compare->isCompatible($actualVersion, '13.0.0');
|
||||
}
|
||||
|
||||
public function testInvalidRequiredVersion() {
|
||||
$actualVersion = '13.0.0';
|
||||
$this->expectException(InvalidArgumentException::class);
|
||||
|
||||
$this->compare->isCompatible($actualVersion, '13.0.0.9');
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in New Issue