From bde0df219c326e039080dcc0f7248ba15cf76ffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jovan=20Kruni=C4=87?= Date: Wed, 9 Feb 2022 20:43:08 +0100 Subject: [PATCH] refactor: initialize config via APP_INITIALIZER Closes #181 --- src/app/app.component.ts | 19 ----------- src/app/app.module.ts | 14 +++++--- .../about/about-page/about-page.component.ts | 15 ++++----- .../modules/about/about-page/about-page.html | 4 +-- .../modules/config/config.provider.spec.ts | 2 -- src/app/modules/config/config.provider.ts | 33 ++++--------------- src/app/modules/map/map.module.ts | 8 +---- src/app/modules/map/map.provider.ts | 10 ++++-- src/app/modules/settings/settings.provider.ts | 4 +-- 9 files changed, 37 insertions(+), 72 deletions(-) diff --git a/src/app/app.component.ts b/src/app/app.component.ts index fd3d2e64..5ba5648c 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -17,8 +17,6 @@ import {Router} from '@angular/router'; import {App, URLOpenListenerEvent} from '@capacitor/app'; import {SplashScreen} from '@capacitor/splash-screen'; import {Platform, ToastController} from '@ionic/angular'; -import {NGXLogger} from 'ngx-logger'; -import {ConfigProvider} from './modules/config/config.provider'; import {SettingsProvider} from './modules/settings/settings.provider'; import {PAIAAuthService} from './modules/auth/paia/paia-auth.service'; import {DefaultAuthService} from './modules/auth/default-auth.service'; @@ -52,8 +50,6 @@ export class AppComponent implements AfterContentInit { * * @param platform TODO * @param settingsProvider TODO - * @param configProvider TODO - * @param logger An angular logger * @param router The angular router * @param zone The angular zone * @param defaultAuth Auth Service @@ -65,8 +61,6 @@ export class AppComponent implements AfterContentInit { constructor( private readonly platform: Platform, private readonly settingsProvider: SettingsProvider, - private readonly configProvider: ConfigProvider, - private readonly logger: NGXLogger, private readonly router: Router, private readonly zone: NgZone, private readonly defaultAuth: DefaultAuthService, @@ -102,19 +96,6 @@ export class AppComponent implements AfterContentInit { await this.paiaAuth.init(); await SplashScreen.hide(); - // initialise the configProvider - try { - await this.configProvider.init(); - } catch (error) { - if ( - typeof error.name !== 'undefined' && - error.name === 'ConfigInitError' - ) { - // TODO: Issue #43 handle initialisation error and inform user - } - this.logger.error(error); - } - // set order of categories in settings this.settingsProvider.setCategoriesOrder([ 'profile', diff --git a/src/app/app.module.ts b/src/app/app.module.ts index dbac2c4e..3a8f733c 100644 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -65,18 +65,21 @@ import {AuthModule} from './modules/auth/auth.module'; import {ScheduleSyncService} from './modules/background/schedule/schedule-sync.service'; import {BackgroundModule} from './modules/background/background.module'; import {LibraryModule} from './modules/library/library.module'; +import {StorageProvider} from './modules/storage/storage.provider'; registerLocaleData(localeDe); /** - * Initializes settings from Config before other components + * Initializes data needed on startup * + * @param storageProvider provider of the saved data (using framework's storage) * @param logger TODO * @param settingsProvider provider of settings (e.g. language that has been set) * @param configProvider TODO * @param translateService TODO */ -export function initSettingsFactory( +export function initializerFactory( + storageProvider: StorageProvider, logger: NGXLogger, settingsProvider: SettingsProvider, configProvider: ConfigProvider, @@ -84,10 +87,12 @@ export function initSettingsFactory( ) { return async () => { initLogger(logger); + await storageProvider.init(); + await configProvider.init(); await settingsProvider.init(); try { - // set language from settings if (configProvider.firstSession) { + // set language from browser await settingsProvider.setSettingValue( 'profile', 'language', @@ -184,13 +189,14 @@ export function createTranslateLoader(http: HttpClient) { provide: APP_INITIALIZER, multi: true, deps: [ + StorageProvider, NGXLogger, SettingsProvider, ConfigProvider, TranslateService, ScheduleSyncService, ], - useFactory: initSettingsFactory, + useFactory: initializerFactory, }, ], }) diff --git a/src/app/modules/about/about-page/about-page.component.ts b/src/app/modules/about/about-page/about-page.component.ts index a6777f16..62914167 100644 --- a/src/app/modules/about/about-page/about-page.component.ts +++ b/src/app/modules/about/about-page/about-page.component.ts @@ -23,7 +23,7 @@ import {ConfigProvider} from '../../config/config.provider'; styleUrls: ['about-page.scss'], }) export class AboutPageComponent implements OnInit { - content: Promise; + content: SCAboutPage; constructor( private readonly route: ActivatedRoute, @@ -33,12 +33,11 @@ export class AboutPageComponent implements OnInit { async ngOnInit() { const route = this.route.snapshot.url.map(it => it.path).join('/'); - this.content = new Promise(resolve => { - this.configProvider - .getValue('aboutPages') - .then(value => - resolve((value as SCAppConfiguration['aboutPages'])[route] ?? {}), - ); - }); + this.content = + ( + this.configProvider.getValue( + 'aboutPages', + ) as SCAppConfiguration['aboutPages'] + )[route] ?? {}; } } diff --git a/src/app/modules/about/about-page/about-page.html b/src/app/modules/about/about-page/about-page.html index 0cf832dd..524296da 100644 --- a/src/app/modules/about/about-page/about-page.html +++ b/src/app/modules/about/about-page/about-page.html @@ -19,7 +19,7 @@ - {{ + {{ 'title' | translateSimple: content }} @@ -29,7 +29,7 @@ - + { expect(storageProviderSpy.has).toHaveBeenCalled(); expect(storageProviderSpy.get).toHaveBeenCalledTimes(0); expect(configProvider.client.handshake).toHaveBeenCalled(); - expect(configProvider.initialised).toBe(true); expect(await configProvider.getValue('name')).toEqual( sampleIndexResponse.app.name, ); @@ -129,7 +128,6 @@ describe('ConfigProvider', () => { expect(error).toEqual(new ConfigFetchError()); expect(storageProviderSpy.has).toHaveBeenCalled(); expect(storageProviderSpy.get).toHaveBeenCalled(); - expect(configProvider.initialised).toBe(true); expect(await configProvider.getValue('name')).toEqual( sampleIndexResponse.app.name, ); diff --git a/src/app/modules/config/config.provider.ts b/src/app/modules/config/config.provider.ts index bb2f06e7..01053a64 100644 --- a/src/app/modules/config/config.provider.ts +++ b/src/app/modules/config/config.provider.ts @@ -50,21 +50,16 @@ export class ConfigProvider { */ config: SCIndexResponse; - /** - * First session indicator - */ - firstSession = true; - - /** - * Initialised status flag of config provider - */ - initialised = false; - /** * Version of the @openstapps/core package that app is using */ scVersion = packageJson.dependencies['@openstapps/core']; + /** + * First session indicator (config not found in storage) + */ + firstSession = true; + /** * Constructor, initialise api client * @@ -100,17 +95,7 @@ export class ConfigProvider { * * @param attribute requested attribute from app configuration */ - public async getValue(attribute: keyof SCAppConfiguration) { - if (!this.initialised) { - try { - await this.init(); - } catch (error) { - // don't throw ConfigFetchError if saved config is available - if (error.name === 'ConfigFetchError' && !this.initialised) { - throw error; - } - } - } + public getValue(attribute: keyof SCAppConfiguration) { if (typeof this.config.app[attribute] !== 'undefined') { return this.config.app[attribute]; } @@ -138,12 +123,10 @@ export class ConfigProvider { async init(): Promise { let loadError; let fetchError; - this.initialised = false; // load saved configuration try { this.config = await this.loadLocal(); this.firstSession = false; - this.initialised = true; this.logger.log(`initialised configuration from storage`); if (this.config.backend.SCVersion !== this.scVersion) { loadError = new WrongConfigVersionInStorage( @@ -159,7 +142,6 @@ export class ConfigProvider { try { const fetchedConfig: SCIndexResponse = await this.fetch(); await this.set(fetchedConfig); - this.initialised = true; this.logger.log(`initialised configuration from remote`); } catch (error) { fetchError = error; @@ -169,7 +151,7 @@ export class ConfigProvider { throw new ConfigInitError(); } if (typeof loadError !== 'undefined') { - throw loadError; + this.logger.warn(loadError); } if (typeof fetchError !== 'undefined') { throw fetchError; @@ -182,7 +164,6 @@ export class ConfigProvider { * @throws SavedConfigNotAvailable if no configuration could be loaded */ async loadLocal(): Promise { - await this.storageProvider.init(); // get local configuration if (await this.storageProvider.has(STORAGE_KEY_CONFIG)) { return this.storageProvider.get(STORAGE_KEY_CONFIG); diff --git a/src/app/modules/map/map.module.ts b/src/app/modules/map/map.module.ts index a7bd5060..c4caff23 100644 --- a/src/app/modules/map/map.module.ts +++ b/src/app/modules/map/map.module.ts @@ -13,7 +13,6 @@ * this program. If not, see . */ import {CommonModule} from '@angular/common'; -import {APP_INITIALIZER, NgModule} from '@angular/core'; import {FormsModule} from '@angular/forms'; import {RouterModule, Routes} from '@angular/router'; import {LeafletModule} from '@asymmetrik/ngx-leaflet'; @@ -34,6 +33,7 @@ import {MapPageComponent} from './page/map-page.component'; import {MapListModalComponent} from './page/modals/map-list-modal.component'; import {MapSingleModalComponent} from './page/modals/map-single-modal.component'; import {MapItemComponent} from './item/map-item.component'; +import {NgModule} from '@angular/core'; /** * Initializes the default area to show in advance (before components are initialized) @@ -86,12 +86,6 @@ const mapRoutes: Routes = [ DataProvider, DataFacetsProvider, StAppsWebHttpClient, - { - provide: APP_INITIALIZER, - multi: true, - deps: [ConfigProvider, MapProvider], - useFactory: initMapConfigFactory, - }, ], }) export class MapModule {} diff --git a/src/app/modules/map/map.provider.ts b/src/app/modules/map/map.provider.ts index 8737d617..b63f6941 100644 --- a/src/app/modules/map/map.provider.ts +++ b/src/app/modules/map/map.provider.ts @@ -26,6 +26,7 @@ import {divIcon, geoJSON, icon, LatLng, Map, marker, Marker} from 'leaflet'; import {DataProvider} from '../data/data.provider'; import {MapPosition, PositionService} from './position.service'; import {hasValidLocation} from '../data/types/place/place-types'; +import {ConfigProvider} from '../config/config.provider'; /** * Provides methods for presenting the map @@ -107,7 +108,12 @@ export class MapProvider { constructor( private dataProvider: DataProvider, private positionService: PositionService, - ) {} + private configProvider: ConfigProvider, + ) { + this.defaultPolygon = this.configProvider.getValue( + 'campusPolygon', + ) as Polygon; + } /** * Provide the specific place by its UID @@ -128,7 +134,7 @@ export class MapProvider { /** * Provide places (buildings and canteens) const result = await this.dataProvider.search(query); - + * * @param contextFilter Additional contextual filter (e.g. from the context menu) * @param queryText Query (text) of the search query diff --git a/src/app/modules/settings/settings.provider.ts b/src/app/modules/settings/settings.provider.ts index 214efc6c..7769483f 100644 --- a/src/app/modules/settings/settings.provider.ts +++ b/src/app/modules/settings/settings.provider.ts @@ -333,9 +333,9 @@ export class SettingsProvider { */ public async init(): Promise { try { - const settings: SCSetting[] = (await this.configProvider.getValue( + const settings: SCSetting[] = this.configProvider.getValue( 'settings', - )) as SCSetting[]; + ) as SCSetting[]; for (const setting of settings) this.addSetting(setting); for (const category of Object.keys(this.settingsCache)) {