Make target-state-cache a singleton

Change-type: patch
Signed-off-by: Cameron Diver <cameron@balena.io>

src/application-manager.js

@@ -24,7 +24,7 @@ import {
 } from './lib/errors';
 import { pathExistsOnHost } from './lib/fs-utils';
 
-import { TargetStateAccessor } from './device-state/target-state-cache';
+import * as targetStateCache from './device-state/target-state-cache';
 
 import { ServiceManager } from './compose/service-manager';
 import { Service } from './compose/service';
@@ -185,8 +185,6 @@ export class ApplicationManager extends EventEmitter {
 		this._targetVolatilePerImageId = {};
 		this._containerStarted = {};
 
-		this.targetStateWrapper = new TargetStateAccessor(this);
-
 		this.actionExecutors = compositionSteps.getExecutors({
 			lockFn: this._lockingIfNecessary,
 			services: this.services,
@@ -225,10 +223,9 @@ export class ApplicationManager extends EventEmitter {
 		return this.emit('change', data);
 	}
 
-	init() {
+	async init() {
-		return Images.initialized
+		await Images.initialized;
-			.then(() => Images.cleanupDatabase())
+		await Images.cleanupDatabase();
-			.then(() => {
 		const cleanup = () => {
 			return docker.listContainers({ all: true }).then((containers) => {
 				return logger.clearOutOfDateDBLogs(_.map(containers, 'Id'));
@@ -242,17 +239,12 @@ export class ApplicationManager extends EventEmitter {
 		// Once a day
 		setInterval(cleanup, 1000 * 60 * 60 * 24);
 		// But also run it in on startup
-				return cleanup();
+		await cleanup();
-			})
+		await this.localModeManager.init();
-			.then(() => {
+		await this.services.attachToRunning();
-				return this.localModeManager.init();
+		this.services.listenToEvents();
-			})
+
-			.then(() => {
+		await targetStateCache.initialized;
-				return this.services.attachToRunning();
-			})
-			.then(() => {
-				return this.services.listenToEvents();
-			});
 	}
 
 	// Returns the status of applications and their services
@@ -409,7 +401,7 @@ export class ApplicationManager extends EventEmitter {
 	}
 
 	getTargetApp(appId) {
-		return this.targetStateWrapper.getTargetApp(appId).then((app) => {
+		return targetStateCache.getTargetApp(appId).then((app) => {
 			if (app == null) {
 				return;
 			}
@@ -1129,7 +1121,7 @@ export class ApplicationManager extends EventEmitter {
 				});
 				return Promise.map(appsArray, this.normaliseAppForDB)
 					.then((appsForDB) => {
-						return this.targetStateWrapper.setTargetApps(appsForDB, trx);
+						return targetStateCache.setTargetApps(appsForDB, trx);
 					})
 					.then(() =>
 						trx('app')
@@ -1219,7 +1211,7 @@ export class ApplicationManager extends EventEmitter {
 
 	getTargetApps() {
 		return Promise.map(
-			this.targetStateWrapper.getTargetApps(),
+			targetStateCache.getTargetApps(),
 			this.normaliseAndExtendAppFromDB,
 		)
 			.map((app) => {

src/device-state/target-state-cache.ts

@@ -4,71 +4,78 @@ import { ApplicationManager } from '../application-manager';
 import * as config from '../config';
 import * as db from '../db';
 
-// Once we have correct types for both applications and the
+// We omit the id (which does appear in the db) in this type, as we don't use it
-// incoming target state this should be changed
+// at all, and we can use the below type for both insertion and retrieval.
-export type DatabaseApp = Dictionary<any>;
+export interface DatabaseApp {
+	name: string;
+	releaseId: number;
+	commit: string;
+	appId: number;
+	services: string;
+	networks: string;
+	volumes: string;
+	source: string;
+}
 export type DatabaseApps = DatabaseApp[];
 
 /*
- * This class is a wrapper around the database setting and
+ * This module is a wrapper around the database fetching and retrieving of
- * receiving of target state. Because the target state can
+ * target state. Because the target state can only be set only be set from a
- * only be set from a single place, but several workflows
+ * single place, but several workflows rely on getting the target state at one
- * rely on getting the target state at one point or another,
+ * point or another, we cache the values using this class. Accessing the
- * we cache the values using this class. Accessing the
+ * database is inherently expensive, and for example the local log backend
- * database is inherently expensive, and for example the
+ * accesses the target state for every log line. This can very quickly cause
- * local log backend accesses the target state for every log
+ * serious memory problems and database connection timeouts.
- * line. This can very quickly cause serious memory problems
- * and database connection timeouts.
  */
-export class TargetStateAccessor {
+let targetState: DatabaseApps | undefined;
-	private targetState?: DatabaseApps;
 
-	public constructor(protected applications: ApplicationManager) {
+export const initialized = (async () => {
+	await db.initialized;
+	await config.initialized;
 	// If we switch backend, the target state also needs to
 	// be invalidated (this includes switching to and from
 	// local mode)
 	config.on('change', (conf) => {
 		if (conf.apiEndpoint != null || conf.localMode != null) {
-				this.targetState = undefined;
+			targetState = undefined;
 		}
 	});
-	}
+})();
 
-	public async getTargetApp(appId: number): Promise<DatabaseApp | undefined> {
+export async function getTargetApp(
-		if (this.targetState == null) {
+	appId: number,
+): Promise<DatabaseApp | undefined> {
+	if (targetState == null) {
 		// TODO: Perhaps only fetch a single application from
 		// the DB, at the expense of repeating code
-			await this.getTargetApps();
+		await getTargetApps();
 	}
 
-		return _.find(this.targetState, (app) => app.appId === appId);
+	return _.find(targetState, (app) => app.appId === appId);
 }
 
-	public async getTargetApps(): Promise<DatabaseApps> {
+export async function getTargetApps(): Promise<DatabaseApps> {
-		if (this.targetState == null) {
+	if (targetState == null) {
 		const { apiEndpoint, localMode } = await config.getMany([
 			'apiEndpoint',
 			'localMode',
 		]);
 
 		const source = localMode ? 'local' : apiEndpoint;
-			this.targetState = await db.models('app').where({ source });
+		targetState = await db.models('app').where({ source });
 	}
-		return this.targetState!;
+	return targetState!;
 }
 
-	public async setTargetApps(
+export async function setTargetApps(
 	apps: DatabaseApps,
-		trx: db.Transaction,
+	trx?: db.Transaction,
 ): Promise<void> {
 	// We can't cache the value here, as it could be for a
 	// different source
-		this.targetState = undefined;
+	targetState = undefined;
 
 	await Promise.all(
 		apps.map((app) => db.upsertModel('app', app, { appId: app.appId }, trx)),
 	);
 }
-}
-
-export default TargetStateAccessor;