[go: nahoru, domu]

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

FIAM: Fix FirebaseInAppMessagingDisplayErrorListener not being called #5719

Merged
merged 7 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion firebase-inappmessaging-display/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

* [fixed] Fixed FirebaseInAppMessagingDisplayErrorListener not being called
(GitHub [#5644](//github.com/firebase/firebase-android-sdk/issues/5644))

# 20.4.0
* [changed] Added Kotlin extensions (KTX) APIs from `com.google.firebase:firebase-inappmessaging-display-ktx`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import com.google.firebase.inappmessaging.display.internal.FiamImageLoader;
import com.google.firebase.inappmessaging.display.internal.FiamWindowManager;
import com.google.firebase.inappmessaging.display.internal.FirebaseInAppMessagingDisplayImpl;
import com.google.firebase.inappmessaging.display.internal.GlideErrorListener;
import com.google.firebase.inappmessaging.display.internal.InAppMessageLayoutConfig;
import com.google.firebase.inappmessaging.display.internal.Logging;
import com.google.firebase.inappmessaging.display.internal.RenewableTimer;
Expand Down Expand Up @@ -144,9 +145,18 @@ public void testMessage(
Activity activity,
InAppMessage inAppMessage,
FirebaseInAppMessagingDisplayCallbacks callbacks) {
setInAppMessageAndCallbacks(inAppMessage, callbacks);
showActiveFiam(activity);
}

private void setInAppMessageAndCallbacks(
InAppMessage inAppMessage, FirebaseInAppMessagingDisplayCallbacks callbacks) {
this.inAppMessage = inAppMessage;
this.callbacks = callbacks;
showActiveFiam(activity);
}

private void clearInAppMessageAndCallbacks() {
setInAppMessageAndCallbacks(null, null);
}

/**
Expand Down Expand Up @@ -204,8 +214,7 @@ private void bindFiamToActivity(Activity activity) {
Logging.logd("Active FIAM exists. Skipping trigger");
return;
}
inAppMessage = iam;
callbacks = cb;
setInAppMessageAndCallbacks(iam, cb);
showActiveFiam(activity);
});
// set the current activity to be the one passed in so that we know not to bind again to the
Expand Down Expand Up @@ -329,8 +338,7 @@ public void onClick(View v) {
// Ensure that we remove the displayed FIAM, and ensure that on re-load, the message
// isn't re-displayed
removeDisplayedFiam(activity);
inAppMessage = null;
callbacks = null;
clearInAppMessageAndCallbacks();
}
};
} else {
Expand Down Expand Up @@ -432,8 +440,7 @@ public void onError(Exception e) {
.removeGlobalOnLayoutListener(layoutListener);
}
cancelTimers(); // Not strictly necessary.
inAppMessage = null;
callbacks = null;
clearInAppMessageAndCallbacks();
}
});
}
Expand Down Expand Up @@ -492,6 +499,7 @@ private void loadNullableImage(
if (isValidImageData(imageData)) {
imageLoader
.load(imageData.getImageUrl())
.addErrorListener(new GlideErrorListener(inAppMessage, callbacks))
.tag(activity.getClass())
.placeholder(R.drawable.image_placeholder)
.into(fiam.getImageView(), callback);
Expand All @@ -506,8 +514,7 @@ private void dismissFiam(Activity activity) {
Logging.logd("Dismissing fiam");
notifyFiamDismiss();
removeDisplayedFiam(activity);
inAppMessage = null;
callbacks = null;
clearInAppMessageAndCallbacks();
}

private void removeDisplayedFiam(Activity activity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,11 @@ public FiamImageRequestCreator placeholder(int placeholderResId) {
return this;
}

public FiamImageRequestCreator addErrorListener(GlideErrorListener glideErrorListener) {
requestBuilder.addListener(glideErrorListener);
return this;
}

public FiamImageRequestCreator tag(Class c) {
tag = c.getSimpleName();
checkAndTag();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,40 +14,28 @@

package com.google.firebase.inappmessaging.display.internal;

// Picasso 's api forces us to listen to errors only using a global listener set on the picasso
// singleton. Since we initialize picasso from a static context and the in app message param to the
// logError method is not available statically, we are forced to introduce a error listener with
// mutable state so that the error from picasso can be translated to a logError on
// fiam headless, with the in app message as a parameter

import android.graphics.drawable.Drawable;
import androidx.annotation.Nullable;
import com.bumptech.glide.load.DataSource;
import com.bumptech.glide.load.engine.GlideException;
import com.bumptech.glide.request.RequestListener;
import com.bumptech.glide.request.target.Target;
import com.google.firebase.inappmessaging.FirebaseInAppMessagingDisplayCallbacks;
import com.google.firebase.inappmessaging.display.internal.injection.scopes.FirebaseAppScope;
import com.google.firebase.inappmessaging.model.InAppMessage;
import javax.inject.Inject;

/** @hide */
@FirebaseAppScope
public class GlideErrorListener implements RequestListener<Object> {
private InAppMessage inAppMessage;
private FirebaseInAppMessagingDisplayCallbacks displayCallbacks;

@Inject
GlideErrorListener() {}
public class GlideErrorListener implements RequestListener<Drawable> {
private final InAppMessage inAppMessage;
private final FirebaseInAppMessagingDisplayCallbacks displayCallbacks;

public void setInAppMessage(
public GlideErrorListener(
InAppMessage inAppMessage, FirebaseInAppMessagingDisplayCallbacks displayCallbacks) {
this.inAppMessage = inAppMessage;
this.displayCallbacks = displayCallbacks;
}

@Override
public boolean onLoadFailed(
@Nullable GlideException e, Object model, Target<Object> target, boolean isFirstResource) {
@Nullable GlideException e, Object model, Target<Drawable> target, boolean isFirstResource) {
Logging.logd("Image Downloading Error : " + e.getMessage() + ":" + e.getCause());

if (inAppMessage != null && displayCallbacks != null) {
Expand All @@ -67,9 +55,9 @@ public boolean onLoadFailed(

@Override
public boolean onResourceReady(
Object resource,
Drawable resource,
Object model,
Target<Object> target,
Target<Drawable> target,
DataSource dataSource,
boolean isFirstResource) {
Logging.logd("Image Downloading Success : " + resource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import com.google.firebase.inappmessaging.display.FirebaseInAppMessagingDisplay;
import com.google.firebase.inappmessaging.display.internal.FiamImageLoader;
import com.google.firebase.inappmessaging.display.internal.GlideErrorListener;
import com.google.firebase.inappmessaging.display.internal.injection.modules.GlideModule;
import com.google.firebase.inappmessaging.display.internal.injection.modules.HeadlessInAppMessagingModule;
import com.google.firebase.inappmessaging.display.internal.injection.scopes.FirebaseAppScope;
Expand All @@ -31,7 +30,5 @@ public interface AppComponent {
@FirebaseAppScope
FirebaseInAppMessagingDisplay providesFirebaseInAppMessagingUI();

GlideErrorListener glideErrorListener();

FiamImageLoader fiamImageLoader();
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import android.app.Application;
import com.bumptech.glide.Glide;
import com.bumptech.glide.RequestManager;
import com.google.firebase.inappmessaging.display.internal.GlideErrorListener;
import com.google.firebase.inappmessaging.display.internal.injection.scopes.FirebaseAppScope;
import dagger.Module;
import dagger.Provides;
Expand All @@ -27,8 +26,7 @@
public class GlideModule {
@Provides
@FirebaseAppScope
RequestManager providesGlideRequestManager(
Application application, GlideErrorListener glideErrorListener) {
return Glide.with(application).addDefaultRequestListener(glideErrorListener);
RequestManager providesGlideRequestManager(Application application) {
return Glide.with(application);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.refEq;
import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -61,6 +62,7 @@
import com.google.firebase.inappmessaging.display.internal.FiamImageLoader;
import com.google.firebase.inappmessaging.display.internal.FiamImageLoader.Callback;
import com.google.firebase.inappmessaging.display.internal.FiamWindowManager;
import com.google.firebase.inappmessaging.display.internal.GlideErrorListener;
import com.google.firebase.inappmessaging.display.internal.InAppMessageLayoutConfig;
import com.google.firebase.inappmessaging.display.internal.RenewableTimer;
import com.google.firebase.inappmessaging.display.internal.bindingwrappers.BannerBindingWrapper;
Expand Down Expand Up @@ -419,6 +421,8 @@ public void streamListener_whenImageUrlExists_loadsImage() {

verify(fiamImageRequestCreator).tag(TestActivity.class);
verify(fiamImageRequestCreator).placeholder(R.drawable.image_placeholder);
verify(fiamImageRequestCreator)
.addErrorListener(refEq(new GlideErrorListener(IMAGE_MESSAGE_MODEL, callbacks)));
verify(fiamImageRequestCreator).into(any(ImageView.class), any(Callback.class));
}

Expand All @@ -428,6 +432,7 @@ public void streamListener_whenNoImageUrlExists_doesNotLoadImage() {
listener.displayMessage(null, callbacks);
verify(fiamImageRequestCreator, times(0)).tag(TestActivity.class);
verify(fiamImageRequestCreator, times(0)).placeholder(R.drawable.image_placeholder);
verify(fiamImageRequestCreator, times(0)).addErrorListener(any(GlideErrorListener.class));
verify(fiamImageRequestCreator, times(0)).into(any(ImageView.class), any(Callback.class));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,14 @@ public void placeholder_setsPlaceholderOnUnderlyingRequestCreator() {
verify(requestBuilder).placeholder(1);
}

@Test
public void addErrorListener_setsErrorListenerOnUnderlyingRequestCreator() {
FiamImageLoader.FiamImageRequestCreator fiamImageRequestCreator = imageLoader.load(IMAGE_URL);
GlideErrorListener errorListener = new GlideErrorListener(null, null);
fiamImageRequestCreator.addErrorListener(errorListener);
verify(requestBuilder).addListener(errorListener);
}

@Test
public void tag_tagsUnderlyingRequestCreator() {
ImageView imageView = mock(ImageView.class);
Expand Down
3 changes: 2 additions & 1 deletion firebase-inappmessaging/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Unreleased

* [fixed] Fixed FirebaseInAppMessagingDisplayErrorListener not being called
(GitHub [#5644](//github.com/firebase/firebase-android-sdk/issues/5644))

# 20.4.0
* [changed] Added Kotlin extensions (KTX) APIs from `com.google.firebase:firebase-inappmessaging-ktx`
Expand Down
Loading