「空になったら物理削除」がゴミ箱を素通り — メモ帳のサイレントデータロス 2 件をレビューで捕まえるまで
| 開発記録 | メモ帳
タグ: #iOS #Swift #Realm #CloudKit #コードレビュー #データ設計
iOS 版メモ帳にゴミ箱を付けました。テストは全部通っていたのに、「本文を空にして保存」「写真メモの最後の 1 枚を削除」の 2 経路がゴミ箱を素通りして物理削除まで走っていました。レビューで P1 として出た経緯と、空メモ保存を新規・既存・ゴミ箱内の 3 分岐に整理した修正の記録です。
ゴミ箱を付けたのに、ゴミ箱を素通りして消える経路が残っていた
iOS 版メモ帳(タブでフォルダ切り替え)の 1.9.0 で、ゴミ箱機能を追加しました。それまでの「削除」は、その場で本当に消える物理削除です。iCloud 同期を入れてからは削除がクラウド側にも伝播します。つまり誤タップ 1 回で全端末からメモが消える構造でした。
ゴミ箱はその安全弁です。TDD で API を固めてから画面を組み、既存分を含む全 14 本のユニットテストを通した状態でコミットしました。
ところが、その直後のコードレビューで P1(データロス)が 2 件 出ました。どちらも「ゴミ箱機能そのもの」ではありません。既存コードに昔からあった自動削除の経路がゴミ箱を素通りしていた という指摘です。テストは全部通っていたのに、なぜ見つからなかったのか。この記事はその記録です。
設計: ゴミ箱は「擬似フォルダ」、削除は「フォルダ移動」
まず今回の設計を簡単に。メモ帳のデータモデルは Realm で、メモ(Note)はフォルダ ID を 1 つ持ちます。「すべて」タブは ID 0 の擬似フォルダとして実装済みでした。なのでゴミ箱も同じパターンで、負の固定 ID を持つ擬似フォルダ にしました。ゴミ箱への移動は folderId の書き換え、つまり 論理削除 です。
struct Const {
// ゴミ箱の擬似フォルダ ID。0 = 未分類/すべて、正の値 = 実フォルダなので衝突しない
static let TRASH_FOLDER_ID = -1
}
extension Note {
var isInTrash: Bool {
return folderId == Const.TRASH_FOLDER_ID
}
/// ゴミ箱へ移動(論理削除)。folderId の変更として CloudKit にも自然に伝播する
func moveToTrash() {
updateFolderId(folderId: Const.TRASH_FOLDER_ID)
}
/// ゴミ箱内を除く全メモ(「すべて」タブ用)
static func loadAllActive() -> [Note] {
let predicate = NSPredicate(format: "folderId != %d", Const.TRASH_FOLDER_ID)
return Array(realm.objects(Note.self).filter(predicate)
.sorted(byKeyPath: "sortOrderAll", ascending: false))
}
}
この方式の利点は、既存の updateFolderId() がそのまま使えることです。この関数は updateDate を更新し、iCloud 同期フラグを立て、CloudKit への同期をスケジュールします。ゴミ箱移動も復元(別フォルダへの移動)も、同期の観点では「フォルダを移した」だけ。CloudKit 側に新しいレコード種別や削除ロジックを足さずに済みました。
テストもこの API に対して書きました。
func testMoveToTrash() {
let note = makeNote(id: 1, folderId: 3, text: "ゴミ箱へ移動するメモ")
note.moveToTrash()
XCTAssertEqual(note.folderId, Const.TRASH_FOLDER_ID)
XCTAssertTrue(note.isInTrash)
XCTAssertTrue(note.exBool0, "needsSync フラグが立つ必要がある")
}
func testLoadAllActiveExcludesTrash() {
makeNote(id: 1, folderId: 0, text: "通常メモ")
let trashed = makeNote(id: 2, folderId: 3, text: "削除済みメモ")
trashed.moveToTrash()
XCTAssertEqual(Note.loadAllActive().count, 1, "ゴミ箱内のメモは除外される")
}
func testDeleteFromTrashIsPhysical() {
let note = makeNote(id: 1, folderId: 3, text: "完全削除されるメモ")
note.moveToTrash()
note.delete()
XCTAssertEqual(testRealm.objects(Note.self).count, 0, "ゴミ箱からの削除は物理削除")
}
移動・除外・検索除外・ゴミ箱一覧・完全削除・復元。ゴミ箱の「仕様」として思いつく操作は一通り書きました。Green にしてから画面側の分岐を組みます。編集画面の削除ボタンは「通常メモならゴミ箱へ、ゴミ箱内なら確認ダイアログの上で完全削除」。一覧の「すべて削除」も同じ 2 分岐です。
1 件目: 空にして保存すると、確認なしで物理削除
レビューの 1 件目は編集画面の保存処理でした。このアプリには昔から「本文も写真も空のメモは、保存時に自動で片付ける」というロジックがあります。新規メモを開いて何も書かずに閉じたとき、空のメモが一覧に残らないようにするための後片付けです。
// 修正前(ゴミ箱導入時点)
func saveNote() {
if !isModifiedNote() { return }
if textView.text.count == 0 && photos.count == 0 {
delete() // ← 新規でも既存でも、ここで物理削除
return
}
// ... 通常の保存
}
問題は、この delete() が 既存メモに対しても走る ことです。既存メモを開いて本文を全部消して保存すると、「空になった」と判定されて物理削除に入ります。しかもこの delete() は Realm から消したうえで、CloudKit のレコード削除まで投げます。他の端末からも消えて、ゴミ箱には一切残りません。
ゴミ箱を付ける前なら、この挙動はまだ「削除ボタンと同じ結果になるだけ」でした。でもゴミ箱を付けた後は違います。削除ボタンはゴミ箱へ行くのに、本文を空にして保存すると即消える。ユーザーから見て説明のつかない非対称です。「間違えて全選択して消してしまい、そのまま閉じた」。メモアプリで一番ありそうな事故だと思います。
修正は 3 分岐にしました。
// 修正後
if textView.text.count == 0 && photos.count == 0 {
if let n = originalNote, n.id > 0, n.realm != nil {
if !n.isInTrash {
// 既存メモを空にして保存 → 物理削除せずゴミ箱へ
moveToTrash()
return
}
// ゴミ箱内のメモは自動削除しない(明示的な削除操作でのみ完全削除)
} else {
// 未保存の新規メモは従来どおり後片付け(Realm 未登録なので実削除は起きない)
delete()
return
}
}
ポイントは、「空だから消す」という 1 つの条件を 「どこにあるメモが空になったのか」で 3 つに割った ことです。
- 未保存の新規メモ: Realm に登録されていないので、従来どおり後片付けしてよい
- 通常タブの既存メモ: ゴミ箱へ移動する。ユーザーの意図が「削除」なら削除ボタンと同じ結果になり、誤操作なら復元できる
- ゴミ箱内の既存メモ: 何もしない。ゴミ箱の中身が勝手に消えるのは、ゴミ箱の存在意義に反する
n.realm != nil の判定は「Realm に管理されているオブジェクトか」を見ています。新規メモは保存前は unmanaged です。ここで新規と既存を確実に区別できます。
2 件目: 写真メモの最後の 1 枚を消すと、メモごと消える
2 件目は一覧側にありました。写真だけのメモ(本文なし)から写真を削除するフローです。最後の 1 枚を消して写真が 0 枚になると、「本文も写真も無いメモ」として Note.delete() が呼ばれていました。
// 修正前
let note = Note.find(id: noteId)
if note?.text.count == 0 {
note?.delete() // ← 確認なしで物理削除 + CloudKit 削除
}
こちらも 1 件目と同じ構造です。ユーザーの操作は「写真を 1 枚消した」だけ。メモを消したつもりはありません。ゴミ箱導入前でも危うい挙動でした。導入後は「削除ボタンより強い削除」が残ることになります。
// 修正後
if let note = note, note.text.count == 0 {
// 最後の写真を削除して空になったメモは物理削除せずゴミ箱へ
// (ゴミ箱内のメモは明示的な削除操作でのみ完全削除する)
if !note.isInTrash {
note.moveToTrash()
}
}
なぜテストを通り抜けたのか
ゴミ箱関連で新たに書いた 7 本のテストは全部通っていました。それでも P1 が 2 件残った理由ははっきりしています。テストが守っていたのは「ゴミ箱 API を呼んだときの振る舞い」であって、「アプリ内で delete() が呼ばれる全経路」ではなかった からです。
ゴミ箱を作るとき、僕の頭にあった「削除」は削除ボタンと「すべて削除」の 2 つでした。その 2 つはきちんとゴミ箱へ振り分けています。でもコードベースには、ユーザーが「削除」と思っていない場所から delete() を呼ぶ経路が 2 つありました。そちらは「削除機能」として認識していないので、分岐の対象に入らない(安全弁を付けた本人が、裏口を開けたままにしていました)。
言い換えます。ゴミ箱を導入するというのは「削除ボタンの挙動を変える」ことではありません。「このアプリで物理削除が許される条件を再定義する」 ことでした。後者として捉えていれば、最初にやるべきことは決まっています。delete() の呼び出し箇所を全部列挙して、1 つずつ「ゴミ箱を経由すべきか」を判定する。
レビューは、まさにこの「delete() の呼び出し元を全部見る」をやってくれました。人間が機能単位で考えて見落とす経路を、呼び出しグラフ単位で機械的に潰していく。AI レビューがいちばん効くのはこういう場面だと改めて思います。
同じ考え方で見直すと、一覧の「すべて削除」にも問題がありました。旧実装は Note.deleteAll() と Photo.deleteAll() を呼びます。つまり タブに関係なく全メモ・全写真を消す 実装です。ゴミ箱対応と同時に「通常タブなら全件ゴミ箱へ移動、ゴミ箱タブなら確認のうえ全件完全削除」に直しました。
学び
- 論理削除の導入は「削除ボタンの改修」ではなく「物理削除が許される条件の再定義」。まず物理削除関数の呼び出し元をすべて列挙し、1 箇所ずつ「ゴミ箱を経由すべきか」を判定する
- ユーザーが「削除」と思っていない操作の裏で走る削除が一番危ない。「空になったら片付ける」「最後の写真を消したらメモも消す」のような暗黙の後片付けは、削除ボタンと同じ扱いにするか、より弱い扱い(ゴミ箱行き)にする
- 「空」の扱いは「どこにあるメモが空になったか」で分ける。未保存の新規 / 通常タブの既存 / ゴミ箱内の既存で、正しい振る舞いはそれぞれ違う
- ゴミ箱の中身を自動で消す経路を作らない。ゴミ箱内のメモは、ユーザーが確認ダイアログを通した明示的な操作でだけ消える。これを不変条件にする
- API のテストが通ることと、アプリの全経路がその API を通ることは別の話。新しい安全機構を入れたら、古い経路がそれを迂回していないかを確認する
ゴミ箱付きの 1.9.0 は App Store で公開済みです。誤って消したメモは、右端のゴミ箱タブから元のフォルダへ戻せます。もう「消えました」の連絡が来ませんように。