Skip to content

Conversation

@raman325
Copy link
Collaborator

Summary

  • Replaces custom JSON file handling with Home Assistant's built-in Store helper
  • Includes automatic migration from the legacy JSON format
  • Zero user-facing changes - existing configurations work unchanged

Background

Previously, keymaster persisted lock data (PINs, code slot settings, etc.) to a custom JSON file at custom_components/keymaster/json_kmlocks/keymaster_kmlocks.json. This worked but had several issues:

  1. No atomic writes - If HA crashed mid-write, the file could be corrupted (as seen in PR refactor: introduce provider abstraction for multi-platform lock support #538 testing)
  2. Sync I/O in executor - Required async_add_executor_job() wrappers for file operations
  3. Non-standard location - Data stored inside the integration folder instead of HA's .storage/ directory
  4. Manual JSON handling - Custom serialization/deserialization code

What Changed

New Architecture

Before: custom_components/keymaster/json_kmlocks/keymaster_kmlocks.json
After:  .storage/keymaster.locks

The new implementation uses homeassistant.helpers.storage.Store:

Old Method New Method
_create_json_folder() Not needed
_get_dict_from_json_file() _async_load_data()
_write_config_to_json() _async_save_data()
delete_json() async_remove_data()

Why Store is Better

  1. Atomic writes - Store writes to a temp file then renames, preventing corruption if HA crashes mid-write
  2. Native async - No executor jobs needed; proper async file I/O
  3. Standard location - Data lives in .storage/ alongside other HA data, making backups easier
  4. Built-in versioning - STORAGE_VERSION constant enables future schema migrations
  5. Battle-tested - Used by HA core and hundreds of integrations

Migration

On first startup after this update:

  1. Keymaster checks if the legacy json_kmlocks/keymaster_kmlocks.json exists
  2. If found, loads the data and saves it to the new Store location
  3. Deletes the legacy JSON file (and folder if empty)
  4. Logs the migration for visibility

Users don't need to do anything - migration is automatic and seamless.

Type of change

  • Refactoring (no functional changes)

Test plan

  • All 271 existing tests pass
  • Verified migration logic loads legacy JSON correctly
  • Verified Store save/load round-trips data correctly

🤖 Generated with Claude Code

Replace custom JSON file handling with Home Assistant's built-in
Store helper for data persistence. This provides:

- Atomic writes (prevents corruption)
- Proper async operations
- Standard `.storage/keymaster.locks` location
- Built-in versioning for migrations

Migration support included: on first startup, any existing
`json_kmlocks/keymaster_kmlocks.json` file is automatically
migrated to the new Store format and deleted.

Changes:
- Add Store import and STORAGE_VERSION/STORAGE_KEY constants
- Replace _json_folder/_json_filename with _store instance
- Add _async_load_data() with legacy JSON migration
- Add _async_save_data() and async_remove_data() methods
- Remove _create_json_folder, _get_dict_from_json_file,
  _write_config_to_json, and delete_json methods
- Update __init__.py to use async_remove_data()
- Update test to patch new method name

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings January 20, 2026 02:42
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 60.31746% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.48%. Comparing base (a9a30ef) to head (78eac2a).
⚠️ Report is 10 commits behind head on beta.

Files with missing lines Patch % Lines
custom_components/keymaster/coordinator.py 60.00% 24 Missing ⚠️
custom_components/keymaster/__init__.py 0.00% 1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             beta     #550      +/-   ##
==========================================
- Coverage   80.86%   80.48%   -0.38%     
==========================================
  Files          19       21       +2     
  Lines        2341     2501     +160     
==========================================
+ Hits         1893     2013     +120     
- Misses        448      488      +40     
Flag Coverage Δ
python 80.48% <60.31%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

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 refactors the keymaster integration to use Home Assistant's built-in Store helper for data persistence instead of custom JSON file handling. The change includes automatic migration from the legacy JSON format located in custom_components/keymaster/json_kmlocks/keymaster_kmlocks.json to the new .storage/keymaster.locks location.

Changes:

  • Replaced manual JSON file operations with Store API for atomic, async data persistence
  • Added migration logic to automatically convert legacy JSON files to the new storage format
  • Updated all method calls from _write_config_to_json() to _async_save_data() and delete_json() to async_remove_data()

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
custom_components/keymaster/coordinator.py Replaces custom JSON file handling with Store API, adds migration logic, removes old sync file operation methods
custom_components/keymaster/init.py Updates coordinator cleanup to use new async_remove_data() method
tests/test_coordinator_lifecycle.py Updates test mock to patch new _async_save_data method
custom_components/keymaster/time.py Removes unused KeymasterCodeSlot import
custom_components/keymaster/services.py Removes unused functools import

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

raman325 and others added 5 commits January 20, 2026 00:02
Change 'if config:' to 'if config is not None:' to ensure empty
legacy JSON files are properly migrated and deleted. An empty dict
is a valid state (no configured locks) and should be migrated.

Addresses Copilot review comment.
Combine the two separate executor calls for file deletion and
folder removal into a single _cleanup_legacy_files() method.
Reduces context switching overhead during migration.
Separate migration logic from cleanup - now the legacy file is always
deleted when found, regardless of whether it contained valid data.
This prevents users with corrupted legacy files from hitting the same
error repeatedly.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Combine load and cleanup into one `_migrate_legacy_json` method to
reduce context switches between async and sync code.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants