「空になったら物理削除」がゴミ箱を素通り — メモ帳のサイレントデータロス 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 つありました。そちらは「削除機能」として認識していないので、分岐の対象に入らない(安全弁を付けた本人が、裏口を開けたままにしていました)。

通常タブで削除に至る 4 つの経路の図。削除ボタンとすべて削除はゴミ箱へ。本文を空にして保存と写真の最後の 1 枚の削除は、修正前は物理削除と CloudKit 削除に直行していた。修正後は通常タブからの 4 経路すべてがゴミ箱へ。ゴミ箱タブでの削除は確認ダイアログのうえ完全削除
図: レビューで見つかった 2 経路(赤)はゴミ箱を素通りして物理削除していた。修正後(緑の破線)は通常タブの既存メモならゴミ箱へ

言い換えます。ゴミ箱を導入するというのは「削除ボタンの挙動を変える」ことではありません。「このアプリで物理削除が許される条件を再定義する」 ことでした。後者として捉えていれば、最初にやるべきことは決まっています。delete() の呼び出し箇所を全部列挙して、1 つずつ「ゴミ箱を経由すべきか」を判定する。

レビューは、まさにこの「delete() の呼び出し元を全部見る」をやってくれました。人間が機能単位で考えて見落とす経路を、呼び出しグラフ単位で機械的に潰していく。AI レビューがいちばん効くのはこういう場面だと改めて思います。

同じ考え方で見直すと、一覧の「すべて削除」にも問題がありました。旧実装は Note.deleteAll()Photo.deleteAll() を呼びます。つまり タブに関係なく全メモ・全写真を消す 実装です。ゴミ箱対応と同時に「通常タブなら全件ゴミ箱へ移動、ゴミ箱タブなら確認のうえ全件完全削除」に直しました。

学び

  • 論理削除の導入は「削除ボタンの改修」ではなく「物理削除が許される条件の再定義」。まず物理削除関数の呼び出し元をすべて列挙し、1 箇所ずつ「ゴミ箱を経由すべきか」を判定する
  • ユーザーが「削除」と思っていない操作の裏で走る削除が一番危ない。「空になったら片付ける」「最後の写真を消したらメモも消す」のような暗黙の後片付けは、削除ボタンと同じ扱いにするか、より弱い扱い(ゴミ箱行き)にする
  • 「空」の扱いは「どこにあるメモが空になったか」で分ける。未保存の新規 / 通常タブの既存 / ゴミ箱内の既存で、正しい振る舞いはそれぞれ違う
  • ゴミ箱の中身を自動で消す経路を作らない。ゴミ箱内のメモは、ユーザーが確認ダイアログを通した明示的な操作でだけ消える。これを不変条件にする
  • API のテストが通ることと、アプリの全経路がその API を通ることは別の話。新しい安全機構を入れたら、古い経路がそれを迂回していないかを確認する

ゴミ箱付きの 1.9.0 は App Store で公開済みです。誤って消したメモは、右端のゴミ箱タブから元のフォルダへ戻せます。もう「消えました」の連絡が来ませんように。