Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simperium: Sync'ing Account Entities #1091

Merged
merged 17 commits into from
Jan 19, 2021

Conversation

jleandroperez
Copy link
Contributor

@jleandroperez jleandroperez commented Jan 15, 2021

Details

  • Updates Simperium to 1.2
  • Enables Sync'ing of the Account bucket, backed by SPJSONStorage
  • Implements the EmailVerification entity

Depends on Simperium PR 606
Ref #1086

cc @eshurakov WDYT? (Feedback definitely more than welcomed!!)


Test: Unverified Account

  1. Update your mobile-secrets first please!
  2. Checkout commit f493053 (contains debug code!)
  3. Log into an unverified account
  • Verify that the legend Email Verification is never printed in your console

Test: Verified Account

  1. Update your mobile-secrets first please!
  2. Checkout commit f493053 (contains debug code!)
  3. Log into an unverified account
  • Verify that the legend Email Verification does get printed in your console

Release

These changes do not require release notes.

@peril-automattic
Copy link

peril-automattic bot commented Jan 15, 2021

Warnings
⚠️ PR is missing at least one label.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@peril-automattic
Copy link

peril-automattic bot commented Jan 15, 2021

You can trigger an installable build for these changes by visiting CircleCI here.

@eshurakov
Copy link
Contributor

I was running some tests and encountered an issue that remote changes are not applied to a local json storage.
I used an account that never requested or verified an email. Then I requested an email verification and was printing a payload (let payload = accountBucket.object(forKey: SPCredentials.simperiumEmailVerificationObjectKey) as? [AnyHashable: Any]) in bucket:didChangeObjectForKey: method.

2021-01-15 11:29:28.260138+0100 Simplenote[10570:205523] Simperium non-local ADD ENTITY received email-verification
2021-01-15 11:29:28.260415+0100 Simplenote[10570:205523] Simperium received version = (null), previous version = 0
2021-01-15 11:29:28.260649+0100 Simplenote[10570:205523] Simperium warning: applyGhostDiff for a member that doesn't exist (status): {
    o = "+";
    v = sent;
}
2021-01-15 11:29:28.261439+0100 Simplenote[10570:205523] Simperium MODIFIED ghost version 1 (Account-Account)
2021-01-15 11:29:28.262617+0100 Simplenote[10570:205523] Simperium applying diff: {
    status =     {
        o = "+";
        v = sent;
    };
}
2021-01-15 11:29:28.269927+0100 Simplenote[10570:205523] Simperium warning: applyDiff for a member that doesn't exist (status): {
    o = "+";
    v = sent;
}
(lldb) po payload
0 elements

If I restart the app and wait for a bucket to finish indexing, the payload will contain correct information.

Copy link
Contributor

@eshurakov eshurakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for bringing things together, it started to get some shape!
Sending you a few comments!

Simplenote/Classes/Simperium+Simplenote.swift Outdated Show resolved Hide resolved
Simplenote/Classes/Simperium+Simplenote.swift Show resolved Hide resolved
Simplenote/EmailVerification.swift Outdated Show resolved Hide resolved
Simplenote/SPAppDelegate+Extensions.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@eshurakov eshurakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
I see that tests are failing on circleCI, but if I run it locally, it works.

@jleandroperez jleandroperez marked this pull request as ready for review January 19, 2021 14:49
@jleandroperez jleandroperez merged commit ff0f23b into hotfix/email-verification Jan 19, 2021
@jleandroperez jleandroperez deleted the issue/1086-account-sync branch January 19, 2021 16:18
@jleandroperez jleandroperez mentioned this pull request Jan 19, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants