diff --git a/app/services/cwmp/main_state_machine.ts b/app/services/cwmp/main_state_machine.ts index 2c408dc..e4104c7 100644 --- a/app/services/cwmp/main_state_machine.ts +++ b/app/services/cwmp/main_state_machine.ts @@ -44,58 +44,66 @@ class IdleState extends DeviceState { async handle(context: DeviceContext): Promise { const { device } = context - // Phase 4: Check GPN circuit breaker before attempting GPN - if (device.gpnStatus === 'unsupported' || !device.isGpnAllowed()) { - logger.info( - { gpnStatus: device.gpnStatus, circuitBreaker: device.gpnCircuitBreaker, deviceId: device.identifier }, - '[IdleState] Skipping GPN — device unsupported or circuit breaker open' - ) - // Skip GPN, go directly to CriticalReadsState - return { nextState: new CriticalReadsState(), response: '' } - } + // Determine which Inform parameters are NOT yet classified. + // A param is "classified" if it's in _parameters (writable) OR in _properties (read-only) + // AND the device has been through classification before (gpnStatus != 'unknown'). + // If gpnStatus is 'unknown', the device has never been classified → all need GPN/catalog. + const tmpParams = device.getTmpParamValues() + const unclassified: string[] = [] - // If device is already classified (GPN supported + has parameters), skip GPN test - if (device.gpnStatus === 'supported' && device.getParameters().length > 0) { - logger.info( - { gpnStatus: device.gpnStatus, paramCount: device.getParameters().length, deviceId: device.identifier }, - '[IdleState] Device already classified — skipping GPN test' - ) - return { nextState: new CriticalReadsState(), response: '' } - } - - logger.info( - { gpnStatus: device.gpnStatus, circuitBreaker: device.gpnCircuitBreaker, deviceId: device.identifier }, - '[IdleState] GPN allowed — proceeding to discovery' - ) - - // Extract unique parent paths from the Inform parameters to test GPN support - // and classify them (writable vs read-only). If GPN fails, the catalog will classify instead. - const allKnown = [...device.getParameters(), ...device.getProperties()] - const parentPaths = new Set() - for (const p of allKnown) { - if (!p?.name || typeof p.name !== 'string') continue - const lastDot = p.name.lastIndexOf('.') - if (lastDot > 0) { - parentPaths.add(p.name.substring(0, lastDot + 1)) + if (device.gpnStatus === 'unknown') { + // Never classified → all tmpParams are unclassified + for (const pv of tmpParams) { + const name = typeof pv?.Name === 'string' ? pv.Name : pv?.Name?._ || null + if (name) unclassified.push(name) + } + } else { + // Already classified before → only params NOT in _parameters AND NOT in _properties are new + for (const pv of tmpParams) { + const name = typeof pv?.Name === 'string' ? pv.Name : pv?.Name?._ || null + if (!name) continue + if (!device.getParameter(name) && !device.getProperty(name)) { + unclassified.push(name) + } } } - // If we have paths from the Inform, use them for GPN test + classification - if (parentPaths.size > 0) { - const paths = Array.from(parentPaths).sort() - logger.info({ paths, deviceId: device.identifier }, '[IdleState] GPN test on Inform parameter paths') - return { nextState: new DiscoveringParametersState(paths), response: '' } + // If all Inform params are already classified → skip GPN/catalog, go to CriticalReads + if (unclassified.length === 0) { + logger.info( + { deviceId: device.identifier, gpnStatus: device.gpnStatus }, + '[IdleState] All Inform params already classified — skipping to CriticalReads' + ) + return { nextState: new CriticalReadsState(), response: '' } } - // Fallback: use known data model root path - const dataModel = device.dataModel - if (dataModel === 'tr-181') { - return { nextState: new DiscoveringParametersState(['Device.ManagementServer.']), response: '' } - } else if (dataModel === 'tr-098') { - return { nextState: new DiscoveringParametersState(['InternetGatewayDevice.ManagementServer.']), response: '' } + // Extract unique parent paths from the UNCLASSIFIED params only + const parentPaths = new Set() + for (const name of unclassified) { + const lastDot = name.lastIndexOf('.') + if (lastDot > 0) { + parentPaths.add(name.substring(0, lastDot + 1)) + } } - // Unknown data model — skip GPN test, go to CriticalReadsState - return { nextState: new CriticalReadsState(), response: '' } + + // Case 1: GPN unsupported or circuit breaker open → classify unclassified via catalog + if (device.gpnStatus === 'unsupported' || !device.isGpnAllowed()) { + logger.info( + { gpnStatus: device.gpnStatus, unclassifiedCount: unclassified.length, deviceId: device.identifier }, + '[IdleState] GPN unavailable — classifying unclassified params via catalog' + ) + reclassifyDeviceFromCatalog(device) + return { nextState: new CriticalReadsState(), response: '' } + } + + // Case 2: GPN supported → send GPN only on parent paths of unclassified params + // Case 3: GPN unknown → send GPN to test support + classify + const paths = Array.from(parentPaths).sort() + logger.info( + { paths, unclassifiedCount: unclassified.length, gpnStatus: device.gpnStatus, deviceId: device.identifier }, + '[IdleState] Classifying unclassified params via GPN' + ) + return { nextState: new DiscoveringParametersState(paths), response: '' } } } @@ -248,11 +256,7 @@ class CriticalReadsState extends DeviceState { } } - // Only reclassify via catalog if GPN is not supported AND device hasn't been classified yet - // (i.e., no parameters exist — all are still stored as properties) - if (context.device.gpnStatus !== 'supported' && context.device.getParameters().length === 0) { - reclassifyDeviceFromCatalog(context.device) - } + // Classification is handled in IdleState — no need to reclassify here return { nextState: new ConfigurationEvaluationState(), response: '' } } @@ -457,10 +461,7 @@ export class OperationalState extends DeviceState { async handle(context: DeviceContext): Promise { const { device } = context - // Consolidated reclassification — only if not already classified - if (device.gpnStatus !== 'supported' && device.getParameters().length === 0) { - reclassifyDeviceFromCatalog(device) - } + // Classification is handled incrementally in IdleState — no need here // Persist device with operational status device.operationalStatus = 'operational' diff --git a/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts b/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts index 0a253b5..627f820 100644 --- a/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts +++ b/tests/unit/services/cwmp/gpn_circuit_breaker.spec.ts @@ -52,6 +52,10 @@ import DeviceModel from '../../../../app/models/device.js' function makeDevice(): Device { const device = new Device(new DeviceId('Sagemcom', 'F8B7FF', 'Livebox 4', 'LB4-TEST-001')) device.setDataModel('tr-181') + // Simulate unclassified Inform params so GPN test triggers + device.setTmpParamValues([ + { Name: 'Device.ManagementServer.ConnectionRequestURL', Value: { _: 'http://cpe/' } }, + ]) return device } 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 28c06c3..c00cafc 100644 --- a/tests/unit/services/cwmp/main_state_machine_extended.spec.ts +++ b/tests/unit/services/cwmp/main_state_machine_extended.spec.ts @@ -31,6 +31,11 @@ import { CWMPTaskQueue } from '../../../../app/services/cwmp/task_queue.js' function makeDevice(dataModel: string = 'tr-181') { const d = new Device(new DeviceId('Mfr', 'OUI', 'PC', 'SN')) d.setDataModel(dataModel as any) + // Simulate Inform params (tmpParamValues) so IdleState can detect unclassified params + const root = dataModel === 'tr-098' ? 'InternetGatewayDevice' : 'Device' + d.setTmpParamValues([ + { Name: `${root}.ManagementServer.ConnectionRequestURL`, Value: { _: 'http://cpe/' } }, + ]) return d } @@ -63,10 +68,12 @@ describe('MainStateMachine', () => { expect(sm.getCurrentStateName()).toBe('critical-reads') }) - it('with unknown data model, skips to critical-reads', async () => { - const sm = new MainStateMachine(makeDevice('unknown'), 'sess-1', new CWMPTaskQueue()) + it('with unknown data model and no unclassified params, skips to critical-reads', async () => { + const device = new Device(new DeviceId('Mfr', 'OUI', 'PC', 'SN')) + device.setDataModel('unknown' as any) + // No tmpParamValues → nothing to classify → skips to critical-reads + const sm = new MainStateMachine(device, 'sess-1', new CWMPTaskQueue()) await sm.start() - // idle → directly to critical-reads (no GPN for unknown model) 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 6ffa5ca..3ccdbbf 100644 --- a/tests/unit/services/cwmp/main_state_machine_extra.spec.ts +++ b/tests/unit/services/cwmp/main_state_machine_extra.spec.ts @@ -50,6 +50,10 @@ function makeDevice(gpnStatus: string = 'unsupported'): Device { const device = new Device(new DeviceId('Sagemcom', 'F8B7FF', 'Livebox 4', 'LB4-TEST-001')) device.setDataModel('tr-181') device.setGpnStatus(gpnStatus as any) + // Simulate unclassified Inform params + device.setTmpParamValues([ + { Name: 'Device.ManagementServer.ConnectionRequestURL', Value: { _: 'http://cpe/' } }, + ]) return device } @@ -62,6 +66,7 @@ describe('MainStateMachine — additional coverage (DiscoveringParameters, GPN f const device = new Device(new DeviceId('Huawei', '202BC1', 'BM632w', 'SN001')) device.setDataModel('tr-098') device.setGpnStatus('unknown') + device.setTmpParamValues([{ Name: 'InternetGatewayDevice.ManagementServer.URL', Value: { _: 'http://acs/' } }]) const taskQueue = new CWMPTaskQueue() const sm = new MainStateMachine(device, 'sess', taskQueue) diff --git a/tests/unit/services/cwmp/two_session_scenario.spec.ts b/tests/unit/services/cwmp/two_session_scenario.spec.ts index 7a46e7d..37dcf66 100644 --- a/tests/unit/services/cwmp/two_session_scenario.spec.ts +++ b/tests/unit/services/cwmp/two_session_scenario.spec.ts @@ -94,12 +94,14 @@ describe('Two-session scenario: same device, first contact then reconnect', () = const d = new Device(new DeviceId('Huawei Technologies Co., Ltd.', '202BC1', 'BM632w', '000000')) d.setDataModel('tr-098') // Simulate Inform parameters (stored by handleInform before state machine starts) - d.updateFromValues([ + const informParams = [ { Name: 'InternetGatewayDevice.DeviceInfo.HardwareVersion', Value: { _: '40501' } }, { Name: 'InternetGatewayDevice.DeviceInfo.SoftwareVersion', Value: { _: 'V100R001' } }, { Name: 'InternetGatewayDevice.ManagementServer.ConnectionRequestURL', Value: { _: 'http://172.20.0.5:37547/' } }, { Name: 'InternetGatewayDevice.ManagementServer.PeriodicInformInterval', Value: { _: '60' } }, - ]) + ] + d.updateFromValues(informParams) + d.setTmpParamValues(informParams) return d } diff --git a/tests/unit/services/main_state_machine.spec.ts b/tests/unit/services/main_state_machine.spec.ts index fdee9c3..f7bde85 100644 --- a/tests/unit/services/main_state_machine.spec.ts +++ b/tests/unit/services/main_state_machine.spec.ts @@ -10,6 +10,11 @@ vi.mock('@adonisjs/core/services/logger', () => ({ }, })) +// Mock reclassifyDeviceFromCatalog +vi.mock('../../../app/services/parameter_writability.js', () => ({ + reclassifyDeviceFromCatalog: vi.fn(), +})) + // Mock DeviceModel to avoid MongoDB dependency vi.mock('../../../app/models/device.js', () => ({ default: { @@ -34,6 +39,7 @@ import { DeviceId } from '../../../app/domain/device/device_id.js' import { DeviceProperty } from '../../../app/domain/device/device_parameter.js' import { CWMPTaskQueue } from '../../../app/services/cwmp/task_queue.js' import { ParameterType } from '../../../app/domain/shared/types.js' +import { reclassifyDeviceFromCatalog } from '../../../app/services/parameter_writability.js' // --------------------------------------------------------------------------- // Helpers @@ -42,12 +48,14 @@ import { ParameterType } from '../../../app/domain/shared/types.js' function makeTr181Device(): Device { const device = new Device(new DeviceId('Sagemcom', 'F8B7FF', 'Livebox 4', 'LB4-TEST-001')) device.setDataModel('tr-181') + device.setTmpParamValues([{ Name: 'Device.ManagementServer.ConnectionRequestURL', Value: { _: 'http://cpe/' } }]) return device } function makeTr098Device(): Device { const device = new Device(new DeviceId('Huawei Technologies Co., Ltd.', '202BC1', 'BM632w', '000000')) device.setDataModel('tr-098') + device.setTmpParamValues([{ Name: 'InternetGatewayDevice.ManagementServer.ConnectionRequestURL', Value: { _: 'http://cpe/' } }]) return device } @@ -216,37 +224,24 @@ describe('MainStateMachine — unknown data model', () => { // MainStateMachine — CompletedState reclassification // --------------------------------------------------------------------------- -describe('MainStateMachine — CompletedState reclassification', () => { - it('reclassifies writable properties on completion when GPN unsupported', async () => { +describe('MainStateMachine — IdleState reclassification via catalog', () => { + it('reclassifies unclassified params via catalog when GPN unsupported', async () => { const device = makeTr181Device() device.setGpnStatus('unsupported') - // Add writable property that should be reclassified - device.setProperty(new DeviceProperty('Device.ManagementServer.PeriodicInformInterval', '60', ParameterType.UNSIGNED_INT)) - // Add read-only property that should stay - device.setProperty(new DeviceProperty('Device.DeviceInfo.Manufacturer', 'Sagemcom', ParameterType.STRING)) + // tmpParamValues has an unclassified param (not in _parameters or _properties) + device.setTmpParamValues([ + { Name: 'Device.ManagementServer.PeriodicInformInterval', Value: { _: '60' } }, + ]) + // This param is NOT in device._parameters or device._properties → unclassified const taskQueue = new CWMPTaskQueue() const sm = new MainStateMachine(device, 'session-1', taskQueue) - // Start → since dataModel is tr-181, it will try GPN + // Start → IdleState detects unclassified param, GPN unsupported → calls catalog await sm.start() - // Simulate GPN failure - await sm.handleResponse(true, {}) - - // The state machine should eventually complete - // Let it run through (no config match → completes) - let guard = 0 - while (!sm.isCompleted() && guard++ < 10) { - await sm.handleResponse(true, {}) - } - - // After completion, PeriodicInformInterval should be a parameter - expect(device.getParameter('Device.ManagementServer.PeriodicInformInterval')).not.toBeNull() - expect(device.getProperty('Device.ManagementServer.PeriodicInformInterval')).toBeNull() - // Manufacturer should stay as property - expect(device.getProperty('Device.DeviceInfo.Manufacturer')).not.toBeNull() - expect(device.getParameter('Device.DeviceInfo.Manufacturer')).toBeNull() + // reclassifyDeviceFromCatalog should have been called + expect(vi.mocked(reclassifyDeviceFromCatalog)).toHaveBeenCalled() }) })