From 77dee0d977e94c0fcf7e8e19d419a0405b8ed9dc Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Wed, 10 Jul 2019 15:54:17 +0200 Subject: [PATCH] Recover-strategy to detect an updated server without a reload --- .../core/core-services/autoupdate.service.ts | 3 ++ .../core/core-services/constants.service.ts | 38 +++++++++++++------ .../data-store-upgrade.service.ts | 36 ++++++++++++------ .../core/core-services/openslides.service.ts | 24 +++++++++++- .../app/core/core-services/ping.service.ts | 2 +- .../core-services/projector-data.service.ts | 2 +- .../core/core-services/websocket.service.ts | 25 +++++++++--- openslides/core/apps.py | 37 +++++++++++++----- openslides/core/config.py | 29 ++++++++++++-- openslides/core/config_variables.py | 9 +++++ tests/conftest.py | 5 ++- 11 files changed, 164 insertions(+), 46 deletions(-) diff --git a/client/src/app/core/core-services/autoupdate.service.ts b/client/src/app/core/core-services/autoupdate.service.ts index 2947ffaa4..46bc9dc8f 100644 --- a/client/src/app/core/core-services/autoupdate.service.ts +++ b/client/src/app/core/core-services/autoupdate.service.ts @@ -166,6 +166,7 @@ export class AutoupdateService { * Does a full update: Requests all data from the server and sets the DS to the fresh data. */ public async doFullUpdate(): Promise { + const oldChangeId = this.DS.maxChangeId; const response = await this.websocketService.sendAndGetResponse<{}, AutoupdateFormat>('getElements', {}); const updateSlot = await this.DSUpdateManager.getNewUpdateSlot(this.DS); @@ -180,5 +181,7 @@ export class AutoupdateService { await this.DS.set(allModels, response.to_change_id); this.DSUpdateManager.commit(updateSlot); + + console.log(`Full update done from ${oldChangeId} to ${response.to_change_id}`); } } diff --git a/client/src/app/core/core-services/constants.service.ts b/client/src/app/core/core-services/constants.service.ts index 3d4c507ad..0ca12df7a 100644 --- a/client/src/app/core/core-services/constants.service.ts +++ b/client/src/app/core/core-services/constants.service.ts @@ -29,11 +29,6 @@ export class ConstantsService { */ private constants: Constants; - /** - * Flag, if the websocket connection is open. - */ - private websocketOpen = false; - /** * Flag, if constants are requested, but the server hasn't send them yet. */ @@ -54,18 +49,26 @@ export class ConstantsService { if (this.pending) { // send constants to subscribers that await constants. this.pending = false; - Object.keys(this.pendingSubject).forEach(key => { - this.pendingSubject[key].next(this.constants[key]); - }); + this.informSubjects(); } }); // We can request constants, if the websocket connection opens. - websocketService.connectEvent.subscribe(() => { - if (!this.websocketOpen && this.pending) { + // On retries, the `refresh()` method is called by the OpenSlidesService, so + // here we do not need to take care about this. + websocketService.noRetryConnectEvent.subscribe(() => { + if (this.pending) { this.websocketService.send('constants', {}); } - this.websocketOpen = true; + }); + } + + /** + * Inform subjects about changes. + */ + private informSubjects(): void { + Object.keys(this.pendingSubject).forEach(key => { + this.pendingSubject[key].next(this.constants[key]); }); } @@ -81,7 +84,7 @@ export class ConstantsService { if (!this.pending) { this.pending = true; // if the connection is open, we directly can send the request. - if (this.websocketOpen) { + if (this.websocketService.isConnected) { this.websocketService.send('constants', {}); } } @@ -91,4 +94,15 @@ export class ConstantsService { return this.pendingSubject[key].asObservable() as Observable; } } + + /** + * Refreshed the constants + */ + public async refresh(): Promise { + if (!this.websocketService.isConnected) { + return; + } + this.constants = await this.websocketService.sendAndGetResponse('constants', {}); + this.informSubjects(); + } } diff --git a/client/src/app/core/core-services/data-store-upgrade.service.ts b/client/src/app/core/core-services/data-store-upgrade.service.ts index 743b76971..f3aad3a75 100644 --- a/client/src/app/core/core-services/data-store-upgrade.service.ts +++ b/client/src/app/core/core-services/data-store-upgrade.service.ts @@ -1,10 +1,12 @@ import { Injectable } from '@angular/core'; +import { take } from 'rxjs/operators'; + import { ConstantsService } from './constants.service'; import { AutoupdateService } from './autoupdate.service'; import { StorageService } from './storage.service'; -const MIGRATIONVERSION = 'MigrationVersion'; +const DB_SCHEMA_VERSION = 'DbSchemaVersion'; /** * Manages upgrading the DataStore, if the migration version from the server is higher than the current one. @@ -19,16 +21,28 @@ export class DataStoreUpgradeService { * @param storageService */ public constructor( - autoupdateService: AutoupdateService, - constantsService: ConstantsService, - storageService: StorageService + private autoupdateService: AutoupdateService, + private constantsService: ConstantsService, + private storageService: StorageService ) { - constantsService.get(MIGRATIONVERSION).subscribe(async version => { - const currentVersion = await storageService.get(MIGRATIONVERSION); - await storageService.set(MIGRATIONVERSION, version); - if (currentVersion && currentVersion !== version) { - autoupdateService.doFullUpdate(); - } - }); + this.checkForUpgrade(); + } + + public async checkForUpgrade(): Promise { + const version = await this.constantsService + .get(DB_SCHEMA_VERSION) + .pipe(take(1)) + .toPromise(); + console.log('DB schema version:', version); + const currentVersion = await this.storageService.get(DB_SCHEMA_VERSION); + await this.storageService.set(DB_SCHEMA_VERSION, version); + const doUpgrade = version !== currentVersion; + + if (doUpgrade) { + console.log(`DB schema version changed from ${currentVersion} to ${version}`); + await this.autoupdateService.doFullUpdate(); + } + + return doUpgrade; } } diff --git a/client/src/app/core/core-services/openslides.service.ts b/client/src/app/core/core-services/openslides.service.ts index 660989099..addaee317 100644 --- a/client/src/app/core/core-services/openslides.service.ts +++ b/client/src/app/core/core-services/openslides.service.ts @@ -6,6 +6,8 @@ import { OperatorService } from './operator.service'; import { StorageService } from './storage.service'; import { AutoupdateService } from './autoupdate.service'; import { DataStoreService } from './data-store.service'; +import { ConstantsService } from './constants.service'; +import { DataStoreUpgradeService } from './data-store-upgrade.service'; /** * Handles the bootup/showdown of this application. @@ -44,7 +46,9 @@ export class OpenSlidesService { private websocketService: WebsocketService, private router: Router, private autoupdateService: AutoupdateService, - private DS: DataStoreService + private DS: DataStoreService, + private constantsService: ConstantsService, + private dataStoreUpgradeService: DataStoreUpgradeService ) { // Handler that gets called, if the websocket connection reconnects after a disconnection. // There might have changed something on the server, so we check the operator, if he changed. @@ -170,8 +174,24 @@ export class OpenSlidesService { await this.reboot(); } else if (requestChanges) { // User is still the same, but check for missed autoupdates. - this.autoupdateService.requestChanges(); + await this.recoverAfterReconnect(); } } } + + /** + * The cache-refresh strategy, if there was an reconnect and the user didn't changed. + */ + private async recoverAfterReconnect(): Promise { + // Reload constants to get either new one (in general) and especially + // the "DbSchemaVersion" one, to check, if the DB has changed (e.g. due + // to an update) + await this.constantsService.refresh(); + + // If the DB schema version didn't change, request normal changes. + // If so, then a full update is implicit triggered, so we do not need to to anything. + if (!(await this.dataStoreUpgradeService.checkForUpgrade())) { + this.autoupdateService.requestChanges(); + } + } } diff --git a/client/src/app/core/core-services/ping.service.ts b/client/src/app/core/core-services/ping.service.ts index a5fe18544..c2872afb5 100644 --- a/client/src/app/core/core-services/ping.service.ts +++ b/client/src/app/core/core-services/ping.service.ts @@ -55,7 +55,7 @@ export class PingService { // Connects the ping-pong mechanism to the opening and closing of the connection. this.websocketService.closeEvent.subscribe(() => this.stopPing()); - this.websocketService.connectEvent.subscribe(() => this.startPing()); + this.websocketService.generalConnectEvent.subscribe(() => this.startPing()); if (this.websocketService.isConnected) { this.startPing(); } diff --git a/client/src/app/core/core-services/projector-data.service.ts b/client/src/app/core/core-services/projector-data.service.ts index 9600e20d2..1773496de 100644 --- a/client/src/app/core/core-services/projector-data.service.ts +++ b/client/src/app/core/core-services/projector-data.service.ts @@ -51,7 +51,7 @@ export class ProjectorDataService { }); }); - this.websocketService.connectEvent.subscribe(() => this.updateProjectorDataSubscription()); + this.websocketService.generalConnectEvent.subscribe(() => this.updateProjectorDataSubscription()); } /** diff --git a/client/src/app/core/core-services/websocket.service.ts b/client/src/app/core/core-services/websocket.service.ts index 0cbf1ade3..13c7b6df9 100644 --- a/client/src/app/core/core-services/websocket.service.ts +++ b/client/src/app/core/core-services/websocket.service.ts @@ -90,16 +90,29 @@ export class WebsocketService { return this._retryReconnectEvent; } + /** + * Subjects that will be called, if connect took place, but not a retry reconnect. + * THis is the complement from the generalConnectEvent to the retryReconnectEvent. + */ + private readonly _noRetryConnectEvent: EventEmitter = new EventEmitter(); + + /** + * Getter for the no-retry connect event. + */ + public get noRetryConnectEvent(): EventEmitter { + return this._noRetryConnectEvent; + } + /** * Listeners will be nofitied, if the wesocket connection is establiched. */ - private readonly _connectEvent: EventEmitter = new EventEmitter(); + private readonly _generalConnectEvent: EventEmitter = new EventEmitter(); /** * Getter for the connect event. */ - public get connectEvent(): EventEmitter { - return this._connectEvent; + public get generalConnectEvent(): EventEmitter { + return this._generalConnectEvent; } /** @@ -234,12 +247,14 @@ export class WebsocketService { return; } + this._connectionOpen = true; if (retry) { this.dismissConnectionErrorNotice(); this._retryReconnectEvent.emit(); + } else { + this._noRetryConnectEvent.emit(); } - this._connectionOpen = true; - this._connectEvent.emit(); + this._generalConnectEvent.emit(); this.sendQueueWhileNotConnected.forEach(entry => { this.websocket.send(entry); }); diff --git a/openslides/core/apps.py b/openslides/core/apps.py index 00969e6bf..4c3367ec1 100644 --- a/openslides/core/apps.py +++ b/openslides/core/apps.py @@ -1,3 +1,5 @@ +import hashlib +import logging import os import sys from collections import OrderedDict @@ -10,6 +12,9 @@ from django.db.models import Max from django.db.models.signals import post_migrate, pre_delete +logger = logging.getLogger("openslides.core") + + class CoreAppConfig(AppConfig): name = "openslides.core" verbose_name = "OpenSlides Core" @@ -60,9 +65,7 @@ class CoreAppConfig(AppConfig): ) post_migrate.connect( - call_save_default_values, - sender=self, - dispatch_uid="core_save_config_default_values", + manage_config, sender=self, dispatch_uid="core_manage_config" ) pre_delete.connect( autoupdate_for_many_to_many_relations, @@ -175,17 +178,33 @@ class CoreAppConfig(AppConfig): # get max migration id -> the "version" of the DB from django.db.migrations.recorder import MigrationRecorder - constants["MigrationVersion"] = MigrationRecorder.Migration.objects.aggregate( - Max("id") - )["id__max"] + migration_version = MigrationRecorder.Migration.objects.aggregate(Max("id"))[ + "id__max" + ] + config_version = config["config_version"] + hash = hashlib.sha1( + f"{migration_version}#{config_version}".encode() + ).hexdigest() + constants["DbSchemaVersion"] = hash + logger.info(f"DbSchemaVersion={hash}") return constants -def call_save_default_values(**kwargs): +def manage_config(**kwargs): + """ + Should be run after every migration. Saves default values + of all non db-existing config objects into the db. Deletes all + unnecessary old config values, e.g. all db entries, that does + not have a config_variable anymore. Increments the config version, + if at least one of the operations altered some data. + """ from .config import config - config.save_default_values() + altered = config.save_default_values() + altered = config.cleanup_old_config_values() or altered + if altered: + config.increment_version() def startup(): @@ -201,6 +220,6 @@ def startup(): from openslides.utils.cache import element_cache from openslides.core.models import History - set_constants(get_constants_from_apps()) element_cache.ensure_cache() + set_constants(get_constants_from_apps()) History.objects.build_history() diff --git a/openslides/core/config.py b/openslides/core/config.py index aba4fafd0..6525531e7 100644 --- a/openslides/core/config.py +++ b/openslides/core/config.py @@ -205,13 +205,14 @@ class ConfigHandler: self.config_variables.update(item_index) - def save_default_values(self) -> None: + def save_default_values(self) -> bool: """ - Saves the default values to the database. + Saves the default values to the database. Does also build the dictonary key_to_id. - Does also build the dictonary key_to_id. + Returns True, if something in the DB was changed. """ self.key_to_id = {} + altered_config = False for item in self.config_variables.values(): try: db_value = ConfigStore.objects.get(key=item.name) @@ -220,7 +221,29 @@ class ConfigHandler: db_value.key = item.name db_value.value = item.default_value db_value.save(skip_autoupdate=True) + altered_config = True self.key_to_id[db_value.key] = db_value.id + return altered_config + + def increment_version(self) -> None: + """ + Increments the config key "config_version" + """ + db_value = ConfigStore.objects.get(key="config_version") + db_value.value = db_value.value + 1 + db_value.save(skip_autoupdate=True) + + def cleanup_old_config_values(self) -> bool: + """ + Deletes all config variable in the database, if the keys are not + in key_to_id. This required a fully build key_to_id! + Returns True, if something in the DB was changed. + """ + key_to_id = key_to_id = cast(Dict[str, int], self.key_to_id) + queryset = ConfigStore.objects.exclude(key__in=key_to_id.keys()) + altered_config = queryset.exists() + queryset.delete() + return altered_config def get_collection_string(self) -> str: """ diff --git a/openslides/core/config_variables.py b/openslides/core/config_variables.py index a5878e7fd..aa4975572 100644 --- a/openslides/core/config_variables.py +++ b/openslides/core/config_variables.py @@ -401,3 +401,12 @@ def get_config_variables(): weight=1000, group="Custom translations", ) + + # Config version + yield ConfigVariable( + name="config_version", + input_type="integer", + default_value=1, + group="Version", + hidden=True, + ) diff --git a/tests/conftest.py b/tests/conftest.py index cc6e793af..7cb15e490 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -55,9 +55,10 @@ def pytest_collection_modifyitems(items): @pytest.fixture(autouse=True) -def constants(request): +def constants(request, reset_cache): """ - Resets the constants on every test. + Resets the constants on every test. The filled cache is needed to + build the constants, because some of them depends on the config. Uses fake constants, if the db is not in use. """