Skip to content

fix: persist MessagePack header after deletion - #69

Merged
kochounoyume merged 3 commits into
mainfrom
feature/fix-messagepack-delete-header
Jul 18, 2026
Merged

fix: persist MessagePack header after deletion#69
kochounoyume merged 3 commits into
mainfrom
feature/fix-messagepack-delete-header

Conversation

@kochounoyume

Copy link
Copy Markdown
Member

概要

MessagePack 実装でキーを削除した後、新しいインスタンスで同じ保存先を開くと、削除前の古いヘッダーを読み込んでしまう問題を修正します。公開 API は変更しません。

発生していた問題

削除処理は、メモリ上のヘッダー更新と値本体の削除までは行っていましたが、更新後ヘッダーの永続化に MessagePackSerializer.Serialize(_header, options) を使用していました。この overload はシリアライズ済みバイト列を返しますが、戻り値が使用されていなかったため、_writer へ新しいヘッダーが書き込まれていませんでした。

同一インスタンスでは更新済みのメモリ上ヘッダーを参照するため正常に見えますが、再オープンすると古いヘッダーが復元されます。その結果、削除したキーが残って見えたり、後続値の位置情報が不整合になる可能性がありました。

修正内容

  • ヘッダーを _writer へ直接シリアライズする overload へ変更
  • a / b / c の中央 b を削除し、別インスタンスで再オープンする回帰テストを追加
  • 再オープン後に ac が保持され、b が存在しないことを確認
  • WebGL Player テストからも同じ永続化ケースを実行

影響範囲

  • MessagePack 実装の削除処理のみ
  • 公開 API と保存形式の変更なし
  • 同一インスタンス内の見かけ上の動作ではなく、実際に永続化されるヘッダーを修正

検証

  • dotnet format を変更 C# ファイルへ実行
  • dotnet test src/LocalPrefs.Tests/LocalPrefs.Tests.csproj --configuration Release --framework net9.0: 81 / 81 passed
  • 統合差分で Rider / Unity コンパイル成功
  • WebGL Player テスト画面で All test(s) succeeded を確認

補足

このブランチは main から独立しており、他の修正PRを前提にしません。MessagePack 3.1.3 の既存 NU1902 / NU1903 警告は別PRの 3.1.8 更新で対応します。

@kochounoyume kochounoyume self-assigned this Jul 18, 2026
# Conflicts:
#	src/LocalPrefs.Tests/Editor/LocalPrefsCoreTest.cs
@kochounoyume
kochounoyume marked this pull request as ready for review July 18, 2026 17:22
Copilot AI review requested due to automatic review settings July 18, 2026 17:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a persistence bug in the MessagePack-backed ILocalPrefs implementation where deleting a key updated the in-memory header but failed to write the updated header back to storage, causing a subsequent instance reopening the same path to read a stale header.

Changes:

  • Fix MessagePackLocalPrefs.DeleteAsync to serialize the updated header directly into the writer (so it’s actually persisted).
  • Add a regression test that deletes the middle key (a/b/c → delete b) and verifies correctness after reopening via a new instance.
  • Run the new regression scenario through both the .NET test suite and the Unity WebGL player test wrapper.

Reviewed changes

Copilot reviewed 4 out of 7 changed files in this pull request and generated no comments.

File Description
src/LocalPrefs.MessagePack/MessagePackLocalPrefs.cs Fixes header persistence by serializing the updated header into _writer during deletion.
src/LocalPrefs.Tests/Runtime/LocalPrefsTest.cs Adds a regression test verifying delete + reopen preserves remaining keys/values.
src/LocalPrefs.Tests/Editor/LocalPrefsCoreTest.cs Wires the new regression test into the editor test suite across factories.
src/LocalPrefs.Unity/Assets/Tests/LSPrefsTest.cs Wires the new regression test into Unity WebGL test execution.

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

@kochounoyume
kochounoyume merged commit f21f5b2 into main Jul 18, 2026
8 checks passed
@kochounoyume
kochounoyume deleted the feature/fix-messagepack-delete-header branch July 18, 2026 17:30
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