Skip to content

Commit

Permalink
Refactor getAppPath
Browse files Browse the repository at this point in the history
  • Loading branch information
@VicDeo
VicDeo authored and Vincent Petry committed Jan 8, 2018
1 parent 46c1dfe commit b07298c
Show file tree
Hide file tree
Showing 5 changed files with 197 additions and 73 deletions.
108 changes: 105 additions & 3 deletions lib/private/App/AppManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,15 @@ class AppManager implements IAppManager {
/** @var IConfig */
private $config;

/**
* Apps as 'appId' => [
* 'path' => '/app/path'
* 'url' => '/app/url'
* ]
* @var string[][]
*/
private $appDirs = [];

/**
* @param IUserSession $userSession
* @param IAppConfig $appConfig
Expand All @@ -86,8 +95,8 @@ class AppManager implements IAppManager {
* @param IConfig $config
*/
public function __construct(IUserSession $userSession = null,
IAppConfig $appConfig,
IGroupManager $groupManager,
IAppConfig $appConfig = null,
IGroupManager $groupManager = null,
ICacheFactory $memCacheFactory,
EventDispatcherInterface $dispatcher,
IConfig $config) {
Expand Down Expand Up @@ -215,7 +224,7 @@ public function isInstalled($appId) {
* @throws \Exception
*/
public function enableApp($appId) {
if(OC_App::getAppPath($appId) === false) {
if($this->getAppPath($appId) === false) {
throw new \Exception("$appId can't be enabled since it is not installed.");
}
$this->canEnableTheme($appId);
Expand Down Expand Up @@ -483,4 +492,97 @@ public function canInstall() {
$appsFolder = OC_App::getInstallPath();
return $appsFolder !== null && is_writable($appsFolder) && is_readable($appsFolder);
}

/**
* Get the directory for the given app.
* If the app exists in multiple directories, the most recent version is taken.
* Returns false if not found
*
* @param string $appId
* @return string|false
* @since 10.0.5
*/
public function getAppPath($appId) {
if (trim($appId) === '') {
return false;
}
if (($appRoot = $this->findAppInDirectories($appId)) !== false) {
return $appRoot['path'];
}
return false;
}

/**
* Get the web path for the given app.
* If the app exists in multiple directories, web path to the most recent version is taken.
* Returns false if not found
*
* @param string $appId
* @return string|false
* @since 10.0.5
*/
public function getAppWebPath($appId) {
if (($appRoot = $this->findAppInDirectories($appId)) !== false) {
return \OC::$WEBROOT . $appRoot['url'];
}
return false;
}

/**
* Search for an app in all app directories
* Returns an app directory as an array with keys
* 'path' - a path to the app with no trailing slash
* 'url' - a web path to the app with no trailing slash
* both are relative to OC root directory and webroot
*
* @param string $appId
* @return false|string[]
*/
protected function findAppInDirectories($appId) {
$sanitizedAppId = \OC_App::cleanAppId($appId);
if ($sanitizedAppId !== $appId) {
return false;
}

if (!isset($this->appDirs[$appId])) {
$possibleAppRoots = [];
foreach ($this->getAppRoots() as $appRoot) {
if (is_dir($appRoot['path'] . '/' . $appId)) {
$possibleAppRoots[] = $appRoot;
}
}

$versionToLoad = [];
foreach ($possibleAppRoots as $possibleAppRoot) {
$version = $this->getAppVersionByPath($possibleAppRoot['path'] . '/' . $appId);
if (empty($versionToLoad) || version_compare($version, $versionToLoad['version'], '>')) {
$versionToLoad = array_merge($possibleAppRoot, ['version' => $version]);
$versionToLoad['path'] .= '/' . $appId;
$versionToLoad['url'] .= '/' . $appId;
}
}
$this->appDirs[$appId] = empty($versionToLoad) ? false : $versionToLoad;
}
return $this->appDirs[$appId];
}

/**
* Get apps roots as an array of path and url
* Wrapper for easy mocking
* @return string[][]
*/
protected function getAppRoots(){
return \OC::$APPSROOTS;
}

/**
* Get app's version based on it's path
* Wrapper for easy mocking
*
* @param string $path
* @return string
*/
protected function getAppVersionByPath($path) {
return \OC_App::getAppVersionByPath($path);
}
}
11 changes: 9 additions & 2 deletions lib/private/Server.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,17 @@ public function __construct($webRoot, \OC\Config $config) {
);
});
$this->registerService('AppManager', function (Server $c) {
if(\OC::$server->getSystemConfig()->getValue('installed', false)) {
$appConfig = $c->getAppConfig();
$groupManager = $c->getGroupManager();
} else {
$appConfig = null;
$groupManager = null;
}
return new \OC\App\AppManager(
$c->getUserSession(),
$c->getAppConfig(),
$c->getGroupManager(),
$appConfig,
$groupManager,
$c->getMemCacheFactory(),
$c->getEventDispatcher(),
$c->getConfig()
Expand Down
79 changes: 11 additions & 68 deletions lib/private/legacy/app.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,73 +550,30 @@ public static function getInstallPath() {
return null;
}


/**
* search for an app in all app-directories
* Get the directory for the given app.
* If the app exists in multiple directories, the most recent version is taken.
* (false if not found)
*
* @param string $appId
* @return false|string
* @return string|false
*/
protected static function findAppInDirectories($appId) {
$sanitizedAppId = self::cleanAppId($appId);
if($sanitizedAppId !== $appId) {
return false;
}
static $app_dir = [];

if (isset($app_dir[$appId])) {
return $app_dir[$appId];
}

$possibleApps = [];
foreach (OC::$APPSROOTS as $dir) {
if (file_exists($dir['path'] . '/' . $appId)) {
$possibleApps[] = $dir;
}
}

if (empty($possibleApps)) {
return false;
} elseif (count($possibleApps) === 1) {
$dir = array_shift($possibleApps);
$app_dir[$appId] = $dir;
return $dir;
} else {
$versionToLoad = [];
foreach ($possibleApps as $possibleApp) {
$version = self::getAppVersionByPath($possibleApp['path'] . '/' . $appId);
if (empty($versionToLoad) || version_compare($version, $versionToLoad['version'], '>')) {
$versionToLoad = [
'dir' => $possibleApp,
'version' => $version,
];
}
}
$app_dir[$appId] = $versionToLoad['dir'];
return $versionToLoad['dir'];
//TODO - write test
}
public static function getAppPath($appId) {
return \OC::$server->getAppManager()->getAppPath($appId);
}

/**
* Get the directory for the given app.
* If the app is defined in multiple directories, the first one is taken. (false if not found)
* Get the web path for the given app.
* If the app exists in multiple directories, the most recent version is taken.
* (false if not found)
*
* @param string $appId
* @return string|false
*/
public static function getAppPath($appId) {
if ($appId === null || trim($appId) === '') {
return false;
}

if (($dir = self::findAppInDirectories($appId)) != false) {
return $dir['path'] . '/' . $appId;
}
return false;
public static function getAppWebPath($appId) {
return \OC::$server->getAppManager()->getAppWebPath($appId);
}


/**
* check if an app's directory is writable
*
Expand All @@ -628,20 +585,6 @@ public static function isAppDirWritable($appId) {
return ($path !== false) ? is_writable($path) : false;
}

/**
* Get the path for the given app on the access
* If the app is defined in multiple directories, the first one is taken. (false if not found)
*
* @param string $appId
* @return string|false
*/
public static function getAppWebPath($appId) {
if (($dir = self::findAppInDirectories($appId)) != false) {
return OC::$WEBROOT . $dir['url'] . '/' . $appId;
}
return false;
}

/**
* get the last version of the app from appinfo/info.xml
*
Expand Down
22 changes: 22 additions & 0 deletions lib/public/App/IAppManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,26 @@ public function readAppPackage($path);
* @since 10.0.3
*/
public function canInstall();

/**
* Get the directory for the given app.
* If the app exists in multiple directories, the most recent version is taken.
* Returns false if not found
*
* @param string $appId
* @return string|false
* @since 10.0.5
*/
public function getAppPath($appId);

/**
* Get the web path for the given app.
* If the app exists in multiple directories, web path to the most recent version is taken.
* Returns false if not found
*
* @param string $appId
* @return string|false
* @since 10.0.5
*/
public function getAppWebPath($appId);
}
50 changes: 50 additions & 0 deletions tests/lib/App/ManagerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use OCP\IUserSession;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Test\TestCase;
use org\bovigo\vfs\vfsStream;

/**
* Class Manager
Expand Down Expand Up @@ -460,4 +461,53 @@ public function providesDataForCanInstall() {
[false, 'clustered-instance'],
];
}

/**
* @dataProvider appInfoDataProvider
*
* @param string $firstDirVersion
* @param string $secondDirVersion
* @param bool $isFirstWinner
*/
public function testTheMostRecentAppIsFound($firstDirVersion, $secondDirVersion, $isFirstWinner) {
$appId = 'bogusapp';
$appsParentDir = vfsStream::setup();
$firstAppDir = vfsStream::newDirectory('apps')->at($appsParentDir);
$appDir1 = vfsStream::newDirectory($appId)->at($firstAppDir);
$secondAppDir = vfsStream::newDirectory('apps2')->at($appsParentDir);
$appDir2 = vfsStream::newDirectory($appId)->at($secondAppDir);

$appManager = $this->getMockBuilder(AppManager::class)
->setMethods(['getAppVersionByPath', 'getAppRoots'])
->disableOriginalConstructor()
->getMock();

$appManager->expects($this->any())
->method('getAppRoots')
->willReturn([
[
'path' => $firstAppDir->url(),
'url' => $firstAppDir->url(),
],
[
'path' => $secondAppDir->url(),
'url' => $secondAppDir->url(),
]
]);

$appManager->expects($this->any())
->method('getAppVersionByPath')
->will($this->onConsecutiveCalls($firstDirVersion, $secondDirVersion));

$expected = $isFirstWinner ? $appDir1->url() : $appDir2->url();
$appPath = $appManager->getAppPath($appId);
$this->assertEquals($expected, $appPath);
}

public function appInfoDataProvider() {
return [
[ '1.2.3', '3.2.4', false ],
[ '2.2.3', '2.2.1', true ]
];
}
}

0 comments on commit b07298c

Please sign in to comment.