Skip to content

Commit 06007d6

Browse files
authored
Revert "Remove impressions for Campaigns delivered by backend service (#1402)"
This reverts commit f03383a.
1 parent f03383a commit 06007d6

File tree

5 files changed

+28
-89
lines changed

5 files changed

+28
-89
lines changed

firebase-inappmessaging/src/androidTest/java/com/google/firebase/inappmessaging/FirebaseInAppMessagingFlowableTest.java

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -596,17 +596,43 @@ public void onUnsupportedCampaign_doesNotNotify() {
596596
}
597597

598598
@Test
599-
public void whenImpressedButReceivedFromBackend_doesNotFilterCampaign()
599+
public void whenImpressed_filtersCampaign()
600600
throws ExecutionException, InterruptedException, TimeoutException {
601+
CampaignMetadata otherMetadata =
602+
new CampaignMetadata("otherCampaignId", "otherName", IS_NOT_TEST_MESSAGE);
603+
BannerMessage otherMessage = createBannerMessageCustomMetadata(otherMetadata);
604+
VanillaCampaignPayload otherCampaign =
605+
VanillaCampaignPayload.newBuilder(vanillaCampaign.build())
606+
.setCampaignId(otherMetadata.getCampaignId())
607+
.setCampaignName(otherMetadata.getCampaignName())
608+
.build();
609+
ThickContent otherContent =
610+
ThickContent.newBuilder(thickContent)
611+
.setContent(BANNER_MESSAGE_PROTO)
612+
.setIsTestCampaign(IS_NOT_TEST_MESSAGE)
613+
.clearVanillaPayload()
614+
.clearTriggeringConditions()
615+
.addTriggeringConditions(
616+
TriggeringCondition.newBuilder().setEvent(Event.newBuilder().setName("event2")))
617+
.setVanillaPayload(otherCampaign)
618+
.build();
619+
FetchEligibleCampaignsResponse response =
620+
FetchEligibleCampaignsResponse.newBuilder(eligibleCampaigns)
621+
.addMessages(otherContent)
622+
.build();
623+
GoodFiamService impl = new GoodFiamService(response);
624+
grpcServerRule.getServiceRegistry().addService(impl);
625+
601626
Task<Void> logImpressionTask =
602627
displayCallbacksFactory
603628
.generateDisplayCallback(MODAL_MESSAGE_MODEL, ANALYTICS_EVENT_NAME)
604629
.impressionDetected();
605630
Tasks.await(logImpressionTask, 1000, TimeUnit.MILLISECONDS);
606631
analyticsConnector.invokeListenerOnEvent(ANALYTICS_EVENT_NAME);
632+
analyticsConnector.invokeListenerOnEvent("event2");
607633
waitUntilNotified(subscriber);
608634

609-
assertSubsriberExactly(MODAL_MESSAGE_MODEL, subscriber);
635+
assertSubsriberExactly(otherMessage, subscriber);
610636
}
611637

612638
// There is not a purely functional way to determine if our clients inject the impressed
@@ -816,7 +842,6 @@ public void logImpression_logsToEngagementMetrics() {
816842
}
817843

818844
@Test
819-
@Ignore("Broken due to Impression Store changes. Needs fixing.")
820845
public void whenlogImpressionFails_doesNotFilterCampaign()
821846
throws ExecutionException, InterruptedException, TimeoutException, FileNotFoundException {
822847
doThrow(new NullPointerException("e1")).when(application).openFileInput(IMPRESSIONS_STORE_FILE);
@@ -943,7 +968,6 @@ public void whenlogEventLimitIncrementSuccess_writesLimitsToDisk() {
943968
}
944969

945970
@Test
946-
@Ignore("Broken due to Impression Store changes. Needs fixing.")
947971
public void onImpressionLog_cachesImpressionsInMemory()
948972
throws ExecutionException, InterruptedException, TimeoutException, FileNotFoundException {
949973
CampaignMetadata otherMetadata =
@@ -999,7 +1023,6 @@ public void onCorruptImpressionStore_doesNotFilter()
9991023
}
10001024

10011025
@Test
1002-
@Ignore("Broken due to Impression Store changes. Needs fixing.")
10031026
public void onImpressionStoreReadFailure_doesNotFilter()
10041027
throws ExecutionException, InterruptedException, TimeoutException, IOException {
10051028
doThrow(new NullPointerException("e1")).when(application).openFileInput(IMPRESSIONS_STORE_FILE);
@@ -1015,7 +1038,6 @@ public void onImpressionStoreReadFailure_doesNotFilter()
10151038
// We work around this by failing hard on the fake service if we do not find an empty impression
10161039
// list
10171040
@Test
1018-
@Ignore("Broken due to Impression Store changes. Needs fixing.")
10191041
public void whenImpressionStorageClientFails_injectsEmptyImpressionListUpstream()
10201042
throws ExecutionException, InterruptedException, TimeoutException, FileNotFoundException {
10211043
VanillaCampaignPayload otherCampaign =

firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/ImpressionStorageClient.java

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,10 @@
1818
import com.google.internal.firebase.inappmessaging.v1.CampaignProto;
1919
import com.google.internal.firebase.inappmessaging.v1.sdkserving.CampaignImpression;
2020
import com.google.internal.firebase.inappmessaging.v1.sdkserving.CampaignImpressionList;
21-
import com.google.internal.firebase.inappmessaging.v1.sdkserving.FetchEligibleCampaignsResponse;
2221
import io.reactivex.Completable;
2322
import io.reactivex.Maybe;
2423
import io.reactivex.Observable;
2524
import io.reactivex.Single;
26-
import java.util.HashSet;
2725
import javax.inject.Inject;
2826
import javax.inject.Singleton;
2927

@@ -98,40 +96,4 @@ public Single<Boolean> isImpressed(CampaignProto.ThickContent content) {
9896
.map(CampaignImpression::getCampaignId)
9997
.contains(campaignId);
10098
}
101-
102-
/**
103-
* Clears impressions for all campaigns found in the provided {@link
104-
* FetchEligibleCampaignsResponse} This is done because we trust the server to deliver campaigns
105-
* which should be shown again for scheduled campaigns.
106-
*/
107-
public Completable clearImpressions(FetchEligibleCampaignsResponse response) {
108-
HashSet<String> idsToClear = new HashSet<>();
109-
for (CampaignProto.ThickContent content : response.getMessagesList()) {
110-
String id =
111-
content.getPayloadCase().equals(CampaignProto.ThickContent.PayloadCase.VANILLA_PAYLOAD)
112-
? content.getVanillaPayload().getCampaignId()
113-
: content.getExperimentalPayload().getCampaignId();
114-
idsToClear.add(id);
115-
}
116-
Logging.logd("Potential impressions to clear: " + idsToClear.toString());
117-
return getAllImpressions()
118-
.defaultIfEmpty(EMPTY_IMPRESSIONS)
119-
.flatMapCompletable(
120-
(storedImpressions) -> {
121-
Logging.logd("Existing impressions: " + storedImpressions.toString());
122-
CampaignImpressionList.Builder clearedImpressionListBuilder =
123-
CampaignImpressionList.newBuilder();
124-
for (CampaignImpression storedImpression :
125-
storedImpressions.getAlreadySeenCampaignsList()) {
126-
if (!idsToClear.contains(storedImpression.getCampaignId())) {
127-
clearedImpressionListBuilder.addAlreadySeenCampaigns(storedImpression);
128-
}
129-
}
130-
CampaignImpressionList clearedImpressionList = clearedImpressionListBuilder.build();
131-
Logging.logd("New cleared impression list: " + clearedImpressionList.toString());
132-
return storageClient
133-
.write(clearedImpressionList)
134-
.doOnComplete(() -> initInMemCache(clearedImpressionList));
135-
});
136-
}
13799
}

firebase-inappmessaging/src/main/java/com/google/firebase/inappmessaging/internal/InAppMessageStreamManager.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,6 @@ public Flowable<TriggeredInAppMessage> createFirebaseInAppMessageStream() {
242242
Locale.US,
243243
"Successfully fetched %d messages from backend",
244244
resp.getMessagesList().size())))
245-
.doOnSuccess(
246-
resp -> impressionStorageClient.clearImpressions(resp).subscribe())
247245
.doOnSuccess(analyticsEventsManager::updateContextualTriggers)
248246
.doOnSuccess(testDeviceHelper::processCampaignFetch)
249247
.doOnError(e -> Logging.logw("Service fetch error: " + e.getMessage()))

firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/ImpressionStorageClientTest.java

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.internal.firebase.inappmessaging.v1.CampaignProto.ThickContent;
2626
import com.google.internal.firebase.inappmessaging.v1.sdkserving.CampaignImpression;
2727
import com.google.internal.firebase.inappmessaging.v1.sdkserving.CampaignImpressionList;
28-
import com.google.internal.firebase.inappmessaging.v1.sdkserving.FetchEligibleCampaignsResponse;
2928
import com.google.protobuf.Parser;
3029
import io.reactivex.Completable;
3130
import io.reactivex.Maybe;
@@ -279,45 +278,5 @@ public void isImpressed_onError_notifiesError() {
279278
subscriber.assertError(IOException.class);
280279
}
281280

282-
@Test
283-
public void clearImpressions_clearsImpressionsForFetchedCampaign() {
284-
// verify campaign is impressed.
285-
TestSubscriber<Boolean> subscriber =
286-
impressionStorageClient.isImpressed(vanillaCampaign).toFlowable().test();
287-
assertThat(subscriber.getEvents().get(0)).containsExactly(true);
288-
289-
// clear impressions for a fetch response containing that campaign.
290-
// This simulates having received the campaign again from the server.
291-
impressionStorageClient
292-
.clearImpressions(
293-
FetchEligibleCampaignsResponse.newBuilder().addMessages(vanillaCampaign).build())
294-
.subscribe();
295-
296-
// Verify campaign is no longer impressed.
297-
TestSubscriber<Boolean> subscriber2 =
298-
impressionStorageClient.isImpressed(vanillaCampaign).toFlowable().test();
299-
assertThat(subscriber2.getEvents().get(0)).containsExactly(false);
300-
}
301-
302-
@Test
303-
public void clearImpressions_doesNotClearImpressionForUnfetchedCampaign() {
304-
// verify initial campaign is impressed.
305-
TestSubscriber<Boolean> subscriber =
306-
impressionStorageClient.isImpressed(vanillaCampaign).toFlowable().test();
307-
assertThat(subscriber.getEvents().get(0)).containsExactly(true);
308-
309-
// clear impressions for a fetch response containing a different campaign.
310-
// This simulates having received the campaign again from the server.
311-
impressionStorageClient
312-
.clearImpressions(
313-
FetchEligibleCampaignsResponse.newBuilder().addMessages(experimentalCampaign).build())
314-
.subscribe();
315-
316-
// Verify campaign is still impressed.
317-
TestSubscriber<Boolean> subscriber2 =
318-
impressionStorageClient.isImpressed(vanillaCampaign).toFlowable().test();
319-
assertThat(subscriber2.getEvents().get(0)).containsExactly(true);
320-
}
321-
322281
interface CampaignImpressionsParser extends Parser<CampaignImpressionList> {}
323282
}

firebase-inappmessaging/src/test/java/com/google/firebase/inappmessaging/internal/InAppMessageStreamManagerTest.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,8 +198,6 @@ public void setup() {
198198
abtIntegrationHelper);
199199
subscriber = streamManager.createFirebaseInAppMessageStream().test();
200200
when(application.getApplicationContext()).thenReturn(application);
201-
when(impressionStorageClient.clearImpressions(any(FetchEligibleCampaignsResponse.class)))
202-
.thenReturn(Completable.complete());
203201
when(rateLimiterClient.isRateLimited(appForegroundRateLimit)).thenReturn(Single.just(false));
204202
when(campaignCacheClient.get()).thenReturn(Maybe.empty());
205203
when(campaignCacheClient.put(any(FetchEligibleCampaignsResponse.class)))

0 commit comments

Comments
 (0)