chore: refactoring & code quality improvements

From SonarCloud suggestions

README.md

@@ -2,6 +2,7 @@
 
 [![Tests](https://github.com/nrkno/sofie-mos-connection/actions/workflows/node.yaml/badge.svg)](https://github.com/nrkno/sofie-mos-connection/actions/workflows/node.yaml)
 [![codecov](https://codecov.io/gh/nrkno/sofie-mos-connection/branch/master/graph/badge.svg?token=LQL02uXajF)](https://codecov.io/gh/nrkno/sofie-mos-connection)
+[![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=nrkno_tv-automation-mos-connection&metric=alert_status)](https://sonarcloud.io/summary/new_code?id=nrkno_tv-automation-mos-connection)
 
 This is the _MOS-Connection_ library of the [**Sofie** TV Automation System](https://github.com/nrkno/Sofie-TV-automation/), used for connecting to a _MOS_ device using the [MOS Protocol](http://mosprotocol.com/).
 

packages/connector/src/MosConnection.ts

@@ -13,9 +13,9 @@ import { IConnectionConfig, IMosConnection, IMOSDeviceConnectionOptions } from '
 import { PROFILE_VALIDNESS_CHECK_WAIT_TIME } from './lib'
 
 export class MosConnection extends EventEmitter implements IMosConnection {
-	static CONNECTION_PORT_LOWER = 10540
-	static CONNECTION_PORT_UPPER = 10541
-	static CONNECTION_PORT_QUERY = 10542
+	static readonly CONNECTION_PORT_LOWER = 10540
+	static readonly CONNECTION_PORT_UPPER = 10541
+	static readonly CONNECTION_PORT_QUERY = 10542
 	static _nextSocketID = 0
 
 	public readonly mosTypes: MosTypes

packages/connector/src/MosDevice.ts

@@ -411,7 +411,7 @@ export class MosDevice implements IMOSDevice {
 					storyID: data.roStoryDelete.storyID,
 				},
 			}
-		} else if (data.roStoryMoveMultiple && data.roStoryMoveMultiple.storyID) {
+		} else if (data.roStoryMoveMultiple?.storyID) {
 			const stories: string[] = Array.isArray(data.roStoryMoveMultiple.storyID)
 				? (data.roStoryMoveMultiple.storyID as string[])
 				: [data.roStoryMoveMultiple.storyID as string]
@@ -504,7 +504,7 @@ export class MosDevice implements IMOSDevice {
 					itemID: data.roItemDelete.itemID,
 				},
 			}
-		} else if (data.roItemMoveMultiple && data.roItemMoveMultiple.itemID && data.roItemMoveMultiple.storyID) {
+		} else if (data.roItemMoveMultiple?.itemID && data.roItemMoveMultiple?.storyID) {
 			const items: string[] = Array.isArray(data.roItemMoveMultiple.itemID)
 				? (data.roItemMoveMultiple.itemID as string[])
 				: [data.roItemMoveMultiple.itemID as string]
@@ -887,8 +887,8 @@ export class MosDevice implements IMOSDevice {
 	}
 
 	getConnectionStatus(): IMOSConnectionStatus {
-		const primary = this._primaryConnection && this._primaryConnection.getConnectedStatus()
-		const secondary = this._secondaryConnection && this._secondaryConnection.getConnectedStatus()
+		const primary = this._primaryConnection?.getConnectedStatus()
+		const secondary = this._secondaryConnection?.getConnectedStatus()
 
 		return {
 			PrimaryConnected: primary ? primary.connected : false,
@@ -946,7 +946,7 @@ export class MosDevice implements IMOSDevice {
 		this._callbackOnMOSObjects = cb
 	}
 
-	async sendRequestAllMOSObjects(pause?: number): Promise<IMOSAck> {
+	async sendRequestAllMOSObjects(pause = 0): Promise<IMOSAck> {
 		if (typeof this._callbackOnMOSObjects !== 'function') {
 			throw new Error('Cannot sent request, because callback onMOSObjects() is required')
 		}

packages/connector/src/__tests__/Profile4.spec.ts

@@ -106,7 +106,7 @@ describe('Profile 4', () => {
 		const messageId = await fakeIncomingMessage(serverSocketMockLower, xmlData.roStorySend)
 		expect(onROStory).toHaveBeenCalledTimes(1)
 
-		const o = Object.assign({}, xmlApiData.roStorySend)
+		const o = { ...xmlApiData.roStorySend }
 		// @ts-ignore optional property
 		delete o.Body
 		expect(onROStory.mock.calls[0][0]).toMatchObject(o)
@@ -134,7 +134,7 @@ describe('Profile 4', () => {
 		const messageId = await fakeIncomingMessage(serverSocketMockLower, xmlData.roStorySendSingle)
 		expect(onROStory).toHaveBeenCalledTimes(1)
 
-		const o = Object.assign({}, xmlApiData.roStorySendSingle)
+		const o = { ...xmlApiData.roStorySendSingle }
 		// @ts-ignore optional property
 		delete o.Body
 		expect(onROStory.mock.calls[0][0]).toMatchObject(o)

packages/connector/src/__tests__/lib.ts

@@ -121,14 +121,14 @@ export function getXMLReply(
 	theirMosId?: string
 ): string {
 	//add field only if messageId exist
-	if(messageId) messageId = '<messageID>' + messageId + '</messageID>'
+	if (messageId) messageId = '<messageID>' + messageId + '</messageID>'
 	return (
 		'<mos>' +
 		'<mosID>' +
-		(ourMosId || 'our.mos.id') +
+		(ourMosId ?? 'our.mos.id') +
 		'</mosID>' +
 		'<ncsID>' +
-		(theirMosId || 'their.mos.id') +
+		(theirMosId ?? 'their.mos.id') +
 		'</ncsID>' +
 		messageId +
 		content +

packages/connector/src/connection/NCSServerConnection.ts

@@ -52,8 +52,8 @@ export class NCSServerConnection extends EventEmitter implements INCSServerConne
 		super()
 		this._id = id
 		this._host = host
-		this._timeout = timeout || DEFAULT_COMMAND_TIMEOUT
-		this._heartBeatsInterval = Math.max(heartbeatsInterval || 0, this._timeout)
+		this._timeout = timeout ?? DEFAULT_COMMAND_TIMEOUT
+		this._heartBeatsInterval = Math.max(heartbeatsInterval ?? 0, this._timeout)
 		this._mosID = mosID
 		this._connected = false
 		this._debug = debug ?? false
@@ -118,6 +118,10 @@ export class NCSServerConnection extends EventEmitter implements INCSServerConne
 		// if (this._callbackOnConnectionChange) this._callbackOnConnectionChange()
 	}
 
+	/**
+	 * Sends a mos message.
+	 * Returns a Promise which resolves when a MOS reply has been received.
+	 */
 	async executeCommand(message: MosModel.MosMessage): Promise<any> {
 		// Fill with clients
 		let clients: Array<MosSocketClient>
@@ -137,7 +141,7 @@ export class NCSServerConnection extends EventEmitter implements INCSServerConne
 			throw Error(`No "${message.port}" ports found`)
 		}
 		return new Promise((resolve, reject) => {
-			if (clients && clients.length) {
+			if (clients?.length) {
 				clients[0].queueCommand(message, (err, data) => {
 					if (err) {
 						reject(err)
@@ -146,7 +150,7 @@ export class NCSServerConnection extends EventEmitter implements INCSServerConne
 					}
 				})
 			} else {
-				reject('executeCommand: No clients found for ' + message.port)
+				reject(new Error('executeCommand: No clients found for ' + message.port))
 			}
 		})
 	}

packages/connector/src/connection/mosSocketClient.ts

@@ -54,7 +54,7 @@ export class MosSocketClient extends EventEmitter {
 		this._host = host
 		this._port = port
 		this._description = description
-		this._commandTimeout = timeout || DEFAULT_COMMAND_TIMEOUT
+		this._commandTimeout = timeout ?? DEFAULT_COMMAND_TIMEOUT
 		this._debug = debug ?? false
 		this._strict = strict ?? false
 
@@ -127,7 +127,7 @@ export class MosSocketClient extends EventEmitter {
 		message.prepare()
 		// this.debugTrace('queueing', message.messageID, message.constructor.name )
 		this._queueCallback[message.messageID + ''] = cb
-		this._queueMessages.push({ time: time || Date.now(), msg: message })
+		this._queueMessages.push({ time: time ?? Date.now(), msg: message })
 
 		this.processQueue()
 	}
@@ -245,7 +245,7 @@ export class MosSocketClient extends EventEmitter {
 		}
 		if (this._sentMessageTimeout) {
 			clearTimeout(this._sentMessageTimeout)
-			this._sentMessageTimeout
+			delete this._sentMessageTimeout
 		}
 		if (this._client) {
 			const client = this._client
@@ -310,11 +310,8 @@ export class MosSocketClient extends EventEmitter {
 
 		const sentMessageId = message.msg.messageID
 
-		// this.debugTrace('executeCommand', message)
-		// message.prepare() // @todo, is prepared? is sent already? logic needed
 		const messageString: string = message.msg.toString()
 		const buf = iconv.encode(messageString, 'utf16-be')
-		// this.debugTrace('sending',this._client.name, str)
 
 		const sendTime = Date.now()
 		// Command timeout:
@@ -371,15 +368,13 @@ export class MosSocketClient extends EventEmitter {
 	/** */
 	private _onConnected() {
 		this.emit(SocketConnectionEvent.ALIVE)
-		// global.clearInterval(this._connectionAttemptTimer)
 		this._clearConnectionAttemptTimer()
 		this.connected = true
 	}
 
 	/** */
 	private _onData(data: Buffer) {
 		this.emit(SocketConnectionEvent.ALIVE)
-		// data = Buffer.from(data, 'ucs2').toString()
 		const messageString: string = iconv.decode(data, 'utf16-be')
 
 		this.emit('rawMessage', 'recieved', messageString)

packages/connector/src/index.ts

@@ -6,7 +6,7 @@ export { ConnectionConfig } from './config/connectionConfig'
 
 export { MosDevice } from './MosDevice'
 
-// Backwards comppatibility
+// Backwards compatibility
 import { xml2js, pad, addTextElement, xmlToObject } from '@mos-connection/helper'
 export const Utils = {
 	pad,

packages/helper/src/__tests__/index.spec.ts

@@ -33,5 +33,6 @@ describe('Index', () => {
 		expect(typeof MOS.xml2js).toBe('function')
 		expect(MOS.MosModel.XMLMosItem.fromXML).toBeTruthy()
 		expect(MOS.MosModel.XMLMosItem.toXML).toBeTruthy()
+		expect(MOS.pad).toBeTruthy()
 	})
 })

packages/helper/src/__tests__/parsing/mosXml2Js.spec.ts

@@ -7,13 +7,13 @@ import { IMOSItem, MosModel, stringifyMosObject } from '../..'
 function parseMosPluginMessageXml(xmlString: string): any | undefined {
 	const doc: any = xml2js(xmlString)
 
-	if (doc && doc.mos) {
+	if (doc?.mos) {
 		const res: any = {}
 		if (doc.mos.ncsReqAppInfo) {
 			res.ncsReqAppInfo = true
 		}
 
-		if (doc.mos.ncsItem && doc.mos.ncsItem.item) {
+		if (doc.mos.ncsItem?.item) {
 			res.item = MosModel.XMLMosItem.fromXML(doc.mos.ncsItem.item, true)
 		}
 

packages/helper/src/index.ts

@@ -1,7 +1,6 @@
 export * from '@mos-connection/model'
 
 export {
-	pad,
 	xml2js,
 	addTextElement,
 	// not addTextElementInternal

packages/helper/src/mosModel/MosMessage.ts

@@ -29,7 +29,6 @@ export abstract class MosMessage {
 	prepare(messageID?: number): void {
 		if (!this.mosID) throw new Error(`Can't prepare message: mosID missing`)
 		if (!this.ncsID) throw new Error(`Can't prepare message: ncsID missing`)
-		// if (!this.port) throw new Error(`Can't prepare message: port missing`)
 		this._messageID = messageID ? messageID : MosMessage.getNewMessageID()
 	}
 

packages/helper/src/mosModel/__tests__/profile1.spec.ts

@@ -135,7 +135,7 @@ ${mosObjForTest0XML}
 </mosReqObj>`)
 	})
 	test('mosReqObjAll', () => {
-		const msg = new ReqMosObjAll(undefined, true)
+		const msg = new ReqMosObjAll(0, true)
 		expect(getXMLString(msg)).toBe(`<mosReqAll>
 <pause>0</pause>
 </mosReqAll>`)

packages/helper/src/mosModel/profile1/reqMosObjAll.ts

@@ -5,7 +5,7 @@ import { MosMessage } from '../MosMessage'
 export class ReqMosObjAll extends MosMessage {
 	private pause: number
 	/** */
-	constructor(pause = 0, strict: boolean) {
+	constructor(pause: number, strict: boolean) {
 		super('lower', strict)
 		this.pause = pause
 	}

packages/helper/src/utils/Utils.ts

@@ -2,13 +2,6 @@ import { xml2js as xmlParser } from 'xml-js'
 import * as XMLBuilder from 'xmlbuilder'
 import { getMosTypes, IMOSDuration, IMOSString128, IMOSTime, stringifyMosType } from '@mos-connection/model'
 
-/** */
-export function pad(n: string, width: number, z?: string): string {
-	z = z || '0'
-	n = n + ''
-	return n.length >= width ? n : new Array(width - n.length + 1).join(z) + n
-}
-
 export function xml2js(messageString: string): { [key: string]: any } {
 	const object = xmlParser(messageString, { compact: false, trim: true, nativeType: true })
 	// common tags we typically want to know the order of the contents of:
@@ -36,7 +29,7 @@ export function xml2js(messageString: string): { [key: string]: any } {
 				concatChildrenAndTraverseObject(element.elements[0])
 
 				const childEl = element.elements[0]
-				const name = childEl.$name || childEl.$type || 'unknownElement'
+				const name = childEl.$name ?? childEl.$type ?? 'unknownElement'
 				if (childEl.$type && childEl.$type === 'text') {
 					if (childEl.$name === undefined) {
 						// pure text node, hoist it up:

packages/model/src/__tests__/index.spec.ts

@@ -2,25 +2,30 @@ import * as MOS from '../index'
 
 describe('Index', () => {
 	test('ensure that types and enums are exposed', () => {
-		let a: any
 		// Type checks:
-		a = (_: MOS.IMOSItem) => null
-		a = (_: MOS.IMOSRunningOrder) => null
-		a = (_: MOS.IMOSRunningOrderBase) => null
-		a = (_: MOS.IMOSRunningOrderStatus) => null
-		a = (_: MOS.IMOSStoryStatus) => null
-		a = (_: MOS.IMOSItemStatus) => null
-		a = (_: MOS.IMOSStoryAction) => null
-		a = (_: MOS.IMOSROStory) => null
-		a = (_: MOS.IMOSItemAction) => null
-		a = (_: MOS.IMOSItem) => null
-		a = (_: MOS.IMOSROAction) => null
-		a = (_: MOS.IMOSROReadyToAir) => null
-		a = (_: MOS.IMOSROFullStory) => null
+		const fcn = (
+			_exposedTypes: [
+				MOS.IMOSItem,
+				MOS.IMOSRunningOrder,
+				MOS.IMOSRunningOrderBase,
+				MOS.IMOSRunningOrderStatus,
+				MOS.IMOSStoryStatus,
+				MOS.IMOSItemStatus,
+				MOS.IMOSStoryAction,
+				MOS.IMOSROStory,
+				MOS.IMOSItemAction,
+				MOS.IMOSItem,
+				MOS.IMOSROAction,
+				MOS.IMOSROReadyToAir,
+				MOS.IMOSROFullStory,
+				MOS.IMOSAck
+			],
+			// @ts-expect-error types test
+			_falsePositiveTest: MOS.ThisDoesNotExist
+		) => null
 
-		// @ts-expect-error types test
-		a = (_: MOS.ThisDoesNotExist) => null
-		if (a) a()
+		// @ts-expect-error
+		fcn()
 
 		expect(MOS.getMosTypes).toBeTruthy()
 
@@ -37,4 +42,7 @@ describe('Index', () => {
 		expect(mosTypes.mosDuration).toBeTruthy()
 		expect(mosTypes.mosTime).toBeTruthy()
 	})
+	test('ensure that helper functions are exposed', () => {
+		expect(MOS.pad).toBeTruthy()
+	})
 })

packages/model/src/index.ts

@@ -1,2 +1,3 @@
 export * from './mosTypes'
 export * from './model'
+export { pad } from './mosTypes/lib'

packages/model/src/mosTypes/lib.ts

@@ -1,5 +1,5 @@
 export function pad(n: string | number, width: number, z?: string): string {
-	z = z || '0'
+	z = z ?? '0'
 	n = '' + n
 	return n.length >= width ? n : new Array(width - n.length + 1).join(z) + n
 }

packages/quick-mos/src/index.ts

@@ -534,7 +534,7 @@ class MOSMonitor {
 
 			newStories[mosTypes.mosString128.stringify(story.ID)] = story
 
-			const localStory = local && local.fullStories[mosTypes.mosString128.stringify(story.ID)]
+			const localStory = local?.fullStories[mosTypes.mosString128.stringify(story.ID)]
 			if (!local || !_.isEqual(localStory, story)) {
 				this.commands.push(async () => {
 					console.log('sendFullStory', story.ID)