From 44f7d9acfc6d274611c4cce7619c3b8666b7b28a Mon Sep 17 00:00:00 2001 From: Kevin L'Hereec Date: Mon, 8 Jun 2026 16:46:38 +0200 Subject: [PATCH] fix --- app/domain/device/device.ts | 4 ++ app/services/cwmp/device_action_executor.ts | 6 ++- app/services/cwmp/main_state_machine.ts | 20 +++++++-- .../cwmp/device_action_executor.spec.ts | 42 ++++++++++++++++++- .../services/cwmp/gpn_circuit_breaker.spec.ts | 40 +++++++++++++----- .../cwmp/main_state_machine_extended.spec.ts | 19 +++++---- .../cwmp/main_state_machine_extra.spec.ts | 14 ++++--- 7 files changed, 116 insertions(+), 29 deletions(-) diff --git a/app/domain/device/device.ts b/app/domain/device/device.ts index 64738f3..267b158 100644 --- a/app/domain/device/device.ts +++ b/app/domain/device/device.ts @@ -251,6 +251,10 @@ export class Device { } } + public removeProperty(name: string): void { + this._properties.delete(name) + } + public getConnectionRequestURL(): URL | null { for (const property of this._properties.values()) { const url = property.getConnectionRequestURL() diff --git a/app/services/cwmp/device_action_executor.ts b/app/services/cwmp/device_action_executor.ts index 2604009..b8e16d2 100644 --- a/app/services/cwmp/device_action_executor.ts +++ b/app/services/cwmp/device_action_executor.ts @@ -1,4 +1,5 @@ import { Device } from '../../domain/device/device.js' +import { DeviceParameter } from '../../domain/device/device_parameter.js' import { ParameterType } from '../../domain/shared/types.js' import { CWMPTaskQueue, ResultStatus, TaskResult } from './task_queue.js' import DeviceModel from '../../models/device.js' @@ -401,7 +402,10 @@ export class DeviceActionExecutor { { parameterValueList: pvStructs, parameterKey }, async (result: TaskResult) => { if (result.status === ResultStatus.Success) { - device.updateFromValues(pvStructs as any) + // ProvisioningCode must always be stored as a writable parameter. + // If it was previously classified as a read-only property (wrong), remove it first. + device.removeProperty(provisioningPath) + device.setParameter(new DeviceParameter(provisioningPath, device.tmpPC, ParameterType.STRING)) await this.persistDevice(device) device.tmpPC = '' return await callback(true) diff --git a/app/services/cwmp/main_state_machine.ts b/app/services/cwmp/main_state_machine.ts index f4bf9f3..6ce1628 100644 --- a/app/services/cwmp/main_state_machine.ts +++ b/app/services/cwmp/main_state_machine.ts @@ -59,12 +59,26 @@ class IdleState extends DeviceState { '[IdleState] GPN allowed — proceeding to discovery' ) - // Test GPN support using the ManagementServer object path + // Discover mandatory subtrees via GPN. + // DeviceInfo is always included so that HardwareVersion, SoftwareVersion, + // ModelName, etc. are classified (writable vs read-only) on every session. const dataModel = device.dataModel if (dataModel === 'tr-181') { - return { nextState: new DiscoveringParametersState(['Device.ManagementServer.']), response: '' } + return { + nextState: new DiscoveringParametersState([ + 'Device.DeviceInfo.', + 'Device.ManagementServer.', + ]), + response: '', + } } else if (dataModel === 'tr-098') { - return { nextState: new DiscoveringParametersState(['InternetGatewayDevice.ManagementServer.']), response: '' } + return { + nextState: new DiscoveringParametersState([ + 'InternetGatewayDevice.DeviceInfo.', + 'InternetGatewayDevice.ManagementServer.', + ]), + response: '', + } } // Unknown data model — skip GPN test, go to CriticalReadsState return { nextState: new CriticalReadsState(), response: '' } diff --git a/tests/unit/services/cwmp/device_action_executor.spec.ts b/tests/unit/services/cwmp/device_action_executor.spec.ts index d9bf9f8..141deef 100644 --- a/tests/unit/services/cwmp/device_action_executor.spec.ts +++ b/tests/unit/services/cwmp/device_action_executor.spec.ts @@ -1,8 +1,10 @@ import { describe, it, expect, beforeEach, vi } from 'vitest' import { DeviceActionExecutor } from '../../../../app/services/cwmp/device_action_executor.js' -import { CWMPTaskQueue, ResultStatus, TaskResult } from '../../../../app/services/cwmp/task_queue.js' +import { CWMPTaskQueue, ResultStatus } from '../../../../app/services/cwmp/task_queue.js' import { Device } from '../../../../app/domain/device/device.js' import { DeviceId } from '../../../../app/domain/device/device_id.js' +import { DeviceProperty } from '../../../../app/domain/device/device_parameter.js' +import { ParameterType } from '../../../../app/domain/shared/types.js' // Mock AdonisJS logger vi.mock('@adonisjs/core/services/logger', () => ({ @@ -139,6 +141,44 @@ describe('DeviceActionExecutor', () => { expect(xml).toContain('SetParameterValues') expect(xml).toContain('ProvisioningCode') }) + + it('stores ProvisioningCode as a writable parameter (not property) after SPV success', async () => { + const device = makeDevice() + device.setDataModel('tr-181') + device.tmpPC = 'code-xyz' + + // Device has no prior ProvisioningCode entry + await executor.insertProvisioningCode(device, async () => '') + const task = queue.getTaskForDevice(device.deviceId) + await queue.executeTaskCallback(task!.id, ResultStatus.Success, {}) + + // Must be in parameters (writable), not properties (read-only) + const asParam = device.getParameter('Device.DeviceInfo.ProvisioningCode') + const asProp = device.getProperty('Device.DeviceInfo.ProvisioningCode') + expect(asParam).not.toBeNull() + expect(asProp).toBeNull() + expect(asParam!.value).toBe('code-xyz') + }) + + it('keeps ProvisioningCode as parameter when it already exists as property (reclassifies)', async () => { + const device = makeDevice() + device.setDataModel('tr-181') + device.tmpPC = 'new-code' + + // Simulate device where ProvisioningCode was incorrectly stored as property + device.setProperty(new DeviceProperty('Device.DeviceInfo.ProvisioningCode', 'old-code', ParameterType.STRING)) + + await executor.insertProvisioningCode(device, async () => '') + const task = queue.getTaskForDevice(device.deviceId) + await queue.executeTaskCallback(task!.id, ResultStatus.Success, {}) + + // After SPV success, must be a parameter, not a property + const asParam = device.getParameter('Device.DeviceInfo.ProvisioningCode') + const asProp = device.getProperty('Device.DeviceInfo.ProvisioningCode') + expect(asParam).not.toBeNull() + expect(asProp).toBeNull() + expect(asParam!.value).toBe('new-code') + }) }) describe('callbacks', () => { diff --git a/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts b/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts index 03c52cc..3ef3897 100644 --- a/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts +++ b/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts @@ -106,41 +106,59 @@ describe('GPN Circuit Breaker — DiscoveringParametersState', () => { }) it('DiscoveringParametersState processes GPN response and stores parameters', async () => { - const device = makeDevice() + const device = makeDevice() // tr-181 device.setGpnStatus('unknown') const taskQueue = new CWMPTaskQueue() const sm = new MainStateMachine(device, 'session-1', taskQueue) - // Start: IdleState → DiscoveringParametersState → sends GPN + // Start: IdleState → DiscoveringParametersState → sends GPN for Device.DeviceInfo. await sm.start() - // Simulate a successful GPN response with ParameterInfoStruct list + // Simulate a successful GPN response for DeviceInfo const gpnResponse = { ParameterList: { ParameterInfoStruct: [ - { Name: 'InternetGatewayDevice.ManagementServer.', Writable: false }, - { Name: 'InternetGatewayDevice.ManagementServer.PeriodicInformInterval', Writable: true }, - { Name: 'InternetGatewayDevice.ManagementServer.PeriodicInformEnable', Writable: true }, - { Name: 'InternetGatewayDevice.ManagementServer.URL', Writable: true }, - { Name: 'InternetGatewayDevice.ManagementServer.ConnectionRequestURL', Writable: false }, + { Name: 'Device.DeviceInfo.', Writable: false }, + { Name: 'Device.DeviceInfo.HardwareVersion', Writable: false }, + { Name: 'Device.DeviceInfo.SoftwareVersion', Writable: false }, + { Name: 'Device.DeviceInfo.ModelName', Writable: false }, + { Name: 'Device.DeviceInfo.ProvisioningCode', Writable: true }, ], }, } + // First GPN response → still discovering (ManagementServer. still in queue) await sm.handleResponse(true, gpnResponse) + expect(sm.getCurrentStateName()).toBe('discovering-parameters') + + // Second GPN response (for Device.ManagementServer.) → transitions to critical-reads + const gpnResponse2 = { + ParameterList: { + ParameterInfoStruct: [ + { Name: 'Device.ManagementServer.', Writable: false }, + { Name: 'Device.ManagementServer.PeriodicInformInterval', Writable: true }, + { Name: 'Device.ManagementServer.ConnectionRequestURL', Writable: false }, + ], + }, + } + await sm.handleResponse(true, gpnResponse2) // gpnStatus should be 'supported' expect(device.gpnStatus).toBe('supported') // Writable params should be stored as parameters - const periodicInterval = device.getParameter('InternetGatewayDevice.ManagementServer.PeriodicInformInterval') + const periodicInterval = device.getParameter('Device.ManagementServer.PeriodicInformInterval') expect(periodicInterval).not.toBeNull() // Read-only params should be stored as properties - const connUrl = device.getProperty('InternetGatewayDevice.ManagementServer.ConnectionRequestURL') + const connUrl = device.getProperty('Device.ManagementServer.ConnectionRequestURL') expect(connUrl).not.toBeNull() + // Writable DeviceInfo param should be parameter + const provCode = device.getParameter('Device.DeviceInfo.ProvisioningCode') + expect(provCode).not.toBeNull() + // Object paths (ending with dot) should NOT be stored - const obj = device.getParameter('InternetGatewayDevice.ManagementServer.') + const obj = device.getParameter('Device.DeviceInfo.') expect(obj).toBeNull() }) diff --git a/tests/unit/services/cwmp/main_state_machine_extended.spec.ts b/tests/unit/services/cwmp/main_state_machine_extended.spec.ts index 0251df4..4bff9d6 100644 --- a/tests/unit/services/cwmp/main_state_machine_extended.spec.ts +++ b/tests/unit/services/cwmp/main_state_machine_extended.spec.ts @@ -44,22 +44,23 @@ describe('MainStateMachine', () => { it('start triggers state execution and returns response', async () => { const sm = new MainStateMachine(makeDevice('tr-181'), 'sess-1', new CWMPTaskQueue()) const response = await sm.start() - // idle → discovering-parameters → sends GPN for Device.ManagementServer. + // idle → discovering-parameters → sends GPN for Device.DeviceInfo. (first mandatory subtree) expect(response).toContain('GetParameterNames') + expect(response).toContain('Device.DeviceInfo.') expect(sm.getCurrentStateName()).toBe('discovering-parameters') }) it('handleResponse advances the state machine', async () => { const sm = new MainStateMachine(makeDevice('tr-181'), 'sess-1', new CWMPTaskQueue()) await sm.start() - // Simulate GPN success response + // Simulate GPN success response for DeviceInfo (first path) await sm.handleResponse(true, { ParameterList: { - ParameterInfoStruct: [{ Name: 'Device.ManagementServer.URL', Writable: '1' }], + ParameterInfoStruct: [{ Name: 'Device.DeviceInfo.HardwareVersion', Writable: '0' }], }, }) - // After GPN success, should transition to critical-reads - expect(sm.getCurrentStateName()).toBe('critical-reads') + // Queue still has ManagementServer. → still in discovering-parameters + expect(sm.getCurrentStateName()).toBe('discovering-parameters') }) it('with unknown data model, skips to critical-reads', async () => { @@ -72,7 +73,7 @@ describe('MainStateMachine', () => { it('with tr-098, sends GPN for InternetGatewayDevice.ManagementServer.', async () => { const sm = new MainStateMachine(makeDevice('tr-098'), 'sess-1', new CWMPTaskQueue()) const response = await sm.start() - expect(response).toContain('InternetGatewayDevice.ManagementServer.') + expect(response).toContain('InternetGatewayDevice.DeviceInfo.') }) it('GPN failure sets gpnStatus unsupported and continues', async () => { @@ -90,7 +91,11 @@ describe('MainStateMachine', () => { await sm.start() expect(sm.getCurrentStateName()).toBe('discovering-parameters') - // Step 2: GPN response → critical-reads + // Step 2a: first GPN response (DeviceInfo.) → still discovering + await sm.handleResponse(true, { ParameterList: { ParameterInfoStruct: [] } }) + expect(sm.getCurrentStateName()).toBe('discovering-parameters') + + // Step 2b: second GPN response (ManagementServer.) → critical-reads await sm.handleResponse(true, { ParameterList: { ParameterInfoStruct: [] } }) expect(sm.getCurrentStateName()).toBe('critical-reads') diff --git a/tests/unit/services/cwmp/main_state_machine_extra.spec.ts b/tests/unit/services/cwmp/main_state_machine_extra.spec.ts index 1e4e08e..358b07b 100644 --- a/tests/unit/services/cwmp/main_state_machine_extra.spec.ts +++ b/tests/unit/services/cwmp/main_state_machine_extra.spec.ts @@ -67,7 +67,7 @@ describe('MainStateMachine — additional coverage (DiscoveringParameters, GPN f const response = await sm.start() expect(response).toContain('GetParameterNames') - expect(response).toContain('InternetGatewayDevice.ManagementServer.') + expect(response).toContain('InternetGatewayDevice.DeviceInfo.') }) it('skips GPN for unknown dataModel and goes to critical-reads', async () => { @@ -83,17 +83,19 @@ describe('MainStateMachine — additional coverage (DiscoveringParameters, GPN f }) it('DiscoveringParametersState handles GPN success and transitions to CriticalReads', async () => { - const device = makeDevice('unknown') + const device = makeDevice('unknown') // tr-181 → DeviceInfo. + ManagementServer. in queue const taskQueue = new CWMPTaskQueue() const sm = new MainStateMachine(device, 'sess', taskQueue) - // Start: sends GPN + // Start: sends GPN for Device.DeviceInfo. await sm.start() - // Simulate successful GPN response (empty parameter list) - const response = await sm.handleResponse(true, { ParameterList: { ParameterInfoStruct: [] } }) + // First GPN response (DeviceInfo) → still in discovering-parameters (ManagementServer. pending) + await sm.handleResponse(true, { ParameterList: { ParameterInfoStruct: [] } }) + expect(sm.getCurrentStateName()).toBe('discovering-parameters') - // Should transition to critical-reads and send GPV + // Second GPN response (ManagementServer) → transitions to critical-reads and sends GPV + const response = await sm.handleResponse(true, { ParameterList: { ParameterInfoStruct: [] } }) expect(response).toContain('GetParameterValues') expect(sm.getCurrentStateName()).toBe('critical-reads') })