Fix Core Data warnings.

Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis

GNU General Public License v2.0

43 stars 12 forks source link

Fix Core Data warnings. #118

Closed etoledom closed 4 years ago

etoledom commented 4 years ago

The warning reads as:

[error] fault: One or more models in this application are using transformable properties with transformer names that are either unset, or set to NSKeyedUnarchiveFromDataTransformerName. Please switch to using "NSSecureUnarchiveFromData" or a subclass of NSSecureUnarchiveFromDataTransformer instead. At some point, Core Data will default to using "NSSecureUnarchiveFromData" when nil is specified, and transformable properties containing classes that do not support NSSecureCoding will become unreadable.

I have followed the proposed solution, using NSSecureUnarchiveFromData as the transformer name.

To test:

CI Tests should pass.
Check out https://github.com/wordpress-mobile/WordPress-iOS/pull/13489 and follow the test steps there.

etoledom commented 4 years ago

@astralbodies - could I bug you with a quick look at this? Thanks!

jkmassel commented 4 years ago

This is super good – thanks for tackling it @etoledom!!

One test case I'd like to propose – does this all work correctly if you:

1) Create a tracks event with the code currently in develop. 2) Switch to this branch 3) Try to read and transmit it?

I'd looked at this issue before and couldn't find any documentation as to whether NSSecureUnarchiveFromData is backward-compatible with existing data. If it's not, I worry it might cause a crash-on-startup bug either when it tries to initialize the Core Data store or when it attempts to read the record into the TracksEvent object.

It's hopefully nothing, but this had been in the back of my head for a while, so now that you've fixed it, just wanted to ensure this wouldn't be an issue :)

etoledom commented 4 years ago

Thank you @jkmassel for the extra test case. This is what I have tried:

Delete the app and build from develop.
Wait for the events flush.
Trigger a new event (in this case switching gutenberg on/off, just because :] )
Kill the app from Xcode to avoid flushing the events.
Checkout and build from this branch.
On app launch, wait for the first events flush.
Check that the gutenberg switch event is there, and it's sent to the backend.
Success 🎉

After performing this test, everything seems to behave as expected 👍

jkmassel commented 4 years ago

Thanks for indulging my worry @etoledom!!

astralbodies commented 4 years ago

@etoledom I really apologize for the delay on this review, I will have it to you by tomorrow!