diff --git a/CHANGELOG.md b/CHANGELOG.md index d1eba19eab..ab23b0e2b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,25 @@ The main work (all changes without a GitHub username in brackets in the below li ## __WORK IN PROGRESS__ --> +## __WORK IN PROGRESS__ + +- @matter/model + - Enhancement: `FeatureSelectionErrors()` assesses a cluster's selected features against the combinations its FeatureMap conformance disallows + - Enhancement: `FeatureSet.resolve()` resolves a feature short code, title or camelized title to a short code + - Fix: A cluster's feature table no longer selects features; a feature the specification makes unconditionally mandatory is always selected + - Fix: Feature selection records against `operationalIsSupported` so a feature's `default` conveys only the specification's fallback value + - Fix: A feature mandated by any of several alternatives, such as `DoorLock.User`, is now reported as required + +- @matter/node + - Breaking: Default server exports no longer inherit the features their base implementation enables internally. `ColorControlServer`, `DoorLockServer`, `ElectricalEnergyMeasurementServer`, `LevelControlServer`, `ModeSelectServer`, `PowerSourceServer`, `PowerTopologyServer`, `SmokeCoAlarmServer`, `SwitchServer`, `ThermostatServer` and `WindowCoveringServer` now select no features. Select the features your device supports with `.with(...)`; `PowerSourceServer`, `PowerTopologyServer`, `SmokeCoAlarmServer`, `SwitchServer`, `ThermostatServer`, `WindowCoveringServer` and `ElectricalEnergyMeasurementServer` now require a selection to be added to an endpoint at all. The `DoorLockDevice`, `SpeakerDevice` and `ModeSelectDevice` device types alias these exports, so their clusters also select no features and advertise a different FeatureMap. Verify the feature set of every device you compose. Persisted cluster state resets once for affected devices because it is keyed by feature selection + - Breaking: A LongIdleTimeSupport ICD must select CheckInProtocolSupport and UserActiveModeTrigger + - Breaking: A node that accepts more than one path per invoke must select the General Diagnostics DataModelTest feature + - Enhancement: Adding a server cluster to an endpoint fails when its selected features violate the conformance of its FeatureMap + - Fix: `ClusterBehavior.with()` rejects a feature the cluster does not define + +- @matter/types + - Fix: `Cluster.with()` rejects a feature the cluster does not define and returns one frozen namespace per selection + ## 0.17.7 (2026-07-27) - @matter/general diff --git a/docs/CLUSTER_DEFAULT_IMPLEMENTATIONS.md b/docs/CLUSTER_DEFAULT_IMPLEMENTATIONS.md index 9d75812fb7..c787ad8db6 100644 --- a/docs/CLUSTER_DEFAULT_IMPLEMENTATIONS.md +++ b/docs/CLUSTER_DEFAULT_IMPLEMENTATIONS.md @@ -9,6 +9,8 @@ For examples for nearly all clusters check the [All Clusters Test App](../suppor Matter.js allows the following clusters to be used directly with a default implementation for all relevant commands - or the clusters do not need any commands and so the standard feature set of Matter.js for attributes is sufficient. All clusters contained in this list are also verified using the CI YAML project-chip certification tests. +The "Features" column lists the features the default implementation provides logic for. It does not list enabled features: your application selects the features its device supports with `ClusterServer.with("FeatureName")`, and a cluster the specification allows no choice for selects itself. + | PICS | Cluster name | Features | Matter-Version | Additional Information | |:------------|-------------------------------------------------------------------|---------------------------|----------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | ACL | Access Control | None | 1.5.1 | Access Restriction List (ARL) feature is not implemented | @@ -51,7 +53,7 @@ All clusters contained in this list are also verified using the CI YAML project- | NDOCONC | Nitrogen Dioxide (NO2) Concentration Measurement | All | 1.5.1 | No command implementations needed. | | MOD | Mode Select | OnOff | 1.5.1 | Default implementation sets the new mode when change command is used and automatically handles StartUpMode and OnMode. | | OCC | Occupancy Sensing | None | 1.5.1 | No command implementations needed. The optional event is not implemented. Default Logic automatically fills the legacy attributes occupancySensorTypeBitmap and occupancySensorType based on the enabled features. | -| OO | OnOff | All | 1.5.1 | LevelControl-For-Lighting feature is enabled automatically in default implementation. `offWithEffect` currently ignores requested effects and turns off. `onWithRecallGlobalScene` currently ignores the global scene and turns on. No special logic is implemented for DeadFrontBehavior feature because too use-case specific but can be used and logic adjusted as needed. | +| OO | OnOff | All | 1.5.1 | Logic for the LevelControl-For-Lighting feature is implemented; select the feature to enable it. `offWithEffect` currently ignores requested effects and turns off. `onWithRecallGlobalScene` currently ignores the global scene and turns on. No special logic is implemented for DeadFrontBehavior feature because too use-case specific but can be used and logic adjusted as needed. | | OPCREDS | Operational Credentials | None | 1.5.1 | Internal Root cluster implemented, no need to customize | | OTAP | OTA Software Update Provider | None | 1.5.1 | Full implementation with additional UIs and components for controller, can be used by developers for their own use cases | | OTAR | OTA Software Update Requestor | None | 1.5.1 | Basically fully functional, but applying the update needs to be implemented by the developer. | diff --git a/packages/model/src/common/FeatureSet.ts b/packages/model/src/common/FeatureSet.ts index 0d2a34b050..3cc90b93f4 100644 --- a/packages/model/src/common/FeatureSet.ts +++ b/packages/model/src/common/FeatureSet.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { isDeepEqual } from "@matter/general"; +import { camelize, isDeepEqual } from "@matter/general"; import type { ValueModel } from "../models/index.js"; /** @@ -63,6 +63,54 @@ export namespace FeatureSet { export type Flags = Iterable; export type Definition = Flags | { [name: string]: boolean | undefined }; + /** + * A feature as named by the specification. + */ + export interface Named { + name: string; + title?: string; + } + + /** + * Resolve feature names to short codes. + * + * Callers name features in several ways: the specification's short code ("LT"), its title ("Lighting") or the + * camelized title generated APIs expose ("lighting"). All resolve here so every entry point accepts the same + * vocabulary. Match is case insensitive. + * + * @returns the resolved short codes and any names that resolve to no feature + */ + export function resolve(features: readonly Named[], names: Iterable) { + const byName = new Map(); + for (const feature of features) { + const title = feature.title ?? feature.name; + for (const alias of [feature.name, title, camelize(title)]) { + byName.set(alias.toLowerCase(), feature.name); + } + } + + const resolved = new FeatureSet(); + const unresolved = new Array(); + + for (const name of names) { + const code = byName.get(name.toLowerCase()); + if (code === undefined) { + unresolved.push(name); + } else { + resolved.add(code); + } + } + + return { features: resolved, unresolved }; + } + + /** + * The names {@link resolve} accepts, as the specification titles them. + */ + export function titlesOf(features: readonly Named[]) { + return features.map(feature => feature.title ?? feature.name); + } + /** * Normalize the feature map and list of supported feature names into sets of "all" and "supported" features by * abbreviation. diff --git a/packages/model/src/logic/cluster-variance/FeatureSelectionErrors.ts b/packages/model/src/logic/cluster-variance/FeatureSelectionErrors.ts new file mode 100644 index 0000000000..8a3514b019 --- /dev/null +++ b/packages/model/src/logic/cluster-variance/FeatureSelectionErrors.ts @@ -0,0 +1,81 @@ +/** + * @license + * Copyright 2022-2026 Matter.js Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describeList, InternalError, Logger } from "@matter/general"; +import { ClusterModel } from "../../models/ClusterModel.js"; +import { FeatureBitmap } from "./FeatureBitmap.js"; +import { IllegalFeatureCombinations } from "./IllegalFeatureCombinations.js"; + +const logger = Logger.get("FeatureSelectionErrors"); + +/** + * Assess a cluster's supported features against the combinations its FeatureMap conformance disallows. + * + * Feature selection is the application's, so these constraints are only knowable once a selection is made. We report + * against {@link IllegalFeatureCombinations} so this assessment cannot diverge from the one codegen uses to decide + * whether an application must select features itself. + * + * @param cluster the cluster whose {@link ClusterModel.supportedFeatures} to assess + * @returns one message per violated combination, empty if the selection conforms + */ +export function FeatureSelectionErrors(cluster: ClusterModel): string[] { + let illegal; + try { + ({ illegal } = IllegalFeatureCombinations(cluster)); + } catch (error) { + // Our ruleset does not cover every conformance the grammar allows. A shape we cannot analyze leaves the + // selection unassessed rather than preventing the cluster from operating + if (!(error instanceof InternalError)) { + throw error; + } + logger.warn(`Cannot assess feature selection for ${cluster.name}: ${error.message}`); + return []; + } + + if (!illegal.length) { + return []; + } + + const supported = cluster.supportedFeatures; + const titles = new Map(cluster.features.map(feature => [feature.name, feature.title ?? feature.name])); + const titleOf = (name: string) => titles.get(name) ?? name; + + const errors = new Set(); + + for (const combination of illegal) { + const names = Object.keys(combination); + if (names.some(name => supported.has(name) !== combination[name])) { + continue; + } + + errors.add(describe(combination, names, titleOf)); + } + + return [...errors]; +} + +function describe(combination: FeatureBitmap, names: string[], titleOf: (name: string) => string) { + const required = names.filter(name => !combination[name]).map(titleOf); + const forbidden = names.filter(name => combination[name]).map(titleOf); + + if (!required.length) { + return forbidden.length > 1 + ? `features ${describeList("and", ...forbidden)} cannot be selected together` + : `feature ${forbidden[0]} is not allowed`; + } + + // Several required features satisfy the combination individually, so the requirement is disjunctive + const requirement = + required.length > 1 + ? `select at least one of ${describeList("or", ...required)}` + : `feature ${required[0]} is mandatory`; + + if (!forbidden.length) { + return requirement; + } + + return `${requirement} when ${describeList("and", ...forbidden)} ${forbidden.length > 1 ? "are" : "is"} selected`; +} diff --git a/packages/model/src/logic/cluster-variance/IllegalFeatureCombinations.ts b/packages/model/src/logic/cluster-variance/IllegalFeatureCombinations.ts index 7e673f301e..69515104fa 100644 --- a/packages/model/src/logic/cluster-variance/IllegalFeatureCombinations.ts +++ b/packages/model/src/logic/cluster-variance/IllegalFeatureCombinations.ts @@ -206,7 +206,7 @@ function addFeatureNode( for (const lhsFeature in lhsFeatures) { add({ - feature: false, + [feature.name]: false, [lhsFeature]: lhsFeatures[lhsFeature], ...rhsFeature, }); @@ -215,8 +215,11 @@ function addFeatureNode( } case Conformance.Operator.OR: { + // The feature is mandatory when any disjunct holds, so each disjunct without it is illegal const features = extractDisjunctFeatures(node); - add(Object.fromEntries(Object.entries(features).map((k, v) => [k, !v]))); + for (const name in features) { + add({ [name]: features[name], [feature.name]: false }); + } break; } diff --git a/packages/model/src/logic/cluster-variance/index.ts b/packages/model/src/logic/cluster-variance/index.ts index 5ab61e51cd..13de7cd43c 100644 --- a/packages/model/src/logic/cluster-variance/index.ts +++ b/packages/model/src/logic/cluster-variance/index.ts @@ -5,6 +5,7 @@ */ export * from "./FeatureBitmap.js"; +export * from "./FeatureSelectionErrors.js"; export * from "./IllegalFeatureCombinations.js"; export * from "./InferredComponents.js"; export * from "./NamedComponents.js"; diff --git a/packages/model/src/models/ClusterModel.ts b/packages/model/src/models/ClusterModel.ts index 9488790f61..c5e010544c 100644 --- a/packages/model/src/models/ClusterModel.ts +++ b/packages/model/src/models/ClusterModel.ts @@ -4,10 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Conformance } from "#aspects/Conformance.js"; +import { Conformance } from "#aspects/Conformance.js"; import { ModelIndex } from "#logic/ModelIndex.js"; import { ModelTraversal } from "#logic/ModelTraversal.js"; -import { camelize, describeList } from "@matter/general"; +import { describeList } from "@matter/general"; import { Access } from "../aspects/Access.js"; import { Quality } from "../aspects/Quality.js"; import { SchemaImplementationError } from "../common/errors.js"; @@ -137,18 +137,47 @@ export class ClusterModel return new FeatureSet(this.features.map(feature => feature.name)); } + /** + * Features the implementation supports. + * + * A feature's `default` conveys the specification's fallback value and never implies support. + */ get supportedFeatures(): FeatureSet { const supported = {} as { [name: string]: boolean | undefined }; for (const feature of this.features) { - if (feature.default) { + if (feature.effectiveIsSupported) { supported[feature.name] = true; } } return new FeatureSet(supported); } + /** + * Features the specification mandates without condition. + * + * These are not an application choice. A server implementation supports them regardless of selection; a client + * defers to the features its peer reports. + */ + get unconditionalFeatures(): FeatureSet { + const features = new FeatureSet(); + for (const feature of this.features) { + if (feature.effectiveConformance.ast.type === Conformance.Flag.Mandatory) { + features.add(feature.name); + } + } + return features; + } + set supportedFeatures(features: FeatureSet.Definition | undefined) { - const featureSet = new FeatureSet(features); + const all = this.features; + const { features: selected, unresolved } = FeatureSet.resolve(all, new FeatureSet(features)); + + if (unresolved.length) { + throw new SchemaImplementationError( + this, + `Cannot set unknown feature${unresolved.length > 1 ? "s" : ""} ${describeList("and", ...unresolved)}`, + ); + } let featureMap = this.featureMap; @@ -157,18 +186,11 @@ export class ClusterModel this.children.push(featureMap); } + // Taking ownership of the feature map shadows the models resolved above, so resolve again for (let feature of this.features) { - const desc = feature.title && camelize(feature.title); - let isSupported; - if (desc !== undefined && featureSet.has(desc)) { - isSupported = true; - featureSet.delete(desc); - } else if (featureSet.has(feature.name)) { - isSupported = true; - featureSet.delete(feature.name); - } + const isSupported = selected.has(feature.name); - if (!!feature.default === isSupported) { + if (feature.effectiveIsSupported === isSupported) { continue; } @@ -177,14 +199,7 @@ export class ClusterModel featureMap.children.push(feature); } - feature.default = isSupported ? true : undefined; - } - - if (featureSet.size) { - throw new SchemaImplementationError( - this, - `Cannot set unknown feature${featureSet.size > 1 ? "s" : ""} ${describeList("and", ...featureSet)}`, - ); + feature.operationalIsSupported = isSupported; } } diff --git a/packages/model/test/logic/FeatureSelectionErrorsTest.ts b/packages/model/test/logic/FeatureSelectionErrorsTest.ts new file mode 100644 index 0000000000..7d9521c078 --- /dev/null +++ b/packages/model/test/logic/FeatureSelectionErrorsTest.ts @@ -0,0 +1,89 @@ +/** + * @license + * Copyright 2022-2026 Matter.js Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ClusterModel, FeatureSelectionErrors, Matter } from "#index.js"; + +function errorsFor(cluster: string, supported: string[]) { + const model = Matter.get(ClusterModel, cluster)!.clone(); + model.supportedFeatures = supported; + return FeatureSelectionErrors(model); +} + +describe("FeatureSelectionErrors", () => { + describe("choice groups", () => { + it("requires a member of an exclusive group", () => { + expect(errorsFor("PowerSource", [])).deep.equals(["select at least one of Wired or Battery"]); + }); + + it("rejects two members of an exclusive group", () => { + expect(errorsFor("PowerSource", ["WIRED", "BAT"])).deep.equals([ + "features Wired and Battery cannot be selected together", + ]); + }); + + it("accepts one member of an exclusive group", () => { + expect(errorsFor("PowerSource", ["BAT"])).deep.equals([]); + }); + + it("requires a member of an inclusive group", () => { + expect(errorsFor("Thermostat", [])).deep.equals(["select at least one of Heating or Cooling"]); + }); + + it("accepts several members of an inclusive group", () => { + expect(errorsFor("Thermostat", ["HEAT", "COOL"])).deep.equals([]); + }); + + it("reports each group of a cluster with several", () => { + expect(errorsFor("ElectricalEnergyMeasurement", [])).deep.equals([ + "select at least one of ImportedEnergy or ExportedEnergy", + "select at least one of CumulativeEnergy or PeriodicEnergy", + ]); + }); + }); + + describe("dependent features", () => { + it("requires a feature another selected feature mandates", () => { + expect(errorsFor("IcdManagement", ["LITS"])).deep.equals([ + "feature CheckInProtocolSupport is mandatory when LongIdleTimeSupport is selected", + "feature UserActiveModeTrigger is mandatory when LongIdleTimeSupport is selected", + ]); + }); + + it("accepts the mandated features when selected", () => { + expect(errorsFor("IcdManagement", ["LITS", "CIP", "UAT"])).deep.equals([]); + }); + + it("requires a feature mandated by any of several alternatives", () => { + expect(errorsFor("DoorLock", ["PIN"])).deep.equals([ + "feature User is mandatory when PinCredential is selected", + ]); + expect(errorsFor("DoorLock", ["PIN", "USR"])).deep.equals([]); + }); + + it("rejects a feature whose gating feature is absent", () => { + expect(errorsFor("IcdManagement", ["CIP", "DSLS"])).deep.equals([ + "feature LongIdleTimeSupport is mandatory when DynamicSitLitSupport is selected", + ]); + }); + + it("rejects a deprecated feature", () => { + expect(errorsFor("Thermostat", ["HEAT", "SB"])).deep.equals(["feature Setback is not allowed"]); + }); + }); + + describe("unconstrained clusters", () => { + it("accepts an empty selection when every feature is independently optional", () => { + expect(errorsFor("ColorControl", [])).deep.equals([]); + expect(errorsFor("DoorLock", [])).deep.equals([]); + expect(errorsFor("LevelControl", [])).deep.equals([]); + }); + + it("requires an unconditionally mandatory feature", () => { + expect(errorsFor("EnergyEvse", [])).deep.equals(["feature ChargingPreferences is mandatory"]); + expect(errorsFor("EnergyEvse", ["PREF"])).deep.equals([]); + }); + }); +}); diff --git a/packages/model/test/standard/MatterDefinitionTest.ts b/packages/model/test/standard/MatterDefinitionTest.ts index 7f7cd64406..a8c1a65761 100644 --- a/packages/model/test/standard/MatterDefinitionTest.ts +++ b/packages/model/test/standard/MatterDefinitionTest.ts @@ -58,4 +58,19 @@ describe("MatterDefinition", () => { } expect(offenders).deep.equal([]); }); + + // Feature selection is the application's, so a cluster selects nothing of its own accord. The exception is a + // feature the specification makes unconditionally mandatory, which offers no choice + it("selects only unconditionally mandatory features", () => { + const offenders = new Array(); + for (const cluster of instantiate().clusters) { + for (const name of cluster.supportedFeatures) { + const feature = cluster.features.find(feature => feature.name === name)!; + if (!feature.effectiveConformance.isMandatory) { + offenders.push(`${cluster.name}.${name}`); + } + } + } + expect(offenders).deep.equal([]); + }); }); diff --git a/packages/node/src/behavior/cluster/ClusterBehavior.ts b/packages/node/src/behavior/cluster/ClusterBehavior.ts index 19ac4b63d0..527e0b0c1a 100644 --- a/packages/node/src/behavior/cluster/ClusterBehavior.ts +++ b/packages/node/src/behavior/cluster/ClusterBehavior.ts @@ -107,6 +107,9 @@ export class ClusterBehavior extends Behavior { * * If you invoke on an existing subclass, you will receive a new implementation with the cluster in the subclass * replaced. You should generally only do this with a namespace with the same cluster ID. + * + * Feature selection comes from {@link ns} if it designates features, otherwise it is unchanged. Use + * {@link withFeatures} to select features. */ static for( this: This, diff --git a/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts b/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts index 928575ec43..9ed7266932 100644 --- a/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts +++ b/packages/node/src/behavior/cluster/ClusterBehaviorCache.ts @@ -8,34 +8,46 @@ import { Behavior } from "#behavior/Behavior.js"; import { Schema } from "@matter/model"; import type { ClusterBehavior } from "./ClusterBehavior.js"; -const behaviorCache = new WeakMap>>(); +type SchemaCache = WeakMap>>; -const clientCache = new WeakMap>>(); +const behaviorCache = new WeakMap(); + +const clientCache = new WeakMap(); /** * To save memory we cache behavior implementations specialized for specific clusters. This allows for efficient * configuration of behaviors with conditional runtime logic. * - * We use the schema as the cache key so this relies on similar caching for schemas. + * We key on the schema, the namespace and the client/server distinction because each is visible on the generated type. + * Keying on schema alone would hand a caller a type reporting a different {@link ClusterBehavior.cluster}. This relies + * on similar caching for schemas. */ export namespace ClusterBehaviorCache { - export function get(base: Behavior.Type, schema: Schema, forClient?: boolean) { + export function get(base: Behavior.Type, schema: Schema, namespace: object, forClient?: boolean) { const cache = forClient ? clientCache : behaviorCache; - const baseCache = cache.get(base); - if (baseCache === undefined) { - return; - } - - return baseCache.get(schema)?.deref(); + return cache.get(base)?.get(schema)?.get(namespace)?.deref(); } - export function set(base: Behavior.Type, schema: Schema, type: ClusterBehavior.Type) { - let baseCache = behaviorCache.get(base); - if (baseCache === undefined) { - behaviorCache.set(base, (baseCache = new WeakMap())); + export function set( + base: Behavior.Type, + schema: Schema, + namespace: object, + type: ClusterBehavior.Type, + forClient?: boolean, + ) { + const cache = forClient ? clientCache : behaviorCache; + + let schemaCache = cache.get(base); + if (schemaCache === undefined) { + cache.set(base, (schemaCache = new WeakMap())); + } + + let namespaceCache = schemaCache.get(schema); + if (namespaceCache === undefined) { + schemaCache.set(schema, (namespaceCache = new WeakMap())); } - baseCache.set(schema, new WeakRef(type)); + namespaceCache.set(namespace, new WeakRef(type)); } } diff --git a/packages/node/src/behavior/cluster/ClusterBehaviorType.ts b/packages/node/src/behavior/cluster/ClusterBehaviorType.ts index 560c36f4b4..aab11dc739 100644 --- a/packages/node/src/behavior/cluster/ClusterBehaviorType.ts +++ b/packages/node/src/behavior/cluster/ClusterBehaviorType.ts @@ -5,7 +5,14 @@ */ import { Events, OfflineEvent, OnlineEvent, QuietEvent } from "#behavior/Events.js"; -import { AsyncObservable, camelize, EventEmitter, GeneratedClass, ImplementationError } from "@matter/general"; +import { + AsyncObservable, + camelize, + describeList, + EventEmitter, + GeneratedClass, + ImplementationError, +} from "@matter/general"; import { ClassSemantics, ClusterModel, @@ -72,15 +79,21 @@ export function ClusterBehaviorType({ schema = syncFeatures(schema, namespace); } + // A feature the specification mandates without condition is not a selection. We support it in our own + // implementations; for a peer we defer to the features it reports + if (!forClient) { + schema = selectUnconditionalFeatures(schema); + } + // Construct namespace from schema if not provided if (!namespace) { namespace = ClusterType(schema); } - const useCache = name === undefined; + const useCache = name === undefined && commandFactory === undefined; if (useCache) { - const cached = ClusterBehaviorCache.get(base, schema, forClient); + const cached = ClusterBehaviorCache.get(base, schema, namespace, forClient); if (cached) { return cached; } @@ -146,7 +159,7 @@ export function ClusterBehaviorType({ schema.finalize(); if (useCache) { - ClusterBehaviorCache.set(base, schema, type); + ClusterBehaviorCache.set(base, schema, namespace, type, forClient); } return type as ClusterBehavior.Type; @@ -419,77 +432,66 @@ function schemaForId(id: number) { * We cache schema for clusters configured for certain features. This in turn enables the RootSupervisor cache which * keys off of the schema. */ -const configuredSchemaCache = new Map>(); +const configuredSchemaCache = new WeakMap>(); + +/** + * Schema that differ from another only by feature selection cache against that schema, so a selection reapplied to a + * variant converges on the original object rather than minting a third. + */ +const featureVariantOrigin = new WeakMap(); /** * Ensure the supported features enumerated by schema align with a namespace's feature selection. */ function syncFeatures(schema: Schema.Cluster, namespace: object | undefined) { - // Determine features from namespace's supportedFeatures or schema - let incomingFeatures: FeatureSet; - const nsSupportedFeatures = (namespace as { supportedFeatures?: Record } | undefined) ?.supportedFeatures; - if (nsSupportedFeatures) { - incomingFeatures = new FeatureSet(nsSupportedFeatures); - } else { - // No feature override — use schema's defaults + if (nsSupportedFeatures === undefined) { return schema; } - if (incomingFeatures.is(schema.supportedFeatures)) { - return schema; - } + return syncFeaturesFromSet(schema, featureSetFor(schema, new FeatureSet(nsSupportedFeatures))); +} - // If we have cached this version of the cluster, return the cached schema - const featureKey = [...incomingFeatures].sort().join(","); - let schemaBucket = configuredSchemaCache.get(schema); - if (schemaBucket === undefined) { - schemaBucket = {}; - configuredSchemaCache.set(schema, schemaBucket); - } else { - if (featureKey in schemaBucket) { - return schemaBucket[featureKey]; - } - } +/** + * Apply a feature selection to a schema. If `features` is `true`, all elements are included regardless of + * conformance ("complete" mode). + */ +function applyFeatureSelection(schema: Schema.Cluster, features: readonly string[] | true): Schema.Cluster { + const names = features === true ? (schema as ClusterModel).features.map(feature => feature.name) : features; - // New schema - schema = schema.clone(); - schema.supportedFeatures = incomingFeatures; + return syncFeaturesFromSet(schema, featureSetFor(schema, names)); +} - // Cache - schemaBucket[featureKey] = schema; +/** + * Map feature names to a schema's FeatureSet short codes. + */ +function featureSetFor(schema: Schema.Cluster, names: Iterable) { + const model = schema as ClusterModel; + const { features } = model; + const { features: resolved, unresolved } = FeatureSet.resolve(features, names); - return schema; + if (unresolved.length) { + throw new ImplementationError( + `${model.name} has no feature ${describeList("and", ...unresolved.map(name => `"${name}"`))}; known features are ${FeatureSet.titlesOf(features).join(", ")}`, + ); + } + + return resolved; } /** - * Apply a feature selection to a schema. If `features` is `true`, all elements are included regardless of - * conformance ("complete" mode). Otherwise, maps feature names to the schema's FeatureSet short codes. + * Ensure features the specification mandates without condition are selected. */ -function applyFeatureSelection(schema: Schema.Cluster, features: readonly string[] | true): Schema.Cluster { - if (features === true) { - // "Complete" mode — clone schema and mark all features as supported - const featureSet = new FeatureSet(); - for (const feature of (schema as ClusterModel).features) { - featureSet.add(feature.name); - } - return syncFeaturesFromSet(schema, featureSet); +function selectUnconditionalFeatures(schema: Schema.Cluster) { + const unconditional = (schema as ClusterModel).unconditionalFeatures; + if (!unconditional.size) { + return schema; } - // Map user-facing feature names to schema short codes - const featureSet = new FeatureSet(); - const model = schema as ClusterModel; - for (const name of features) { - for (const feature of model.features) { - if ((feature.title ?? feature.name) === name || feature.name === name) { - featureSet.add(feature.name); - break; - } - } - } + const selection = new FeatureSet([...schema.supportedFeatures, ...unconditional]); - return syncFeaturesFromSet(schema, featureSet); + return syncFeaturesFromSet(schema, selection); } /** @@ -500,20 +502,26 @@ function syncFeaturesFromSet(schema: Schema.Cluster, featureSet: FeatureSet): Sc return schema; } + const origin = featureVariantOrigin.get(schema) ?? schema; + if (featureSet.is(origin.supportedFeatures)) { + return origin; + } + const featureKey = [...featureSet].sort().join(","); - let schemaBucket = configuredSchemaCache.get(schema); + let schemaBucket = configuredSchemaCache.get(origin); if (schemaBucket === undefined) { schemaBucket = {}; - configuredSchemaCache.set(schema, schemaBucket); + configuredSchemaCache.set(origin, schemaBucket); } else if (featureKey in schemaBucket) { return schemaBucket[featureKey]; } - schema = schema.clone(); - schema.supportedFeatures = featureSet; - schemaBucket[featureKey] = schema; + const variant = origin.clone(); + variant.supportedFeatures = featureSet; + schemaBucket[featureKey] = variant; + featureVariantOrigin.set(variant, origin); - return schema; + return variant; } /** @@ -521,9 +529,10 @@ function syncFeaturesFromSet(schema: Schema.Cluster, featureSet: FeatureSet): Sc */ function computeFeatureFlags(schema: ClusterModel): Record { const flags: Record = {}; + const supported = schema.supportedFeatures; for (const child of schema.featureMap.children) { const key = camelize(child.title ?? child.name); - flags[key] = schema.supportedFeatures.has(child.name); + flags[key] = supported.has(child.name); } return flags; } diff --git a/packages/node/src/behavior/cluster/ValidatedElements.ts b/packages/node/src/behavior/cluster/ValidatedElements.ts index 36c7b69cd3..1c473f694c 100644 --- a/packages/node/src/behavior/cluster/ValidatedElements.ts +++ b/packages/node/src/behavior/cluster/ValidatedElements.ts @@ -5,7 +5,7 @@ */ import { Diagnostic, ImplementationError, Logger, MatterAggregateError, Observable } from "@matter/general"; -import { ClusterModel, Conformance, Schema } from "@matter/model"; +import { ClusterModel, Conformance, FeatureSelectionErrors, Schema } from "@matter/model"; import type { ClusterType } from "@matter/types"; import { Behavior } from "../Behavior.js"; import { introspectionInstanceOf } from "./cluster-behavior-utils.js"; @@ -15,6 +15,11 @@ import { NameDependentElements } from "./NameDependentElements.js"; const logger = Logger.get("ValidatedElements"); +/** + * Schema whose feature selection we have already assessed. + */ +const validatedFeatureSelections = new WeakSet(); + /** * Thrown when a {@link ClusterBehavior} cannot be constructed due to fatal errors. */ @@ -121,6 +126,7 @@ export class ValidatedElements { this.error("instance", "Is not an object", true); } + this.#validateFeatures(); this.#validateAttributes(); this.#validateCommands(); this.#validateEvents(); @@ -158,6 +164,25 @@ export class ValidatedElements { } } + /** + * Feature selection is a property of the schema, so assess each schema once however many behaviors share it. + */ + #validateFeatures() { + if (validatedFeatureSelections.has(this.#schema)) { + return; + } + + const errors = FeatureSelectionErrors(this.#schema); + if (errors.length) { + for (const error of errors) { + this.error("features", error, true); + } + return; + } + + validatedFeatureSelections.add(this.#schema); + } + #validateAttributes() { let state; diff --git a/packages/node/src/behaviors/color-control/ColorControlServer.ts b/packages/node/src/behaviors/color-control/ColorControlServer.ts index 7c8c75093a..df0a5d3247 100644 --- a/packages/node/src/behaviors/color-control/ColorControlServer.ts +++ b/packages/node/src/behaviors/color-control/ColorControlServer.ts @@ -2016,6 +2016,5 @@ export namespace ColorControlBaseServer { }; } -// We had turned on some more features to provide a default implementation, but export the cluster with default -// Features again. -export class ColorControlServer extends ColorControlBaseServer.for(ColorControl) {} +// Drop the features the base implementation enables internally so consumers select their own +export class ColorControlServer extends ColorControlBaseServer.with() {} diff --git a/packages/node/src/behaviors/door-lock/DoorLockServer.ts b/packages/node/src/behaviors/door-lock/DoorLockServer.ts index 9866ed211f..b731781348 100644 --- a/packages/node/src/behaviors/door-lock/DoorLockServer.ts +++ b/packages/node/src/behaviors/door-lock/DoorLockServer.ts @@ -1160,4 +1160,4 @@ export namespace DoorLockBaseServer { /** * Default DoorLock server with no features enabled. Use `.with()` to enable features. */ -export class DoorLockServer extends DoorLockBaseServer.for(DoorLock) {} +export class DoorLockServer extends DoorLockBaseServer.with() {} diff --git a/packages/node/src/behaviors/electrical-energy-measurement/ElectricalEnergyMeasurementServer.ts b/packages/node/src/behaviors/electrical-energy-measurement/ElectricalEnergyMeasurementServer.ts index af822c22d7..34cfae8292 100644 --- a/packages/node/src/behaviors/electrical-energy-measurement/ElectricalEnergyMeasurementServer.ts +++ b/packages/node/src/behaviors/electrical-energy-measurement/ElectricalEnergyMeasurementServer.ts @@ -89,7 +89,5 @@ export namespace ElectricalEnergyMeasurementBaseServer { }; } -// Reset all Features -export class ElectricalEnergyMeasurementServer extends ElectricalEnergyMeasurementBaseServer.for( - ElectricalEnergyMeasurement, -) {} +// Drop the features the base implementation enables internally so consumers select their own +export class ElectricalEnergyMeasurementServer extends ElectricalEnergyMeasurementBaseServer.with() {} diff --git a/packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts b/packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts index 0c7991ae63..7b8b26148b 100644 --- a/packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts +++ b/packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts @@ -7,6 +7,7 @@ import type { ValueSupervisor } from "#behavior/supervision/ValueSupervisor.js"; import { NetworkRuntime } from "#behavior/system/network/NetworkRuntime.js"; import type { ServerNetworkRuntime } from "#behavior/system/network/ServerNetworkRuntime.js"; +import { BasicInformationBehavior } from "#behaviors/basic-information"; import { NetworkCommissioningServer } from "#behaviors/network-commissioning"; import { TimeSynchronizationBehavior } from "#behaviors/time-synchronization"; import type { Endpoint } from "#endpoint/Endpoint.js"; @@ -32,7 +33,15 @@ import { } from "@matter/general"; import { FieldElement, Specification } from "@matter/model"; import { assertRemoteActor, MdnsService, SessionManager, Val } from "@matter/protocol"; -import { CommandId, FabricIndex, Status, StatusResponseError, TlvInvokeResponse, TlvOfModel } from "@matter/types"; +import { + CommandId, + DEFAULT_MAX_PATHS_PER_INVOKE, + FabricIndex, + Status, + StatusResponseError, + TlvInvokeResponse, + TlvOfModel, +} from "@matter/types"; import { GeneralDiagnostics } from "@matter/types/clusters/general-diagnostics"; import { GeneralDiagnosticsBehavior } from "./GeneralDiagnosticsBehavior.js"; @@ -96,6 +105,32 @@ export class GeneralDiagnosticsServer extends Base { this.maybeReactTo(this.events.activeNetworkFaults$Changed, this.#triggerActiveNetworkFaultsChangedEvent); } + /** + * DataModelTest is mandatory above a maxPathsPerInvoke of one. + * + * @see {@link MatterSpecification.v16.Core} § 11.12.4.1 + * @throws {@link ImplementationError} if the feature is absent above a maxPathsPerInvoke of one + */ + #assertDataModelTest() { + const maxPathsPerInvoke = + this.endpoint.maybeStateOf(BasicInformationBehavior)?.maxPathsPerInvoke ?? DEFAULT_MAX_PATHS_PER_INVOKE; + + if (!this.features.dataModelTest) { + if (maxPathsPerInvoke > 1) { + throw new ImplementationError( + `The DataModelTest feature is mandatory with a maxPathsPerInvoke of ${maxPathsPerInvoke}; select it with GeneralDiagnosticsServer.with("DataModelTest") or set maxPathsPerInvoke to 1`, + ); + } + return; + } + + if (maxPathsPerInvoke === 1) { + logger.info( + "The DataModelTest feature is enabled but is only required when maxPathsPerInvoke is greater than 1; disable it with GeneralDiagnosticsServer.with() if you do not want to advertise it", + ); + } + } + #validateTestEnabledKey(enableKey: Bytes) { const keyData = Bytes.of(enableKey); if (keyData.every(byte => byte === 0)) { @@ -303,6 +338,8 @@ export class GeneralDiagnosticsServer extends Base { } async #online() { + this.#assertDataModelTest(); + this.events.bootReason.emit( { bootReason: this.state.bootReason ?? GeneralDiagnostics.BootReason.Unspecified }, this.context, @@ -412,6 +449,10 @@ export namespace GeneralDiagnosticsServer { deviceTestEnableKey: Bytes = new Uint8Array(16).fill(0); [Val.properties](endpoint: Endpoint, session: ValueSupervisor.Session) { + // Invoked on the underlying state, so read our own values from here. Reading them via the endpoint would + // require naming this behavior's type, and a type naming features the application did not select is absent + const state = this; + return { /** * Report uptime @@ -447,7 +488,7 @@ export namespace GeneralDiagnosticsServer { Time.nowMs, ).duration; - const timeAsOfLastUpdate = endpoint.stateOf(GeneralDiagnosticsServer).totalOperationalHoursCounter; + const timeAsOfLastUpdate = state.totalOperationalHoursCounter; const totalOperationalTime = Millis(timeAsOfLastUpdate + timeSinceLastUpdate); diff --git a/packages/node/src/behaviors/icd-management/IcdManagementServer.ts b/packages/node/src/behaviors/icd-management/IcdManagementServer.ts index 4aed582936..c1ca865c77 100644 --- a/packages/node/src/behaviors/icd-management/IcdManagementServer.ts +++ b/packages/node/src/behaviors/icd-management/IcdManagementServer.ts @@ -154,9 +154,10 @@ const INSTRUCTION_REQUIRED_TRIGGER_HINTS = [ * ## Long Idle Time (LITS) * * Enable LITS with `IcdManagementServer.with(IcdManagement.Feature.CheckInProtocolSupport, - * IcdManagement.Feature.LongIdleTimeSupport)` — CIP is mandatory under LITS and `.with` replaces (does not augment) - * the feature set, so both must be listed. The mandatory `operatingMode` attribute has no spec-defined default and - * must be configured by the application (initialization throws otherwise); set it to + * IcdManagement.Feature.LongIdleTimeSupport, IcdManagement.Feature.UserActiveModeTrigger)` — CIP and UAT are mandatory + * under LITS and `.with` replaces (does not augment) the feature set, so all three must be listed. The mandatory + * `operatingMode` attribute has no spec-defined default and must be configured by the application (initialization + * throws otherwise); set it to * {@link IcdManagement.OperatingMode.Sit} unless the device starts in LIT. DSLS devices may additionally call * {@link setOperatingMode} to switch SIT↔LIT at runtime. * diff --git a/packages/node/src/behaviors/level-control/LevelControlServer.ts b/packages/node/src/behaviors/level-control/LevelControlServer.ts index 01595a7da0..f584709f51 100644 --- a/packages/node/src/behaviors/level-control/LevelControlServer.ts +++ b/packages/node/src/behaviors/level-control/LevelControlServer.ts @@ -730,9 +730,8 @@ export namespace LevelControlBaseServer { }; } -// We had turned on some more features to provide the default implementation, but export the cluster with no -// Features again. -export class LevelControlServer extends LevelControlBaseServer.for(LevelControl) {} +// Drop the features the base implementation enables internally so consumers select their own +export class LevelControlServer extends LevelControlBaseServer.with() {} function asIntOrNull(value: number | null) { if (value === null) { diff --git a/packages/node/src/behaviors/mode-select/ModeSelectServer.ts b/packages/node/src/behaviors/mode-select/ModeSelectServer.ts index 7e04643e46..00405f1929 100644 --- a/packages/node/src/behaviors/mode-select/ModeSelectServer.ts +++ b/packages/node/src/behaviors/mode-select/ModeSelectServer.ts @@ -108,4 +108,5 @@ export class ModeSelectBaseServer extends ModeSelectBase { } } -export class ModeSelectServer extends ModeSelectBaseServer.for(ModeSelect) {} +// Drop the features the base implementation enables internally so consumers select their own +export class ModeSelectServer extends ModeSelectBaseServer.with() {} diff --git a/packages/node/src/behaviors/on-off/OnOffServer.ts b/packages/node/src/behaviors/on-off/OnOffServer.ts index b06342f1f0..64b9c4178f 100644 --- a/packages/node/src/behaviors/on-off/OnOffServer.ts +++ b/packages/node/src/behaviors/on-off/OnOffServer.ts @@ -341,6 +341,5 @@ export namespace OnOffBaseServer { } } -// We had turned on some more features to provide a default implementation, but export the cluster with default -// Features again. +// Drop the features the base implementation enables internally so consumers select their own export class OnOffServer extends OnOffBaseServer.with() {} diff --git a/packages/node/src/behaviors/power-source/PowerSourceServer.ts b/packages/node/src/behaviors/power-source/PowerSourceServer.ts index 0312f21e93..155c4fd3c8 100644 --- a/packages/node/src/behaviors/power-source/PowerSourceServer.ts +++ b/packages/node/src/behaviors/power-source/PowerSourceServer.ts @@ -62,6 +62,5 @@ export class PowerSourceBaseServer extends PowerSourceLevelBase { } } -// We had turned on some more features to provide the default implementation, but export the cluster with default -// Features again. -export class PowerSourceServer extends PowerSourceBaseServer.for(PowerSource) {} +// Drop the features the base implementation enables internally so consumers select their own +export class PowerSourceServer extends PowerSourceBaseServer.with() {} diff --git a/packages/node/src/behaviors/power-topology/PowerTopologyServer.ts b/packages/node/src/behaviors/power-topology/PowerTopologyServer.ts index 0394166d75..86503d26e8 100644 --- a/packages/node/src/behaviors/power-topology/PowerTopologyServer.ts +++ b/packages/node/src/behaviors/power-topology/PowerTopologyServer.ts @@ -48,4 +48,5 @@ export class PowerTopologyBaseServer extends PowerTopologyBase { } } -export class PowerTopologyServer extends PowerTopologyBaseServer.for(PowerTopology) {} +// Drop the features the base implementation enables internally so consumers select their own +export class PowerTopologyServer extends PowerTopologyBaseServer.with() {} diff --git a/packages/node/src/behaviors/service-area/ServiceAreaServer.ts b/packages/node/src/behaviors/service-area/ServiceAreaServer.ts index c5cc877130..441e00e0ce 100644 --- a/packages/node/src/behaviors/service-area/ServiceAreaServer.ts +++ b/packages/node/src/behaviors/service-area/ServiceAreaServer.ts @@ -294,6 +294,5 @@ export namespace ServiceAreaBaseServer { }; } -// We had turned on some more features to provide the default implementation, but export the cluster with no -// Features again. +// Drop the features the base implementation enables internally so consumers select their own export class ServiceAreaServer extends ServiceAreaBaseServer.with() {} diff --git a/packages/node/src/behaviors/smoke-co-alarm/SmokeCoAlarmServer.ts b/packages/node/src/behaviors/smoke-co-alarm/SmokeCoAlarmServer.ts index 71c80339af..8e43706ddd 100644 --- a/packages/node/src/behaviors/smoke-co-alarm/SmokeCoAlarmServer.ts +++ b/packages/node/src/behaviors/smoke-co-alarm/SmokeCoAlarmServer.ts @@ -41,4 +41,5 @@ export class SmokeCoAlarmBaseServer extends SmokeCoAlarmBase { } } -export class SmokeCoAlarmServer extends SmokeCoAlarmBaseServer.for(SmokeCoAlarm) {} +// Drop the features the base implementation enables internally so consumers select their own +export class SmokeCoAlarmServer extends SmokeCoAlarmBaseServer.with() {} diff --git a/packages/node/src/behaviors/switch/SwitchServer.ts b/packages/node/src/behaviors/switch/SwitchServer.ts index 0040864a49..da9e779cfa 100644 --- a/packages/node/src/behaviors/switch/SwitchServer.ts +++ b/packages/node/src/behaviors/switch/SwitchServer.ts @@ -358,5 +358,5 @@ export namespace SwitchBaseServer { } } -// Reset all Features -export class SwitchServer extends SwitchBaseServer.for(Switch) {} +// Drop the features the base implementation enables internally so consumers select their own +export class SwitchServer extends SwitchBaseServer.with() {} diff --git a/packages/node/src/behaviors/thermostat/ThermostatServer.ts b/packages/node/src/behaviors/thermostat/ThermostatServer.ts index e6bcbd10e5..b14feeca67 100644 --- a/packages/node/src/behaviors/thermostat/ThermostatServer.ts +++ b/packages/node/src/behaviors/thermostat/ThermostatServer.ts @@ -1671,6 +1671,5 @@ export namespace ThermostatBaseServer { } } -// We had turned on some more features to provide a default implementation, but export the cluster with default -// Features again. -export class ThermostatServer extends ThermostatBaseServer.for(Thermostat) {} +// Drop the features the base implementation enables internally so consumers select their own +export class ThermostatServer extends ThermostatBaseServer.with() {} diff --git a/packages/node/src/behaviors/window-covering/WindowCoveringServer.ts b/packages/node/src/behaviors/window-covering/WindowCoveringServer.ts index 7c2b9009dc..b6637c9949 100644 --- a/packages/node/src/behaviors/window-covering/WindowCoveringServer.ts +++ b/packages/node/src/behaviors/window-covering/WindowCoveringServer.ts @@ -648,4 +648,5 @@ export namespace WindowCoveringBaseServer { }; } -export class WindowCoveringServer extends WindowCoveringBaseServer.for(WindowCovering) {} +// Drop the features the base implementation enables internally so consumers select their own +export class WindowCoveringServer extends WindowCoveringBaseServer.with() {} diff --git a/packages/node/test/behavior/cluster/ClusterBehaviorCacheTest.ts b/packages/node/test/behavior/cluster/ClusterBehaviorCacheTest.ts index 3e7922d065..a9365860ef 100644 --- a/packages/node/test/behavior/cluster/ClusterBehaviorCacheTest.ts +++ b/packages/node/test/behavior/cluster/ClusterBehaviorCacheTest.ts @@ -4,7 +4,20 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { LevelControlBaseServer, LevelControlServer } from "#behaviors/level-control"; import { OnOffServer } from "#behaviors/on-off"; +import { PowerSourceServer } from "#behaviors/power-source"; +import { ClusterModel } from "@matter/model"; +import { ClusterType } from "@matter/types"; +import { OnOff } from "@matter/types/clusters/on-off"; +import { PowerSource } from "@matter/types/clusters/power-source"; + +// The namespace `with()` shim is a deprecated compat layer with no declared type +const LegacyPowerSource = PowerSource as ClusterType.WithCompat; + +function constraintOf(type: { schema: ClusterModel }) { + return String(type.schema.attributes.find(attribute => attribute.name === "CurrentLevel")?.constraint); +} describe("ClusterBehaviorCache", () => { it("caches for with", () => { @@ -29,4 +42,69 @@ describe("ClusterBehaviorCache", () => { const Type2 = OnOffServer.with("DeadFrontBehavior", "Lighting"); expect(Type1).equals(Type2); }); + + it("doesn't confuse namespaces sharing a schema", () => { + const variant = Object.create(OnOff) as typeof OnOff; + + const Type1 = OnOffServer.for(OnOff); + const Type2 = OnOffServer.for(variant); + + expect(Type1).not.equals(Type2); + expect(Type1.cluster).equals(OnOff); + expect(Type2.cluster).equals(variant); + }); + + it("caches per namespace", () => { + expect(OnOffServer.for(OnOff)).equals(OnOffServer.for(OnOff)); + }); + + // One schema and one behavior per distinct feature selection. Losing any of these properties multiplies + // ClusterModel and RootSupervisor allocations + describe("sharing", () => { + it("shares one schema per feature selection", () => { + expect(LevelControlBaseServer.with("Lighting").schema).equals( + LevelControlBaseServer.with("Lighting").schema, + ); + }); + + it("shares one behavior per feature selection", () => { + expect(LevelControlServer.with("Lighting", "OnOff")).equals(LevelControlServer.with("Lighting", "OnOff")); + }); + + it("converges on the original schema when a selection is reapplied to a variant", () => { + const cleared = LevelControlBaseServer.with(); + + expect(cleared.schema).not.equals(LevelControlBaseServer.schema); + expect(cleared.with("Lighting", "OnOff").schema).equals(LevelControlBaseServer.schema); + }); + + it("shares one behavior across repeated legacy namespace selection", () => { + const namespaces = new Set(); + const types = new Set(); + + for (let i = 0; i < 100; i++) { + const ns = LegacyPowerSource.with("Battery"); + namespaces.add(ns); + types.add(PowerSourceServer.for(ns)); + } + + expect(namespaces.size).equals(1); + expect(types.size).equals(1); + }); + + it("does not collapse an altered schema into the schema it derives from", () => { + const altered = LevelControlServer.with("Lighting", "OnOff").alter({ + attributes: { currentLevel: { min: 5, max: 200 } }, + }); + + expect(altered.schema).not.equals(LevelControlBaseServer.schema); + expect(constraintOf(altered)).equals("5 to 200"); + + // Convergence on an origin schema must follow feature variants only, never discard alterations + const roundTripped = altered.with().with("Lighting", "OnOff"); + + expect(roundTripped.schema).not.equals(LevelControlBaseServer.schema); + expect(constraintOf(roundTripped)).equals("5 to 200"); + }); + }); }); diff --git a/packages/node/test/behavior/cluster/ClusterBehaviorFeaturesTest.ts b/packages/node/test/behavior/cluster/ClusterBehaviorFeaturesTest.ts new file mode 100644 index 0000000000..2e57309767 --- /dev/null +++ b/packages/node/test/behavior/cluster/ClusterBehaviorFeaturesTest.ts @@ -0,0 +1,254 @@ +/** + * @license + * Copyright 2022-2026 Matter.js Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { GroupsServer } from "#behaviors/groups"; +import { LevelControlBaseServer, LevelControlBehavior, LevelControlServer } from "#behaviors/level-control"; +import { OnOffBaseServer, OnOffServer } from "#behaviors/on-off"; +import { PowerSourceServer } from "#behaviors/power-source"; +import { ServiceAreaBaseServer, ServiceAreaServer } from "#behaviors/service-area"; +import { ThermostatBaseServer, ThermostatServer } from "#behaviors/thermostat"; +import { WindowCoveringBaseServer, WindowCoveringServer } from "#behaviors/window-covering"; +import { camelize, ImplementationError, MatterAggregateError } from "@matter/general"; +import { ClusterModel } from "@matter/model"; +import { ClusterType } from "@matter/types"; +import { LevelControl } from "@matter/types/clusters/level-control"; +import * as behaviors from "../../../src/behaviors/index.js"; +import { MockEndpoint } from "../../endpoint/mock-endpoint.js"; +import { MockEndpointType } from "../mock-behavior.js"; + +/** + * Published behaviors that enable features unconditionally because matter.js implements them for every consumer. Every + * other published behavior must enable nothing so consumers select the features their device supports. + */ +const INTENTIONALLY_ENABLED: Record = { + AccessControlServer: ["extension"], + BooleanStateServer: ["changeEvent"], + GeneralDiagnosticsServer: ["dataModelTest"], + GroupsServer: ["groupNames"], + IcdManagementServer: ["checkInProtocolSupport"], + ScenesManagementServer: ["sceneNames"], + TimeFormatLocalizationServer: ["calendarFormat"], + UnitLocalizationServer: ["temperatureUnit"], +}; + +/** + * Endpoint construction reports behavior failures as an aggregate, so assert against the whole cause chain. + */ +async function causeChainOf(promise: Promise) { + try { + await promise; + } catch (error) { + const messages = new Array(); + const pending = [error]; + while (pending.length) { + const next = pending.shift(); + if (!(next instanceof Error)) { + continue; + } + messages.push(next.message); + pending.push(next.cause, ...(next instanceof MatterAggregateError ? next.errors : [])); + } + return messages.join(" / "); + } + + throw new Error("Expected endpoint construction to fail"); +} + +function enabledFeaturesOf(type: { features: Record }) { + return Object.entries(type.features) + .filter(([, enabled]) => enabled) + .map(([name]) => name) + .sort(); +} + +describe("cluster behavior feature selection", () => { + describe("with()", () => { + it("selects exactly the named features", () => { + expect(LevelControlBaseServer.with("Lighting", "Frequency").features).deep.equals({ + onOff: false, + lighting: true, + frequency: true, + }); + }); + + it("drops all features when invoked with no features", () => { + expect(LevelControlBaseServer.with().features).deep.equals({ + onOff: false, + lighting: false, + frequency: false, + }); + }); + + it("drops elements gated by a dropped feature", () => { + expect(LevelControlBaseServer.defaults.remainingTime).equals(0); + expect(LevelControlBaseServer.defaults.startUpCurrentLevel).equals(null); + + const Dropped = LevelControlBaseServer.with(); + expect(Dropped.schema.supportedFeatures.has("LT")).equals(false); + expect(Dropped.defaults).not.property("remainingTime", 0); + expect(Dropped.defaults).not.property("startUpCurrentLevel", null); + }); + + it("rejects a feature the cluster does not define", () => { + expect(() => (LevelControlBaseServer.with as (...features: string[]) => unknown)("Lightning")).throws( + ImplementationError, + /no feature "Lightning"/, + ); + }); + }); + + describe("for()", () => { + // The namespace `with()` shim is a deprecated compat layer with no declared type + const LegacyLevelControl = LevelControl as ClusterType.WithCompat; + + it("leaves feature selection alone for a namespace that designates no features", () => { + expect((LevelControl as { supportedFeatures?: unknown }).supportedFeatures).equals(undefined); + expect(enabledFeaturesOf(LevelControlBaseServer.for(LevelControl))).deep.equals( + enabledFeaturesOf(LevelControlBaseServer), + ); + }); + + it("adopts the feature selection a namespace designates", () => { + const ns = LegacyLevelControl.with("Lighting"); + + expect(enabledFeaturesOf(LevelControlBaseServer.for(ns))).deep.equals(["lighting"]); + }); + + it("caches per designated feature selection", () => { + expect(LegacyLevelControl.with("Lighting")).equals(LegacyLevelControl.with("Lighting")); + expect(LevelControlBaseServer.for(LegacyLevelControl.with("Lighting"))).equals( + LevelControlBaseServer.for(LegacyLevelControl.with("Lighting")), + ); + }); + }); + + describe("alter() and enable()", () => { + it("alter retains feature selection", () => { + const Altered = LevelControlBaseServer.alter({ attributes: { currentLevel: { min: 1, max: 254 } } }); + + expect(Altered.features).deep.equals(LevelControlBaseServer.features); + }); + + it("enable retains feature selection", () => { + const Enabled = LevelControlBaseServer.enable({ attributes: { onOffTransitionTime: true } }); + + expect(Enabled.features).deep.equals(LevelControlBaseServer.features); + }); + }); + + describe("published behaviors", () => { + it("LevelControl enables nothing, though the spec assigns OnOff a fallback of 1", () => { + // The model carries the specification's fallback; it conveys no selection + expect(LevelControl.schema.features.find(feature => feature.name === "OO")?.default).equals(1); + + expect(enabledFeaturesOf(LevelControlBehavior)).deep.equals([]); + expect(enabledFeaturesOf(LevelControlServer)).deep.equals([]); + }); + + it("publish nothing of what their base implementation enables", () => { + expect(enabledFeaturesOf(OnOffBaseServer)).deep.equals(["lighting"]); + expect(enabledFeaturesOf(OnOffServer)).deep.equals([]); + + expect(enabledFeaturesOf(ServiceAreaBaseServer)).deep.equals(["maps", "progressReporting"]); + expect(enabledFeaturesOf(ServiceAreaServer)).deep.equals([]); + + expect(enabledFeaturesOf(ThermostatBaseServer)).deep.equals([ + "autoMode", + "cooling", + "heating", + "occupancy", + "presets", + ]); + expect(enabledFeaturesOf(ThermostatServer)).deep.equals([]); + + expect(enabledFeaturesOf(WindowCoveringBaseServer)).deep.equals([ + "lift", + "positionAwareLift", + "positionAwareTilt", + "tilt", + ]); + expect(enabledFeaturesOf(WindowCoveringServer)).deep.equals([]); + }); + + it("retain features enabled as a matter.js implementation decision", () => { + expect(enabledFeaturesOf(GroupsServer)).deep.equals(["groupNames"]); + }); + + it("do not serve Lighting elements on an endpoint", async () => { + const endpoint = await MockEndpoint.create(MockEndpointType.with(LevelControlServer), { + levelControl: { minLevel: 0, currentLevel: 0 }, + }); + + expect(endpoint.state.levelControl.minLevel).equals(0); + + const served = endpoint.behaviors.elementsOf(LevelControlServer).attributes; + expect(served.has("currentLevel")).equals(true); + expect(served.has("remainingTime")).equals(false); + expect(served.has("startUpCurrentLevel")).equals(false); + + await endpoint.close(); + }); + + it("are rejected on an endpoint when the selection violates FeatureMap conformance", async () => { + const powerSource = { status: 1, order: 1, description: "test" }; + + expect( + await causeChainOf(MockEndpoint.create(MockEndpointType.with(PowerSourceServer), { powerSource })), + ).match(/select at least one of Wired or Battery/); + + expect( + await causeChainOf( + MockEndpoint.create(MockEndpointType.with(PowerSourceServer.with("Wired", "Battery")), { + powerSource: { + ...powerSource, + wiredCurrentType: 0, + batChargeLevel: 0, + batReplacementNeeded: false, + batReplaceability: 0, + }, + }), + ), + ).match(/features Wired and Battery cannot be selected together/); + + const endpoint = await MockEndpoint.create(MockEndpointType.with(PowerSourceServer.with("Wired")), { + powerSource: { ...powerSource, wiredCurrentType: 0 }, + }); + await endpoint.close(); + }); + + it("enable only features matter.js implements unconditionally", () => { + const unexpected = new Array(); + + for (const [name, type] of Object.entries(behaviors)) { + // A *BaseServer carries the features its default logic implements so subclasses may override the + // corresponding methods. It is an extension point, not an endpoint-ready export + if (typeof type !== "function" || name.endsWith("BaseServer")) { + continue; + } + + const schema = (type as { schema?: unknown }).schema; + if (!(schema instanceof ClusterModel) || schema.id === undefined) { + continue; + } + + const mandatory = schema.features + .filter(feature => feature.effectiveConformance.isMandatory) + .map(feature => camelize(feature.title ?? feature.name)); + + const expected = [...new Set([...(INTENTIONALLY_ENABLED[name] ?? []), ...mandatory])] + .map(feature => camelize(feature)) + .sort(); + const actual = enabledFeaturesOf(type as { features: Record }); + + if (actual.join(",") !== expected.join(",")) { + unexpected.push(`${name}: expected [${expected}], got [${actual}]`); + } + } + + expect(unexpected).deep.equals([]); + }); + }); +}); diff --git a/packages/node/test/behavior/system/icd/IcdClientTest.ts b/packages/node/test/behavior/system/icd/IcdClientTest.ts index cc43aee605..ba3d682796 100644 --- a/packages/node/test/behavior/system/icd/IcdClientTest.ts +++ b/packages/node/test/behavior/system/icd/IcdClientTest.ts @@ -32,12 +32,14 @@ const RootWithIcd = ServerNode.RootEndpoint.with(IcdManagementServer); const LitIcdServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); const RootWithLitIcd = ServerNode.RootEndpoint.with(LitIcdServer); const DslsIcdServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, IcdManagement.Feature.DynamicSitLitSupport, ); const RootWithDslsIcd = ServerNode.RootEndpoint.with(DslsIcdServer); diff --git a/packages/node/test/behavior/system/icd/litSupportedTest.ts b/packages/node/test/behavior/system/icd/litSupportedTest.ts index e6321f6ae0..fe966a5d9d 100644 --- a/packages/node/test/behavior/system/icd/litSupportedTest.ts +++ b/packages/node/test/behavior/system/icd/litSupportedTest.ts @@ -17,6 +17,7 @@ const { litSupported, MIN_LIT_SPECIFICATION_VERSION } = IcdClient; const LitIcdServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); const RootWithLitIcd = ServerNode.RootEndpoint.with(LitIcdServer); diff --git a/packages/node/test/behaviors/general-diagnostics/GeneralDiagnosticsFeaturesTest.ts b/packages/node/test/behaviors/general-diagnostics/GeneralDiagnosticsFeaturesTest.ts new file mode 100644 index 0000000000..9bef6991df --- /dev/null +++ b/packages/node/test/behaviors/general-diagnostics/GeneralDiagnosticsFeaturesTest.ts @@ -0,0 +1,78 @@ +/** + * @license + * Copyright 2022-2026 Matter.js Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { GeneralDiagnosticsServer } from "#behaviors/general-diagnostics"; +import { ServerNode } from "#node/ServerNode.js"; +import { LogDestination, Logger, LogLevel } from "@matter/general"; +import { MockServerNode } from "../../node/mock-server-node.js"; + +async function countAdvisories(maxPathsPerInvoke?: number) { + const captured = new Array(); + Logger.destinations.capture = LogDestination({ + level: LogLevel.INFO, + write: text => captured.push(text), + }); + + try { + const node = await MockServerNode.create(ServerNode.RootEndpoint, { + basicInformation: maxPathsPerInvoke === undefined ? {} : { maxPathsPerInvoke }, + }); + await node.start(); + await node.close(); + } finally { + delete Logger.destinations.capture; + } + + return captured.filter(line => line.includes("DataModelTest feature is enabled")).length; +} + +describe("GeneralDiagnosticsServer features", () => { + it("constructs with a feature selection of its own", async () => { + // State getters must not name this behavior's type; a type naming unselected features is absent + const node = await MockServerNode.create(ServerNode.RootEndpoint.with(GeneralDiagnosticsServer.with()), { + basicInformation: { maxPathsPerInvoke: 1 }, + }); + await node.start(); + + expect(node.stateOf(GeneralDiagnosticsServer.with()).totalOperationalHours).equals(0); + + await node.close(); + }); + + it("rejects a node that accepts several paths per invoke without DataModelTest", async () => { + const node = await MockServerNode.create(ServerNode.RootEndpoint.with(GeneralDiagnosticsServer.with()), {}); + + let message = "no throw"; + try { + await node.start(); + } catch (error) { + message = (error as Error).message; + for (let cause = error as Error; cause instanceof Error; cause = cause.cause as Error) { + message = cause.message; + } + } + await node.close(); + + expect(message).match(/DataModelTest feature is mandatory with a maxPathsPerInvoke of 10/); + }); + + it("accepts a node that accepts one path per invoke without DataModelTest", async () => { + const node = await MockServerNode.create(ServerNode.RootEndpoint.with(GeneralDiagnosticsServer.with()), { + basicInformation: { maxPathsPerInvoke: 1 }, + }); + await node.start(); + await node.close(); + }); + + it("advises disabling DataModelTest when only one path per invoke is accepted", async () => { + expect(await countAdvisories(1)).equals(1); + }); + + it("stays silent when more than one path per invoke is accepted", async () => { + expect(await countAdvisories()).equals(0); + expect(await countAdvisories(3)).equals(0); + }); +}); diff --git a/packages/node/test/behaviors/icd-management/IcdManagementServerTest.ts b/packages/node/test/behaviors/icd-management/IcdManagementServerTest.ts index 97c7baa248..0cfbfe82ff 100644 --- a/packages/node/test/behaviors/icd-management/IcdManagementServerTest.ts +++ b/packages/node/test/behaviors/icd-management/IcdManagementServerTest.ts @@ -520,6 +520,7 @@ describe("IcdManagementServer", () => { const litServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); const RootWithLit = ServerNode.RootEndpoint.with(litServer); @@ -691,6 +692,7 @@ describe("IcdManagementServer", () => { const litServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); const RootWithLit = ServerNode.RootEndpoint.with(litServer); @@ -912,6 +914,7 @@ describe("IcdManagementServer", () => { const dslsServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, IcdManagement.Feature.DynamicSitLitSupport, ); const RootWithDsls = ServerNode.RootEndpoint.with(dslsServer); @@ -1026,6 +1029,7 @@ describe("IcdManagementServer", () => { const litServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); await using site = new MockSite(); const { device } = await site.addCommissionedPair({ @@ -1041,6 +1045,7 @@ describe("IcdManagementServer", () => { const litServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); await using site = new MockSite(); const { device } = await site.addCommissionedPair({ @@ -1062,6 +1067,7 @@ describe("IcdManagementServer", () => { const litServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, ); await using site = new MockSite(); const { device } = await site.addCommissionedPair({ diff --git a/packages/node/test/devices/DeviceFeatureSelectionTest.ts b/packages/node/test/devices/DeviceFeatureSelectionTest.ts new file mode 100644 index 0000000000..dd7ed88527 --- /dev/null +++ b/packages/node/test/devices/DeviceFeatureSelectionTest.ts @@ -0,0 +1,43 @@ +/** + * @license + * Copyright 2022-2026 Matter.js Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ClusterBehavior } from "#behavior/cluster/ClusterBehavior.js"; +import { EndpointType } from "#endpoint/type/EndpointType.js"; +import { ClusterModel, FeatureSelectionErrors } from "@matter/model"; +import * as devices from "../../src/devices/index.js"; + +/** + * A device type's default behaviors must present a legal feature selection. Codegen omits a cluster from the defaults + * when the application has to choose features, so anything remaining must stand on its own. + */ +describe("device type feature selection", () => { + it("every device type's default behaviors conform", () => { + const offenders = new Array(); + + for (const [name, device] of Object.entries(devices)) { + if (typeof device !== "function" || !("behaviors" in device)) { + continue; + } + + for (const type of Object.values((device as EndpointType).behaviors ?? {})) { + if (!ClusterBehavior.is(type)) { + continue; + } + + const schema = type.schema; + if (!(schema instanceof ClusterModel)) { + continue; + } + + for (const error of FeatureSelectionErrors(schema)) { + offenders.push(`${name}.${type.id}: ${error}`); + } + } + } + + expect(offenders).deep.equals([]); + }); +}); diff --git a/packages/node/test/node/ServerSubscriptionTest.ts b/packages/node/test/node/ServerSubscriptionTest.ts index 892c951ec5..8da6fff6af 100644 --- a/packages/node/test/node/ServerSubscriptionTest.ts +++ b/packages/node/test/node/ServerSubscriptionTest.ts @@ -13,7 +13,11 @@ import { LIT_CONFIG } from "./icd-helpers.js"; import { MockServerNode } from "./mock-server-node.js"; const RootWithLitIcd = MockServerNode.RootEndpoint.with( - IcdManagementServer.with(IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport), + IcdManagementServer.with( + IcdManagement.Feature.CheckInProtocolSupport, + IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, + ), ); describe("ServerSubscription", () => { diff --git a/packages/node/test/node/client/ClientNodeInteractionIcdTest.ts b/packages/node/test/node/client/ClientNodeInteractionIcdTest.ts index 1b2aa45e57..9c7db5ac17 100644 --- a/packages/node/test/node/client/ClientNodeInteractionIcdTest.ts +++ b/packages/node/test/node/client/ClientNodeInteractionIcdTest.ts @@ -22,6 +22,7 @@ import { subscribedPeer } from "../node-helpers.js"; const DslsIcdServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, IcdManagement.Feature.DynamicSitLitSupport, ); const RootWithDslsIcd = ServerNode.RootEndpoint.with(DslsIcdServer); diff --git a/packages/node/test/node/client/ClientNodePhysicalPropertiesIcdTest.ts b/packages/node/test/node/client/ClientNodePhysicalPropertiesIcdTest.ts index 938968782d..9a2d3c3e88 100644 --- a/packages/node/test/node/client/ClientNodePhysicalPropertiesIcdTest.ts +++ b/packages/node/test/node/client/ClientNodePhysicalPropertiesIcdTest.ts @@ -17,6 +17,7 @@ const RootWithCipIcd = ServerNode.RootEndpoint.with(IcdManagementServer); const LitIcdServer = IcdManagementServer.with( IcdManagement.Feature.CheckInProtocolSupport, IcdManagement.Feature.LongIdleTimeSupport, + IcdManagement.Feature.UserActiveModeTrigger, IcdManagement.Feature.DynamicSitLitSupport, ); const RootWithLitIcd = ServerNode.RootEndpoint.with(LitIcdServer); diff --git a/packages/types/src/cluster/ClusterType.ts b/packages/types/src/cluster/ClusterType.ts index 8a3d1bc6c4..a712ac992c 100644 --- a/packages/types/src/cluster/ClusterType.ts +++ b/packages/types/src/cluster/ClusterType.ts @@ -4,9 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { camelize, capitalize, decamelize } from "@matter/general"; +import { camelize, capitalize, decamelize, ImplementationError } from "@matter/general"; import type { AttributeModel, CommandModel, EventModel } from "@matter/model"; -import { ClusterModel, ClusterModifier, GeneratorScope, GLOBAL_IDS, Metatype, ValueModel } from "@matter/model"; +import { + ClusterModel, + ClusterModifier, + FeatureSet, + GeneratorScope, + GLOBAL_IDS, + Metatype, + ValueModel, +} from "@matter/model"; import { StatusResponseError } from "../common/StatusResponseError.js"; import type { AttributeId } from "../datatype/AttributeId.js"; import { ClusterId } from "../datatype/ClusterId.js"; @@ -164,24 +172,57 @@ function installLazyProperties(ns: object, model: ClusterModel) { return Object.freeze(result); }); - // Compat shim for pre-PR #3466 call sites: `PowerSource.Cluster.with(Feature.X, Feature.Y)`. - // Returns a shallow prototype-based clone of the namespace with `supportedFeatures` set. Feature - // selection has no other runtime effect (attribute maps are not culled by conformance). + // Compat shim for pre-PR #3466 call sites: `PowerSource.Cluster.with(Feature.X, Feature.Y)`. Returns a + // shallow prototype-based clone of the namespace with `supportedFeatures` set. The namespace's own element + // maps are not culled; the selection takes effect when a behavior derives its schema from the namespace. // @deprecated Removal tracked for 0.18. - lazy("with", () => (...features: string[]) => { - const clone = Object.create(ns) as Record; - const supportedFeatures: Record = {}; - for (const feature of features) { - // Feature enum values are PascalCase; SupportedFeatures keys are camelCase. - supportedFeatures[feature.charAt(0).toLowerCase() + feature.slice(1)] = true; - } - clone.supportedFeatures = Object.freeze(supportedFeatures); - // Preserve the `Cluster`/`Complete` self-reference invariant on the clone; prototype-chain lookup would - // otherwise resolve these to the source namespace and drop the `supportedFeatures` marker. Must use - // defineProperty because the inherited props are non-writable (installed via lazy() with no `writable`). - Object.defineProperty(clone, "Cluster", { value: clone, enumerable: true, configurable: true }); - Object.defineProperty(clone, "Complete", { value: clone, enumerable: true, configurable: true }); - return clone; + lazy("with", () => { + // ClusterBehaviorCache keys on namespace identity, so returning a clone per call would defeat it. One + // clone per selection is load-bearing rather than an optimization + const clones = new Map(); + + return (...features: string[]) => { + // Resolve live: the model may gain features after this property installs. Unresolved names would also + // grow the clone cache without bound + const all = model.features; + const { features: resolved, unresolved } = FeatureSet.resolve(all, features); + + if (unresolved.length) { + throw new ImplementationError( + `${model.name} has no feature ${unresolved.map(name => `"${name}"`).join(", ")}; known features are ${FeatureSet.titlesOf(all).join(", ")}`, + ); + } + + // SupportedFeatures keys are the camelized title + const names = [...resolved].map(code => + camelize(all.find(feature => feature.name === code)!.title ?? code), + ); + + const key = names.sort().join(","); + const existing = clones.get(key); + if (existing) { + return existing; + } + + const clone = Object.create(ns) as Record; + const supportedFeatures: Record = {}; + for (const name of names) { + supportedFeatures[name] = true; + } + clone.supportedFeatures = Object.freeze(supportedFeatures); + // Preserve the `Cluster`/`Complete` self-reference invariant on the clone; prototype-chain lookup would + // otherwise resolve these to the source namespace and drop the `supportedFeatures` marker. Must use + // defineProperty because the inherited props are non-writable (installed via lazy() with no `writable`). + Object.defineProperty(clone, "Cluster", { value: clone, enumerable: true, configurable: true }); + Object.defineProperty(clone, "Complete", { value: clone, enumerable: true, configurable: true }); + + // Holders share this object, so a write by one would otherwise corrupt it for every other + Object.freeze(clone); + + clones.set(key, clone); + + return clone; + }; }); } @@ -329,8 +370,8 @@ export namespace ClusterType { /** * Compat layer for pre-PR #3466 call sites that pin features via `Cluster.with(...)`. Returns the namespace - * shape with a `with()` method that shifts `Typing.SupportedFeatures`. Feature selection has no further runtime - * effect; the shim exists only to keep existing call sites typing correctly during the 0.17 → 0.18 migration. + * shape with a `with()` method that shifts `Typing.SupportedFeatures`. The shim exists to keep existing call + * sites typing correctly during the 0.17 → 0.18 migration. * * @deprecated Scheduled for removal in 0.18. New code should type the cluster via * {@link WithSupportedFeatures} directly. diff --git a/packages/types/test/cluster/ClusterTypeWithCompatTest.ts b/packages/types/test/cluster/ClusterTypeWithCompatTest.ts index 67bf501c4d..d5eca57e1c 100644 --- a/packages/types/test/cluster/ClusterTypeWithCompatTest.ts +++ b/packages/types/test/cluster/ClusterTypeWithCompatTest.ts @@ -75,10 +75,33 @@ describe("ClusterType.Cluster.with() compat shim", () => { expect(selected.Cluster.supportedFeatures).equal(selected.supportedFeatures); }); - it("produces a fresh clone per call", () => { - const a = PowerSource.Cluster.with(PowerSource.Feature.Battery); - const b = PowerSource.Cluster.with(PowerSource.Feature.Battery); + // Consumers cache behavior types against namespace identity, so a clone per call would defeat them + it("produces one clone per feature selection", () => { + expect(PowerSource.Cluster.with(PowerSource.Feature.Battery)).equal( + PowerSource.Cluster.with(PowerSource.Feature.Battery), + ); + + expect(PowerSource.Cluster.with(PowerSource.Feature.Battery, PowerSource.Feature.Rechargeable)).equal( + PowerSource.Cluster.with(PowerSource.Feature.Rechargeable, PowerSource.Feature.Battery), + ); + + expect(PowerSource.Cluster.with(PowerSource.Feature.Battery, PowerSource.Feature.Battery)).equal( + PowerSource.Cluster.with(PowerSource.Feature.Battery), + ); + }); + + it("rejects a feature the cluster does not define", () => { + // The message must echo the caller's own spelling and list features as the caller names them + expect(() => (PowerSource.Cluster.with as (...features: string[]) => unknown)("Batery")).throws( + 'PowerSource has no feature "Batery"; known features are Wired, Battery, Rechargeable, Replaceable', + ); + }); + + it("produces distinct clones for distinct feature selections", () => { + expect(PowerSource.Cluster.with(PowerSource.Feature.Battery)).not.equal( + PowerSource.Cluster.with(PowerSource.Feature.Wired), + ); - expect(a).not.equal(b); + expect(PowerSource.Cluster.with(PowerSource.Feature.Battery)).not.equal(PowerSource.Cluster); }); });