Skip to content

Commit fa409a1

Browse files
lfkellogglfkellogg
committed
Support multiple progress listeners on an UpdateTask (#4523)
* Support multiple progress listeners on an UpdateTask * Remove unnecessary usage
1 parent b462b47 commit fa409a1

File tree

3 files changed

+166
-15
lines changed

3 files changed

+166
-15
lines changed

‎firebase-appdistribution/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Unreleased
22

3+
# 16.0.0-beta04
4+
* [fixed] Fixed a bug where only the last listener added to an `UpdateTask` using
5+
`addOnProgressListener()` would receive updates.
6+
37
# 16.0.0-beta05
48
* [unchanged] Updated to accommodate the release of the updated
59
[appdistro] Kotlin extensions library.

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/UpdateTaskImpl.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,17 @@
3232
import com.google.firebase.appdistribution.UpdateProgress;
3333
import com.google.firebase.appdistribution.UpdateTask;
3434
import com.google.firebase.concurrent.FirebaseExecutors;
35+
import java.util.ArrayList;
36+
import java.util.List;
3537
import java.util.concurrent.Executor;
3638

3739
/** Implementation of UpdateTask, the return type of updateApp. */
3840
// TODO(b/261013814): Use an explicit executor in continuations.
3941
class UpdateTaskImpl extends UpdateTask {
42+
4043
@Nullable
4144
@GuardedBy("lock")
42-
private ManagedListener listener = null;
45+
private List<ManagedListener> listeners = new ArrayList<>();
4346

4447
private final Object lock = new Object();
4548
private final Object taskCompletionLock = new Object();
@@ -52,15 +55,15 @@ class UpdateTaskImpl extends UpdateTask {
5255

5356
UpdateTaskImpl() {
5457
synchronized (taskCompletionLock) {
55-
this.taskCompletionSource = new TaskCompletionSource<>();
58+
taskCompletionSource = new TaskCompletionSource<>();
5659
}
5760
}
5861

5962
void updateProgress(@NonNull UpdateProgress updateProgress) {
6063
synchronized (lock) {
6164
snapshot = updateProgress;
62-
if (this.listener != null) {
63-
this.listener.invoke(updateProgress);
65+
for (ManagedListener listener : listeners) {
66+
listener.invoke(updateProgress);
6467
}
6568
}
6669
}
@@ -72,13 +75,13 @@ void updateProgress(@NonNull UpdateProgress updateProgress) {
7275
void shadow(UpdateTask updateTask) {
7376
updateTask
7477
.addOnProgressListener(FirebaseExecutors.directExecutor(), this::updateProgress)
75-
.addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> this.setResult())
78+
.addOnSuccessListener(FirebaseExecutors.directExecutor(), unused -> setResult())
7679
.addOnFailureListener(FirebaseExecutors.directExecutor(), this::setException);
7780
}
7881

7982
private Task<Void> getTask() {
80-
synchronized (this.taskCompletionLock) {
81-
return this.taskCompletionSource.getTask();
83+
synchronized (taskCompletionLock) {
84+
return taskCompletionSource.getTask();
8285
}
8386
}
8487

@@ -94,9 +97,9 @@ public UpdateTask addOnProgressListener(
9497
@Nullable Executor executor, @NonNull OnProgressListener listener) {
9598
ManagedListener managedListener = new ManagedListener(executor, listener);
9699
synchronized (lock) {
97-
this.listener = managedListener;
100+
listeners.add(managedListener);
98101
if (snapshot != null) {
99-
this.listener.invoke(snapshot);
102+
managedListener.invoke(snapshot);
100103
}
101104
}
102105
return this;
@@ -264,12 +267,12 @@ public <TContinuationResult> Task<TContinuationResult> onSuccessTask(
264267
public void setResult() {
265268

266269
synchronized (lock) {
267-
this.listener = null;
270+
listeners.clear();
268271
}
269272

270273
synchronized (taskCompletionLock) {
271-
if (!this.taskCompletionSource.getTask().isComplete()) {
272-
this.taskCompletionSource.setResult(null);
274+
if (!taskCompletionSource.getTask().isComplete()) {
275+
taskCompletionSource.setResult(null);
273276
}
274277
}
275278
}
@@ -278,12 +281,12 @@ public void setResult() {
278281
public void setException(@NonNull Exception exception) {
279282

280283
synchronized (lock) {
281-
this.listener = null;
284+
listeners.clear();
282285
}
283286

284287
synchronized (taskCompletionLock) {
285-
if (!this.taskCompletionSource.getTask().isComplete()) {
286-
this.taskCompletionSource.setException(exception);
288+
if (!taskCompletionSource.getTask().isComplete()) {
289+
taskCompletionSource.setException(exception);
287290
}
288291
}
289292
}

‎firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/UpdateTaskImplTest.java

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.appdistribution.impl;
16+
17+
import static com.google.common.truth.Truth.assertThat;
18+
19+
import com.google.firebase.appdistribution.UpdateProgress;
20+
import com.google.firebase.appdistribution.UpdateStatus;
21+
import com.google.firebase.concurrent.FirebaseExecutors;
22+
import java.util.ArrayList;
23+
import java.util.List;
24+
import org.junit.Test;
25+
import org.junit.runner.RunWith;
26+
import org.robolectric.RobolectricTestRunner;
27+
28+
@RunWith(RobolectricTestRunner.class)
29+
public class UpdateTaskImplTest {
30+
31+
private static final UpdateProgress PROGRESS_DOWNLOADING =
32+
UpdateProgressImpl.builder()
33+
.setUpdateStatus(UpdateStatus.DOWNLOADING)
34+
.setApkBytesDownloaded(100)
35+
.setApkFileTotalBytes(123)
36+
.build();
37+
38+
private static final UpdateProgress PROGRESS_DOWNLOADED =
39+
UpdateProgressImpl.builder()
40+
.setUpdateStatus(UpdateStatus.DOWNLOADED)
41+
.setApkBytesDownloaded(123)
42+
.setApkFileTotalBytes(123)
43+
.build();
44+
45+
@Test
46+
public void addOnProgressListener_supportsMultipleListeners() {
47+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
48+
List<UpdateProgress> progressEvents1 = new ArrayList<>();
49+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents1::add);
50+
List<UpdateProgress> progressEvents2 = new ArrayList<>();
51+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents2::add);
52+
53+
updateTaskImpl.updateProgress(PROGRESS_DOWNLOADING);
54+
55+
assertThat(progressEvents1).containsExactly(PROGRESS_DOWNLOADING);
56+
assertThat(progressEvents2).containsExactly(PROGRESS_DOWNLOADING);
57+
}
58+
59+
@Test
60+
public void setException_clearsProgressListeners() {
61+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
62+
List<UpdateProgress> progressEvents1 = new ArrayList<>();
63+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents1::add);
64+
List<UpdateProgress> progressEvents2 = new ArrayList<>();
65+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents2::add);
66+
67+
// Set the result
68+
updateTaskImpl.setException(new RuntimeException("Error"));
69+
70+
// Has no effect on the listeners after the exception is set
71+
updateTaskImpl.updateProgress(PROGRESS_DOWNLOADING);
72+
73+
assertThat(progressEvents1).isEmpty();
74+
assertThat(progressEvents2).isEmpty();
75+
}
76+
77+
@Test
78+
public void setResult_clearsProgressListeners() {
79+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
80+
List<UpdateProgress> progressEvents1 = new ArrayList<>();
81+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents1::add);
82+
List<UpdateProgress> progressEvents2 = new ArrayList<>();
83+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents2::add);
84+
85+
// Set the result
86+
updateTaskImpl.setResult();
87+
88+
// Has no effect on the listeners after the result is set
89+
updateTaskImpl.updateProgress(PROGRESS_DOWNLOADING);
90+
91+
assertThat(progressEvents1).isEmpty();
92+
assertThat(progressEvents2).isEmpty();
93+
}
94+
95+
@Test
96+
public void shadow_completesWithShadowedException() {
97+
UpdateTaskImpl taskToShadow = new UpdateTaskImpl();
98+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
99+
100+
// Shadow the task
101+
updateTaskImpl.shadow(taskToShadow);
102+
103+
// Complete the shadowed task
104+
RuntimeException expectedException = new RuntimeException("Error");
105+
taskToShadow.setException(expectedException);
106+
107+
assertThat(updateTaskImpl.isComplete()).isEqualTo(true);
108+
assertThat(updateTaskImpl.isSuccessful()).isEqualTo(false);
109+
assertThat(updateTaskImpl.getException()).isEqualTo(expectedException);
110+
}
111+
112+
@Test
113+
public void shadow_completesWithShadowedResult() {
114+
UpdateTaskImpl taskToShadow = new UpdateTaskImpl();
115+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
116+
117+
// Shadow the task
118+
updateTaskImpl.shadow(taskToShadow);
119+
120+
// Complete the shadowed task
121+
taskToShadow.setResult();
122+
123+
assertThat(updateTaskImpl.isComplete()).isEqualTo(true);
124+
assertThat(updateTaskImpl.isSuccessful()).isEqualTo(true);
125+
}
126+
127+
@Test
128+
public void shadow_updatesWithShadowedProgress() {
129+
UpdateTaskImpl taskToShadow = new UpdateTaskImpl();
130+
UpdateTaskImpl updateTaskImpl = new UpdateTaskImpl();
131+
List<UpdateProgress> progressEvents = new ArrayList<>();
132+
updateTaskImpl.addOnProgressListener(FirebaseExecutors.directExecutor(), progressEvents::add);
133+
134+
// Shadow the task
135+
updateTaskImpl.shadow(taskToShadow);
136+
137+
// Update the shadowed task
138+
taskToShadow.updateProgress(PROGRESS_DOWNLOADING);
139+
taskToShadow.updateProgress(PROGRESS_DOWNLOADED);
140+
141+
assertThat(updateTaskImpl.isComplete()).isEqualTo(false);
142+
assertThat(progressEvents).containsExactly(PROGRESS_DOWNLOADING, PROGRESS_DOWNLOADED);
143+
}
144+
}

0 commit comments

Comments
 (0)