Merge pull request #1598 from balena-io/1595-refactor-truthy-check

Refactor checkTruthy to return more predictable values

src/config/types.ts

@@ -3,7 +3,7 @@ import * as t from 'io-ts';
 import * as _ from 'lodash';
 
 import { InternalInconsistencyError } from '../lib/errors';
-import { checkTruthy } from '../lib/validation';
+import { checkBooleanish, checkTruthy } from '../lib/validation';
 
 const permissiveValue = t.union([
 	t.boolean,
@@ -23,11 +23,10 @@ export const PermissiveBoolean = new t.Type<boolean, t.TypeOf<PermissiveType>>(
 				case 'string':
 				case 'boolean':
 				case 'number':
-					const val = checkTruthy(v);
+					if (!checkBooleanish(v)) {
-					if (val == null) {
 						return t.failure(v, c);
 					}
-					return t.success(val);
+					return t.success(checkTruthy(v));
 				case 'undefined':
 					return t.success(false);
 				case 'object':

src/device-api/v1.ts

@@ -15,7 +15,7 @@ import { getApp } from '../device-state/db-format';
 export function createV1Api(router: express.Router) {
 	router.post('/v1/restart', (req: AuthorizedRequest, res, next) => {
 		const appId = checkInt(req.body.appId);
-		const force = checkTruthy(req.body.force) ?? false;
+		const force = checkTruthy(req.body.force);
 		eventTracker.track('Restart container (v1)', { appId });
 		if (appId == null) {
 			return res.status(400).send('Missing app id');
@@ -42,7 +42,7 @@ export function createV1Api(router: express.Router) {
 		action: 'start' | 'stop',
 	) => {
 		const appId = checkInt(req.params.appId);
-		const force = checkTruthy(req.body.force) ?? false;
+		const force = checkTruthy(req.body.force);
 		if (appId == null) {
 			return res.status(400).send('Missing app id');
 		}
@@ -168,7 +168,7 @@ export function createV1Api(router: express.Router) {
 
 	router.post('/v1/purge', (req: AuthorizedRequest, res, next) => {
 		const appId = checkInt(req.body.appId);
-		const force = checkTruthy(req.body.force) ?? false;
+		const force = checkTruthy(req.body.force);
 		if (appId == null) {
 			const errMsg = 'Invalid or missing appId';
 			return res.status(400).send(errMsg);

src/device-api/v2.ts

@@ -577,8 +577,8 @@ export function createV2Api(router: Router) {
 	});
 
 	router.post('/v2/journal-logs', (req, res) => {
-		const all = checkTruthy(req.body.all) || false;
+		const all = checkTruthy(req.body.all);
-		const follow = checkTruthy(req.body.follow) || false;
+		const follow = checkTruthy(req.body.follow);
 		const count = checkInt(req.body.count, { positive: true }) || undefined;
 		const unit = req.body.unit;
 		const format = req.body.format || 'short';

src/device-config.ts

@@ -633,10 +633,8 @@ async function isVPNEnabled(): Promise<boolean> {
 	}
 }
 
-async function setVPNEnabled(value?: string | boolean) {
+async function setVPNEnabled(value: string | boolean = true) {
-	const v = checkTruthy(value || true);
+	const enable = checkTruthy(value);
-	const enable = v != null ? v : true;
-
 	if (enable) {
 		await dbus.startService(vpnServiceName);
 	} else {

src/lib/contracts.ts

@@ -225,8 +225,7 @@ export function validateTargetContracts(
 
 					serviceContracts[svc.serviceName] = {
 						contract: svc.contract,
-						optional:
+						optional: checkTruthy(svc.labels?.['io.balena.features.optional']),
-							checkTruthy(svc.labels?.['io.balena.features.optional']) || false,
 					};
 				} catch (e) {
 					throw new ContractValidationError(svc.serviceName, e.message);

src/lib/validation.ts

@@ -13,6 +13,8 @@ export interface CheckIntOptions {
 const ENV_VAR_KEY_REGEX = /^[a-zA-Z_][a-zA-Z0-9_]*$/;
 const LABEL_NAME_REGEX = /^[a-zA-Z][a-zA-Z0-9\.\-]*$/;
 const NUMERALS_REGEX = /^-?[0-9]+\.?0*$/; // Allows trailing 0 decimals
+const TRUTHY = ['1', 'true', true, 'on', 1];
+const FALSEY = ['0', 'false', false, 'off', 0];
 
 /**
  * checkInt
@@ -55,32 +57,40 @@ export function checkString(s: unknown): string | void {
 	return s;
 }
 
+/**
+ * checkBooleanish
+ *
+ * Given an unknown value, determine if it can be evaluated to truthy/falsey.
+ *
+ */
+export function checkBooleanish(v: unknown): boolean {
+	return checkTruthy(v) || checkFalsey(v);
+}
+
 /**
  * checkTruthy
  *
- * Given a value which can be a string, boolean or number, return a boolean
+ * Given an unknown value, determine if it evaluates to true.
- * which represents if the input was truthy
+ *
  */
-export function checkTruthy(v: unknown): boolean | undefined {
+export function checkTruthy(v: unknown): boolean {
-	if (_.isString(v)) {
+	if (typeof v === 'string') {
 		v = v.toLowerCase();
 	}
-	switch (v) {
+	return TRUTHY.includes(v as any);
-		case '1':
+}
-		case 'true':
+
-		case true:
+/**
-		case 'on':
+ * checkFalsey
-		case 1:
+ *
-			return true;
+ * Given an unknown value, determine if it evaluates to false.
-		case '0':
+ *
-		case 'false':
+ */
-		case false:
+export function checkFalsey(v: unknown): boolean {
-		case 'off':
+	if (typeof v === 'string') {
-		case 0:
+		v = v.toLowerCase();
-			return false;
-		default:
-			return;
 	}
+	return FALSEY.includes(v as any);
 }
 
 /*

src/network.ts

@@ -7,7 +7,7 @@ import * as url from 'url';
 
 import * as constants from './lib/constants';
 import { EEXIST } from './lib/errors';
-import { checkTruthy } from './lib/validation';
+import { checkFalsey } from './lib/validation';
 
 import blink = require('./lib/blink');
 
@@ -106,8 +106,8 @@ export const startConnectivityCheck = _.once(
 );
 
 export function enableConnectivityCheck(enable: boolean) {
-	const boolEnable = checkTruthy(enable);
+	// Only disable if value explicitly matches falsey
-	enable = boolEnable != null ? boolEnable : true;
+	enable = !checkFalsey(enable);
 	enableCheck(enable);
 	log.debug(`Connectivity check enabled: ${enable}`);
 }

test/06-validation.spec.ts

@@ -6,6 +6,59 @@ import * as validation from '../src/lib/validation';
 const almostTooLongText = _.times(255, () => 'a').join('');
 
 describe('validation', () => {
+	describe('checkBooleanish', () => {
+		it('returns true for a truthy or falsey value', () => {
+			expect(validation.checkBooleanish(true)).to.equal(true);
+			expect(validation.checkBooleanish('true')).to.equal(true);
+			expect(validation.checkBooleanish('1')).to.equal(true);
+			expect(validation.checkBooleanish(1)).to.equal(true);
+			expect(validation.checkBooleanish('on')).to.equal(true);
+			expect(validation.checkBooleanish(false)).to.equal(true);
+			expect(validation.checkBooleanish('false')).to.equal(true);
+			expect(validation.checkBooleanish('0')).to.equal(true);
+			expect(validation.checkBooleanish(0)).to.equal(true);
+			expect(validation.checkBooleanish('off')).to.equal(true);
+		});
+
+		it('returns false for invalid values', () => {
+			expect(validation.checkBooleanish({})).to.equal(false);
+			expect(validation.checkBooleanish(10)).to.equal(false);
+			expect(validation.checkBooleanish('on1')).to.equal(false);
+			expect(validation.checkBooleanish('foo')).to.equal(false);
+			expect(validation.checkBooleanish(undefined)).to.equal(false);
+			expect(validation.checkBooleanish(null)).to.equal(false);
+			expect(validation.checkBooleanish('')).to.equal(false);
+		});
+	});
+
+	describe('checkFalsey', () => {
+		it('returns false for a truthy value', () => {
+			expect(validation.checkFalsey(true)).to.equal(false);
+			expect(validation.checkFalsey('true')).to.equal(false);
+			expect(validation.checkFalsey('1')).to.equal(false);
+			expect(validation.checkFalsey(1)).to.equal(false);
+			expect(validation.checkFalsey('on')).to.equal(false);
+		});
+
+		it('returns true for a falsey value', () => {
+			expect(validation.checkFalsey(false)).to.equal(true);
+			expect(validation.checkFalsey('false')).to.equal(true);
+			expect(validation.checkFalsey('0')).to.equal(true);
+			expect(validation.checkFalsey(0)).to.equal(true);
+			expect(validation.checkFalsey('off')).to.equal(true);
+		});
+
+		it('returns false for invalid values', () => {
+			expect(validation.checkFalsey({})).to.equal(false);
+			expect(validation.checkFalsey(10)).to.equal(false);
+			expect(validation.checkFalsey('on1')).to.equal(false);
+			expect(validation.checkFalsey('foo')).to.equal(false);
+			expect(validation.checkFalsey(undefined)).to.equal(false);
+			expect(validation.checkFalsey(null)).to.equal(false);
+			expect(validation.checkFalsey('')).to.equal(false);
+		});
+	});
+
 	describe('checkTruthy', () => {
 		it('returns true for a truthy value', () => {
 			expect(validation.checkTruthy(true)).to.equal(true);
@@ -15,7 +68,7 @@ describe('validation', () => {
 			expect(validation.checkTruthy('on')).to.equal(true);
 		});
 
-		it('returns false for a falsy value', () => {
+		it('returns false for a falsey value', () => {
 			expect(validation.checkTruthy(false)).to.equal(false);
 			expect(validation.checkTruthy('false')).to.equal(false);
 			expect(validation.checkTruthy('0')).to.equal(false);
@@ -23,14 +76,14 @@ describe('validation', () => {
 			expect(validation.checkTruthy('off')).to.equal(false);
 		});
 
-		it('returns undefined for invalid values', () => {
+		it('returns false for invalid values', () => {
-			expect(validation.checkTruthy({})).to.be.undefined;
+			expect(validation.checkTruthy({})).to.equal(false);
-			expect(validation.checkTruthy(10)).to.be.undefined;
+			expect(validation.checkTruthy(10)).to.equal(false);
-			expect(validation.checkTruthy('on1')).to.be.undefined;
+			expect(validation.checkTruthy('on1')).to.equal(false);
-			expect(validation.checkTruthy('foo')).to.be.undefined;
+			expect(validation.checkTruthy('foo')).to.equal(false);
-			expect(validation.checkTruthy(undefined)).to.be.undefined;
+			expect(validation.checkTruthy(undefined)).to.equal(false);
-			expect(validation.checkTruthy(null)).to.be.undefined;
+			expect(validation.checkTruthy(null)).to.equal(false);
-			expect(validation.checkTruthy('')).to.be.undefined;
+			expect(validation.checkTruthy('')).to.equal(false);
 		});
 	});