refactor(wan_external): migrate JNAP logic to service layer

[PeterJhongLinksys]

User description

Summary

Refactor WanExternal provider to comply with constitution guidelines by migrating JNAP logic to service layer.

Changes

• New Service: WanExternalService encapsulating JNAP communication
• New UI Model: WanExternalUIModel isolating JNAP model from provider layer
• Updated Provider: WANExternalNotifier now uses service instead of direct JNAP calls
• Updated State: Uses UI model instead of JNAP model
• Updated Consumer: instant_verify_state.dart uses UI model type

Tests

• 33 unit tests covering service, provider, state, and UI model
• Test data builder for reusable mock responses

Architecture Compliance

• ✅ No JNAP model imports in provider layer
• ✅ Service layer handles all JNAP communication
• ✅ Follows three-layer architecture (Article V/VI)

---

PR Type

Enhancement

---

Description

• Create WanExternalService encapsulating JNAP communication logic

• Create WanExternalUIModel isolating JNAP model from provider layer

• Refactor WANExternalNotifier to use service instead of direct JNAP calls

• Update state and consumers to use UI model type

• Add 33 comprehensive unit tests covering service, provider, state, and UI model

---

Diagram Walkthrough

flowchart LR
  A["JNAP Model<br/>WanExternal"] -->|"fromJnap()"| B["UI Model<br/>WanExternalUIModel"]
  C["RouterRepository"] -->|"send()"| D["WanExternalService"]
  D -->|"fetchWanExternal()"| B
  B -->|"used by"| E["WANExternalNotifier"]
  E -->|"updates"| F["WANExternalState"]
  F -->|"consumed by"| G["InstantVerifyState"]

Loading

File Walkthrough

	Relevant files
Enhancement	5 files wan_external_ui_model.dart New UI model isolating JNAP data from presentation              +77/-0    wan_external_service.dart New service layer encapsulating JNAP communication              +57/-0    wan_external_provider.dart Refactor provider to use service layer                                      +11/-15  wan_external_state.dart Update state to use UI model instead of JNAP model              +4/-4      instant_verify_state.dart Update consumer to use UI model type                                          +4/-4
Tests	5 files wan_external_ui_model_test.dart Add comprehensive UI model unit tests                                        +125/-0  wan_external_service_test.dart Add service layer unit tests with error handling                  +159/-0  wan_external_provider_test.dart Add provider unit tests with cache and error scenarios      +181/-0  wan_external_state_test.dart Add state serialization and equality unit tests                    +157/-0  wan_external_test_data.dart Add reusable test data builder for WAN external tests        +54/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive data logging Description: The service logs WAN external data via logger.d('[Service]:[WanExternal]: Fetched ${uiModel.toJson()}'), which can expose sensitive network information (public/private IPv4/IPv6 addresses) into application logs that may be accessible in production or through log aggregation. wan_external_service.dart [34-44] Referred Code Future<WanExternalUIModel> fetchWanExternal({bool force = false}) async { try { final result = await _routerRepository.send( JNAPAction.getWANExternal, fetchRemote: force, timeoutMs: 30000, ); final wanExternal = WanExternal.fromMap(result.output); final uiModel = WanExternalUIModel.fromJnap(wanExternal); logger.d('[Service]:[WanExternal]: Fetched ${uiModel.toJson()}'); return uiModel;
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Swallowed service errors: The provider catches all errors in fetch() and silently returns a state update (only lastUpdate), preventing callers from distinguishing recoverable vs. critical failures (e.g., auth errors) and losing actionable context. Referred Code } catch (error) { logger.d('[WanExternal]: error fetch wan external data: $error'); state = state.copyWith(lastUpdate: DateTime.now().millisecondsSinceEpoch); return state; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Logs sensitive IPs: The service logs uiModel.toJson() which includes public/private WAN IPv4/IPv6 addresses, potentially exposing sensitive network information in application logs. Referred Code logger.d('[Service]:[WanExternal]: Fetched ${uiModel.toJson()}'); return uiModel; Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Unvalidated map decoding: Deserialization passes dynamic map['wanExternal'] directly into WanExternalUIModel.fromMap(...) without type checking/casting, which may allow malformed persisted data to cause runtime failures and should be validated at the boundary. Referred Code factory WANExternalState.fromMap(Map<String, dynamic> map) { return WANExternalState( wanExternal: map['wanExternal'] != null ? WanExternalUIModel.fromMap(map['wanExternal']) : null, lastUpdate: map['lastUpdate']?.toInt() ?? 0, Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ Fix caching logic to respect force parameter Suggestion Impact: The cache condition was changed to include `!force`, ensuring that when `force` is true the function will not early-return cached state. code diff: - if (DateTime.now().millisecondsSinceEpoch - state.lastUpdate < - 3600 * 1000) { + if (!force && + DateTime.now().millisecondsSinceEpoch - state.lastUpdate < + 3600 * 1000) { return state; Modify the cache check to respect the force parameter, ensuring that setting force: true bypasses the cache and fetches fresh data. lib/core/jnap/providers/wan_external_provider.dart [23-26] -if (DateTime.now().millisecondsSinceEpoch - state.lastUpdate < - 3600 * 1000) { +if (!force && + DateTime.now().millisecondsSinceEpoch - state.lastUpdate < + 3600 * 1000) { return state; } [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug where the force parameter is ignored in the caching logic, preventing forced data refreshes.	Medium
General	unify non-JNAP exceptions Add a generic catch block to the fetchWanExternal method to handle any non-JNAP exceptions and wrap them as an UnexpectedError. lib/core/jnap/services/wan_external_service.dart [34-48] Future<WanExternalUIModel> fetchWanExternal({bool force = false}) async { try { final result = await _routerRepository.send( JNAPAction.getWANExternal, fetchRemote: force, timeoutMs: 30000, ); final wanExternal = WanExternal.fromMap(result.output); final uiModel = WanExternalUIModel.fromJnap(wanExternal); logger.d('[Service]:[WanExternal]: Fetched ${uiModel.toJson()}'); return uiModel; } on JNAPError catch (e) { throw _mapJnapError(e); + } catch (error, stack) { + logger.e('[Service]:[WanExternal]: Unexpected error', error, stack); + throw UnexpectedError(originalError: error, message: error.toString()); } } Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion improves robustness by adding a generic catch block to handle and normalize non-JNAP errors into a consistent ServiceError type, which is a good practice for error handling in a service layer.	Medium
	fix async return type Change the return type of the fetch method from FutureOr to Future for type accuracy, as it is an async method. lib/core/jnap/providers/wan_external_provider.dart [19] -FutureOr<WANExternalState> fetch({bool force = false}) async { +Future<WANExternalState> fetch({bool force = false}) async { Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that since the method is async, its return type should be Future<WANExternalState> instead of FutureOr<WANExternalState> for better type clarity and correctness.	Low
Update

[HankYuLinksys]

Looks good!