store ios: Exclude the database file from iOS backup by rajveermalviya · Pull Request #2195 · zulip/zulip-flutter

rajveermalviya · 2026-03-03T19:44:21Z

Tested by generating iOS backups from Finder app on macOS, and then using iTunes-Backup-Explorer to read the backup files:

Before	After

chrisbobbe

Thanks! Comments below.

chrisbobbe · 2026-03-05T20:05:16Z

ios/Runner.xcodeproj/project.pbxproj

-		37D847A22F4CE7C20020EB99 /* RunnerUITests */ = {isa = PBXFileSystemSynchronizedRootGroup; explicitFileTypes = {}; explicitFolders = (); path = RunnerUITests; sourceTree = "<group>"; };
+		37D847A22F4CE7C20020EB99 /* RunnerUITests */ = {
+			isa = PBXFileSystemSynchronizedRootGroup;
+			exceptions = (
+			);
+			explicitFileTypes = {
+			};
+			explicitFolders = (
+			);
+			path = RunnerUITests;
+			sourceTree = "<group>";
+		};


The substantive change here is just to add an empty exceptions entry, right? It looks like this section was added in 509d88f (Patrol setup); I suspect it's not related to the intended change for your commit.

Would you take a few minutes to try and reproduce this change on its own, and either discard it or make a separate commit for it depending on what you learn? (E.g. probably we want to keep this change if it happens naturally when you clear an Xcode cache and rebuild, because in that case it probably belonged in 509d88f and was just left out accidentally.)

chrisbobbe · 2026-03-05T20:58:59Z

lib/model/store.dart

+    if (defaultTargetPlatform == TargetPlatform.iOS) {
+      // Exclude the database file from backups on iOS.
+      await IosNativeHostApi().setExcludedFromBackup(file.path);
+    }


If this throws it'll block loading the app (i.e. stop before creating the LiveGlobalStore) with no feedback to the user. Maybe instead run this unawaited() with a TODO(log) on error?

Also how about putting this call immediately after the await _dbFile() line? It sets an attribute on the file; the intervening database queries (await db.getGlobalSettings() etc.) seem unrelated and not prerequisites for this filesystem action.

That proximity should also help us remember to handle this detail carefully in the migration in your other PR #2156.

We can't put this after _dbFile(), because the database file may not exists yet. We can though put it after AppDatabase(NativeDatabase.createInBackground(file));, moved there.

Hmm but will the file exist right after the AppDatabase(NativeDatabase.createInBackground(file)) line? There's no await there, so maybe that line creates the file synchronously…which seems unusual? But then why is the method's name createInBackground?

I wonder if, in your previous revision, await db.getGlobalSettings() and friends were actually giving time for the file to finish being created asynchronously, before the setExcludedFromBackup call? Causing that call to succeed basically by accident.

A call like await db.getGlobalSettings() will wait for the database to be opened, which I believe will involve creating the database file if it doesn't exist. (Ultimately it calls sqlite3_open_v2 with flags SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, via [Sqlite3Implementation.open].)

I just spent a few minutes looking around Drift's API, and I don't see a totally clean way to say "wait for the database to be opened, then do X" short of actually making a query. (The closest is await db.executor.ensureOpen(db); but the type of executor is documented to be internal to Drift and unstable.)

NativeDatabase.createInBackground(file) returns the QueryExecutor which is same as db.executor. So, AIUI it should be safe to use executor.ensureOpen via the returned QueryExecutor?

Alternatively, there's db.doWhenOpened but it also says:

Calling this method directly might circumvent the current transaction. For that reason, it should only be called inside drift.

Reverted to the previous revision for now, where the call is made at the end of the load method.

chrisbobbe · 2026-03-05T21:01:06Z

ios/Runner/AppDelegate.swift

    notificationTapEventListener = NotificationTapEventListener()
    NotificationTapEventsStreamHandler.register(with: controller.binaryMessenger, streamHandler: notificationTapEventListener!)
+
+    IosNativeHostApiSetup.setUp(binaryMessenger: controller.binaryMessenger, api: IosNativeHostApiImpl())


nit: how about moving this above the // Retrieve the remote notification payload comment, so it doesn't interrupt notification-related code

chrisbobbe · 2026-03-05T21:04:39Z

pigeon/ios_native.dart

+
+@HostApi()
+abstract class IosNativeHostApi {
+  /// Sets the flag to exclude from iOS backups for the given filepath.


nit: can be more transparent about what this method does:

Suggested change

/// Sets the flag to exclude from iOS backups for the given filepath.

/// Sets UrlResourceValues.isExcludedFromBackup for the given file path.

///

/// See doc:

/// https://developer.apple.com/documentation/foundation/urlresourcevalues/isexcludedfrombackup

chrisbobbe · 2026-03-05T21:17:14Z

ios/Runner/AppDelegate.swift

+    var resourceValues = URLResourceValues()
+    resourceValues.isExcludedFromBackup = true
+
+    var url = URL(fileURLWithPath: path)


Suggested change

var url = URL(fileURLWithPath: path)

var url = URL(fileURLWithPath: path, isDirectory: false)

I think? In case this affects app startup time; see doc:

https://developer.apple.com/documentation/foundation/improving-performance-and-stability-when-accessing-the-file-system#Provide-directory-hints-for-file-URLs-to-avoid-wasteful-IO

For earlier versions [than iOS 16], opt for the nonblocking variants of the methods in the following table.

rajveermalviya · 2026-03-06T15:20:05Z

Thanks for the review @chrisbobbe! Pushed an update, PTAL.

chrisbobbe

Thanks!! Comments below.

chrisbobbe · 2026-03-06T19:04:44Z

ios/Runner/AppDelegate.swift

-
+

nit: indentation

chrisbobbe · 2026-03-06T19:16:55Z

lib/model/store.dart

+    if (defaultTargetPlatform == TargetPlatform.iOS) {
+      // Exclude the database file from backups on iOS.
+      await IosNativeHostApi().setExcludedFromBackup(file.path);
+    }


Hmm but will the file exist right after the AppDatabase(NativeDatabase.createInBackground(file)) line? There's no await there, so maybe that line creates the file synchronously…which seems unusual? But then why is the method's name createInBackground?

I wonder if, in your previous revision, await db.getGlobalSettings() and friends were actually giving time for the file to finish being created asynchronously, before the setExcludedFromBackup call? Causing that call to succeed basically by accident.

rajveermalviya · 2026-03-09T04:58:23Z

Thanks for the review @chrisbobbe! Pushed an update, PTAL.

chrisbobbe

Thanks! Bump on something small below, and marking for Greg's review.

chrisbobbe · 2026-03-09T17:14:44Z

lib/model/store.dart

+    // Disable OS backups for the database file, see:
+    //   https://github.com/zulip/zulip-flutter/issues/2158
+    unawaited(_maybeDisableOsBackup(file));


Suggested change

// Disable OS backups for the database file, see:

// https://github.com/zulip/zulip-flutter/issues/2158

unawaited(_maybeDisableOsBackup(file));

// Disable OS backups for the database file, see:

// https://github.com/zulip/zulip-flutter/issues/2158

unawaited(_maybeDisableOsBackup(file)); // TODO(log) on error

Just running `flutter build ios --config-only` wasn't sufficient, there were no changes, I had to trigger Xcode's edits to the `project.pbxproj` first, by changing the order of build phases (moved "Compile Sources" phase above "Run Script") and then again reverted the order of those build phases. The Xcode's edits ended up being changing the `objectVersion = 54;` to `objectVersion = 70;`, and addition of couple of empty `inputPaths = ();` and `outputPaths = ();`. After Xcode's edits to `project.pbxproj`, I ran `flutter build ios --config-only` which resulted in this commit. This tells that this was probably missed in merge/rebase conflict in 509d88f (Patrol setup). Which changed the `objectVersion` entry to `objectVersion = 54;` but missed the change to `37D847A22F4CE7C20020EB99 /* RunnerUITests */ = {` block. So, on the next `flutter run/build`, it probably doesn't bother to verify or make changes to the `project.pbxproj` because `objectVersion` is already set to a value that `flutter run/build` likes.

Fixes zulip#2158.

gnprice · 2026-03-09T21:00:55Z

Thanks @rajveermalviya for taking care of this, and @chrisbobbe for the detailed reviews!

Looks good; merging, with a few added comments including the one Chris suggested above:

diff --git lib/model/store.dart lib/model/store.dart
index 7c8fe3ecd..1084a2f01 100644
--- lib/model/store.dart
+++ lib/model/store.dart
@@ -1120,7 +1120,9 @@ class LiveGlobalStore extends GlobalStore {
 
     // Disable OS backups for the database file, see:
     //   https://github.com/zulip/zulip-flutter/issues/2158
-    unawaited(_maybeDisableOsBackup(file));
+    // This comes after the queries above, because it must come after
+    // the database file has been created on disk.
+    unawaited(_maybeDisableOsBackup(file)); // TODO(log) on error
 
     return LiveGlobalStore._(
       backend: LiveGlobalStoreBackend._(db: db),
diff --git pigeon/ios_native.dart pigeon/ios_native.dart
index f33261120..40bbebe8d 100644
--- pigeon/ios_native.dart
+++ pigeon/ios_native.dart
@@ -12,6 +12,9 @@ import 'package:pigeon/pigeon.dart';
 abstract class IosNativeHostApi {
   /// Sets UrlResourceValues.isExcludedFromBackup for the given file path.
   ///
+  /// The file at this path must already exist,
+  /// and be a regular file (not a directory).
+  ///
   /// See doc:
   ///   https://developer.apple.com/documentation/foundation/urlresourcevalues/isexcludedfrombackup
   ///   https://developer.apple.com/documentation/foundation/optimizing-your-app-s-data-for-icloud-backup

Those two points seemed like things that would otherwise be easy to miss when making future edits to this code.

These changes belonged in 1d03e57. Before merging that PR zulip#2195, I edited this comment in the source file of the pigeon but didn't rerun generating the files (nor CI), oops.

These changes belonged in 19d67b4. Before merging that PR zulip#2195, I edited this comment in the source file of the pigeon but didn't rerun generating the files (nor CI), oops.

rajveermalviya force-pushed the pr-ios-backup branch 2 times, most recently from 9012d25 to 0f3b485 Compare March 5, 2026 17:12

rajveermalviya marked this pull request as ready for review March 5, 2026 17:17

rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Mar 5, 2026

rajveermalviya assigned chrisbobbe Mar 5, 2026

rajveermalviya requested a review from chrisbobbe March 5, 2026 17:17

rajveermalviya force-pushed the pr-ios-backup branch from 0f3b485 to ace6b25 Compare March 5, 2026 17:54

chrisbobbe reviewed Mar 5, 2026

View reviewed changes

rajveermalviya force-pushed the pr-ios-backup branch 2 times, most recently from 6fe5c64 to b91fe25 Compare March 6, 2026 15:19

rajveermalviya requested a review from chrisbobbe March 6, 2026 15:20

chrisbobbe reviewed Mar 6, 2026

View reviewed changes

rajveermalviya force-pushed the pr-ios-backup branch 2 times, most recently from 8c8c0a9 to e85346a Compare March 9, 2026 04:57

rajveermalviya requested a review from chrisbobbe March 9, 2026 04:58

chrisbobbe reviewed Mar 9, 2026

View reviewed changes

chrisbobbe requested a review from gnprice March 9, 2026 17:15

chrisbobbe assigned gnprice and unassigned chrisbobbe Mar 9, 2026

chrisbobbe added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Mar 9, 2026

rajveermalviya added 2 commits March 9, 2026 13:59

store ios: Exclude the database file from iOS backup

1d03e57

Fixes zulip#2158.

gnprice force-pushed the pr-ios-backup branch from e85346a to 1d03e57 Compare March 9, 2026 21:01

gnprice merged commit 1d03e57 into zulip:main Mar 9, 2026
1 of 2 checks passed

chrisbobbe mentioned this pull request Mar 9, 2026

changelog: Describe changes since 30.0.270 #2198

Merged

gnprice mentioned this pull request Mar 9, 2026

ios [nfc]: Update comments generated from Pigeon #2210

Merged

gnprice mentioned this pull request Mar 30, 2026

notif ios: Decrypt E2EE notifications #2230

Open

	var url = URL(fileURLWithPath: path)
	var url = URL(fileURLWithPath: path, isDirectory: false)

Conversation

rajveermalviya commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisbobbe Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Mar 6, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rajveermalviya commented Mar 9, 2026

Uh oh!

chrisbobbe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice commented Mar 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rajveermalviya commented Mar 3, 2026 •

edited

Loading

chrisbobbe Mar 5, 2026 •

edited

Loading

rajveermalviya Mar 9, 2026 •

edited

Loading