[Core Data] Make new Core Data migration path. Remove models below 30 #15987

iamgabrielma · 2025-08-07T05:24:50Z

Apologies for the 15k LOC, it belongs mostly to deleting the .xcdatamodels and associated .xcmappings, the actual changes are the ~200 additions.

Description

The app currently has 124+ Core Data model versions, which represents 7+ years of incremental migrations. This PR starts the work on clearing the app from core data old models as part of p91TBi-drE-p2 by triggering a new migration path when a merchant hits the threshold (model 30). In future PRs we'll be raising the threshold to get it closer to the latest version.

If a merchant is below the threshold (<30), the new path is triggered: The database is destroyed, a fresh one set to the latest model is created, and the merchant is migrated directly, instead of performing iterative migrations.
If a merchant is above the threshold (30+), the existing path of iterative migration is triggered.

I left a bunch of DDLogs with what's happening, following the style of other core data handling functions, and through the migration steps.

Changes

Created a CoreDataMigratorUtils to encapsulate the logic related to finding the current DB version
Updated CoreDataIterativeMigrator with the new migration path. This relies on the existing persistent store coordinator to automatically create a fresh store with the latest model as needed.
Deleted oldest 30 .xcdatamodel
Deleted associated .xcmappings migrations between several of these models
Updated existing tests, added new tests, and updated documentation.

Steps to reproduce/Testing information

This is not very straight-forward to test, ultimately we'd want the merchant to not crash if they happen to be on a model version that has been deleted when a migration is performed. This crash is expected with an iterative migration if a step is missing, but won't happen with the direct migration.

If the worst happens and they crash due being unable to perform iterative migration, the app already performs a direct migration as a fallback, which is what we leverage directly via the threshold check when we can.

I added instructions on the P2 at p91TBi-drE-p2 on how I initially tested it by deleting 120 models instead, since this is the oldest available in TestFlight it allows us to simulate what would happen when a migration is needed and the threshold is hit.

At the moment of writing this PR the latest available TF version is 22.3, which as per MIGRATIONS.md was using model 120, so I'll use that as example. You can:

Install oldest TF version, login and play around, this should set the model 120 in your device
Set oldestSupportedDataModel to 122 or higher (including the model version that your TF build should be in), so it determines that anything below is an old model and should be purged entirely.
Delete the remaining .xcdatamodels up till 122 in Xcode, then run the app from Xcode in the same device you previously ran the TF build.
Following the traditional path, we would see a crash here since the iterative migration is broken (missing steps). Instead, since the model is below the threshold, we should migrate directly to the latest version, skipping the iterative migration path.
Observe there is no crash when running the app, and that works normally. From testing, the Order list will appear empty, a simple pull-to-refresh should repopulate it.

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

dangermattic · 2025-08-07T05:25:23Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

this will make some test fail (expected), ie: test_iterativeMigrate_can_iteratively_migrate_from_model_1_to_model_10 since we cannot iterateively migrate anymore below 30

wpmobilebot · 2025-08-07T06:14:30Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`pr15987-644059f`
	Version	`23.0`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`644059f`
	Installation URL	51qp04occ6j5g

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

Modules/Sources/Storage/CoreData/CoreDataMigratorUtils.swift

Modules/Sources/Storage/Model/MIGRATIONS.md

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift

iamgabrielma · 2025-08-07T08:59:46Z

Only one review is needed. No rush to review this before 23.0 freeze.

jaclync · 2025-08-08T09:22:50Z

Apology for not getting to it today. I will take a look first thing next Monday if it's still open.

jaclync

Thanks for taking on this task to reduce the app size. I had some questions marked with ❓ , especially the one about whether it can detect a deleted model version. Let me know what you think.

Modules/Sources/Storage/Model/MIGRATIONS.md

Modules/Sources/Storage/CoreData/CoreDataIterativeMigrator.swift

jaclync · 2025-08-11T00:52:10Z

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigrator+MigrationStepTests.swift

-        let modelVersion23 = ModelVersion(name: "Model 23")
-        let modelVersion31 = ModelVersion(name: "Model 31")
+        let modelVersion23 = ModelVersion(name: "Model 33")
+        let modelVersion31 = ModelVersion(name: "Model 41")


❓ I'm a bit confused by why these model versions need to be incremented by 10, instead of removing the ones < 30?

nit: for all the diffs in this file, the constant names can be updated to match the model version name

I'm a bit confused by why these model versions need to be incremented by 10, instead of removing the ones < 30?

No specific reason, I just added +10 to all of the cases for consistency. Since model 23 is below the current threshold it will return nil and not trigger the iterative migration for the test. Or are you referring to something else?

the constant names can be updated to match the model version name

Uggh of course I forgot, thanks! 😅 Updated 66da437

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigrator+MigrationStepTests.swift

jaclync · 2025-08-11T01:04:32Z

Modules/Sources/Storage/CoreData/CoreDataIterativeMigrator.swift

+        guard let sourceVersion = CoreDataMigratorUtils.findSourceVersion(for: sourceModel, in: modelsInventory) else {
+            DDLogInfo("Could not determine source model version. Using iterative migration.")
+            return false
+        }
+        // If the merchant is on a model version older than this, nuke the DB and migrate them to the latest one without iterative process
+        let oldestModelThreshold = Constants.oldestSupportedDataModel
+        let versionNumber = CoreDataMigratorUtils.extractVersionNumber(from: sourceVersion.name)


nit: how about combining CoreDataMigratorUtils.findSourceVersion and CoreDataMigratorUtils.extractVersionNumber into one function to simplify the code a bit? It looks like the two methods are only called here.

Suggested change

guard let sourceVersion = CoreDataMigratorUtils.findSourceVersion(for: sourceModel, in: modelsInventory) else {

DDLogInfo("Could not determine source model version. Using iterative migration.")

return false

}

// If the merchant is on a model version older than this, nuke the DB and migrate them to the latest one without iterative process

let oldestModelThreshold = Constants.oldestSupportedDataModel

let versionNumber = CoreDataMigratorUtils.extractVersionNumber(from: sourceVersion.name)

guard let versionNumber = CoreDataMigratorUtils.findSourceVersionNumber(for: sourceModel, in: modelsInventory) else {

DDLogInfo("Could not determine source model version. Using iterative migration.")

return false

}

// If the merchant is on a model version older than this, nuke the DB and migrate them to the latest one without iterative process

let oldestModelThreshold = Constants.oldestSupportedDataModel

In general I'd prefer to keep these separated, but with the simplification you suggested on 6080964 we can also remove extractVersionNumber entirely.

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift

jaclync · 2025-08-11T01:29:41Z

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift

+        XCTAssertEqual(versionNumber, 0)
+    }
+
+    func test_extractVersionNumber__when_version_number_then_handles_numbered_models() {


super nit:

Suggested change

func test_extractVersionNumber__when_version_number_then_handles_numbered_models() {

func test_extractVersionNumber_when_version_number_then_handles_numbered_models() {

Removed with 6080964

jaclync · 2025-08-11T01:35:10Z

Modules/Tests/StorageTests/CoreData/CoreDataManagerTests.swift

-        XCTAssertNil(NSEntityDescription.entity(forEntityName: Note.entityName,
+        // The OrderFeeLine entity does not exist in Model 30, was introduced in Model 42
+        // This proves that the store was reset to Model 30.
+        XCTAssertNil(NSEntityDescription.entity(forEntityName: OrderFeeLine.entityName,


nice replacement 👍

Rather than relying on a set constant and comparing model versions, we can rely on the existence of the model itself in the build to simplify the logic

jaclync

Thanks for the updates, LGTM Tested with TestFlight 22.3 build then PR build with model versions up to 121 deleted. Didn't see the DDLogInfo messages, but the app didn't crash and recovered after a bit 👍

Modules/Sources/Storage/CoreData/CoreDataIterativeMigrator.swift

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift

Co-authored-by: Jaclyn Chen <[email protected]>

Create CoreDataMigratorUtils and tests

5466c91

iamgabrielma added this to the 23.1 milestone Aug 7, 2025

iamgabrielma added type: task An internally driven task. type: technical debt Represents or solves tech debt of the project. labels Aug 7, 2025

iamgabrielma added 3 commits August 7, 2025 13:01

check for destroyPersistentStore. Extract utils to own file

b23e5e9

lint

8804c6c

update oldest supported DB threshold to 30

d72dd96

this will make some test fail (expected), ie: test_iterativeMigrate_can_iteratively_migrate_from_model_1_to_model_10 since we cannot iterateively migrate anymore below 30

iamgabrielma added 6 commits August 7, 2025 14:21

remove models 1 to 29

1bd0ede

updat ManagedObjectModelsInventoryTests

5f4e8fe

remove mapping models below 30 and related migration tests

67ba1af

lint

5439a99

update docs

65144c7

update tests that rely on state from removed models

91088d0

iamgabrielma commented Aug 7, 2025

View reviewed changes

Modules/Sources/Storage/CoreData/CoreDataMigratorUtils.swift Outdated Show resolved Hide resolved

iamgabrielma commented Aug 7, 2025

View reviewed changes

Modules/Sources/Storage/Model/MIGRATIONS.md Show resolved Hide resolved

iamgabrielma commented Aug 7, 2025

View reviewed changes

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift Show resolved Hide resolved

iamgabrielma marked this pull request as ready for review August 7, 2025 08:58

iamgabrielma requested review from itsmeichigo, jaclync and staskus August 7, 2025 08:58

jaclync reviewed Aug 11, 2025

View reviewed changes

iamgabrielma added 5 commits August 11, 2025 10:09

Merge branch 'trunk' into task/WOOMOB-1006-remove-cd-models-1-to-30

41ffc50

remove duplication due to wrapper

d1d463b

update var names to match model name

66da437

Restore test case when when migrating to the same model version

dc6431f

simplify shouldDestroyPersistentStore check

6080964

Rather than relying on a set constant and comparing model versions, we can rely on the existence of the model itself in the build to simplify the logic

iamgabrielma added 2 commits August 11, 2025 11:14

update model names to fit their number

08de9bf

lint

f544d68

iamgabrielma requested a review from jaclync August 11, 2025 04:39

jaclync approved these changes Aug 12, 2025

View reviewed changes

Modules/Sources/Storage/CoreData/CoreDataIterativeMigrator.swift Outdated Show resolved Hide resolved

Modules/Tests/StorageTests/CoreData/CoreDataIterativeMigratorTests.swift Show resolved Hide resolved

iamgabrielma and others added 2 commits August 12, 2025 10:45

Update Modules/Sources/Storage/CoreData/CoreDataIterativeMigrator.swift

35a6fd2

Co-authored-by: Jaclyn Chen <[email protected]>

Merge branch 'trunk' into task/WOOMOB-1006-remove-cd-models-1-to-30

644059f

iamgabrielma enabled auto-merge August 12, 2025 03:46

iamgabrielma merged commit d051c95 into trunk Aug 12, 2025
14 checks passed

iamgabrielma deleted the task/WOOMOB-1006-remove-cd-models-1-to-30 branch August 12, 2025 04:07

iamgabrielma mentioned this pull request Sep 9, 2025

[Core Data] Remove old models. Part 2 (30-59) #16109

Merged

iamgabrielma mentioned this pull request Sep 30, 2025

[Core Data] Remove old models. Part 3 (60-79) #16181

Merged

	func test_extractVersionNumber__when_version_number_then_handles_numbered_models() {
	func test_extractVersionNumber_when_version_number_then_handles_numbered_models() {

[Core Data] Make new Core Data migration path. Remove models below 30 #15987

[Core Data] Make new Core Data migration path. Remove models below 30 #15987

Uh oh!

Conversation

iamgabrielma commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Steps to reproduce/Testing information

Uh oh!

dangermattic commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

iamgabrielma commented Aug 7, 2025

Uh oh!

jaclync commented Aug 8, 2025

Uh oh!

jaclync left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jaclync Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

iamgabrielma Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jaclync Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

iamgabrielma Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaclync Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

iamgabrielma Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

jaclync Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

jaclync left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

iamgabrielma commented Aug 7, 2025 •

edited

Loading

dangermattic commented Aug 7, 2025 •

edited

Loading

wpmobilebot commented Aug 7, 2025 •

edited

Loading