Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: only install config updates in external config directory #7365

Merged
merged 2 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ zwave-js.tgz
# ZWave cache
*/cache
packages/*/cache
/test/config

# Temporary (import) config files
.tmp
Expand Down
2 changes: 2 additions & 0 deletions docs/api/driver.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,8 @@ installConfigUpdate(): Promise<boolean>

Checks whether there is a compatible update for the currently installed config package and tries to install it. Returns `true` when an update was installed, `false` otherwise.

This requires an external configuration directory to be configured using the `deviceConfigExternalDir` driver option or the `ZWAVEJS_EXTERNAL_CONFIG` environment variable.

> [!NOTE] Although the updated config gets loaded after the update, bugfixes and changes to device configuration generally require either a driver restart or re-interview of the changed devices to take effect.

## Driver properties
Expand Down
10 changes: 7 additions & 3 deletions docs/getting-started/migrating/v14.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ The `ZWaveHost` and `ZWaveApplicationHost` interfaces have been replaced by mult

Furthermore, `Message` and `CommandClass` implementations are no longer bound to a specific host instance. Instead, their methods that need access to host functionality (like value DBs, home ID, device configuration, etc.) now receive a method-specific context object. Parsing of those instances no longer happens in the constructor, but in a separate `from` method.

In an attempt to make Z-Wave JS more portable, almost all usages of Node.js's `Buffer` class have been replaced with the native `Uint8Array`, or our new `Bytes` class that acts as a `Buffer` replacement.

Last but not least, the `npm` packages of Z-Wave JS are now hybrid ES Module & CommonJS. This means that consumers of Z-Wave JS can now benefit from the advantages of ES Modules, like tree-shaking or improved browser compatibility, without breaking compatibility with CommonJS-based projects.

All in all, this release contains a huge list of breaking changes, but most of those are limited low-level APIs.
All in all, this release contains a huge list of breaking changes, but most of those are limited to low-level or sparingly used APIs.

## Replaced Node.js `Buffer` with `Uint8Array` or portable `Bytes` class

Expand All @@ -27,6 +25,12 @@ Both `Uint8Array` and `Bytes` can easily be converted to a `Buffer` instance if

To test whether something is a `Uint8Array`, use the `isUint8Array` function exported from `node:util/types` (not portable) or `@zwave-js/shared` (portable).

## Configuration DB updates require an external config directory

Previously, when a new version of the config DB should be installed, Z-Wave JS would attempt to update `@zwave-js/config` inside `node_modules`, unless it was installed in Docker, or an external configuration directory was configured. This exception is however true for a majority of our installations, so the added complexity of dealing with package managers was not worth it.

Going forward, the `Driver.installConfigUpdate()` method will only install configuration updates if an external config directory is configured using either the `deviceConfigExternalDir` driver option or the `ZWAVEJS_EXTERNAL_CONFIG` environment variable.

## Changed some default paths

A few default paths have been changed to be relative to the cwd of the current process, rather than Z-Wave JS's source files:
Expand Down
9 changes: 3 additions & 6 deletions packages/config/src/ConfigManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ test.sequential(
const { tempDir, logger } = context;

const configDir = path.join(tempDir, "extconfig");
process.env.ZWAVEJS_EXTERNAL_CONFIG = configDir;
await syncExternalConfigDir(logger);
await syncExternalConfigDir(configDir, logger);

expect(await pathExists(configDir)).toBe(true);
expect(
Expand All @@ -68,7 +67,6 @@ test.sequential(
const { tempDir, logger } = context;

const configDir = path.join(tempDir, "extconfig");
process.env.ZWAVEJS_EXTERNAL_CONFIG = configDir;
const otherVersion = semver.inc(ownVersion, "major");

await fs.mkdir(configDir, { recursive: true });
Expand All @@ -78,7 +76,7 @@ test.sequential(
"utf8",
);

await syncExternalConfigDir(logger);
await syncExternalConfigDir(configDir, logger);

expect(await pathExists(configDir)).toBe(true);

Expand All @@ -95,7 +93,6 @@ test.sequential(
const { tempDir, logger } = context;

const configDir = path.join(tempDir, "extconfig");
process.env.ZWAVEJS_EXTERNAL_CONFIG = configDir;
const otherVersion = semver.inc(ownVersion, "prerelease")!;

await fs.mkdir(configDir, { recursive: true });
Expand All @@ -105,7 +102,7 @@ test.sequential(
"utf8",
);

await syncExternalConfigDir(logger);
await syncExternalConfigDir(configDir, logger);

expect(await pathExists(configDir)).toBe(true);

Expand Down
32 changes: 25 additions & 7 deletions packages/config/src/ConfigManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,17 @@ import {
loadFulltextDeviceIndexInternal,
} from "./devices/DeviceConfig.js";
import {
type SyncExternalConfigDirResult,
configDir,
externalConfigDir,
getDeviceEntryPredicate,
getExternalConfigDirEnvVariable,
syncExternalConfigDir,
} from "./utils.js";

export interface ConfigManagerOptions {
logContainer?: ZWaveLogContainer;
deviceConfigPriorityDir?: string;
deviceConfigExternalDir?: string;
}

export class ConfigManager {
Expand All @@ -41,6 +43,8 @@ export class ConfigManager {
options.logContainer ?? new ZWaveLogContainer({ enabled: false }),
);
this.deviceConfigPriorityDir = options.deviceConfigPriorityDir;
this.deviceConfigExternalDir = options.deviceConfigExternalDir;

this._configVersion = PACKAGE_VERSION;
}

Expand All @@ -63,6 +67,12 @@ export class ConfigManager {
}

private deviceConfigPriorityDir: string | undefined;
private deviceConfigExternalDir: string | undefined;
public get externalConfigDir(): string | undefined {
return this.deviceConfigExternalDir
?? getExternalConfigDirEnvVariable();
}

private index: DeviceConfigIndex | undefined;
private fulltextIndex: FulltextDeviceConfigIndex | undefined;

Expand All @@ -74,11 +84,19 @@ export class ConfigManager {
public async loadAll(): Promise<void> {
// If the environment option for an external config dir is set
// try to sync it and then use it
const syncResult = await syncExternalConfigDir(this.logger);
if (syncResult.success) {
let syncResult: SyncExternalConfigDirResult | undefined;
const externalConfigDir = this.externalConfigDir;
if (externalConfigDir) {
syncResult = await syncExternalConfigDir(
externalConfigDir,
this.logger,
);
}

if (syncResult?.success) {
this._useExternalConfig = true;
this.logger.print(
`Using external configuration dir ${externalConfigDir()}`,
`Using external configuration dir ${externalConfigDir}`,
);
this._configVersion = syncResult.version;
} else {
Expand All @@ -94,7 +112,7 @@ export class ConfigManager {
public async loadManufacturers(): Promise<void> {
try {
this._manufacturers = await loadManufacturersInternal(
this._useExternalConfig,
this._useExternalConfig && this.externalConfigDir || undefined,
);
} catch (e) {
// If the config file is missing or invalid, don't try to find it again
Expand Down Expand Up @@ -163,7 +181,7 @@ export class ConfigManager {
// The index of config files included in this package
const embeddedIndex = await loadDeviceIndexInternal(
this.logger,
this._useExternalConfig,
this._useExternalConfig && this.externalConfigDir || undefined,
);
// A dynamic index of the user-defined priority device config files
const priorityIndex: DeviceConfigIndex = [];
Expand Down Expand Up @@ -251,7 +269,7 @@ export class ConfigManager {

if (indexEntry) {
const devicesDir = getDevicesPaths(
this._useExternalConfig ? externalConfigDir()! : configDir,
this._useExternalConfig && this.externalConfigDir || configDir,
).devicesDir;
const filePath = path.isAbsolute(indexEntry.filename)
? indexEntry.filename
Expand Down
6 changes: 3 additions & 3 deletions packages/config/src/Manufacturers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import { isObject } from "alcalzone-shared/typeguards/index.js";
import JSON5 from "json5";
import fs from "node:fs/promises";
import path from "node:path";
import { configDir, externalConfigDir } from "./utils.js";
import { configDir } from "./utils.js";
import { hexKeyRegex4Digits, throwInvalidConfig } from "./utils_safe.js";

export type ManufacturersMap = Map<number, string>;

/** @internal */
export async function loadManufacturersInternal(
externalConfig?: boolean,
externalConfigDir?: string,
): Promise<ManufacturersMap> {
const configPath = path.join(
(externalConfig && externalConfigDir()) || configDir,
externalConfigDir || configDir,
"manufacturers.json",
);

Expand Down
6 changes: 3 additions & 3 deletions packages/config/src/devices/DeviceConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import path from "node:path";
import semver from "semver";
import { clearTemplateCache, readJsonWithTemplate } from "../JsonTemplate.js";
import type { ConfigLogger } from "../Logger.js";
import { configDir, externalConfigDir } from "../utils.js";
import { configDir } from "../utils.js";
import { hexKeyRegex4Digits, throwInvalidConfig } from "../utils_safe.js";
import {
type AssociationConfig,
Expand Down Expand Up @@ -305,10 +305,10 @@ export async function generatePriorityDeviceIndex(
*/
export async function loadDeviceIndexInternal(
logger?: ConfigLogger,
externalConfig?: boolean,
externalConfigDir?: string,
): Promise<DeviceConfigIndex> {
const { devicesDir, indexPath } = getDevicesPaths(
(externalConfig && externalConfigDir()) || configDir,
externalConfigDir || configDir,
);

return loadDeviceIndexShared(
Expand Down
1 change: 0 additions & 1 deletion packages/config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,3 @@ export * from "./devices/DeviceMetadata.js";
export * from "./devices/EndpointConfig.js";
export * from "./devices/ParamInformation.js";
export * from "./devices/shared.js";
export { externalConfigDir } from "./utils.js";
5 changes: 3 additions & 2 deletions packages/config/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ export const configDir = path.resolve(
path.dirname(require.resolve("@zwave-js/config/package.json")),
"config",
);

/** The (optional) absolute path of an external configuration directory */
export function externalConfigDir(): string | undefined {
export function getExternalConfigDirEnvVariable(): string | undefined {
return process.env.ZWAVEJS_EXTERNAL_CONFIG;
}

Expand Down Expand Up @@ -59,9 +60,9 @@ export type SyncExternalConfigDirResult =
* Synchronizes or updates the external config directory and returns whether the directory is in a state that can be used
*/
export async function syncExternalConfigDir(
extConfigDir: string,
logger: ConfigLogger,
): Promise<SyncExternalConfigDirResult> {
const extConfigDir = externalConfigDir();
if (!extConfigDir) return { success: false };

// Make sure the config dir exists
Expand Down
1 change: 0 additions & 1 deletion packages/zwave-js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@
},
"dependencies": {
"@alcalzone/jsonl-db": "^3.1.1",
"@alcalzone/pak": "^0.11.0",
"@homebridge/ciao": "^1.3.1",
"@zwave-js/cc": "workspace:*",
"@zwave-js/config": "workspace:*",
Expand Down
47 changes: 19 additions & 28 deletions packages/zwave-js/src/lib/driver/Driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,7 @@ import {
isMultiEncapsulatingCommandClass,
isTransportServiceEncapsulation,
} from "@zwave-js/cc";
import {
ConfigManager,
type DeviceConfig,
externalConfigDir,
} from "@zwave-js/config";
import { ConfigManager, type DeviceConfig } from "@zwave-js/config";
import {
type CCId,
CommandClasses,
Expand Down Expand Up @@ -174,7 +170,6 @@ import {
cloneDeep,
createWrappingCounter,
getErrorMessage,
isDocker,
isUint8Array,
mergeDeep,
noop,
Expand Down Expand Up @@ -260,11 +255,7 @@ import {
type TransportServiceRXInterpreter,
createTransportServiceRXMachine,
} from "./TransportServiceMachine.js";
import {
checkForConfigUpdates,
installConfigUpdate,
installConfigUpdateInDocker,
} from "./UpdateConfig.js";
import { checkForConfigUpdates, installConfigUpdate } from "./UpdateConfig.js";
import { mergeUserAgent, userAgentComponentsToString } from "./UserAgent.js";
import type {
EditableZWaveOptions,
Expand Down Expand Up @@ -705,6 +696,8 @@ export class Driver extends TypedEventEmitter<DriverEventCallbacks>
logContainer: this._logContainer,
deviceConfigPriorityDir:
this._options.storage.deviceConfigPriorityDir,
deviceConfigExternalDir:
this._options.storage.deviceConfigExternalDir,
});

const self = this;
Expand Down Expand Up @@ -7213,25 +7206,23 @@ ${handlers.length} left`,
const newVersion = await this.checkForConfigUpdates(true);
if (!newVersion) return false;

try {
const extConfigDir = this.configManager.externalConfigDir;
if (!this.configManager.useExternalConfig || !extConfigDir) {
this.driverLog.print(
`Installing version ${newVersion} of configuration DB...`,
`Cannot update configuration DB update - external config directory is not set`,
"error",
);
// We have 3 variants of this.
const extConfigDir = externalConfigDir();
if (this.configManager.useExternalConfig && extConfigDir) {
// 1. external config dir, leave node_modules alone
await installConfigUpdateInDocker(newVersion, {
cacheDir: this._options.storage.cacheDir,
configDir: extConfigDir,
});
} else if (isDocker()) {
// 2. Docker, but no external config dir, extract into node_modules
await installConfigUpdateInDocker(newVersion);
} else {
// 3. normal environment, use npm/yarn to install a new version of @zwave-js/config
await installConfigUpdate(newVersion);
}
return false;
}

this.driverLog.print(
`Installing version ${newVersion} of configuration DB...`,
);
try {
await installConfigUpdate(newVersion, {
cacheDir: this.cacheDir,
configDir: extConfigDir,
});
} catch (e) {
this.driverLog.print(getErrorMessage(e), "error");
return false;
Expand Down
Loading
Loading