Skip to content

Commit

Permalink
Prevent AddSourceActivity's SourceCallback from being GCed (#791)
Browse files Browse the repository at this point in the history
**Summary**
`Stripe#CreateSourceTask` holds a `WeakReference` to the
`SourceCallback`. This reference was being garbage collected,
causing callback (either onSuccess or onError) to never happen.

Making the SourceCallback an instance variable seems to fix
this issue.

**Motivation**
Because the callback was never happening, the user would be
stuck on the "Add a Card" screen.

**Testing**
Tested manually on various API levels.
  • Loading branch information
mshafrir-stripe committed Jan 30, 2019
1 parent df83cf2 commit 2b74328
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 35 deletions.
6 changes: 4 additions & 2 deletions stripe/src/main/java/com/stripe/android/SourceCallback.java
@@ -1,5 +1,7 @@
package com.stripe.android;

import androidx.annotation.NonNull;

import com.stripe.android.model.Source;

/**
Expand All @@ -12,11 +14,11 @@ public interface SourceCallback {
* Error callback method.
* @param error the error that occurred.
*/
void onError(Exception error);
void onError(@NonNull Exception error);

/**
* Success callback method.
* @param source the {@link Source} that was found or created.
*/
void onSuccess(Source source);
void onSuccess(@NonNull Source source);
}
17 changes: 9 additions & 8 deletions stripe/src/main/java/com/stripe/android/Stripe.java
Expand Up @@ -763,7 +763,8 @@ private void validateKey(@NonNull @Size(min = 1) String publishableKey) {
}
}

private void executeTask(Executor executor, AsyncTask<Void, Void, ResponseWrapper> task) {
private void executeTask(@Nullable Executor executor,
@NonNull AsyncTask<Void, Void, ResponseWrapper> task) {
if (executor != null) {
task.executeOnExecutor(executor);
} else {
Expand All @@ -772,23 +773,23 @@ private void executeTask(Executor executor, AsyncTask<Void, Void, ResponseWrappe
}

private static class ResponseWrapper {
final Source source;
final Token token;
final Exception error;
@Nullable final Source source;
@Nullable final Token token;
@Nullable final Exception error;

private ResponseWrapper(Token token) {
private ResponseWrapper(@Nullable Token token) {
this.token = token;
this.source = null;
this.error = null;
}

private ResponseWrapper(Source source) {
private ResponseWrapper(@Nullable Source source) {
this.source = source;
this.error = null;
this.token = null;
}

private ResponseWrapper(Exception error) {
private ResponseWrapper(@NonNull Exception error) {
this.error = error;
this.source = null;
this.token = null;
Expand Down Expand Up @@ -850,7 +851,7 @@ protected ResponseWrapper doInBackground(Void... params) {
}

@Override
protected void onPostExecute(ResponseWrapper responseWrapper) {
protected void onPostExecute(@NonNull ResponseWrapper responseWrapper) {
final SourceCallback sourceCallback = mSourceCallbackRef.get();
if (sourceCallback != null) {
if (responseWrapper.source != null) {
Expand Down
54 changes: 29 additions & 25 deletions stripe/src/main/java/com/stripe/android/view/AddSourceActivity.java
Expand Up @@ -34,15 +34,16 @@
public class AddSourceActivity extends StripeActivity {

public static final String EXTRA_NEW_SOURCE = "new_source";

static final String ADD_SOURCE_ACTIVITY = "AddSourceActivity";
static final String EXTRA_SHOW_ZIP = "show_zip";
static final String EXTRA_PROXY_DELAY = "proxy_delay";
static final String EXTRA_UPDATE_CUSTOMER = "update_customer";

CardMultilineWidget mCardMultilineWidget;
CustomerSessionProxy mCustomerSessionProxy;
FrameLayout mErrorLayout;
StripeProvider mStripeProvider;
@Nullable private CardMultilineWidget mCardMultilineWidget;
@Nullable private CustomerSessionProxy mCustomerSessionProxy;
private FrameLayout mErrorLayout;
@Nullable private StripeProvider mStripeProvider;

private boolean mStartedFromPaymentSession;
private boolean mUpdatesCustomer;
Expand All @@ -63,6 +64,24 @@ public boolean onEditorAction(TextView v, int actionId, KeyEvent event) {
}
};

@NonNull private final SourceCallback mSourceCallback = new SourceCallback() {
@Override
public void onError(@NonNull Exception error) {
setCommunicatingProgress(false);
// This error is independent of the CustomerSession, so
// we have to surface it here.
showError(error.getLocalizedMessage());
}

@Override
public void onSuccess(@NonNull Source source) {
if (mUpdatesCustomer) {
attachCardToCustomer(source);
} else {
finishWithSource(source);
}
}
};

/**
* Create an {@link Intent} to start a {@link AddSourceActivity}.
Expand Down Expand Up @@ -136,28 +155,12 @@ protected void onActionSave() {
final SourceParams sourceParams = SourceParams.createCardParams(card);
setCommunicatingProgress(true);

stripe.createSource(sourceParams, new SourceCallback() {
@Override
public void onError(Exception error) {
setCommunicatingProgress(false);
// This error is independent of the CustomerSession, so
// we have to surface it here.
showError(error.getLocalizedMessage());
}
stripe.createSource(sourceParams, mSourceCallback);

@Override
public void onSuccess(Source source) {
if (mUpdatesCustomer) {
attachCardToCustomer(source);
} else {
finishWithSource(source);
}
}
});
}

private void attachCardToCustomer(StripePaymentSource source) {
CustomerSession.SourceRetrievalListener listener =
private void attachCardToCustomer(@NonNull final StripePaymentSource source) {
final CustomerSession.SourceRetrievalListener listener =
new CustomerSession.SourceRetrievalListener() {
@Override
public void onSourceRetrieved(@NonNull Source source) {
Expand Down Expand Up @@ -250,8 +253,9 @@ interface StripeProvider {
}

interface CustomerSessionProxy {
void addProductUsageTokenIfValid(String token);
void addProductUsageTokenIfValid(@NonNull String token);

void addCustomerSource(String sourceId, CustomerSession.SourceRetrievalListener listener);
void addCustomerSource(String sourceId,
@NonNull CustomerSession.SourceRetrievalListener listener);
}
}

0 comments on commit 2b74328

Please sign in to comment.