Remove side effects for module imports

The supervisor uses the following pattern for async module
initialization

```typescript
// module.ts

export const initialised = (async () => {
    // do some async initialization
})();

// somewhere else
import * as module from 'module';

async function setup() {
  await module.initialise;
}
```

The above pattern means that whenever the module is imported, the
initialisation procedure will be ran, which is an anti-pattern.

This converts any instance of this pattern into a function

```typescript
export const initialised = _.once(async () => {
    // do some async initialization
});
```

And anywhere else on the code it replaces the call with a

```typescript
await module.initialised();
```

Change-type: patch
This commit is contained in:
Felipe Lalanne 2022-09-06 14:03:23 -04:00
parent 6b36ccfddf
commit 48e0733c7e
38 changed files with 122 additions and 119 deletions

View File

@ -547,10 +547,10 @@ async function reportInitialName(
export let balenaApi: PinejsClientRequest | null = null;
export const initialized = (async () => {
await config.initialized;
await eventTracker.initialized;
await deviceState.initialized;
export const initialized = _.once(async () => {
await config.initialized();
await eventTracker.initialized();
await deviceState.initialized();
const { unmanaged, apiEndpoint, currentApiKey } = await config.getMany([
'unmanaged',
@ -573,7 +573,7 @@ export const initialized = (async () => {
});
log.info(`API Binder bound to: ${baseUrl}`);
})();
});
export const router = express.Router();
router.use(express.urlencoded({ limit: '10mb', extended: true }));

View File

@ -115,8 +115,8 @@ let targetVolatilePerImageId: {
[imageId: number]: Partial<Service['config']>;
} = {};
export const initialized = (async () => {
await config.initialized;
export const initialized = _.once(async () => {
await config.initialized();
await imageManager.cleanImageData();
const cleanup = async () => {
@ -140,7 +140,7 @@ export const initialized = (async () => {
imageManager.on('change', reportCurrentState);
serviceManager.on('change', reportCurrentState);
})();
});
export function getDependentState() {
return proxyvisor.getCurrentStates();

View File

@ -142,7 +142,7 @@ export class ConfigFs extends ConfigBackend {
log.success('Initialised ConfigFS');
} catch (error) {
log.error(error);
await logger.initialized;
await logger.initialized();
logger.logSystemMessage(
'Unable to initialise ConfigFS',
{ error },

View File

@ -343,7 +343,7 @@ function checkValueDecode(
return true;
}
export const initialized = (async () => {
await db.initialized;
export const initialized = _.once(async () => {
await db.initialized();
await generateRequiredFields();
})();
});

View File

@ -1,5 +1,6 @@
import * as Knex from 'knex';
import * as path from 'path';
import * as _ from 'lodash';
import * as constants from './lib/constants';
@ -16,7 +17,7 @@ const knex = Knex({
useNullAsDefault: true,
});
export const initialized = (async () => {
export const initialized = _.once(async () => {
try {
await knex('knex_migrations_lock').update({ is_locked: 0 });
} catch {
@ -25,7 +26,7 @@ export const initialized = (async () => {
return knex.migrate.latest({
directory: path.join(__dirname, 'migrations'),
});
})();
});
export function models(modelName: string): Knex.QueryBuilder {
return knex(modelName);

View File

@ -11,8 +11,8 @@ import { lock } from '../lib/update-lock';
import { appNotFoundMessage } from '../lib/messages';
export async function doRestart(appId, force) {
await deviceState.initialized;
await applicationManager.initialized;
await deviceState.initialized();
await applicationManager.initialized();
return lock(appId, { force }, () =>
deviceState.getCurrentState().then(function (currentState) {
@ -38,8 +38,8 @@ export async function doRestart(appId, force) {
}
export async function doPurge(appId, force) {
await deviceState.initialized;
await applicationManager.initialized;
await deviceState.initialized();
await applicationManager.initialized();
logger.logSystemMessage(
`Purging data for app ${appId}`,

View File

@ -285,9 +285,9 @@ events.on('apply-target-state-end', function (err) {
}
});
export const initialized = (async () => {
await config.initialized;
await applicationManager.initialized;
export const initialized = _.once(async () => {
await config.initialized();
await applicationManager.initialized();
applicationManager.on('change', (d) => reportCurrentState(d));
createDeviceStateRouter();
@ -301,7 +301,7 @@ export const initialized = (async () => {
maxPollTime = changedConfig.appUpdatePollInterval;
}
});
})();
});
export function isApplyInProgress() {
return applyInProgress;
@ -374,8 +374,8 @@ async function saveInitialConfig() {
}
export async function loadInitialState() {
await applicationManager.initialized;
await apiKeys.initialized;
await applicationManager.initialized();
await apiKeys.initialized();
const conf = await config.getMany([
'initialConfigSaved',
@ -466,8 +466,8 @@ function usingInferStepsLock<T extends () => any, U extends ReturnType<T>>(
}
export async function setTarget(target: TargetState, localSource?: boolean) {
await db.initialized;
await config.initialized;
await db.initialized();
await config.initialized();
// When we get a new target state, clear any built up apply errors
// This means that we can attempt to apply the new state instantly

View File

@ -147,7 +147,7 @@ export async function getTargetJson(): Promise<TargetApps> {
function getDBEntry(): Promise<DatabaseApp[]>;
function getDBEntry(appId: number): Promise<DatabaseApp>;
async function getDBEntry(appId?: number) {
await targetStateCache.initialized;
await targetStateCache.initialized();
return appId != null
? targetStateCache.getTargetApp(appId)

View File

@ -52,9 +52,9 @@ export type DatabaseService = {
*/
let targetState: DatabaseApp[] | undefined;
export const initialized = (async () => {
await db.initialized;
await config.initialized;
export const initialized = _.once(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)
@ -63,7 +63,7 @@ export const initialized = (async () => {
targetState = undefined;
}
});
})();
});
export async function getTargetApp(
appId: number,

View File

@ -45,18 +45,6 @@ let cache: CachedResponse;
*/
let appUpdatePollInterval: number;
/**
* Listen for config changes to appUpdatePollInterval
*/
(async () => {
await config.initialized;
config.on('change', (changedConfig) => {
if (changedConfig.appUpdatePollInterval) {
appUpdatePollInterval = changedConfig.appUpdatePollInterval;
}
});
})();
/**
* Emit target state event based on if the CacheResponse has/was emitted.
*
@ -115,7 +103,7 @@ export const update = async (
force = false,
isFromApi = false,
): Promise<void> => {
await config.initialized;
await config.initialized();
return Bluebird.using(lockGetTarget(), async () => {
const {
uuid,
@ -228,6 +216,17 @@ export const get = async (): Promise<TargetState> => {
export const startPoll = async (): Promise<void> => {
let instantUpdates;
try {
await config.initialized();
/**
* Listen for config changes to appUpdatePollInterval
*/
config.on('change', (changedConfig) => {
if (changedConfig.appUpdatePollInterval) {
appUpdatePollInterval = changedConfig.appUpdatePollInterval;
}
});
// Query and set config values we need to avoid multiple db hits
const {
instantUpdates: updates,

View File

@ -29,8 +29,8 @@ let defaultProperties: EventTrackProperties;
// to it within the rest of the supervisor codebase
export let client: mixpanel.Mixpanel | null = null;
export const initialized = (async () => {
await config.initialized;
export const initialized = _.once(async () => {
await config.initialized();
const {
unmanaged,
@ -57,13 +57,13 @@ export const initialized = (async () => {
host: mixpanelHost.host,
path: mixpanelHost.path,
});
})();
});
export async function track(
event: string,
properties: EventTrackProperties | Error = {},
) {
await initialized;
await initialized();
if (properties instanceof Error) {
properties = { error: properties };

View File

@ -153,8 +153,8 @@ export const provision = async (
balenaApi: PinejsClientRequest,
opts: KeyExchangeOpts,
) => {
await config.initialized;
await eventTracker.initialized;
await config.initialized();
await eventTracker.initialized();
let device: Device | null = null;

View File

@ -92,12 +92,12 @@ export type AuthorizedRequestHandler = (
export let cloudApiKey: string = '';
// should be called before trying to use this singleton
export const initialized = (async () => {
await db.initialized;
export const initialized = _.once(async () => {
await db.initialized();
// make sure we have an API key which the cloud will use to call us
await generateCloudKey();
})();
});
/**
* This middleware will extract an API key used to make a call, and then expand it out to provide
@ -139,7 +139,7 @@ export const authMiddleware: AuthorizedRequestHandler = async (
// if we have a key, find the scopes and add them to the request
if (apiKey && apiKey !== '') {
await initialized;
await initialized();
const scopes = await getScopesForKey(apiKey);
if (scopes != null) {
@ -200,7 +200,7 @@ export async function generateScopedKey(
serviceName: string,
options?: Partial<GenerateKeyOptions>,
): Promise<string> {
await initialized;
await initialized();
return await generateKey(appId, serviceName, options);
}
@ -243,7 +243,7 @@ export async function refreshKey(key: string): Promise<string> {
*/
const getApiKeyForService = memoizee(
async (appId: number, serviceName: string | null): Promise<DbApiSecret[]> => {
await db.initialized;
await db.initialized();
return await db.models('apiSecret').where({ appId, serviceName }).select();
},
@ -259,7 +259,7 @@ const getApiKeyForService = memoizee(
*/
const getApiKeyByKey = memoizee(
async (key: string): Promise<DbApiSecret> => {
await db.initialized;
await db.initialized();
const [apiKey] = await db.models('apiSecret').where({ key }).select();
return apiKey;

View File

@ -1,10 +1,12 @@
import * as config from '../config';
import * as dbus from './dbus';
import * as _ from 'lodash';
import log from './supervisor-console';
export const initialized = (async () => {
await config.initialized;
export const initialized = _.once(async () => {
await config.initialized();
config.on('change', (conf) => {
if (conf.hostDiscoverability != null) {
@ -13,7 +15,7 @@ export const initialized = (async () => {
});
await switchDiscoverability(await config.get('hostDiscoverability'));
})();
});
async function switchDiscoverability(discoverable: boolean) {
try {

View File

@ -8,8 +8,8 @@ import { logSystemMessage } from '../logger';
import * as dbFormat from '../device-state/db-format';
export const initialised = (async () => {
await config.initialized;
export const initialised = _.once(async () => {
await config.initialized();
await applyFirewall();
// apply firewall whenever relevant config changes occur...
@ -18,7 +18,7 @@ export const initialised = (async () => {
applyFirewall({ firewallMode, localMode });
}
});
})();
});
const BALENA_FIREWALL_CHAIN = 'BALENA-FIREWALL';

View File

@ -61,8 +61,8 @@ async function createVolumeFromLegacyData(
* Gets proper database ids from the cloud for the app and app services
*/
export async function normaliseLegacyDatabase() {
await apiBinder.initialized;
await deviceState.initialized;
await apiBinder.initialized();
await deviceState.initialized();
if (apiBinder.balenaApi == null) {
throw new InternalInconsistencyError(
@ -217,7 +217,7 @@ export async function normaliseLegacyDatabase() {
await serviceManager.killAllLegacy();
log.debug('Migrating legacy app volumes');
await applicationManager.initialized;
await applicationManager.initialized();
const targetApps = await applicationManager.getTargetApps();
for (const appUuid of Object.keys(targetApps)) {
@ -275,8 +275,8 @@ export async function fromV2TargetApps(
apps: TargetAppsV2,
local = false,
): Promise<TargetApps> {
await apiBinder.initialized;
await deviceState.initialized;
await apiBinder.initialized();
await deviceState.initialized();
if (apiBinder.balenaApi == null) {
throw new InternalInconsistencyError(

View File

@ -24,8 +24,8 @@ let backend: LogBackend | null = null;
let balenaBackend: BalenaLogBackend | null = null;
let localBackend: LocalLogBackend | null = null;
export const initialized = (async () => {
await config.initialized;
export const initialized = _.once(async () => {
await config.initialized();
const {
apiEndpoint,
uuid,
@ -71,7 +71,7 @@ export const initialized = (async () => {
}
});
}
})();
});
export function switchBackend(localMode: boolean) {
if (localMode) {

View File

@ -367,7 +367,8 @@ export class Proxyvisor {
this.router = createProxyvisorRouter(this);
this.actionExecutors = {
updateDependentTargets: (step) => {
return config.initialized
return config
.initialized()
.then(() => config.getMany(['currentApiKey', 'apiTimeout']))
.then(({ currentApiKey, apiTimeout }) => {
// - take each of the step.devices and update dependentDevice with it (targetCommit, targetEnvironment, targetConfig)

View File

@ -80,8 +80,8 @@ export class SupervisorAPI {
this.api.post(
'/v1/regenerate-api-key',
async (req: apiKeys.AuthorizedRequest, res) => {
await deviceState.initialized;
await apiKeys.initialized;
await deviceState.initialized();
await apiKeys.initialized();
// check if we're updating the cloud API key
const updateCloudKey = req.auth.apiKey === apiKeys.cloudApiKey;

View File

@ -34,12 +34,12 @@ export class Supervisor {
public async init() {
log.info(`Supervisor v${version} starting up...`);
await db.initialized;
await config.initialized;
await eventTracker.initialized;
await avahi.initialized;
await db.initialized();
await config.initialized();
await eventTracker.initialized();
await avahi.initialized();
log.debug('Starting logging infrastructure');
await logger.initialized;
await logger.initialized();
const conf = await config.getMany(startupConfigFields);
@ -50,12 +50,12 @@ export class Supervisor {
});
log.info('Starting firewall');
await firewall.initialised;
await firewall.initialised();
log.debug('Starting api binder');
await apiBinder.initialized;
await apiBinder.initialized();
await deviceState.initialized;
await deviceState.initialized();
logger.logSystemMessage('Supervisor starting', {}, 'Supervisor start');
if (conf.legacyAppsPresent && apiBinder.balenaApi != null) {

View File

@ -59,7 +59,7 @@ describe('Database Migrations', () => {
delete require.cache[require.resolve('~/src/db')];
const testDb = await import('~/src/db');
await testDb.initialized;
await testDb.initialized();
expect(await exists(databasePath)).to.be.true;
});
@ -71,7 +71,7 @@ describe('Database Migrations', () => {
constants.databasePath = databasePath;
delete require.cache[require.resolve('~/src/db')];
const testDb = await import('~/src/db');
await testDb.initialized;
await testDb.initialized();
await Bluebird.all([
expect(knexForDB.schema.hasColumn('app', 'appId')).to.eventually.be.true,
expect(knexForDB.schema.hasColumn('app', 'releaseId')).to.eventually.be
@ -104,7 +104,7 @@ describe('Database', () => {
db = await import('~/src/db');
});
it('initializes correctly, running the migrations', () => {
return expect(db.initialized).to.be.fulfilled;
return expect(db.initialized()).to.be.fulfilled;
});
it('creates a database at the path from an env var', async () => {
expect(await exists(process.env.DATABASE_PATH!)).to.be.true;

View File

@ -13,7 +13,7 @@ import { fnSchema } from '~/src/config/functions';
describe('Config', () => {
before(async () => {
await prepare();
await conf.initialized;
await conf.initialized();
});
it('reads and exposes values from the config.json', async () => {

View File

@ -44,7 +44,7 @@ describe('device-state', () => {
before(async () => {
testDb = await dbHelper.createDB();
await config.initialized;
await config.initialized();
// Prevent side effects from changes in config
sinon.stub(config, 'on');
@ -52,7 +52,7 @@ describe('device-state', () => {
// Set the device uuid
await config.set({ uuid: 'local' });
await deviceState.initialized;
await deviceState.initialized();
// disable log output during testing
sinon.stub(log, 'debug');

View File

@ -46,7 +46,7 @@ describe('EventTracker', () => {
});
it('initializes in unmanaged mode', () => {
expect(eventTracker.initialized).to.be.fulfilled.then(() => {
expect(eventTracker.initialized()).to.be.fulfilled.then(() => {
expect(eventTracker.client).to.be.null;
});
});
@ -89,7 +89,7 @@ describe('EventTracker', () => {
});
it('initializes a mixpanel client when not in unmanaged mode', () => {
expect(eventTracker.initialized).to.be.fulfilled.then(() => {
expect(eventTracker.initialized()).to.be.fulfilled.then(() => {
expect(mixpanel.init).to.have.been.calledWith('someToken');
// @ts-ignore
expect(eventTracker.client.token).to.equal('someToken');
@ -120,7 +120,7 @@ describe('EventTracker', () => {
} as any);
eventTracker = await import('~/src/event-tracker');
await eventTracker.initialized;
await eventTracker.initialized();
});
after(() => {
@ -197,7 +197,7 @@ describe('EventTracker', () => {
track: stub(),
} as any);
eventTracker = await import('~/src/event-tracker');
await eventTracker.initialized;
await eventTracker.initialized();
});
after(() => {

View File

@ -41,10 +41,10 @@ const initModels = async (obj: Dictionary<any>, filename: string) => {
} as any;
ApiBinder = await import('~/src/api-binder');
await ApiBinder.initialized;
await ApiBinder.initialized();
obj.apiBinder = ApiBinder;
await deviceState.initialized;
await deviceState.initialized();
obj.deviceState = deviceState;
};

View File

@ -38,7 +38,7 @@ describe('Logger', function () {
// delete the require cache for the logger module so we can force a refresh
delete require.cache[require.resolve('~/src/logger')];
logger = await import('~/src/logger');
await logger.initialized;
await logger.initialized();
});
afterEach(function () {

View File

@ -19,7 +19,7 @@ describe('Startup', () => {
startStub = stub(apiBinder as any, 'start').resolves();
deviceStateStub = stub(deviceState, 'applyTarget').resolves();
// @ts-expect-error
applicationManager.initialized = Promise.resolve();
applicationManager.initialized = () => Promise.resolve();
vpnStatusPathStub = stub(constants, 'vpnStatusPath').returns('');
dockerStub = stub(docker, 'listContainers').returns(Promise.resolve([]));
});

View File

@ -21,8 +21,8 @@ describe('SupervisorAPI', () => {
const request = supertest(`http://127.0.0.1:${mockedOptions.listenPort}`);
before(async () => {
await apiBinder.initialized;
await deviceState.initialized;
await apiBinder.initialized();
await deviceState.initialized();
// The mockedAPI contains stubs that might create unexpected results
// See the module to know what has been stubbed
@ -32,7 +32,7 @@ describe('SupervisorAPI', () => {
await api.listen(mockedOptions.listenPort, mockedOptions.timeout);
// Create a scoped key
await apiKeys.initialized;
await apiKeys.initialized();
await apiKeys.generateCloudKey();
});
@ -109,7 +109,7 @@ describe('SupervisorAPI', () => {
const scopes = await apiKeys.getScopesForKey(apiKeys.cloudApiKey);
const key = 'not-a-normal-key';
await db.initialized;
await db.initialized();
await db
.models('apiSecret')
.update({

View File

@ -29,7 +29,7 @@ describe('LocalModeManager', () => {
});
before(async () => {
await db.initialized;
await db.initialized();
dockerStub = sinon.stub(docker);

View File

@ -21,7 +21,7 @@ describe('db-format', () => {
before(async () => {
testDb = await dbHelper.createDB();
await config.initialized;
await config.initialized();
// Prevent side effects from changes in config
sinon.stub(config, 'on');

View File

@ -40,8 +40,8 @@ describe('Host Firewall', function () {
},
} as Docker.Image);
await targetStateCache.initialized;
await firewall.initialised;
await targetStateCache.initialized();
await firewall.initialised();
apiEndpoint = await config.get('apiEndpoint');
listenPort = await config.get('listenPort');

View File

@ -4,7 +4,7 @@ import * as commitStore from '~/src/compose/commit';
import * as db from '~/src/db';
describe('compose/commit', () => {
before(async () => await db.initialized);
before(async () => await db.initialized());
describe('Fetching commits', () => {
beforeEach(async () => {

View File

@ -80,9 +80,9 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
});
before(async () => {
await apiBinder.initialized;
await deviceState.initialized;
await targetStateCache.initialized;
await apiBinder.initialized();
await deviceState.initialized();
await targetStateCache.initialized();
// Do not apply target state
stub(deviceState, 'applyStep').resolves();
@ -107,7 +107,7 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
targetStateCacheMock = stub(targetStateCache, 'getTargetApp');
// Create a scoped key
await apiKeys.initialized;
await apiKeys.initialized();
await apiKeys.generateCloudKey();
// Stub logs for all API methods

View File

@ -32,8 +32,8 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
let loggerStub: SinonStub;
before(async () => {
await apiBinder.initialized;
await deviceState.initialized;
await apiBinder.initialized();
await deviceState.initialized();
// The mockedAPI contains stubs that might create unexpected results
// See the module to know what has been stubbed
@ -46,7 +46,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
);
// Create a scoped key
await apiKeys.initialized;
await apiKeys.initialized();
await apiKeys.generateCloudKey();
serviceManagerMock = stub(serviceManager, 'getAll').resolves([]);
imagesMock = stub(images, 'getState').resolves([]);

View File

@ -203,7 +203,7 @@ describe('compose/application-manager', () => {
});
it('should init', async () => {
await applicationManager.initialized;
await applicationManager.initialized();
});
// TODO: missing tests for getCurrentApps

View File

@ -16,7 +16,7 @@ export async function createDB() {
delete require.cache[require.resolve('~/src/db')];
// Initialize the database module
await db.initialized;
await db.initialized();
// Get the knex instance to allow queries to the db
const { models, upsertModel } = db;

View File

@ -157,8 +157,8 @@ async function cleanUp(): Promise<void> {
}
async function createAPIOpts(): Promise<void> {
await db.initialized;
await deviceState.initialized;
await db.initialized();
await deviceState.initialized();
// Initialize and set values for mocked Config
await initConfig();
@ -166,7 +166,7 @@ async function createAPIOpts(): Promise<void> {
async function initConfig(): Promise<void> {
// Initialize this config
await config.initialized;
await config.initialized();
// Set a currentCommit
for (const [id, commit] of Object.entries(STUBBED_VALUES.commits)) {

View File

@ -3,8 +3,8 @@ import * as db from '~/src/db';
import * as config from '~/src/config';
export = async function () {
await db.initialized;
await config.initialized;
await db.initialized();
await config.initialized();
await db.transaction(async (trx) => {
const result = await trx.raw(`