From 481d5376444d1ecba953def7f7c7a20431cb66f7 Mon Sep 17 00:00:00 2001 From: HJD Date: Thu, 4 Jul 2024 13:38:30 -0500 Subject: [PATCH 1/2] fix: Update characteristic naming convention warning message text and associated tests. --- src/lib/Accessory.spec.ts | 72 +++++++++++++++++++++++++--------- src/lib/Accessory.ts | 18 +++------ src/lib/util/checkName.spec.ts | 39 +++++++++--------- src/lib/util/checkName.ts | 9 ++--- 4 files changed, 82 insertions(+), 56 deletions(-) diff --git a/src/lib/Accessory.spec.ts b/src/lib/Accessory.spec.ts index 56d297df2..f462f0cb3 100644 --- a/src/lib/Accessory.spec.ts +++ b/src/lib/Accessory.spec.ts @@ -409,14 +409,14 @@ describe("Accessory", () => { }); describe("Accessory and Service naming checks", () => { - let consoleLogSpy: jest.SpyInstance; + let consoleWarnSpy: jest.SpyInstance; beforeEach(() => { - consoleLogSpy = jest.spyOn(console, "warn"); + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); }); afterEach(() => { - consoleLogSpy.mockRestore(); + consoleWarnSpy.mockRestore(); }); test("Accessory Name ending with !", async () => { @@ -431,7 +431,7 @@ describe("Accessory", () => { await accessoryBadName.publish(publishInfo); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'Bad Name! 7430' is getting published with the characteristic 'Name' not following HomeKit naming rules ('Bad Name! 7430'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'Bad Name!' has an invalid 'Name' characteristic ('Bad Name!'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -450,7 +450,7 @@ describe("Accessory", () => { await accessoryBadName.publish(publishInfo); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'Bad ! Name 7430' is getting published with the characteristic 'Name' not following HomeKit naming rules ('Bad ! Name 7430'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'Bad ! Name' has an invalid 'Name' characteristic ('Bad ! Name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -468,7 +468,7 @@ describe("Accessory", () => { }; await accessoryBadName.publish(publishInfo); - expect(consoleLogSpy).toBeCalledTimes(0); + expect(consoleWarnSpy).toBeCalledTimes(0); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -487,9 +487,9 @@ describe("Accessory", () => { await accessoryBadName.publish(publishInfo); expect(accessoryBadName.displayName.startsWith(TEST_DISPLAY_NAME)); - expect(consoleLogSpy).toBeCalledTimes(2); + expect(consoleWarnSpy).toBeCalledTimes(2); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory ''Bad Name 7430' is getting published with the characteristic 'Name' not following HomeKit naming rules (''Bad Name 7430'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory ''Bad Name' has an invalid 'Name' characteristic (''Bad Name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -510,7 +510,7 @@ describe("Accessory", () => { await accessoryBadName.publish(publishInfo); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad ! Switch' is getting published with the characteristic 'Name' not following HomeKit naming rules ('My Bad ! Switch'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad ! Switch' has an invalid 'Name' characteristic ('My Bad ! Switch'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -530,9 +530,9 @@ describe("Accessory", () => { }; await accessoryBadName.publish(publishInfo); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad Switch!' is getting published with the characteristic 'Name' not following HomeKit naming rules ('My Bad Switch!'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad Switch!' has an invalid 'Name' characteristic ('My Bad Switch!'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -552,7 +552,7 @@ describe("Accessory", () => { }; await accessoryBadName.publish(publishInfo); - expect(consoleLogSpy).toBeCalledTimes(0); + expect(consoleWarnSpy).toBeCalledTimes(0); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -572,9 +572,9 @@ describe("Accessory", () => { }; await accessoryBadName.publish(publishInfo); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad Switch'' is getting published with the characteristic 'Name' not following HomeKit naming rules ('My Bad Switch''). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'My Bad Switch'' has an invalid 'Name' characteristic ('My Bad Switch''). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -594,9 +594,9 @@ describe("Accessory", () => { }; await accessoryBadName.publish(publishInfo); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory ''My Bad Switch' is getting published with the characteristic 'Name' not following HomeKit naming rules (''My Bad Switch'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory ''My Bad Switch' has an invalid 'Name' characteristic (''My Bad Switch'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -620,9 +620,9 @@ describe("Accessory", () => { switchService.getCharacteristic(Characteristic.ConfiguredName).updateValue("'Bad Name"); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'unknown' is getting published with the characteristic 'Configured Name' not following HomeKit naming rules (''Bad Name'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'Configured Name' has an invalid 'ConfiguredName' characteristic (''Bad Name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); await awaitEventOnce(accessoryBadName, AccessoryEventTypes.ADVERTISED); await accessoryBadName?.unpublish(); @@ -1228,6 +1228,18 @@ describe("Accessory", () => { }); describe("handleSetCharacteristic", () => { + let consoleWarnSpy: jest.SpyInstance; + + beforeEach(() => { + // Mock console.warn before each test that needs it + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + // Restore console.warn after each test + consoleWarnSpy.mockRestore(); + }); + const testRequestResponse = async ( request: Partial, ...expectedReadData: CharacteristicWriteData[] @@ -1948,6 +1960,18 @@ describe("Accessory", () => { }); describe("characteristicWarning", () => { + let consoleWarnSpy: jest.SpyInstance; + + beforeEach(() => { + // Mock console.warn before each test that needs it + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + // Restore console.warn after each test + consoleWarnSpy.mockRestore(); + }); + test("emit characteristic warning", () => { accessory.on(AccessoryEventTypes.CHARACTERISTIC_WARNING, callback); @@ -2006,6 +2030,18 @@ describe("Accessory", () => { }); describe("deserialize", () => { + let consoleWarnSpy: jest.SpyInstance; + + beforeEach(() => { + // Mock console.warn before each test that needs it + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + // Restore console.warn after each test + consoleWarnSpy.mockRestore(); + }); + test("deserialize legacy json from homebridge", () => { const json = JSON.parse("{\"plugin\":\"homebridge-samplePlatform\",\"platform\":\"SamplePlatform\"," + "\"displayName\":\"2020-01-17T18:45:41.049Z\",\"UUID\":\"dc3951d8-662e-46f7-b6fe-d1b5b5e1a995\",\"category\":1," + diff --git a/src/lib/Accessory.ts b/src/lib/Accessory.ts index 7a1ab721d..5b09318d4 100644 --- a/src/lib/Accessory.ts +++ b/src/lib/Accessory.ts @@ -1051,18 +1051,12 @@ export class Accessory extends EventEmitter { } }; - const model = service.getCharacteristic(Characteristic.Model).value; - const serialNumber = service.getCharacteristic(Characteristic.SerialNumber).value; - const firmwareRevision = service.getCharacteristic(Characteristic.FirmwareRevision).value; - const name = service.getCharacteristic(Characteristic.Name).value; - const manufacturer = service.getCharacteristic(Characteristic.Manufacturer).value; - - checkValue("Model", model); - checkValue("SerialNumber", serialNumber); - checkValue("FirmwareRevision", firmwareRevision); - checkValue("Name", name); - checkName(this.displayName, "Name", name); - checkValue("Manufacturer", manufacturer); + checkName(this.displayName, "Name", service.getCharacteristic(Characteristic.Name).value); + checkValue("FirmwareRevision", service.getCharacteristic(Characteristic.FirmwareRevision).value); + checkValue("Manufacturer", service.getCharacteristic(Characteristic.Manufacturer).value); + checkValue("Model", service.getCharacteristic(Characteristic.Model).value); + checkValue("Name", service.getCharacteristic(Characteristic.Name).value); + checkValue("SerialNumber", service.getCharacteristic(Characteristic.SerialNumber).value); } if (mainAccessory) { diff --git a/src/lib/util/checkName.spec.ts b/src/lib/util/checkName.spec.ts index 52b8140eb..f4f9858a6 100644 --- a/src/lib/util/checkName.spec.ts +++ b/src/lib/util/checkName.spec.ts @@ -1,54 +1,51 @@ import { checkName } from "./checkName"; describe("#checkName()", () => { - let consoleLogSpy: jest.SpyInstance; + let consoleWarnSpy: jest.SpyInstance; beforeEach(() => { - consoleLogSpy = jest.spyOn(console, "warn"); + consoleWarnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); }); afterEach(() => { - consoleLogSpy.mockRestore(); + consoleWarnSpy.mockRestore(); }); test("Accessory Name ending with !", async () => { checkName("displayName", "Name", "bad name!"); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' is getting published with the characteristic 'Name' not following HomeKit naming rules ('bad name!'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' has an invalid 'Name' characteristic ('bad name!'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); }); - test("Accessory Name begining with !", async () => { + test("Accessory Name beginning with !", async () => { checkName("displayName", "Name", "!bad name"); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' is getting published with the characteristic 'Name' not following HomeKit naming rules ('!bad name'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' has an invalid 'Name' characteristic ('!bad name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); }); test("Accessory Name containing !", async () => { checkName("displayName", "Name", "bad ! name"); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' is getting published with the characteristic 'Name' not following HomeKit naming rules ('bad ! name'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' has an invalid 'Name' characteristic ('bad ! name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); }); - test("Accessory Name begining with '", async () => { - checkName("displayName", "Name", "'bad name"); + test("Accessory Name beginning with '", async () => { + checkName("displayName", "Name", " 'bad name"); - expect(consoleLogSpy).toBeCalledTimes(1); + expect(consoleWarnSpy).toBeCalledTimes(1); // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' is getting published with the characteristic 'Name' not following HomeKit naming rules (''bad name'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' has an invalid 'Name' characteristic (' 'bad name'). Please use only alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent the accessory from being added in the Home App or cause unresponsiveness."); }); - test("Accessory Name ends with !", async () => { - checkName("displayName", "Name", "bad name!"); + test("Accessory Name containing '", async () => { + checkName("displayName", "Name", "good ' name"); - expect(consoleLogSpy).toBeCalledTimes(1); - // eslint-disable-next-line max-len - expect(consoleLogSpy).toHaveBeenCalledWith("HAP-NodeJS WARNING: The accessory 'displayName' is getting published with the characteristic 'Name' not following HomeKit naming rules ('bad name!'). Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + expect(consoleWarnSpy).toBeCalledTimes(0); }); - -}); \ No newline at end of file +}); diff --git a/src/lib/util/checkName.ts b/src/lib/util/checkName.ts index 0aa37c474..49ca3142a 100644 --- a/src/lib/util/checkName.ts +++ b/src/lib/util/checkName.ts @@ -6,14 +6,13 @@ import { CharacteristicValue, Nullable } from "../../types"; * @private Private API */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types +// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types export function checkName(displayName: string, name: string, value: Nullable): void { // Ensure the string starts and ends with a Unicode letter or number and allow any combination of letters, numbers, spaces, and apostrophes in the middle. if (typeof value === "string" && !(new RegExp(/^[\p{L}\p{N}][\p{L}\p{N} ']*[\p{L}\p{N}]$/u)).test(value)) { - console.warn("HAP-NodeJS WARNING: The accessory '" + displayName + "' is getting published with the characteristic '" + - name + "'" + " not following HomeKit naming rules ('" + value + "'). " + - "Use only alphanumeric, space, and apostrophe characters, start and end with an alphabetic or numeric character, and don't include emojis. " + - "This might prevent the accessory from being added to the Home App or leading to the accessory being unresponsive!"); + console.warn("HAP-NodeJS WARNING: The accessory '" + displayName + "' has an invalid '" + name + "' characteristic ('" + value + "'). Please use only " + + "alphanumeric, space, and apostrophe characters. Ensure it starts and ends with an alphabetic or numeric character, and avoid emojis. This may prevent " + + "the accessory from being added in the Home App or cause unresponsiveness."); } } From df8347573a648b5b17ffa2fea85ac47567533332 Mon Sep 17 00:00:00 2001 From: HJD Date: Thu, 4 Jul 2024 13:43:27 -0500 Subject: [PATCH 2/2] Remove the eslint hinting since we now typecheck fully. --- src/lib/util/checkName.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/util/checkName.ts b/src/lib/util/checkName.ts index 49ca3142a..626603269 100644 --- a/src/lib/util/checkName.ts +++ b/src/lib/util/checkName.ts @@ -6,7 +6,6 @@ import { CharacteristicValue, Nullable } from "../../types"; * @private Private API */ -// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types export function checkName(displayName: string, name: string, value: Nullable): void { // Ensure the string starts and ends with a Unicode letter or number and allow any combination of letters, numbers, spaces, and apostrophes in the middle.