Refactor: Migrate LocalNetworkSettings to Service Layer Architecture

[PeterJhongLinksys]

User description

Summary

This PR completes the architectural refactoring of the LocalNetworkSettings feature to comply with Constitution Article V Section 5.3 (架構層次與職責分離). The refactoring achieves complete separation of concerns across Provider, Service, and View layers.

Key Changes

• State Layer Migration: Changed LocalNetworkStatus.dhcpReservationList from JNAP DHCPReservation to UI DHCPReservationUIModel
• Service Layer Encapsulation: All JNAP communication and model transformations now handled by LocalNetworkSettingsService through private conversion methods
• Provider Layer Cleanup: Removed all JNAP dependencies from Provider and State layers
• DHCP Reservations: Updated DHCPReservationsProvider to work exclusively with UI models
• View Layer Updates:
  • Updated device_detail_view to use DHCPReservationUIModel
  • Fixed use_build_context_synchronously warning in local_network_settings_view
• Model Refactoring: Renamed reservation_item_ui_model.dart to dhcp_reservation_ui_model.dart for clarity
• Test Coverage: Updated all tests and removed obsolete mocks (535 lines deleted)

Architecture Compliance

Constitution Article V Section 5.3 - 架構層次與職責分離

✅ Provider/State Layers

• Zero JNAP dependencies
• Uses UI models exclusively
• No knowledge of data layer implementation

✅ Service Layer

• Encapsulates all JNAP communication
• Handles JNAP ↔ UI model transformations via private methods
• Performs network calculations (IP ranges, subnet masks, etc.)

✅ View Layer

• Pure presentation logic
• Works with UI models only
• No data layer coupling

Files Changed

Core Implementation (8 files)

• lib/page/advanced_settings/local_network_settings/models/dhcp_reservation_ui_model.dart (new)
• lib/page/advanced_settings/local_network_settings/providers/dhcp_reservations_provider.dart
• lib/page/advanced_settings/local_network_settings/providers/local_network_settings_provider.dart (-145 lines)
• lib/page/advanced_settings/local_network_settings/providers/local_network_settings_state.dart
• lib/page/advanced_settings/local_network_settings/services/dhcp_reservations_service.dart (new)
• lib/page/advanced_settings/local_network_settings/services/local_network_settings_service.dart (+189 lines)
• lib/page/advanced_settings/local_network_settings/views/dhcp_reservations_view.dart
• lib/page/instant_device/views/device_detail_view.dart

Test Coverage (8 files)

• Added comprehensive service layer tests
• Updated provider and view tests
• Removed obsolete mocks

Test Plan

• All functional unit tests pass
• Architecture compliance verified (zero JNAP imports in Provider/State layers)
• View layer verification passed
• Manual testing of DHCP reservations functionality
• Manual testing of device detail IP reservation flow

Breaking Changes

None - This is a pure architectural refactoring with no API changes.

🤖 Generated with Claude Code

---

PR Type

Enhancement, Tests

---

Description

• Migrate LocalNetworkSettings to service layer with UI models

  • Extract JNAP communication into LocalNetworkSettingsService
  • Create DHCPReservationUIModel for presentation layer
  • Remove JNAP dependencies from Provider and State layers

• Implement comprehensive service layer architecture

  • DHCPReservationsService handles reservation-specific logic
  • LocalNetworkSettingsService manages JNAP ↔ UI model conversions
  • Private conversion methods encapsulate data transformation

• Add extensive test coverage for new services

  • DHCPReservationUIModel: 11 tests (100% coverage)
  • LocalNetworkSettingsService: 11 tests (90.9% coverage)
  • DHCPReservationsService: 6 tests (conflict detection)

• Fix use_build_context_synchronously warnings in views

---

Diagram Walkthrough

flowchart LR
  JNAP["JNAP API<br/>DHCPReservation"]
  Service["LocalNetworkSettingsService<br/>Conversion & JNAP Comm"]
  DHCPService["DHCPReservationsService<br/>Business Logic"]
  Provider["Provider/State<br/>DHCPReservationUIModel"]
  View["View Layer<br/>UI Models Only"]
  
  JNAP -- "fetch/save" --> Service
  Service -- "UI Models" --> DHCPService
  Service -- "UI Models" --> Provider
  Provider -- "UI Models" --> View
  DHCPService -- "conflict check" --> Provider

Loading

File Walkthrough

	Relevant files
Enhancement	9 files dhcp_reservation_ui_model.dart Create new UI model for DHCP reservations                                +62/-0    local_network_settings_service.dart Extract JNAP communication and model conversions                  +189/-0  dhcp_reservations_service.dart Create service for DHCP reservation business logic              +100/-0  local_network_settings_provider.dart Remove JNAP dependencies, delegate to service layer            +44/-101 local_network_settings_state.dart Replace JNAP DHCPReservation with UI model                              +6/-5      dhcp_reservations_provider.dart Update to use UI models and service layer                                +7/-9      dhcp_reservations_state.dart Replace JNAP model with DHCPReservationUIModel                      +5/-4      dhcp_reservations_view.dart Update to use DHCPReservationUIModel directly                        +11/-5    device_detail_view.dart Update to use DHCPReservationUIModel                                          +2/-2
Bug fix	1 files local_network_settings_view.dart Fix use_build_context_synchronously warnings                          +2/-0
Tests	8 files dhcp_reservation_ui_model_test.dart Add comprehensive unit tests for UI model                                +164/-0  local_network_settings_service_test.dart Add service layer tests for JNAP communication                      +274/-0  dhcp_reservations_service_test.dart Add tests for DHCP reservation business logic                        +88/-0    dhcp_reservations_test_data.dart Create test data builder for DHCP reservations                      +78/-0    local_network_settings_test_data.dart Create test data builder for LAN settings                                +97/-0    dhcp_reservations_notifier_mocks.dart Update mocks to use DHCPReservationUIModel                              +11/-8    local_network_settings_notifier_mocks.dart Update mocks to use DHCPReservationUIModel                              +8/-6      device_detail_view_test.dart Update test to use DHCPReservationUIModel                                +2/-2

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
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: 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: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unsafe map casts: DHCPReservationUIModel.fromMap directly casts map fields to String without null/type checks, which can throw at runtime on malformed/partial persisted JSON (edge cases not handled). Referred Code factory DHCPReservationUIModel.fromMap(Map<String, dynamic> map) { return DHCPReservationUIModel( macAddress: map['macAddress'] as String, ipAddress: map['ipAddress'] as String, description: map['description'] as String, ); 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 list decode: LocalNetworkStatus.fromMap constructs dhcpReservationList from dynamic JSON without validating element shape/types (e.g., DHCPReservationUIModel.fromMap(x)), risking crashes or unsafe state if persisted/external input is malformed. Referred Code factory LocalNetworkStatus.fromMap(Map<String, dynamic> map) { return LocalNetworkStatus( maxUserLimit: map['maxUserLimit']?.toInt() ?? 0, minAllowDHCPLeaseMinutes: map['minAllowDHCPLeaseMinutes']?.toInt() ?? 0, maxAllowDHCPLeaseMinutes: map['maxAllowDHCPLeaseMinutes']?.toInt() ?? 0, minNetworkPrefixLength: map['minNetworkPrefixLength']?.toInt() ?? 0, maxNetworkPrefixLength: map['maxNetworkPrefixLength']?.toInt() ?? 0, dhcpReservationList: List<DHCPReservationUIModel>.from( map['dhcpReservationList'] ?.map((x) => DHCPReservationUIModel.fromMap(x))), errorTextMap: Map<String, String>.from(map['errorTextMap'] ?? {}), Learn more about managing compliance generic rules or creating your own custom rules
⚪

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
High-level	Consolidate duplicated business logic The isReservationOverlap method in LocalNetworkSettingsProvider duplicates the logic of the new isConflict method in DHCPReservationsService. To adhere to the new service-layer architecture, remove the provider method and centralize conflict-checking in the service. Examples: lib/page/advanced_settings/local_network_settings/services/dhcp_reservations_service.dart [82-99] bool isConflict( DHCPReservationUIModel item, List<DHCPReservationUIModel> existingList, { int? indexToExclude, }) { for (int i = 0; i < existingList.length; i++) { if (indexToExclude != null && i == indexToExclude) { continue; } ... (clipped 8 lines) lib/page/advanced_settings/local_network_settings/providers/local_network_settings_provider.dart [219-230] bool isReservationOverlap( {required DHCPReservationUIModel item, int? index}) { final overlap = state.status.dhcpReservationList.where((element) { // Not compare with self if on editing if (index != null && state.status.dhcpReservationList.indexOf(element) == index) { return false; } return element.ipAddress == item.ipAddress || element.macAddress == item.macAddress; ... (clipped 2 lines) Solution Walkthrough: Before: // lib/page/advanced_settings/local_network_settings/services/dhcp_reservations_service.dart class DHCPReservationsService { bool isConflict(DHCPReservationUIModel item, List<DHCPReservationUIModel> existingList, ...) { // ... loop to check for mac/ip conflict } } // lib/page/advanced_settings/local_network_settings/providers/local_network_settings_provider.dart class LocalNetworkSettingsNotifier extends ... { bool isReservationOverlap({required DHCPReservationUIModel item, int? index}) { // ... uses `where().isNotEmpty` to check for mac/ip conflict } bool updateDHCPReservationOfIndex(DHCPReservationUIModel item, int index) { // ... if (!isReservationOverlap(item: item, index: index)) { ... } // ... } } After: // lib/page/advanced_settings/local_network_settings/services/dhcp_reservations_service.dart class DHCPReservationsService { bool isConflict(DHCPReservationUIModel item, List<DHCPReservationUIModel> existingList, ...) { // ... loop to check for mac/ip conflict } } // lib/page/advanced_settings/local_network_settings/providers/local_network_settings_provider.dart class LocalNetworkSettingsNotifier extends ... { // The `isReservationOverlap` method is removed. bool updateDHCPReservationOfIndex(DHCPReservationUIModel item, int index) { final service = ref.read(dhcpReservationsServiceProvider); final list = state.status.dhcpReservationList; // ... if (!service.isConflict(item, list, indexToExclude: index)) { ... } // ... } } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies duplicated business logic for conflict checking in isConflict and isReservationOverlap, which undermines the PR's primary goal of architectural separation and centralization of logic in the service layer.	Medium
Possible issue	Handle nullable reservation list Prevent a potential crash in LocalNetworkStatus.fromMap by handling cases where dhcpReservationList is null, defaulting to an empty list instead. lib/page/advanced_settings/local_network_settings/providers/local_network_settings_state.dart [300-302] -dhcpReservationList: List<DHCPReservationUIModel>.from( - map['dhcpReservationList'] - ?.map((x) => DHCPReservationUIModel.fromMap(x))), +dhcpReservationList: (map['dhcpReservationList'] as List<dynamic>?) + ?.map((x) => DHCPReservationUIModel.fromMap(x as Map<String, dynamic>)) + .toList() + ?? [], Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential null pointer exception in the fromMap factory method if map['dhcpReservationList'] is null, and provides a robust fix that prevents a runtime crash during deserialization.	Medium
General	Add explicit cast in fromMap Add an explicit cast to Map<String, dynamic> for map['data'] in ReservedListItem.fromMap to improve type safety. lib/page/advanced_settings/local_network_settings/providers/dhcp_reservations_state.dart [178] -data: DHCPReservationUIModel.fromMap(map['data']), +data: DHCPReservationUIModel.fromMap( + map['data'] as Map<String, dynamic>), Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly points out a missing explicit cast which improves type safety and prevents potential runtime errors, which is a good practice in Dart.	Medium
	Encapsulate DNS/WINS empty string logic Encapsulate the logic for converting empty DNS/WINS server strings to null within the DHCPSettings model to improve code reuse and maintainability. lib/page/advanced_settings/local_network_settings/services/local_network_settings_service.dart [133-148] final setLANSettings = SetRouterLANSettings( ipAddress: routerIp, networkPrefixLength: networkPrefixLength, hostName: hostName, isDHCPEnabled: isDHCPEnabled, dhcpSettings: DHCPSettings( firstClientIPAddress: firstClientIP, lastClientIPAddress: lastClientIP, leaseMinutes: leaseMinutes, - dnsServer1: dns1?.isEmpty == true ? null : dns1, - dnsServer2: dns2?.isEmpty == true ? null : dns2, - dnsServer3: dns3?.isEmpty == true ? null : dns3, - winsServer: wins?.isEmpty == true ? null : wins, + dnsServer1: dns1, + dnsServer2: dns2, + dnsServer3: dns3, + winsServer: wins, reservations: _toJNAPList(reservations), ), ); Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies that the logic for handling empty DNS/WINS strings has been moved but not fully encapsulated, and proposes a valid architectural improvement to move this logic into the DHCPSettings model for better code reuse and maintainability.	Low
	Simplify reservation read Simplify reading the dhcpReservationList by accessing it directly in the provider's select method, removing an intermediate variable. lib/page/advanced_settings/local_network_settings/views/dhcp_reservations_view.dart [50-54] -final localNetworkStatus = - ref.read(localNetworkSettingProvider.select((state) => state.status)); -// Direct access - LocalNetworkStatus.dhcpReservationList is already UI Model -final reservations = localNetworkStatus.dhcpReservationList; +final reservations = ref.read( + localNetworkSettingProvider.select( + (state) => state.status.dhcpReservationList, + ), +); Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion offers a valid and slightly more concise way to read the nested state from the provider, which improves code readability.	Low
More

[HankYuLinksys]

Looks good!