Skip to content

feat: migrate legacy fields.json on first 0.2 launch#15

Merged
nedseb merged 3 commits into
mainfrom
feat/settings-schema-migration
Apr 28, 2026
Merged

feat: migrate legacy fields.json on first 0.2 launch#15
nedseb merged 3 commits into
mainfrom
feat/settings-schema-migration

Conversation

@nedseb
Copy link
Copy Markdown
Contributor

@nedseb nedseb commented Apr 28, 2026

Summary

Closes #10.

Settings persisted by 0.1.x stored EasyDapLink directly at the JSON top level. After the wireless-stack work landed, the new reader expected {tab_daplink: {...}, tab_ws: {...}} and silently failed on the legacy shape — users upgrading from 0.1 to 0.2 started fresh with all paths and target name lost.

This PR adds a one-shot fallback inside load_settings:

  1. Try serde_json::from_slice::<MainWindow>(content) (current shape).
  2. On failure, try serde_json::from_slice::<TabDaplink>(content) (legacy shape — TabDaplink has the exact same fields as the old EasyDapLink, since it was just renamed).
  3. If the legacy shape parses, lift it into self.tab_daplink. tab_ws keeps its default.

The file is then rewritten in the new shape on the next CloseRequested, so the migration is transparent and runs at most once per user.

Test plan

  • Drop a legacy fields.json ({"bootloader_path": "...", "firmware_path": "...", "user_file_path": "...", "target_waiting_time": 10, "target_name": "STEAMI"}) into the settings dir, launch — confirm the fields appear in the DapLink tab and the log shows Legacy settings migrated to the new schema.
  • Close + reopen the app, inspect the file — should now be in the new shape.
  • Drop a malformed JSON, confirm the existing error message still fires (Failed to deserialize settings. Error: ...).
  • Fresh install with no settings file: same behavior as before.

Settings persisted by 0.1.x stored an `EasyDapLink` directly at the
JSON top level (`bootloader_path`, `firmware_path`, ...). After the
wireless-stack tabbed-window refactor, the 0.2 reader expected the
new shape (`{tab_daplink: {...}, tab_ws: {...}}`) and silently failed
on the old one — users started fresh with empty fields.

Add a one-shot fallback in load_settings: if the new shape fails,
try deserializing the buffer as `TabDaplink` (which has exactly the
legacy field set since the 0.1 EasyDapLink was just renamed) and
lift it into `self.tab_daplink`. `tab_ws` keeps its default. The
file is rewritten in the new shape on the next `CloseRequested`,
making the migration transparent and one-shot.

Closes #10.
Copilot AI review requested due to automatic review settings April 28, 2026 10:36
Copy link
Copy Markdown

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

Adds a backward-compatible settings loader so users upgrading from 0.1.x to 0.2 keep their saved DAPLink paths/target settings when the persisted fields.json schema changed.

Changes:

  • Refactors settings loading into MainWindow::load_settings and adds a legacy-deserialization fallback (0.1.x top-level schema -> tab_daplink).
  • Keeps tab_ws at default when migrating legacy settings; rewritten in new shape on next close via existing save path.
  • Ignores CLAUDE.md via .gitignore.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/ui/main_window.rs Implements one-shot legacy schema fallback when deserializing fields.json and centralizes settings load logic.
.gitignore Adds CLAUDE.md to ignored files.

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

Comment thread src/ui/main_window.rs Outdated
Comment on lines +120 to +125
if let Ok(legacy) = serde_json::from_slice::<TabDaplink>(content) {
self.tab_daplink = legacy;
println!("Legacy settings migrated to the new schema");
return;
}
eprintln!("Failed to deserialize settings. Error: {new_err}");
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

When deserialization fails for the new schema and the legacy fallback also fails, the code logs only new_err (from attempting MainWindow). In that situation the message can be misleading (e.g., “missing field tab_daplink”) and hides why the legacy parse failed. Consider capturing and logging the legacy error too (or logging a combined message indicating both attempts failed).

Suggested change
if let Ok(legacy) = serde_json::from_slice::<TabDaplink>(content) {
self.tab_daplink = legacy;
println!("Legacy settings migrated to the new schema");
return;
}
eprintln!("Failed to deserialize settings. Error: {new_err}");
match serde_json::from_slice::<TabDaplink>(content) {
Ok(legacy) => {
self.tab_daplink = legacy;
println!("Legacy settings migrated to the new schema");
return;
}
Err(legacy_err) => {
eprintln!(
"Failed to deserialize settings. New schema error: {new_err}. Legacy schema error: {legacy_err}"
);
}
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ca587eb — applied your suggestion. Both errors are now reported when both schemas fail.

Comment thread src/ui/main_window.rs Outdated
Comment on lines +108 to +110
/// schema (a flat `EasyDapLink` serialized at the top level — now mapped
/// onto `tab_daplink`). Successful migrations get rewritten in the new
/// shape on the next `CloseRequested`.
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The doc comment references a legacy EasyDapLink type, but that type doesn’t exist in the current codebase (only TabDaplink does). This can be confusing for future maintenance; consider rewording to describe the legacy schema as “top-level TabDaplink fields” (or similar) without naming a removed type.

Suggested change
/// schema (a flat `EasyDapLink` serialized at the top level — now mapped
/// onto `tab_daplink`). Successful migrations get rewritten in the new
/// shape on the next `CloseRequested`.
/// schema where `TabDaplink` fields were serialized directly at the top
/// level and are now mapped onto `tab_daplink`. Successful migrations get
/// rewritten in the new shape on the next `CloseRequested`.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ca587eb — reworded as suggested, no more reference to the removed EasyDapLink type.

nedseb added 2 commits April 28, 2026 12:43
- Log both the new-schema and legacy-schema errors when both attempts
  fail. Previously only the new-schema error was reported, which often
  said something like "missing field tab_daplink" and hid the actual
  reason a legacy file couldn't be parsed either.

- Reword the doc comment on load_settings: it referenced a removed
  EasyDapLink type that no longer exists in the codebase. Now describes
  the legacy shape as "TabDaplink fields serialized at the top level".

- Drop an unrelated CLAUDE.md addition to .gitignore that slipped into
  the previous commit by accident.
Four unit tests on `MainWindow::load_settings` covering the test plan
items announced on the PR:

- loads_new_schema: 0.2 fields.json round-trips correctly.
- legacy_schema_migrates_and_serializes_in_new_shape: a 0.1.x flat
  fields.json is loaded and re-serializing the resulting MainWindow
  produces the new tab_daplink/tab_ws shape with the legacy values
  preserved. This is the same code path as the CloseRequested save
  handler, so it exercises the on-disk migration without needing
  the GUI runtime (winit 0.30 panics on X11 close on this system,
  so a live end-to-end test wasn't viable).
- malformed_json_keeps_defaults: error path doesn't corrupt state.
- empty_content_keeps_defaults: same, against an empty buffer.

`cargo test` now runs 4 tests where there were 0.
@nedseb nedseb merged commit d23adac into main Apr 28, 2026
3 checks passed
@nedseb nedseb deleted the feat/settings-schema-migration branch April 28, 2026 11:17
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.

fields.json schema changed in 0.2.0: existing users lose saved settings on upgrade

2 participants