Fix WordPressData runtime issues #24494

kean · 2025-04-23T16:49:20Z

No description provided.

dangermattic · 2025-04-23T16:50:02Z

	1 Warning
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.

Generated by 🚫 Danger

kean · 2025-04-23T16:50:29Z

Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 155.xcdatamodel/contents

@@ -55,7 +55,7 @@
            <fetchIndexElement property="blogs" type="Binary" order="ascending"/>
        </fetchIndex>
    </entity>
-    <entity name="AccountSettings" representedClassName="ManagedAccountSettings" syncable="YES">
+    <entity name="AccountSettings" representedClassName=".ManagedAccountSettings" syncable="YES">


Swift-only types like ManagedAccountSettings are not present in the global namespace. You have to specify the module. . means "current module".

public class ManagedAccountSettings: NSManagedObject

Does that mean the migration from version 154 (which has an incorrect module) to 155 is broken?

Should we make this change (adding .) to all modal files?

I see, my mistake was not making the leap in understanding that Objective-C types are handled differently from Swift ones.

Does that mean the migration from version 154 (which has an incorrect module) to 155 is broken?

Maybe a unit test could clear this up? #24496

@crazytonyli you are onto it! ☝️

In version 154, we crash upon trying to init BlogAuthor with

Could not cast value of type 'NSManagedObject_BlogAuthor_' (0x6000012d5aa0) to 'WordPressData.BlogAuthor' (0x1616aea20).

The solution is to apply the same representedClassName change in 154:

diff --git a/Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 154.xcdatamodel/contents b/Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 154.xcdatamodel/contents index 2267cb2313..292045b17d 100644 --- a/Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 154.xcdatamodel/contents +++ b/Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 154.xcdatamodel/contents @@ -204,7 +204,7 @@ </fetchIndex> <userInfo/> </entity> - <entity name="BlogAuthor" representedClassName="WordPress.BlogAuthor" syncable="YES"> + <entity name="BlogAuthor" representedClassName=".BlogAuthor" syncable="YES"> <attribute name="avatarURL" optional="YES" attributeType="String" syncable="YES"/> <attribute name="deletedFromBlog" attributeType="Boolean" defaultValueString="NO" usesScalarValueType="YES" syncable="YES"/> <attribute name="displayName" optional="YES" attributeType="String" syncable="YES"/>

See also CI from #24496

I pushed 2a17e73 in my PR that updates all the models. I used two bash commands, so I might have messed up something. Still, the test above now passes. Progress.

The first thing the app does is migrate from the production model (154). It never loads the represented entities in memory. It uses NSManagedObject and KVC for migration. It's impossible to retroactively change the models and there is also no need to. The app has representedClassName values in the previous models with class name that no longer even exist in the codebase – it's what the migration is for.

kean · 2025-04-23T16:53:57Z

Sources/WordPressData/Swift/PublicizeInfo+CoreDataClass.swift

@@ -8,7 +8,8 @@ import WordPressKit
 /// Furthermore, sites eligible for unlimited sharing will still return a `PublicizeInfo` along with its sharing
 /// limitations, but the numbers should be ignored (at least for now).
 ///
-@objc public class PublicizeInfo: NSManagedObject {
+@objc(PublicizeInfo)
+public class PublicizeInfo: NSManagedObject {


The following annotation implicitly adds a module name to the Objective-C class name.

@objc open class PublicizeInfo (lldb) po NSStringFromClass(PublicizeInfo.self) "WordPressData.PublicizeInfo"

This breaks most of our code that works with entities (it uses classNameWithoutNamespaces).

The fix is to manually set the global name:

@objc(PublicizeInfo) open class PublicizeInfo

kean · 2025-04-23T16:54:09Z

Sources/WordPressData/Swift/ReaderTeamTopic.swift

+import Foundation
+
+@objc(ReaderTeamTopic)
+open class ReaderTeamTopic: ReaderAbstractTopic {


Moved from Keystone.

Ouch. I was sure to have moved all the types but clearly missed this. I think the fact that it does not inherit from NSManagedObject tricked me.

I greped the model and checked every representedClassName – we should be good now.

wpmobilebot · 2025-04-23T16:59:42Z

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

	App Name	WordPress
	Configuration	Release-Alpha
	Build Number	`27489`
	Version	`PR #24494`
	Bundle ID	`org.wordpress.alpha`
	Commit	`43032c5`
	Installation URL	08arvsmm9pr90

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

wpmobilebot · 2025-04-23T16:59:45Z

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

	App Name	Jetpack
	Configuration	Release-Alpha
	Build Number	`27489`
	Version	`PR #24494`
	Bundle ID	`com.jetpack.alpha`
	Commit	`43032c5`
	Installation URL	63nc4j8m52ps8

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

sonarqubecloud · 2025-04-23T20:51:23Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mokagio

Legend!

mokagio · 2025-04-24T05:01:56Z

Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 155.xcdatamodel/contents

@@ -55,7 +55,7 @@
            <fetchIndexElement property="blogs" type="Binary" order="ascending"/>
        </fetchIndex>
    </entity>
-    <entity name="AccountSettings" representedClassName="ManagedAccountSettings" syncable="YES">
+    <entity name="AccountSettings" representedClassName=".ManagedAccountSettings" syncable="YES">


I see, my mistake was not making the leap in understanding that Objective-C types are handled differently from Swift ones.

mokagio · 2025-04-24T05:06:31Z

Sources/WordPressData/Swift/PublicizeInfo+CoreDataClass.swift

@@ -8,7 +8,8 @@ import WordPressKit
 /// Furthermore, sites eligible for unlimited sharing will still return a `PublicizeInfo` along with its sharing
 /// limitations, but the numbers should be ignored (at least for now).
 ///
-@objc public class PublicizeInfo: NSManagedObject {
+@objc(PublicizeInfo)
+public class PublicizeInfo: NSManagedObject {


mokagio · 2025-04-24T05:09:55Z

Sources/WordPressData/Swift/ReaderTeamTopic.swift

+import Foundation
+
+@objc(ReaderTeamTopic)
+open class ReaderTeamTopic: ReaderAbstractTopic {


Ouch. I was sure to have moved all the types but clearly missed this. I think the fact that it does not inherit from NSManagedObject tricked me.

First, convert all entities to "Global Namespace" with ``` sed -i '' -E 's/representedClassName="(WordPress)?(\.)?([^"]+)"/representedClassName="\3"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ *.xcdatamodel/contents ``` Then, set the entities that need the "Current Product Module", using #24494 as the source for which to pick ``` sed -i '' -E 's/representedClassName="(ManagedAccountSettings|BlogAuthor|BloggingPrompt|BloggingPromptSettings|BloggingPromptSettingsReminderDays|BlogSettings|DiffAbstractValue|DiffContentValue|DiffTitleValue|ManagedDomain|ManagedPerson|Plan|PlanFeature|PlanGroup|PublicizeConnection|PublicizeService|ReaderCard|Revision|RevisionDiff|Role)"/representedClassName=".\1"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ *.xcdatamodel/contents ``` Notice that `Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress 155.xcdatamodel/contents` did not change, which is a point in favor of showing this scripted change being correct.

* Move remaining types to WordPressData * Use Current Module for Swift-only types * Fix ReaderTeamTopic compilation * Add missing class names * Remove unused SiteInfo * Add Bundle.wordPressData and update DataMigratorTests * Update CoreDataMigrationTests * Fix unit tests (AppEnvironment required)

* Configure `Package.swift` with WordPressData framework deps * Add `WordPress-Swift.h` to WordPressData to work around build fail * Add prefix header * Add all required sources to WordPressData * Add required imports to Swift layer * Extract `defaultWordPressComAccountRestAPI` method to WordPressData Only the method, not the rest of the extension to avoid dependencies galore. * Add `Blog+SelfHosted` to WordPressData, including dependencies * Hack `ContextManager.swift` to compile * Replicate Keystone preprocessor macro hack for WordPressData-Swift.h * Add `Media+Blog.swift` to WordPressData to compile `PostHelper.m` * Add `RemotePost+Metadata` source to WordPressData for `PostHelper.m` * Add `PostServiceRemoteFactory` to WordPressData to fix `PostService.m` * Add `PostHelper+JetpackSocial` to WordPressData to fix `PostHelper.m` * Add `WordPressOrgRestApi+WordPress` to WordPressData Made possible by #24351 * Add `.m` file in WordPressData only to config CocoaLumberJack * Add some `import CocoaLumberJackSwift` required by WordPressData * Use `_private_wordPressComRestApi` in `Blog+WordPressComRestAPI.swift` to build WordPressData * Add CocoaLumberjack as a dependency to the extensions to fix compilation Otherwise, we'd get: ``` Undefined symbols for architecture arm64: "_OBJC_CLASS_$_DDLog", referenced from: in AppExtensionsService.o ld: symbol(s) not found for architecture arm64 clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` * Remove `WordPress-Swift.h` hack from WordPressData * Add `Blog+Lookup` to WordPressData * Add `WordPress.xcdatamodeld` to WordPressData * Duplicate test infra in WordPressData in order to run `BlogTests` * Remove unnecessary `CocoaLumberJackConfiguration.h` header We don't need the header, the implementation file is enough for the CocoaLumberJack shenanigans to take place. * Remove WordPressData types from WordPress and Jetpack Not Keystone yet because I don't know if that compiles okay to verify the change. * Update imports after removing WordPressData files from WP and Jetpack * Disambiguate `Notification` usages — `/: Notification\)/` * Disambiguate `Notification` usages — `/: Notification$/` * Disambiguate `Notification` usages — `/, Notification(?!s)/` * Disambiguate `Notification` usages — `/as\? Notification/` * Disambiguate `Notification` usages — `/ Notification \{/` * Disambiguate `Notification` usages — `/ Notification\?/` * Disambiguate `Notification` usages — `/ Notification,$/` * Disambiguate `Notification` usages — `/\[Notification\]/` * Disambiguate `Notification` usages — `/ Notification\:/` * Disambiguate `Notification` usages — `/: Notification,/` * Disambiguate `Notification` usages — `/$Notification$/` * Disambiguate `Notification` usages — `/Notification else/` * Disambiguate `Notification` usages — `/Notification\!/` * Disambiguate `Notification` usages — `/as\? Notification/` * Add CocoaLumberjack to notification service extensions * Update imports after `Notification` disambiguation * Replace all `#import` of headers now in WordPressData with `@import WordPressData` * Work around (temporarily?!) a build failure after the WordPressData move * Remove unused `AccountService` import from `Blog.m` It matters because once `Blog` is in WordPressData, `AccountService` will no longer be available. * Move some `MenuItem` imports from `.h` to `.m` `MenuItem` was not used in the interface. Moving the import in the implementation will make it cleaner to update once WordPressData is integrated. * Remove WordPressData types from `WordPress-Bridging-Header.h` * Address module verifier warnings in WordPressData * Extract Swift-dependant logic from `Theme` * Make `Media` extension `public` * Remove unused WordPressData import from NotificationServiceExtension * Add CocoaLumberjack to extensions Jetpack uses to avoid compile failure Undefined symbol: _OBJC_CLASS_$_DDLog * Make WordPressData Swift API available to ObjC framework consumers * Add `testable import WordPressData` to unit tests * Update `WordPress.Notification` to `WordPressData.Notification` in tests * Add more required `@testable import WordPressData` Via ``` find WordPress/WordPressTest \ -name "*.swift" \ -type f \ -exec perl -i -0pe 's/(@testable import WordPress\n)(\nclass (\w+): CoreDataTestCase \{)/\1\@testable import WordPressData\n\nclass \3: CoreDataTestCase {/g' \ {} \; ``` and ``` find WordPress/WordPressTest \ -name "*.swift" \ -type f \ -exec perl -i -0pe 's/(@testable import WordPress\n)(\nfinal class (\w+): CoreDataTestCase \{)/\1\@testable import WordPressData\n\nfinal class \3: CoreDataTestCase {/g' \ {} \; ``` * Add WordPressData as a WordPressTests dependency * Add missing dependencies to WordPressTesting post WordPressData addition * Load model from the `ContextManager` bundle in the unit tests * Replace `AppEnvironment` with `WordPressComRestApi.apiBaseURL` in RestAPI Same rationale as 09990b8 * No longer access `_private_wordPressComRestApi` in `Blog` * Fix WordPressData linkage in Keystone and move sources * Update access of types required by Keystone for WordPressData link * Explicitly add WebKit as a Keystone dependency Triggered by a few errors like ``` In file included from /Users/gio/Developer/a8c/wpios/WordPress/Classes/ViewRelated/Suggestions/SuggestionsTableViewCell.m:3: /Users/gio/Developer/a8c/wpios/DerivedData/WordPress/Build/Intermediates.noindex/WordPress.build/Debug-iphonesimulator/Keystone.build/DerivedSources/Keystone-Swift.h:295:9: fatal error: module 'WebKit' not found 295 | @import WebKit; | ~~~~~~~^~~~~~ ``` which I got after linking WordPressData against Keystone and moving all the sources there. * Revert `CookieJar` `public` access level Apparently unnecessary, see #24420 (comment) * REVERT ME - Only run WordPress tests * Switch all models to use "Global Namespace" Via ``` sed -i '' -E 's/representedClassName="(WordPress)?(\.)?([^"]+)"/representedClassName=".\3"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ 155.xcdatamodel/contents ``` which switched to "Current Product Module" but resulted in unit tests failure in WordPressData, followed by ``` ➜ sed -i '' -E 's/representedClassName="\.([^"]+)"/representedClassName="\1"/g' \ Sources/WordPressData/Resources/WordPress.xcdatamodeld/WordPress\ 155.xcdatamodel/contents ``` * Replace all string values passed to `forEntityName` with `entityName()` Done via ``` find . \ $ -path './DerivedData' -o -path './.git' -o -path './vendor' $ \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: "([A-Za-z_][A-Za-z0-9_]*)"/forEntityName: \1.entityName()/g' {} + ``` * Replace all `NSStringFromClass` values passed to `forEntityName` Done via ``` find . \ $ -path './DerivedData' -o -path './.git' -o -path './vendor' $ \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: NSStringFromClass$([A-Za-z_][A-Za-z0-9_]*)\.self$/forEntityName: \1.entityName()/g' {} + ``` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Replace all `Self.classNameWithoutNamespaces()` with `entityName()` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Replace all `classNameWithoutNamespace()` calls with `entityName()` Done via ``` find . $ -path './DerivedData' -o -path './.git' -o -path './vendor' $ \ -prune -o -type f -name "*.swift" \ -exec sed -i '' -E 's/forEntityName: ([A-Za-z_][A-Za-z0-9_]*)\.classNameWithoutNamespaces/forEntityName: \1.entityName()/g' {} + ``` The goal is to have a single way to get the entity name so we can adapt it to the new WordPressData setup in one go. * Fix WordPressData runtime issues (#24494) * Move remaining types to WordPressData * Use Current Module for Swift-only types * Fix ReaderTeamTopic compilation * Add missing class names * Remove unused SiteInfo * Add Bundle.wordPressData and update DataMigratorTests * Update CoreDataMigrationTests * Fix unit tests (AppEnvironment required) * Add `BlobEntity` to WordPressData * Address straightforward `FIXME` notes --------- Co-authored-by: Alex Grebenyuk <[email protected]>

kean added 5 commits April 23, 2025 12:10

Move remaining types to WordPressData

ae36b86

Use Current Module for Swift-only types

f09fef4

Fix ReaderTeamTopic compilation

b2e2666

Add missing class names

56f5e02

Remove unused SiteInfo

2eacc63

kean added the General label Apr 23, 2025

kean added this to the 25.9 milestone Apr 23, 2025

kean commented Apr 23, 2025

View reviewed changes

kean added 2 commits April 23, 2025 12:57

Add Bundle.wordPressData and update DataMigratorTests

e2b433b

Update CoreDataMigrationTests

393cf58

kean requested a review from mokagio April 23, 2025 17:01

Fix unit tests (AppEnvironment required)

43032c5

mokagio approved these changes Apr 24, 2025

View reviewed changes

mokagio mentioned this pull request Apr 24, 2025

Add bare bone test for migration to 154 to 155 #24496

Closed

kean merged commit 200048d into mokagio/wordpressdata-target-xcdatamodeld-workbench-2-move-files Apr 24, 2025
32 of 34 checks passed

kean deleted the task/wpdata-fdsaf branch April 24, 2025 13:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix WordPressData runtime issues #24494

Fix WordPressData runtime issues #24494

kean commented Apr 23, 2025

dangermattic commented Apr 23, 2025

kean Apr 23, 2025

crazytonyli Apr 24, 2025

crazytonyli Apr 24, 2025

mokagio Apr 24, 2025

mokagio Apr 24, 2025

mokagio Apr 24, 2025

mokagio Apr 24, 2025

kean Apr 24, 2025 •

edited

Loading

kean Apr 23, 2025

mokagio Apr 24, 2025

kean Apr 23, 2025

mokagio Apr 24, 2025

kean Apr 24, 2025

wpmobilebot commented Apr 23, 2025 •

edited

Loading

wpmobilebot commented Apr 23, 2025 •

edited

Loading

sonarqubecloud bot commented Apr 23, 2025

mokagio left a comment

mokagio Apr 24, 2025

mokagio Apr 24, 2025

mokagio Apr 24, 2025

Fix WordPressData runtime issues #24494

Fix WordPressData runtime issues #24494

Conversation

kean commented Apr 23, 2025

dangermattic commented Apr 23, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kean Apr 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpmobilebot commented Apr 23, 2025 • edited Loading

wpmobilebot commented Apr 23, 2025 • edited Loading

sonarqubecloud bot commented Apr 23, 2025

Quality Gate passed

mokagio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kean Apr 24, 2025 •

edited

Loading

wpmobilebot commented Apr 23, 2025 •

edited

Loading

wpmobilebot commented Apr 23, 2025 •

edited

Loading