Skip to content

Core Architecture: DataUpdateCoordinator & Diagnostics Integration - #98

Closed
sirkirby with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-92
Closed

Core Architecture: DataUpdateCoordinator & Diagnostics Integration#98
sirkirby with Copilot wants to merge 7 commits into
mainfrom
copilot/fix-92

Conversation

Copilot AI commented Aug 20, 2025

Copy link
Copy Markdown

This PR refactors the data refresh logic to use Home Assistant's DataUpdateCoordinator patterns more effectively and adds comprehensive diagnostics export for easier troubleshooting.

Key Improvements

🔄 Enhanced DataUpdateCoordinator Integration

  • Improved refresh mechanisms: Replaced custom semaphore handling in _controlled_refresh_wrapper() and _force_refresh_with_cache_clear() with DataUpdateCoordinator's built-in async_request_refresh()
  • Better failure handling: Updated _handle_auth_failure() to leverage coordinator's automatic backoff and throttling instead of manual timing
  • Cleaner architecture: Added documentation clarifying how DataUpdateCoordinator's built-in concurrency control reduces need for manual locks

🔍 Comprehensive Diagnostics System

  • Smart sanitization: Auto-detects and masks passwords, tokens, keys, and other sensitive fields using pattern recognition
  • Rich data collection: Exports coordinator stats, WebSocket events, API session info, rule counts, and health metrics
  • Home Assistant integration: Added proper diagnostics.py file that integrates with HA's built-in diagnostics framework
  • Real-time monitoring: Tracks authentication state, consecutive errors, rate limiting, and entity management

🛡️ Security & Privacy

The diagnostics system automatically sanitizes sensitive data:

# Input
{"username": "admin", "password": "secret123", "api_token": "abc123def456"}

# Sanitized output  
{"username": "admin", "password": "***REDACTED***", "api_token": "***REDACTED***"}

🧪 Robust Testing

  • 36/36 tests passing with no regressions
  • 11 new test cases covering coordinator failure scenarios, diagnostics schema validation, and edge cases
  • Comprehensive coverage of sanitization logic, coordinator stats, and WebSocket event tracking

Benefits

  • More resilient to API hiccups through built-in backoff and failure handling
  • Easier troubleshooting - users can attach diagnostics zips when filing issues
  • Enhanced security - automatic credential sanitization prevents data leakage
  • Better monitoring - detailed insights into integration health and performance
  • Future-proof architecture - aligns with HA core integration patterns for 2025

Usage

Users can now export comprehensive diagnostics through Home Assistant's standard diagnostics interface, providing sanitized snapshots of configuration, recent events, and coordinator health metrics for troubleshooting.

Fixes #92.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits August 20, 2025 19:15
…n and sanitization

Co-authored-by: sirkirby <740137+sirkirby@users.noreply.github.com>
Co-authored-by: sirkirby <740137+sirkirby@users.noreply.github.com>
Copilot AI changed the title [WIP] Core Architecture: DataUpdateCoordinator & Diagnostics Integration Core Architecture: DataUpdateCoordinator & Diagnostics Integration Aug 20, 2025
Copilot AI requested a review from sirkirby August 20, 2025 19:20
@sirkirby

Copy link
Copy Markdown
Owner

@copilot this branch should be merging into dev and not main

Copilot AI commented Aug 21, 2025

Copy link
Copy Markdown
Author

@copilot this branch should be merging into dev and not main

Branch has been rebased onto dev instead of main. All 36 tests continue to pass after the rebase (commit f665766).

@sirkirby sirkirby left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good

@sirkirby
sirkirby marked this pull request as ready for review August 25, 2025 18:13
Copilot AI review requested due to automatic review settings August 25, 2025 18:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the UniFi Network Rules integration by improving DataUpdateCoordinator usage and adding comprehensive diagnostics capabilities. The changes focus on leveraging Home Assistant's built-in coordinator patterns more effectively while providing better troubleshooting tools through automated data sanitization and diagnostics export.

  • Refactored refresh mechanisms to use DataUpdateCoordinator's async_request_refresh() instead of custom semaphore handling
  • Added comprehensive diagnostics system with automatic sensitive data sanitization
  • Enhanced failure handling by leveraging coordinator's built-in backoff and throttling mechanisms

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/test_coordinator_diagnostics.py Adds comprehensive test coverage for coordinator failure scenarios and diagnostics functionality
tests/conftest.py Updates test fixture to remove deprecated event_loop parameter
custom_components/unifi_network_rules/utils/diagnostics.py Implements sanitization, coordinator stats collection, and diagnostics export functionality
custom_components/unifi_network_rules/diagnostics.py Adds Home Assistant integration for diagnostics export
custom_components/unifi_network_rules/coordinator.py Updates coordinator to use DataUpdateCoordinator patterns and improved error handling

Comment thread custom_components/unifi_network_rules/utils/diagnostics.py Outdated
Comment thread tests/test_coordinator_diagnostics.py Outdated
Comment thread custom_components/unifi_network_rules/coordinator.py Outdated
sirkirby and others added 3 commits August 25, 2025 14:15
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@sirkirby

Copy link
Copy Markdown
Owner

@copilot the test are now failing based on committing review suggestions. I see that in utils/diagnostics.py that the HEX_TOKEN_PATTERN and the ALNUM_TOKEN_PATTERN constants which should contain the regular expressions are not defined. There may be other issues as well

Co-authored-by: sirkirby <740137+sirkirby@users.noreply.github.com>

Copilot AI commented Aug 25, 2025

Copy link
Copy Markdown
Author

@copilot the test are now failing based on committing review suggestions. I see that in utils/diagnostics.py that the HEX_TOKEN_PATTERN and the ALNUM_TOKEN_PATTERN constants which should contain the r...

Fixed both issues in commit 9529485:

  1. Missing regex constants: Added HEX_TOKEN_PATTERN and ALNUM_TOKEN_PATTERN to utils/diagnostics.py for proper token detection and sanitization
  2. Coordinator concurrency: Removed manual _update_lock usage and leveraged DataUpdateCoordinator's built-in concurrency control as intended

All 36 tests are now passing with no regressions.

Copilot AI requested a review from sirkirby August 25, 2025 18:31
@sirkirby sirkirby closed this Sep 18, 2025
@sirkirby
sirkirby deleted the copilot/fix-92 branch September 18, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core Architecture: DataUpdateCoordinator & Diagnostics Integration

3 participants