Skip to content

Commit

Permalink
BrowserSwitchRequest Should Notify Cancellation (#45)
Browse files Browse the repository at this point in the history 
* Add shouldNotify property to BrowserSwitchRequest.

Signed-off-by: Steven Shropshire <[email protected]>

* Rename shouldNotify to shouldNotifyCancellation in BrowserSwitchRequest.

Signed-off-by: Steven Shropshire <[email protected]>

* Fix nullability annotations in BrowserSwitchOptions.

Signed-off-by: Steven Shropshire <[email protected]>

* Update BrowserSwitchClient to notify browser switch result cancellation only once, and keep cancelled request dormant in case a success flow is still possible.

Signed-off-by: Steven Shropshire <[email protected]>

* Refactor demo app to clear result text when browser switch is started.

Signed-off-by: Steven Shropshire <[email protected]>

* Fix typo in shared preferences key.

Signed-off-by: Steven Shropshire <[email protected]>

* Update CHANGELOG.

Signed-off-by: Steven Shropshire <[email protected]>

* Re-order CHANGELOG.

Signed-off-by: Steven Shropshire <[email protected]>

* Update comment string in BrowserSwitchClient.

Signed-off-by: Steven Shropshire <[email protected]>

* Update browser-switch/src/test/java/com/braintreepayments/api/BrowserSwitchClientTest.java

Co-authored-by: Sarah Koop <[email protected]>

Co-authored-by: Susan Stevens <[email protected]>
Co-authored-by: Sarah Koop <[email protected]>
  • Loading branch information
@sshropshire @sestevens @sarahkoop
3 people authored Jun 22, 2021
1 parent 7db76e1 commit e8c7c85
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 39 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# browser-switch-android Release Notes

## unreleased

* Update `BrowserSwitchClient#deliverResult()` to allow canceled browser switches a chance to reach the success state (See braintree_android [#409](https://github.com/braintree/braintree_android/issues/409))
* Fix nullability annotations in `BrowserSwitchOptions` setters

## 2.0.0

Expand Down
13 changes: 8 additions & 5 deletions browser-switch/src/main/java/com/braintreepayments/api/BrowserSwitchClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void start(@NonNull FragmentActivity activity, @NonNull BrowserSwitchOpti

JSONObject metadata = browserSwitchOptions.getMetadata();
BrowserSwitchRequest request =
new BrowserSwitchRequest(requestCode, browserSwitchUrl, metadata, returnUrlScheme);
new BrowserSwitchRequest(requestCode, browserSwitchUrl, metadata, returnUrlScheme, true);
persistentStore.putActiveRequest(request, appContext);

customTabsInternalClient.launchUrl(activity, browserSwitchUrl);
Expand Down Expand Up @@ -116,17 +116,20 @@ public BrowserSwitchResult deliverResult(@NonNull FragmentActivity activity) {
return null;
}

BrowserSwitchResult result;
BrowserSwitchResult result = null;

Uri deepLinkUrl = intent.getData();
if (deepLinkUrl != null && request.matchesDeepLinkUrlScheme(deepLinkUrl)) {
result = new BrowserSwitchResult(BrowserSwitchStatus.SUCCESS, request, deepLinkUrl);
} else {
persistentStore.clearActiveRequest(appContext);
} else if (request.getShouldNotifyCancellation()) {
// ensure that cancellation result is delivered exactly one time
result = new BrowserSwitchResult(BrowserSwitchStatus.CANCELED, request);

request.setShouldNotifyCancellation(false);
persistentStore.putActiveRequest(request, activity);
}

// ensure that browser switch result is delivered exactly one time
persistentStore.clearActiveRequest(appContext);
return result;
}
}
6 changes: 3 additions & 3 deletions browser-switch/src/main/java/com/braintreepayments/api/BrowserSwitchOptions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class BrowserSwitchOptions {
* {@link BrowserSwitchResult} when the app has re-entered the foreground
* @return {@link BrowserSwitchOptions} reference to instance to allow setter invocations to be chained
*/
public BrowserSwitchOptions metadata(@NonNull JSONObject metadata) {
public BrowserSwitchOptions metadata(@Nullable JSONObject metadata) {
this.metadata = metadata;
return this;
}
Expand All @@ -49,7 +49,7 @@ public BrowserSwitchOptions requestCode(int requestCode) {
* @param url The target url to use for browser switch
* @return {@link BrowserSwitchOptions} reference to instance to allow setter invocations to be chained
*/
public BrowserSwitchOptions url(@NonNull Uri url) {
public BrowserSwitchOptions url(@Nullable Uri url) {
this.url = url;
return this;
}
Expand All @@ -61,7 +61,7 @@ public BrowserSwitchOptions url(@NonNull Uri url) {
* after browser switch
* @return {@link BrowserSwitchOptions} reference to instance to allow setter invocations to be chained
*/
public BrowserSwitchOptions returnUrlScheme(@NonNull String returnUrlScheme) {
public BrowserSwitchOptions returnUrlScheme(@Nullable String returnUrlScheme) {
this.returnUrlScheme = returnUrlScheme;
return this;
}
Expand Down
16 changes: 14 additions & 2 deletions browser-switch/src/main/java/com/braintreepayments/api/BrowserSwitchRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,24 @@ class BrowserSwitchRequest {
private final int requestCode;
private final JSONObject metadata;
private final String returnUrlScheme;
private boolean shouldNotifyCancellation;

static BrowserSwitchRequest fromJson(String json) throws JSONException {
JSONObject jsonObject = new JSONObject(json);
int requestCode = jsonObject.getInt("requestCode");
String url = jsonObject.getString("url");
String returnUrlScheme = jsonObject.getString("returnUrlScheme");
JSONObject metadata = jsonObject.optJSONObject("metadata");
return new BrowserSwitchRequest(requestCode, Uri.parse(url), metadata, returnUrlScheme);
boolean shouldNotify = jsonObject.optBoolean("shouldNotify", true);
return new BrowserSwitchRequest(requestCode, Uri.parse(url), metadata, returnUrlScheme, shouldNotify);
}

BrowserSwitchRequest(int requestCode, Uri url, JSONObject metadata, String returnUrlScheme) {
BrowserSwitchRequest(int requestCode, Uri url, JSONObject metadata, String returnUrlScheme, boolean shouldNotifyCancellation) {
this.url = url;
this.requestCode = requestCode;
this.metadata = metadata;
this.returnUrlScheme = returnUrlScheme;
this.shouldNotifyCancellation = shouldNotifyCancellation;
}

Uri getUrl() {
Expand All @@ -42,11 +45,20 @@ JSONObject getMetadata() {
return metadata;
}

boolean getShouldNotifyCancellation() {
return shouldNotifyCancellation;
}

void setShouldNotifyCancellation(boolean shouldNotifyCancellation) {
this.shouldNotifyCancellation = shouldNotifyCancellation;
}

String toJson() throws JSONException {
JSONObject result = new JSONObject();
result.put("requestCode", requestCode);
result.put("url", url.toString());
result.put("returnUrlScheme", returnUrlScheme);
result.put("shouldNotify", shouldNotifyCancellation);
if (metadata != null) {
result.put("metadata", metadata);
}
Expand Down
2 changes: 1 addition & 1 deletion browser-switch/src/main/java/com/braintreepayments/api/PersistentStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class PersistentStore {

@VisibleForTesting
static final String PREFERENCES_KEY =
"com.braintreepayament.browserswitch.persistentstore";
"com.braintreepayment.browserswitch.persistentstore";

static void put(String key, String value, Context context) {
Context applicationContext = context.getApplicationContext();
Expand Down
63 changes: 56 additions & 7 deletions browser-switch/src/test/java/com/braintreepayments/api/BrowserSwitchClientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@
import org.robolectric.RobolectricTestRunner;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
Expand Down Expand Up @@ -82,6 +85,7 @@ public void start_createsBrowserSwitchIntentAndInitiatesBrowserSwitch() throws B
assertEquals(browserSwitchRequest.getRequestCode(), 123);
assertEquals(browserSwitchRequest.getUrl(), browserSwitchDestinationUrl);
assertSame(browserSwitchRequest.getMetadata(), metadata);
assertTrue(browserSwitchRequest.getShouldNotifyCancellation());
}

@Test
Expand Down Expand Up @@ -181,7 +185,7 @@ public void deliverResult_whenDeepLinkUrlExistsAndReturnUrlSchemeMatches_clearsR
when(activity.getIntent()).thenReturn(deepLinkIntent);

JSONObject requestMetadata = new JSONObject();
BrowserSwitchRequest request = new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme");
BrowserSwitchRequest request = new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme", false);
when(persistentStore.getActiveRequest(applicationContext)).thenReturn(request);

BrowserSwitchClient sut = new BrowserSwitchClient(browserSwitchInspector, persistentStore, customTabsInternalClient);
Expand All @@ -198,15 +202,15 @@ public void deliverResult_whenDeepLinkUrlExistsAndReturnUrlSchemeMatches_clearsR
}

@Test
public void deliverResult_whenDeepLinkUrlExistsAndReturnUrlSchemeDoesNotMatch_clearsResultStoreAndNotifiesResultCANCELED() {
public void deliverResult_whenDeepLinkUrlExists_AndReturnUrlSchemeDoesNotMatch_AndShouldNotifyCancellation_notifiesResultCANCELED_AndSetsRequestShouldNotifyCancellationToFalse() {
when(activity.getApplicationContext()).thenReturn(applicationContext);

Uri deepLinkUrl = Uri.parse("another-return-url-scheme://test");
Intent deepLinkIntent = new Intent().setData(deepLinkUrl);
when(activity.getIntent()).thenReturn(deepLinkIntent);

JSONObject requestMetadata = new JSONObject();
BrowserSwitchRequest request = new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme");
BrowserSwitchRequest request = new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme", true);
when(persistentStore.getActiveRequest(applicationContext)).thenReturn(request);

BrowserSwitchClient sut = new BrowserSwitchClient(browserSwitchInspector, persistentStore, customTabsInternalClient);
Expand All @@ -219,18 +223,41 @@ public void deliverResult_whenDeepLinkUrlExistsAndReturnUrlSchemeDoesNotMatch_cl
assertSame(result.getRequestMetadata(), requestMetadata);
assertNull(result.getDeepLinkUrl());

verify(persistentStore).clearActiveRequest(applicationContext);
ArgumentCaptor<BrowserSwitchRequest> captor = ArgumentCaptor.forClass(BrowserSwitchRequest.class);
verify(persistentStore).putActiveRequest(captor.capture(), same(activity));

BrowserSwitchRequest updatedRequest = captor.getValue();
assertSame(request, updatedRequest);
assertFalse(updatedRequest.getShouldNotifyCancellation());
}

@Test
public void deliverResult_whenDeepLinkUrlExistsAndReturnUrlSchemeDoesNotMatchAndShouldNotNotifyCancellation_returnsNull() {
when(activity.getApplicationContext()).thenReturn(applicationContext);

Uri deepLinkUrl = Uri.parse("another-return-url-scheme://test");
Intent deepLinkIntent = new Intent().setData(deepLinkUrl);
when(activity.getIntent()).thenReturn(deepLinkIntent);

JSONObject requestMetadata = new JSONObject();
BrowserSwitchRequest request = new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme", false);
when(persistentStore.getActiveRequest(applicationContext)).thenReturn(request);

BrowserSwitchClient sut = new BrowserSwitchClient(browserSwitchInspector, persistentStore, customTabsInternalClient);
BrowserSwitchResult result = sut.deliverResult(activity);

assertNull(result);
verify(persistentStore, never()).putActiveRequest(any(BrowserSwitchRequest.class), any(FragmentActivity.class));
}

@Test
public void deliverResult_whenDeepLinkUrlDoesNotExist_clearsResultStoreAndNotifiesResultCANCELED() {
public void deliverResult_whenDeepLinkUrlDoesNotExistAndShouldNotifyCancellation_notifiesResultCANCELEDAndSetsRequestShouldNotifyCancellationToFalse() {
when(activity.getApplicationContext()).thenReturn(applicationContext);
when(activity.getIntent()).thenReturn(new Intent());

JSONObject requestMetadata = new JSONObject();
BrowserSwitchRequest request =
new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme");
new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme", true);
when(persistentStore.getActiveRequest(applicationContext)).thenReturn(request);

BrowserSwitchClient sut = new BrowserSwitchClient(browserSwitchInspector, persistentStore, customTabsInternalClient);
Expand All @@ -243,7 +270,29 @@ public void deliverResult_whenDeepLinkUrlDoesNotExist_clearsResultStoreAndNotifi
assertSame(result.getRequestMetadata(), requestMetadata);
assertNull(result.getDeepLinkUrl());

verify(persistentStore).clearActiveRequest(applicationContext);
ArgumentCaptor<BrowserSwitchRequest> captor = ArgumentCaptor.forClass(BrowserSwitchRequest.class);
verify(persistentStore).putActiveRequest(captor.capture(), same(activity));

BrowserSwitchRequest updatedRequest = captor.getValue();
assertSame(request, updatedRequest);
assertFalse(updatedRequest.getShouldNotifyCancellation());
}

@Test
public void deliverResult_whenDeepLinkUrlDoesNotExistAndShouldNotNotifyCancellation_returnsNull() {
when(activity.getApplicationContext()).thenReturn(applicationContext);
when(activity.getIntent()).thenReturn(new Intent());

JSONObject requestMetadata = new JSONObject();
BrowserSwitchRequest request =
new BrowserSwitchRequest(123, browserSwitchDestinationUrl, requestMetadata, "return-url-scheme", false);
when(persistentStore.getActiveRequest(applicationContext)).thenReturn(request);

BrowserSwitchClient sut = new BrowserSwitchClient(browserSwitchInspector, persistentStore, customTabsInternalClient);
BrowserSwitchResult result = sut.deliverResult(activity);

assertNull(result);
verify(persistentStore, never()).putActiveRequest(any(BrowserSwitchRequest.class), any(FragmentActivity.class));
}

@Test
Expand Down
23 changes: 21 additions & 2 deletions browser-switch/src/test/java/com/braintreepayments/api/BrowserSwitchRequestUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,30 @@
public class BrowserSwitchRequestUnitTest {

@Test
public void fromJson_withoutMetadata() throws JSONException {
public void fromJson_withoutShouldNotifyProperty_defaultsShouldNotifyToTrue() throws JSONException {
String json = "{\n" +
" \"requestCode\": 123,\n" +
" \"url\": \"https://example.com\",\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\"\n" +
"}";
BrowserSwitchRequest sut = BrowserSwitchRequest.fromJson(json);
assertTrue(sut.getShouldNotifyCancellation());
}

@Test
public void fromJson_withoutMetadata() throws JSONException {
String json = "{\n" +
" \"requestCode\": 123,\n" +
" \"url\": \"https://example.com\",\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\",\n" +
" \"shouldNotify\": true\n" +
"}";
BrowserSwitchRequest sut = BrowserSwitchRequest.fromJson(json);

assertEquals(sut.getRequestCode(), 123);
assertEquals(sut.getUrl().toString(), "https://example.com");
assertNull(sut.getMetadata());
assertTrue(sut.getShouldNotifyCancellation());

assertTrue(sut.matchesDeepLinkUrlScheme(Uri.parse("my-return-url-scheme://test")));
assertFalse(sut.matchesDeepLinkUrlScheme(Uri.parse("another-return-url-scheme://test")));
Expand All @@ -39,14 +52,16 @@ public void toJson_withoutMetadata() throws JSONException {
String json = "{\n" +
" \"requestCode\": 123,\n" +
" \"url\": \"https://example.com\",\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\"\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\",\n" +
" \"shouldNotify\": false\n" +
"}";
BrowserSwitchRequest original = BrowserSwitchRequest.fromJson(json);
BrowserSwitchRequest restored = BrowserSwitchRequest.fromJson(original.toJson());

assertEquals(restored.getRequestCode(), original.getRequestCode());
assertEquals(restored.getUrl(), original.getUrl());
assertNull(restored.getMetadata());
assertFalse(restored.getShouldNotifyCancellation());

assertTrue(restored.matchesDeepLinkUrlScheme(Uri.parse("my-return-url-scheme://test")));
assertFalse(restored.matchesDeepLinkUrlScheme(Uri.parse("another-return-url-scheme://test")));
Expand All @@ -58,6 +73,7 @@ public void fromJson_withMetadata() throws JSONException {
" \"requestCode\": 123,\n" +
" \"url\": \"https://example.com\",\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\",\n" +
" \"shouldNotify\": true,\n" +
" \"metadata\": {\n" +
" \"testKey\": \"testValue\"" +
" }" +
Expand All @@ -66,6 +82,7 @@ public void fromJson_withMetadata() throws JSONException {

assertEquals(sut.getRequestCode(), 123);
assertEquals(sut.getUrl().toString(), "https://example.com");
assertTrue(sut.getShouldNotifyCancellation());
JSONAssert.assertEquals(sut.getMetadata(), new JSONObject().put("testKey", "testValue"), true);

assertTrue(sut.matchesDeepLinkUrlScheme(Uri.parse("my-return-url-scheme://test")));
Expand All @@ -78,6 +95,7 @@ public void toJson_withMetadata() throws JSONException {
" \"requestCode\": 123,\n" +
" \"url\": \"https://example.com\",\n" +
" \"returnUrlScheme\": \"my-return-url-scheme\",\n" +
" \"shouldNotify\": false,\n" +
" \"metadata\": {\n" +
" \"testKey\": \"testValue\"" +
" }" +
Expand All @@ -87,6 +105,7 @@ public void toJson_withMetadata() throws JSONException {

assertEquals(restored.getRequestCode(), original.getRequestCode());
assertEquals(restored.getUrl(), original.getUrl());
assertFalse(restored.getShouldNotifyCancellation());
JSONAssert.assertEquals(restored.getMetadata(), original.getMetadata(), true);

assertTrue(restored.matchesDeepLinkUrlScheme(Uri.parse("my-return-url-scheme://test")));
Expand Down
30 changes: 11 additions & 19 deletions demo/src/main/java/com/braintreepayments/api/demo/DemoFragment.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,38 +53,24 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
public void onClick(View v) {
@IdRes int viewId = v.getId();
if (viewId == R.id.browser_switch) {
startBrowserSwitch();
startBrowserSwitch(null);
} else if (viewId == R.id.browser_switch_with_metadata) {
JSONObject metadata = buildMetadataObject();
startBrowserSwitchWithMetadata(metadata);
startBrowserSwitch(metadata);
}
}

private void startBrowserSwitch() {
Uri url = buildBrowserSwitchUrl();
BrowserSwitchOptions browserSwitchOptions = new BrowserSwitchOptions()
.requestCode(1)
.url(url)
.returnUrlScheme(getReturnUrlScheme());

try {
getDemoActivity().startBrowserSwitch(browserSwitchOptions);
} catch (BrowserSwitchException e) {
String statusText = "Browser Switch Error: " + e.getMessage();
mBrowserSwitchStatusTextView.setText(statusText);
e.printStackTrace();
}
}

private void startBrowserSwitchWithMetadata(JSONObject metadata) {
private void startBrowserSwitch(@Nullable JSONObject metadata) {
Uri url = buildBrowserSwitchUrl();
BrowserSwitchOptions browserSwitchOptions = new BrowserSwitchOptions()
.metadata(metadata)
.requestCode(1)
.url(url)
.returnUrlScheme(getReturnUrlScheme());

try {
getDemoActivity().startBrowserSwitch(browserSwitchOptions);
clearTextViews();
} catch (BrowserSwitchException e) {
String statusText = "Browser Switch Error: " + e.getMessage();
mBrowserSwitchStatusTextView.setText(statusText);
Expand All @@ -108,6 +94,12 @@ private JSONObject buildMetadataObject() {
return null;
}

private void clearTextViews() {
mBrowserSwitchStatusTextView.setText("");
mSelectedColorTextView.setText("");
mMetadataTextView.setText("");
}

public void onBrowserSwitchResult(BrowserSwitchResult result) {
String resultText = null;
String selectedColorText = "";
Expand Down

0 comments on commit e8c7c85

Please sign in to comment.