Check for cached identity on cold opens#1336
Conversation
ReferenceSDK-XXX -- Check for cached identity on cold opens. DescriptionSummary By MatterAI
🔄 What ChangedSynchronized the clearing of 🔍 Impact of the ChangeEnsures that the in-memory developer identity is fully purged during logout, preventing stale data from persisting in the static SDK state and maintaining consistency with the cleared local preferences. 📁 Total Files ChangedClick to Expand
🧪 Test Added/RecommendedRecommended
🔒 Security VulnerabilitiesN/A Testing InstructionsN/A Risk Assessment [
|
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
There was a problem hiding this comment.
Hi Gabe. Tested by commenting out Branch.getInstance().setIdentity("Identity1"); in the Branch-SDK-TestBet main activity. Added Branch Loggers to the If and Else If statements to confirm their firing. In most scenarios the code seemed to work fine, but I found one issue.
If I set an identity using the setIdentity Method, upon first Cold Open, the code sometimes successfully fires and I was able to see the previously set ID. However, when I force closed it again and restarted from cold open, neither IF statement was called.
Despite neither being called (assuming no ID was found), when I click Clear User ID (I was able to see the previously set ID still there. This indicates that the currentUserId value was still there on cold open, but the new code didn't recognize it. Is this expected or am I misunderstanding some part of the logout button code? Video attached in next comment.
- Tested Cold Open without an Identity set in onCreate
- Tested Warm Open without an identity set in onCrete
- Tested Cold Open with identity set in onCreate
- Tested Warm Open with Identity set in onCreate
- Force closed app and re-ran tests to ensure ID was cleared
logout Button Code
` findViewById(R.id.cmdClearUser).setOnClickListener(new OnClickListener() {
@OverRide
public void onClick(View v) {
String currentUserId = PrefHelper.getInstance(MainActivity.this).getIdentity();
Branch.getInstance().logout(new Branch.LogoutStatusListener() {
@OverRide
public void onLogoutFinished(boolean loggedOut, BranchError error) {
if (error != null) {
Log.e("BranchSDK_Tester", "onLogoutFinished Error: " + error);
Toast.makeText(getApplicationContext(), "Error Logging Out: " + error.getMessage(), Toast.LENGTH_LONG).show();
} else {
Log.d("BranchSDK_Tester", "onLogoutFinished succeeded: " + loggedOut);
Toast.makeText(getApplicationContext(), "Cleared User ID: " + currentUserId, Toast.LENGTH_LONG).show();
}
}
});
}
});`
|
Cold Open check potentially malfunctioning Check.for.cached.Identity.on.cold.open.mp4 |
|
Important PR Review SkippedPR review skipped as per the configuration setting. Run a manually review by commenting /matter review 💡Tips to use MatterAICommand List
|
bboothe-branch
left a comment
There was a problem hiding this comment.
Hey @gdeluna-branch I tested this update and I'm finding that the two blocks of code in ServerRequestInitSession.java only fire on warm open. I believe this is intended, but wanted to confirm before approving.
I have found that the first IF statement fires when the String identity = Branch.installDeveloperId; value is already populated, and the ELSE IF statement fires when only the prefHelper_.getIdentity() code is populated but the String identity = Branch.installDeveloperId; is empty.
If this is the intended flow, then we're good to approve.
Also side note, I found that we have Branch.getInstance().setIdentity("Identity1"); in the Testbed app's MainActivity. I commented this out to test, but it initially confused me as to why my ID kept being set to Identity1. Do we want to keep this code in the MainActivity or comment it / remove it? I think commenting it out would make the most sense so that we remember we have the option, but to set it to this as standard when we have the setIdentity() buttons seems redundant and potentially confusing for other devs.
Videos attached:
Identity1.Example.mp4
ExampleIDs.used.mp4
bboothe-branch
left a comment
There was a problem hiding this comment.
Approved after confirming intended behavior after discussion.
Reference
SDK-XXX -- <TITLE>.
Description
Testing Instructions
Risk Assessment [
HIGH||MEDIUM||LOW]Reviewer Checklist (To be checked off by the reviewer only)
cc @BranchMetrics/saas-sdk-devs for visibility.