identity v3 #250

aboedo · 2021-01-27T18:47:55Z

Introduces a new take on identity.

Public changes overview

New methods

Introduces logIn, a new way of identifying users, which also returns whether a new user has been registered in the system.
logIn uses a new backend endpoint.
Introduces logOut, a replacement for reset.

Deprecations / removals

removes createAlias
deprecates identify in favor of logIn
deprecates reset in favor of logOut
deprecates allowSharingAppStoreAccount in favor of dashboard-side configuration

Note: the methods are kept internal in this PR so that it can safely be merged before the backend support for it is ready.
The deprecations are also not included in this PR for the same reason. Those changes are done in a separate PR

common/src/test/java/com/revenuecat/purchases/common/BackendTest.kt

feature/identity/src/main/java/com/revenuecat/purchases/identity/IdentityManager.kt

purchases/src/main/java/com/revenuecat/purchases/interfaces/LogInListener.java

purchases/src/main/kotlin/com/revenuecat/purchases/Purchases.kt

vegaro · 2021-02-10T01:54:38Z

purchases/src/main/kotlin/com/revenuecat/purchases/Purchases.kt

+     */
+    @JvmOverloads
+    internal fun logOut(listener: ReceivePurchaserInfoListener? = null) {
+        val error: PurchasesError? = identityManager.logOut()


I noticed reset also does

deviceCache.clearLatestAttributionData(identityManager.currentAppUserID)

Do we need that here?

I forgot to add a comment to request feedback for this, but it's something I'm torn on. I didn't do it on iOS, since reset doesn't do it there either.
I wanted to maintain consistency with the reset method.
However, it's not technically needed for a reset or logout.
and, if after logging out you log in as the same user, attribution could be re-sent needlessly.

note that logOut does call clearLatestAttributionData, but it happens in identityManager instead of Purchases, since it does seem to pertain to identity management.
I just am not sure whether we should be doing it at all in a reset / logout scenario. @vegaro thoughts?

I think the only reason we had it was to not let the SharedPreferences grow forever. Note that if we never remove it, it will always stay cached since it's dependent on the identityManager.currentAppUserID

I'm not quite sure what I was thinking when I replied before 😂 but yeah, I actually am clearing attribution data, both on iOS and Android. It's just done in IdentityManager instead of purchases.
And thinking about it further, I don't see a reason not to do it - it's just a cache, so the worst case here is that if you log in again with the same user and we've cleared the cache, we just send it an extra time.
I think it's a good tradeoff for having cleaner shared preferences.

common/src/test/java/com/revenuecat/purchases/common/BackendTest.kt

vegaro · 2021-02-17T00:45:41Z

feature/identity/src/main/java/com/revenuecat/purchases/identity/IdentityManager.kt

+                    subscriberAttributesCache.clearSubscriberAttributesIfSyncedForSubscriber(
+                        oldAppUserID
+                    )


Being nitpicky here, but the line breaks are not needed

Suggested change

subscriberAttributesCache.clearSubscriberAttributesIfSyncedForSubscriber(

oldAppUserID

)

subscriberAttributesCache.clearSubscriberAttributesIfSyncedForSubscriber(oldAppUserID)

the formatter is set to 120 characters, so it auto-adds them. I'll fix for now, but maybe we should have a strict set of rules for this? I thought detekt was set to 120 as well

120 is good, that's what I have too. My formatter is not auto-adding the breaks. Maybe there is another config thing going on? Let me know if you want to compare settings to figure out what's going on

yeah, would love to have shared settings. we should probably even share them somewhere so they can be reused by other RC members

you can export just the code style stuff

vegaro · 2021-02-17T00:54:27Z

purchases/src/test/java/com/revenuecat/purchases/PurchasesTest.kt

+
+        purchases.logIn(appUserID, mockCompletion)
+
+        verify {


I like to specify the number of calls that should be verified. It's possible that mockBackend.getPurchaserInfo is called from another part of the code, and logIn never calls it, and this test will still pass. I think making sure that what is being passed to the mockCompletion is what's fetched from the backend could also work.

agreed that explicit > implicit. I was just following the style that was already in the file, but I'll update for all the stuff in this PR

purchases/src/test/java/com/revenuecat/purchases/PurchasesTest.kt

…ct the backend. Added tests for logIn in identityManager

…Info can't be parsed from the backend's response

…d for the old app user id instead of the new one after a successful login

…a full import

feature/identity/src/test/java/com/revenuecat/purchases/identity/IdentityManagerTests.kt

aboedo · 2021-02-19T16:56:54Z

@vegaro I believe that's all comments addressed, give it another look if you'd like and we should be ready to 🚢

vegaro

🚢

aboedo self-assigned this Jan 27, 2021

aboedo force-pushed the feature/identityv3 branch from 62dd6f5 to 89b5c22 Compare February 4, 2021 17:05

aboedo requested a review from vegaro February 4, 2021 17:08

aboedo mentioned this pull request Feb 4, 2021

identity V3: public facing changes #252

Merged

3 tasks

aboedo marked this pull request as ready for review February 4, 2021 17:09

aboedo force-pushed the feature/identityv3 branch from 58dfa2c to 44b1592 Compare February 9, 2021 18:11

vegaro requested changes Feb 10, 2021

View reviewed changes

vegaro reviewed Feb 17, 2021

View reviewed changes

aboedo added 22 commits February 17, 2021 13:47

added starter code for logIn

eaa686d

added basic logic to IdentityManager for logIn

2b6bbfa

added basic backend logic for logIn

0e3ba34

fix imports

e3ae64d

fixed more imports, removed magic number

910ae88

added logOut

fd98265

detekt

7cd75be

made logIn and logOut internal temporarily

6671c28

added logInWith listener conversion

d68c2a4

fixed a bug where if the appUserID was empty, login would still conta…

dd17c27

…ct the backend. Added tests for logIn in identityManager

added tests for identifierManager.logOut

41db91e

wip adding backend tests for logIn + considering case where purchaser…

b267771

…Info can't be parsed from the backend's response

added more tests for backend

de71fb9

added final backend tests for the login endpoint

1363508

added skeleton for unit tests for login in purchases

9f9ba9c

added logout test skeletons

d356e56

more test cases for login in Purchases

e77c826

added more test cases and fixed a bug where offerings would be fetche…

a0fdfe4

…d for the old app user id instead of the new one after a successful login

re-wrote logOut to use the identityManager's logic and added more tests

80d3d07

fix tests

b367431

fixed more tests

919d14c

added error log for login errors in backend.kt

77dac7b

aboedo added 8 commits February 17, 2021 13:47

more logging cleanup

053a047

removed unused import

9c69c14

fixed formatting in a few places

363ef8b

more fixed indentation

34b28ee

added @JvmSynthetic in a couple of places

0ce91f8

updated imports for identity strings to use the namespace instead of …

2b4a37a

…a full import

updated loginListener -> LogInCallback and moved it to kotlin

a75edc9

removed unnecessary line breaks

e48287e

aboedo force-pushed the feature/identityv3 branch from 5f16f25 to e48287e Compare February 17, 2021 16:48

extracted log string into identity strings

82e33a8

aboedo requested a review from vegaro February 17, 2021 21:14

vegaro approved these changes Feb 18, 2021

View reviewed changes

feature/identity/src/test/java/com/revenuecat/purchases/identity/IdentityManagerTests.kt Outdated Show resolved Hide resolved

vegaro approved these changes Feb 18, 2021

View reviewed changes

aboedo added 4 commits February 19, 2021 13:28

fail<String> -> fail

5b7f298

removed unnecessary mock completion

bfbd780

small cleanup

fcf2b9d

added explicit call count to verify calls

73f375a

vegaro approved these changes Feb 19, 2021

View reviewed changes

aboedo merged commit 7d98ce7 into develop Feb 19, 2021

aboedo deleted the feature/identityv3 branch February 19, 2021 18:31

vegaro added this to the 4.0.4 milestone Mar 3, 2021

aboedo mentioned this pull request May 27, 2021

Release: 4.3.0-beta1 #329

Closed

aboedo mentioned this pull request Jul 15, 2021

Release/4.3.0 #339

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

identity v3 #250

identity v3 #250

aboedo commented Jan 27, 2021 •

edited

Loading

vegaro Feb 10, 2021

aboedo Feb 16, 2021

aboedo Feb 16, 2021

vegaro Feb 17, 2021

aboedo Feb 19, 2021

vegaro Feb 17, 2021

aboedo Feb 17, 2021

vegaro Feb 18, 2021

aboedo Feb 19, 2021

aboedo Feb 19, 2021

vegaro Feb 17, 2021

aboedo Feb 19, 2021

aboedo commented Feb 19, 2021

vegaro left a comment

identity v3 #250

identity v3 #250

Conversation

aboedo commented Jan 27, 2021 • edited Loading

Public changes overview

New methods

Deprecations / removals

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

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

aboedo commented Feb 19, 2021

vegaro left a comment

Choose a reason for hiding this comment

aboedo commented Jan 27, 2021 •

edited

Loading