Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 'updates.check_updates_branch' setting #1971

Merged
merged 5 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion application/config/ConfigManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ protected function setDefaultValues()
$this->setEmpty('general.tags_separator', ' ');

$this->setEmpty('updates.check_updates', true);
$this->setEmpty('updates.check_updates_branch', 'latest');
$this->setEmpty('updates.check_updates_interval', 86400);

$this->setEmpty('feed.rss_permalinks', true);
Expand Down
2 changes: 1 addition & 1 deletion application/front/controller/admin/ServerController.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public function index(Request $request, Response $response): Response
}

$currentVersion = ApplicationUtils::getVersion('./shaarli_version.php');
$currentVersion = $currentVersion === 'dev' ? $currentVersion : 'v' . $currentVersion;
$currentVersion = ApplicationUtils::isDevVersion($currentVersion) ? $currentVersion : 'v' . $currentVersion;
$phpEol = new \DateTimeImmutable(ApplicationUtils::getPhpEol(PHP_VERSION));

$permissions = array_merge(
Expand Down
53 changes: 26 additions & 27 deletions application/helper/ApplicationUtils.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ApplicationUtils

public static $GITHUB_URL = 'https://github.com/shaarli/Shaarli';
public static $GIT_RAW_URL = 'https://raw.githubusercontent.com/shaarli/Shaarli';
public static $GIT_BRANCHES = ['latest', 'stable'];
public static $RELEASE_BRANCH = 'release';
private static $VERSION_START_TAG = '<?php /* ';
private static $VERSION_END_TAG = ' */ ?>';

Expand Down Expand Up @@ -89,7 +89,6 @@ public static function getVersion($remote, $timeout = 2)
* @param int $checkInterval the minimum interval between update checks (in seconds
* @param bool $enableCheck whether to check for new versions
* @param bool $isLoggedIn whether the user is logged in
* @param string $branch check update for the given branch
*
* @throws Exception an invalid branch has been set for update checks
*
Expand All @@ -100,13 +99,12 @@ public static function checkUpdate(
$updateFile,
$checkInterval,
$enableCheck,
$isLoggedIn,
$branch = 'stable'
$isLoggedIn
) {
// Do not check versions for visitors
// Do not check if the user doesn't want to
// Do not check with dev version
if (!$isLoggedIn || empty($enableCheck) || $currentVersion === 'dev') {
if (!$isLoggedIn || empty($enableCheck) || self::isDevVersion($currentVersion)) {
return false;
}

Expand All @@ -120,16 +118,10 @@ public static function checkUpdate(
return false;
}

if (!in_array($branch, self::$GIT_BRANCHES)) {
throw new Exception(
'Invalid branch selected for updates: "' . $branch . '"'
);
}

// Late Static Binding allows overriding within tests
// See http://php.net/manual/en/language.oop5.late-static-bindings.php
$latestVersion = static::getVersion(
self::$GIT_RAW_URL . '/' . $branch . '/' . self::$VERSION_FILE
self::$GIT_RAW_URL . '/' . self::$RELEASE_BRANCH . '/' . self::$VERSION_FILE
);

if (!$latestVersion) {
Expand Down Expand Up @@ -189,11 +181,11 @@ public static function checkResourcePermissions(ConfigManager $conf, bool $minim
// Check script and template directories are readable
foreach (
[
'application',
'inc',
'plugins',
$rainTplDir,
$rainTplDir . '/' . $conf->get('resource.theme'),
'application',
'inc',
'plugins',
$rainTplDir,
$rainTplDir . '/' . $conf->get('resource.theme'),
] as $path
) {
if (!is_readable(realpath($path))) {
Expand All @@ -208,10 +200,10 @@ public static function checkResourcePermissions(ConfigManager $conf, bool $minim
];
} else {
$folders = [
$conf->get('resource.thumbnails_cache'),
$conf->get('resource.data_dir'),
$conf->get('resource.page_cache'),
$conf->get('resource.raintpl_tmp'),
$conf->get('resource.thumbnails_cache'),
$conf->get('resource.data_dir'),
$conf->get('resource.page_cache'),
$conf->get('resource.raintpl_tmp'),
];
}

Expand All @@ -231,12 +223,12 @@ public static function checkResourcePermissions(ConfigManager $conf, bool $minim
// Check configuration files are readable and writable
foreach (
[
$conf->getConfigFileExt(),
$conf->get('resource.datastore'),
$conf->get('resource.ban_file'),
$conf->get('resource.log'),
$conf->get('resource.update_check'),
] as $path
$conf->getConfigFileExt(),
$conf->get('resource.datastore'),
$conf->get('resource.ban_file'),
$conf->get('resource.log'),
$conf->get('resource.update_check'),
] as $path
) {
if (!is_string($path) || !is_file(realpath($path))) {
# the file may not exist yet
Expand Down Expand Up @@ -334,4 +326,11 @@ public static function getPhpEol(string $fullVersion): string
'8.2' => '2025-12-08',
][$matches[1]] ?? (new \DateTime('+2 year'))->format('Y-m-d');
}

public static function isDevVersion(string $version): bool
{
return strpos($version, 'dev') !== false
|| preg_match('/[a-f0-9]{7}/', $version) === 1
;
}
}
3 changes: 1 addition & 2 deletions application/render/PageBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,7 @@ private function initialize()
$this->conf->get('resource.update_check'),
$this->conf->get('updates.check_updates_interval'),
$this->conf->get('updates.check_updates'),
$this->isLoggedIn,
$this->conf->get('updates.check_updates_branch')
$this->isLoggedIn
);
$this->tpl->assign('newVersion', escape($version));
$this->tpl->assign('versionError', '');
Expand Down
9 changes: 9 additions & 0 deletions application/updater/Updater.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,15 @@ public function updateMethodMigrateExistingNotesUrl(): bool
return true;
}

public function updateMethodRemoveSettingRemoteBranch(): bool
{
if ($this->conf->exists('updates.check_updates_branch')) {
$this->conf->remove('updates.check_updates_branch', true, true);
}

return true;
}

public function setBasePath(string $basePath): self
{
$this->basePath = $basePath;
Expand Down
4 changes: 0 additions & 4 deletions doc/md/Shaarli-configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,9 @@ Some settings can be configured directly from a web browser by accesing the `Too
"page_cache": "pagecache"
},
"general": {
"check_updates": true,
"rss_permalinks": true,
"links_per_page": 20,
"default_private_links": true,
"check_updates_branch": "stable",
"check_updates_interval": 86400,
"download_max_size": 4194304,
"download_timeout": 30,
Expand All @@ -88,7 +86,6 @@ Some settings can be configured directly from a web browser by accesing the `Too
"formatter": "markdown",
"updates": {
"check_updates": true,
"check_updates_branch": "stable",
"check_updates_interval": 86400
},
"feed": {
Expand Down Expand Up @@ -207,7 +204,6 @@ Must be an associative array: `translation domain => translation path`.
### Updates

- **check_updates**: Enable or disable update check to the git repository.
- **check_updates_branch**: Git branch used to check updates (e.g. `stable` or `master`).
- **check_updates_interval**: Look for new version every N seconds (default: every day).

### Privacy
Expand Down
22 changes: 11 additions & 11 deletions tests/helper/ApplicationUtilsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,6 @@ public function testCheckUpdateNewVersionUnavailable()
$this->assertFalse($version);
}

/**
* Test update checks - invalid Git branch
*/
public function testCheckUpdateInvalidGitBranch()
{
$this->expectException(\Exception::class);
$this->expectExceptionMessageRegExp('/Invalid branch selected for updates/');

ApplicationUtils::checkUpdate('', 'null', 0, true, true, 'unstable');
}

/**
* Shaarli is up-to-date
*/
Expand Down Expand Up @@ -379,6 +368,17 @@ public function testCheckUpdateDev()
);
}

/**
* Check update with a short git object name as curent version (Docker build).
* It should always return false.
*/
public function testCheckUpdateDevHash()
{
$this->assertFalse(
ApplicationUtils::checkUpdate('abc123d', self::$testUpdateFile, 100, true, true)
);
}

/**
* Basic test of getPhpExtensionsRequirement()
*/
Expand Down
1 change: 0 additions & 1 deletion tests/utils/config/configJson.json.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
},
"updates": {
"check_updates": false,
"check_updates_branch": "stable",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think updateMethodCheckUpdateRemoteBranch() in application/legacy/LegacyUpdater.php still refers to this setting.
Ref. #1964

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but this piece of code if from the LegacyUpdater which would update things from older versions, so it doesn't need to be updated/maintained as long as it's not breaking anything.

However I forgot to write an update here to remove the file in existing config files.

"check_updates_interval": 86400
},
"feed": {
Expand Down
Loading