Merge pull request #3565 from nextcloud/bugfix/noid/hpb-multi-setup-protection

Don't allow multiple HPB without clustering and add a cache warning

lib/Config.php

@@ -235,7 +235,7 @@ class Config {
 		];
 	}
 
-	public function getSignalingMode(): string {
+	public function getSignalingMode($cleanExternalSignaling = true): string {
 		$validModes = [
 			self::SIGNALING_INTERNAL,
 			self::SIGNALING_EXTERNAL,
@@ -252,6 +252,7 @@ class Config {
 			return self::SIGNALING_INTERNAL;
 		}
 		if ($numSignalingServers === 1
+			&& $cleanExternalSignaling
 			&& $this->config->getAppValue('spreed', 'signaling_dev', 'no') === 'no') {
 			return self::SIGNALING_EXTERNAL;
 		}

lib/Settings/Admin/AdminSettings.php

@@ -29,6 +29,7 @@ use OCA\Talk\Participant;
 use OCA\Talk\Room;
 use OCA\Talk\Service\CommandService;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IInitialStateService;
 use OCP\Settings\ISettings;
@@ -43,15 +44,19 @@ class AdminSettings implements ISettings {
 	private $commandService;
 	/** @var IInitialStateService */
 	private $initialStateService;
+	/** @var ICacheFactory */
+	private $memcacheFactory;
 
 	public function __construct(Config $talkConfig,
 								IConfig $serverConfig,
 								CommandService $commandService,
-								IInitialStateService $initialStateService) {
+								IInitialStateService $initialStateService,
+								ICacheFactory $memcacheFactory) {
 		$this->talkConfig = $talkConfig;
 		$this->serverConfig = $serverConfig;
 		$this->commandService = $commandService;
 		$this->initialStateService = $initialStateService;
+		$this->memcacheFactory = $memcacheFactory;
 	}
 
 	/**
@@ -100,6 +105,8 @@ class AdminSettings implements ISettings {
 	}
 
 	protected function initSignalingServers(): void {
+		$this->initialStateService->provideInitialState('talk', 'has_cache_configured', $this->memcacheFactory->isAvailable());
+		$this->initialStateService->provideInitialState('talk', 'signaling_mode', $this->talkConfig->getSignalingMode(false));
 		$this->initialStateService->provideInitialState('talk', 'signaling_servers', [
 			'servers' => $this->talkConfig->getSignalingServers(),
 			'secret' => $this->talkConfig->getSignalingSecret(),

lib/TInitialState.php

@@ -55,11 +55,11 @@ trait TInitialState {
 		);
 
 		$signalingMode = $this->talkConfig->getSignalingMode();
-		if ($signalingMode !== Config::SIGNALING_INTERNAL
+		if ($signalingMode === Config::SIGNALING_CLUSTER_CONVERSATION
 			&& !$this->memcacheFactory->isAvailable()
 			&& $this->serverConfig->getAppValue('spreed', 'signaling_dev', 'no') === 'no') {
 			throw new HintException(
-				'External signaling is only supported with a distributed cache'
+				'High Performance Back-end clustering is only supported with a distributed cache!'
 			);
 		}
 

src/components/AdminSettings/SignalingServers.vue

@@ -25,13 +25,13 @@
 		<h2>
 			{{ t('spreed', 'Signaling servers') }}
 			<span v-if="saved" class="icon icon-checkmark-color" :title="t('spreed', 'Saved')" />
-			<a v-else-if="!loading"
+			<a v-else-if="!loading && showAddServerButton"
 				v-tooltip.auto="t('spreed', 'Add a new server')"
 				class="icon icon-add"
 				@click="newServer">
 				<span class="hidden-visually">{{ t('spreed', 'Add a new server') }}</span>
 			</a>
-			<span v-else class="icon icon-loading-small" />
+			<span v-else-if="loading" class="icon icon-loading-small" />
 		</h2>
 
 		<p class="settings-hint">
@@ -39,6 +39,12 @@
 			<span v-if="!servers.length">{{ t('spreed', 'Please note that calls with more than 4 participants without external signaling server, participants can experience connectivity issues and cause high load on participating devices.') }}</span>
 		</p>
 
+		<p
+			v-if="!isCacheConfigured"
+			class="settings-hint warning">
+			{{ t('spreed', 'It is highly recommended to set up a distributed cache when using Nextcloud Talk together with a High Performance Back-end.') }}
+		</p>
+
 		<div v-if="!servers.length" class="signaling-warning">
 			<input id="hide_warning"
 				v-model="hideWarning"
@@ -83,6 +89,7 @@ import SignalingServer from '../../components/AdminSettings/SignalingServer'
 import Tooltip from '@nextcloud/vue/dist/Directives/Tooltip'
 import { loadState } from '@nextcloud/initial-state'
 import debounce from 'debounce'
+import { SIGNALING } from '../../constants'
 
 export default {
 	name: 'SignalingServers',
@@ -102,9 +109,17 @@ export default {
 			hideWarning: false,
 			loading: false,
 			saved: false,
+			isCacheConfigured: loadState('talk', 'has_cache_configured'),
+			isClusteredMode: loadState('talk', 'signaling_mode') === SIGNALING.MODE.CLUSTER_CONVERSATION,
 		}
 	},
 
+	computed: {
+		showAddServerButton() {
+			return this.isClusteredMode || this.servers.length === 0
+		},
+	},
+
 	beforeMount() {
 		const state = loadState('talk', 'signaling_servers')
 		this.servers = state.servers

src/constants.js

@@ -17,6 +17,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
+export const SIGNALING = {
+	MODE: {
+		INTERNAL: 'internal',
+		EXTERNAL: 'external',
+		CLUSTER_CONVERSATION: 'conversation_cluster',
+	},
+}
 export const CONVERSATION = {
 	START_CALL: {
 		EVERYONE: 0,

tests/php/Settings/Admin/AdminSettingsTest.php

@@ -26,6 +26,7 @@ namespace OCA\Talk\Tests\php\Settings\Admin;
 use OCA\Talk\Config;
 use OCA\Talk\Service\CommandService;
 use OCA\Talk\Settings\Admin\AdminSettings;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IInitialStateService;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -40,7 +41,9 @@ class AdminSettingsTest extends \Test\TestCase {
 	protected $commandService;
 	/** @var IInitialStateService|MockObject */
 	protected $initialState;
-	/** @var StunServer */
+	/** @var ICacheFactory|MockObject */
+	protected $cacheFactory;
+	/** @var AdminSettings */
 	protected $admin;
 
 	public function setUp(): void {
@@ -50,13 +53,9 @@ class AdminSettingsTest extends \Test\TestCase {
 		$this->serverConfig = $this->createMock(IConfig::class);
 		$this->commandService = $this->createMock(CommandService::class);
 		$this->initialState = $this->createMock(IInitialStateService::class);
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 
-		$this->admin = new AdminSettings(
-			$this->talkConfig,
-			$this->serverConfig,
-			$this->commandService,
-			$this->initialState
-		);
+		$this->admin = $this->getAdminSettings();
 	}
 
 	/**
@@ -69,7 +68,8 @@ class AdminSettingsTest extends \Test\TestCase {
 				$this->talkConfig,
 				$this->serverConfig,
 				$this->commandService,
-				$this->initialState
+				$this->initialState,
+				$this->cacheFactory
 			);
 		}
 
@@ -79,6 +79,7 @@ class AdminSettingsTest extends \Test\TestCase {
 				$this->serverConfig,
 				$this->commandService,
 				$this->initialState,
+				$this->cacheFactory,
 			])
 			->onlyMethods($methods)
 			->getMock();