Skip to content

Commit dc1a63c

Browse files
diwu-aretediwu-arete
committed
Addressed a few comments
1 parent 8b39c31 commit dc1a63c

File tree

2 files changed

+114
-149
lines changed

2 files changed

+114
-149
lines changed

‎firebase-segmentation/src/main/java/com/google/firebase/segmentation/FirebaseSegmentation.java

Lines changed: 113 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,9 @@
1616

1717
import androidx.annotation.NonNull;
1818
import androidx.annotation.Nullable;
19-
import androidx.annotation.RestrictTo;
19+
import androidx.annotation.WorkerThread;
2020
import com.google.android.gms.common.internal.Preconditions;
2121
import com.google.android.gms.tasks.Task;
22-
import com.google.android.gms.tasks.TaskCompletionSource;
2322
import com.google.android.gms.tasks.Tasks;
2423
import com.google.firebase.FirebaseApp;
2524
import com.google.firebase.iid.FirebaseInstanceId;
@@ -29,6 +28,7 @@
2928
import com.google.firebase.segmentation.local.CustomInstallationIdCacheEntryValue;
3029
import com.google.firebase.segmentation.remote.SegmentationServiceClient;
3130
import com.google.firebase.segmentation.remote.SegmentationServiceClient.Code;
31+
import java.util.concurrent.ExecutionException;
3232
import java.util.concurrent.Executor;
3333
import java.util.concurrent.Executors;
3434

@@ -42,14 +42,13 @@ public class FirebaseSegmentation {
4242
private final Executor executor;
4343

4444
FirebaseSegmentation(FirebaseApp firebaseApp) {
45-
this.firebaseApp = firebaseApp;
46-
this.firebaseInstanceId = FirebaseInstanceId.getInstance(firebaseApp);
47-
this.localCache = new CustomInstallationIdCache(firebaseApp);
48-
this.backendServiceClient = new SegmentationServiceClient();
49-
this.executor = Executors.newFixedThreadPool(4);
45+
this(
46+
firebaseApp,
47+
FirebaseInstanceId.getInstance(firebaseApp),
48+
new CustomInstallationIdCache(firebaseApp),
49+
new SegmentationServiceClient());
5050
}
5151

52-
@RestrictTo(RestrictTo.Scope.TESTS)
5352
FirebaseSegmentation(
5453
FirebaseApp firebaseApp,
5554
FirebaseInstanceId firebaseInstanceId,
@@ -88,9 +87,9 @@ public static FirebaseSegmentation getInstance(@NonNull FirebaseApp app) {
8887
@NonNull
8988
public synchronized Task<Void> setCustomInstallationId(@Nullable String customInstallationId) {
9089
if (customInstallationId == null) {
91-
return clearCustomInstallationId();
90+
return Tasks.call(executor, () -> clearCustomInstallationId());
9291
}
93-
return updateCustomInstallationId(customInstallationId);
92+
return Tasks.call(executor, () -> updateCustomInstallationId(customInstallationId));
9493
}
9594

9695
/**
@@ -112,89 +111,73 @@ public synchronized Task<Void> setCustomInstallationId(@Nullable String customIn
112111
* return
113112
* </pre>
114113
*/
115-
private Task<Void> updateCustomInstallationId(String customInstallationId) {
116-
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
117-
118-
executor.execute(
119-
() -> {
120-
CustomInstallationIdCacheEntryValue cacheEntryValue = localCache.readCacheEntryValue();
121-
if (cacheEntryValue != null
122-
&& cacheEntryValue.getCustomInstallationId().equals(customInstallationId)
123-
&& cacheEntryValue.getCacheStatus() == CustomInstallationIdCache.CacheStatus.SYNCED) {
124-
// If the given custom installation id matches up the cached
125-
// value, there's no need to update.
126-
taskCompletionSource.setResult(null);
127-
return;
128-
}
129-
130-
InstanceIdResult instanceIdResult;
131-
try {
132-
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
133-
} catch (Exception e) {
134-
taskCompletionSource.setException(
135-
new SetCustomInstallationIdException(
136-
"Failed to get Firebase instance id", Status.CLIENT_ERROR));
137-
return;
138-
}
114+
@WorkerThread
115+
private Void updateCustomInstallationId(String customInstallationId)
116+
throws SetCustomInstallationIdException {
117+
CustomInstallationIdCacheEntryValue cacheEntryValue = localCache.readCacheEntryValue();
118+
if (cacheEntryValue != null
119+
&& cacheEntryValue.getCustomInstallationId().equals(customInstallationId)
120+
&& cacheEntryValue.getCacheStatus() == CustomInstallationIdCache.CacheStatus.SYNCED) {
121+
// If the given custom installation id matches up the cached
122+
// value, there's no need to update.
123+
return null;
124+
}
139125

140-
boolean firstUpdateCacheResult =
141-
localCache.insertOrUpdateCacheEntry(
142-
CustomInstallationIdCacheEntryValue.create(
143-
customInstallationId,
144-
instanceIdResult.getId(),
145-
CustomInstallationIdCache.CacheStatus.PENDING_UPDATE));
126+
InstanceIdResult instanceIdResult;
127+
try {
128+
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
129+
} catch (ExecutionException | InterruptedException e) {
130+
throw new SetCustomInstallationIdException(
131+
"Failed to get Firebase instance id", Status.CLIENT_ERROR);
132+
}
146133

147-
if (!firstUpdateCacheResult) {
148-
taskCompletionSource.setException(
149-
new SetCustomInstallationIdException(
150-
"Failed to update client side cache", Status.CLIENT_ERROR));
151-
return;
152-
}
134+
boolean firstUpdateCacheResult =
135+
localCache.insertOrUpdateCacheEntry(
136+
CustomInstallationIdCacheEntryValue.create(
137+
customInstallationId,
138+
instanceIdResult.getId(),
139+
CustomInstallationIdCache.CacheStatus.PENDING_UPDATE));
153140

154-
// Start requesting backend when first cache updae is done.
155-
String iid = instanceIdResult.getId();
156-
String iidToken = instanceIdResult.getToken();
157-
Code backendRequestResult =
158-
backendServiceClient.updateCustomInstallationId(
159-
Utils.getProjectNumberFromAppId(firebaseApp.getOptions().getApplicationId()),
160-
firebaseApp.getOptions().getApiKey(),
161-
customInstallationId,
162-
iid,
163-
iidToken);
141+
if (!firstUpdateCacheResult) {
142+
throw new SetCustomInstallationIdException(
143+
"Failed to update client side cache", Status.CLIENT_ERROR);
144+
}
164145

165-
boolean finalUpdateCacheResult;
166-
switch (backendRequestResult) {
167-
case OK:
168-
finalUpdateCacheResult =
169-
localCache.insertOrUpdateCacheEntry(
170-
CustomInstallationIdCacheEntryValue.create(
171-
customInstallationId,
172-
instanceIdResult.getId(),
173-
CustomInstallationIdCache.CacheStatus.SYNCED));
174-
break;
175-
case HTTP_CLIENT_ERROR:
176-
taskCompletionSource.setException(
177-
new SetCustomInstallationIdException(Status.CLIENT_ERROR));
178-
return;
179-
case CONFLICT:
180-
taskCompletionSource.setException(
181-
new SetCustomInstallationIdException(Status.DUPLICATED_CUSTOM_INSTALLATION_ID));
182-
return;
183-
default:
184-
taskCompletionSource.setException(
185-
new SetCustomInstallationIdException(Status.BACKEND_ERROR));
186-
return;
187-
}
146+
// Start requesting backend when first cache updae is done.
147+
String iid = instanceIdResult.getId();
148+
String iidToken = instanceIdResult.getToken();
149+
Code backendRequestResult =
150+
backendServiceClient.updateCustomInstallationId(
151+
Utils.getProjectNumberFromAppId(firebaseApp.getOptions().getApplicationId()),
152+
firebaseApp.getOptions().getApiKey(),
153+
customInstallationId,
154+
iid,
155+
iidToken);
156+
157+
boolean finalUpdateCacheResult;
158+
switch (backendRequestResult) {
159+
case OK:
160+
finalUpdateCacheResult =
161+
localCache.insertOrUpdateCacheEntry(
162+
CustomInstallationIdCacheEntryValue.create(
163+
customInstallationId,
164+
instanceIdResult.getId(),
165+
CustomInstallationIdCache.CacheStatus.SYNCED));
166+
break;
167+
case HTTP_CLIENT_ERROR:
168+
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR);
169+
case CONFLICT:
170+
throw new SetCustomInstallationIdException(Status.DUPLICATED_CUSTOM_INSTALLATION_ID);
171+
default:
172+
throw new SetCustomInstallationIdException(Status.BACKEND_ERROR);
173+
}
188174

189-
if (finalUpdateCacheResult) {
190-
taskCompletionSource.setResult(null);
191-
} else {
192-
taskCompletionSource.setException(
193-
new SetCustomInstallationIdException(
194-
"Failed to update client side cache", Status.CLIENT_ERROR));
195-
}
196-
});
197-
return taskCompletionSource.getTask();
175+
if (finalUpdateCacheResult) {
176+
return null;
177+
} else {
178+
throw new SetCustomInstallationIdException(
179+
"Failed to update client side cache", Status.CLIENT_ERROR);
180+
}
198181
}
199182

200183
/**
@@ -214,68 +197,51 @@ private Task<Void> updateCustomInstallationId(String customInstallationId) {
214197
* return
215198
* </pre>
216199
*/
217-
private Task<Void> clearCustomInstallationId() {
218-
219-
TaskCompletionSource<Void> taskCompletionSource = new TaskCompletionSource<>();
220-
221-
executor.execute(
222-
() -> {
223-
InstanceIdResult instanceIdResult;
224-
try {
225-
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
226-
} catch (Exception e) {
227-
taskCompletionSource.setException(
228-
new SetCustomInstallationIdException(
229-
"Failed to get Firebase instance id", Status.CLIENT_ERROR));
230-
return;
231-
}
232-
233-
boolean firstUpdateCacheResult =
234-
localCache.insertOrUpdateCacheEntry(
235-
CustomInstallationIdCacheEntryValue.create(
236-
"",
237-
instanceIdResult.getId(),
238-
CustomInstallationIdCache.CacheStatus.PENDING_CLEAR));
200+
@WorkerThread
201+
private Void clearCustomInstallationId() throws SetCustomInstallationIdException {
202+
InstanceIdResult instanceIdResult;
203+
try {
204+
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
205+
} catch (ExecutionException | InterruptedException e) {
206+
throw new SetCustomInstallationIdException(
207+
"Failed to get Firebase instance id", Status.CLIENT_ERROR);
208+
}
239209

240-
if (!firstUpdateCacheResult) {
241-
taskCompletionSource.setException(
242-
new SetCustomInstallationIdException(
243-
"Failed to update client side cache", Status.CLIENT_ERROR));
244-
return;
245-
}
210+
boolean firstUpdateCacheResult =
211+
localCache.insertOrUpdateCacheEntry(
212+
CustomInstallationIdCacheEntryValue.create(
213+
"", instanceIdResult.getId(), CustomInstallationIdCache.CacheStatus.PENDING_CLEAR));
246214

247-
String iid = instanceIdResult.getId();
248-
String iidToken = instanceIdResult.getToken();
249-
Code backendRequestResult =
250-
backendServiceClient.clearCustomInstallationId(
251-
Utils.getProjectNumberFromAppId(firebaseApp.getOptions().getApplicationId()),
252-
firebaseApp.getOptions().getApiKey(),
253-
iid,
254-
iidToken);
215+
if (!firstUpdateCacheResult) {
216+
throw new SetCustomInstallationIdException(
217+
"Failed to update client side cache", Status.CLIENT_ERROR);
218+
}
255219

256-
boolean finalUpdateCacheResult;
257-
switch (backendRequestResult) {
258-
case OK:
259-
finalUpdateCacheResult = localCache.clear();
260-
break;
261-
case HTTP_CLIENT_ERROR:
262-
taskCompletionSource.setException(
263-
new SetCustomInstallationIdException(Status.CLIENT_ERROR));
264-
return;
265-
default:
266-
taskCompletionSource.setException(
267-
new SetCustomInstallationIdException(Status.BACKEND_ERROR));
268-
return;
269-
}
220+
String iid = instanceIdResult.getId();
221+
String iidToken = instanceIdResult.getToken();
222+
Code backendRequestResult =
223+
backendServiceClient.clearCustomInstallationId(
224+
Utils.getProjectNumberFromAppId(firebaseApp.getOptions().getApplicationId()),
225+
firebaseApp.getOptions().getApiKey(),
226+
iid,
227+
iidToken);
228+
229+
boolean finalUpdateCacheResult;
230+
switch (backendRequestResult) {
231+
case OK:
232+
finalUpdateCacheResult = localCache.clear();
233+
break;
234+
case HTTP_CLIENT_ERROR:
235+
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR);
236+
default:
237+
throw new SetCustomInstallationIdException(Status.BACKEND_ERROR);
238+
}
270239

271-
if (finalUpdateCacheResult) {
272-
taskCompletionSource.setResult(null);
273-
} else {
274-
taskCompletionSource.setException(
275-
new SetCustomInstallationIdException(
276-
"Failed to update client side cache", Status.CLIENT_ERROR));
277-
}
278-
});
279-
return taskCompletionSource.getTask();
240+
if (finalUpdateCacheResult) {
241+
return null;
242+
} else {
243+
throw new SetCustomInstallationIdException(
244+
"Failed to update client side cache", Status.CLIENT_ERROR);
245+
}
280246
}
281247
}

‎firebase-segmentation/src/main/java/com/google/firebase/segmentation/remote/SegmentationServiceClient.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public Code updateCustomInstallationId(
9090
} finally {
9191
gzipOutputStream.close();
9292
}
93-
httpsURLConnection.connect();
93+
9494
int httpResponseCode = httpsURLConnection.getResponseCode();
9595
switch (httpResponseCode) {
9696
case 200:
@@ -150,7 +150,6 @@ public Code clearCustomInstallationId(
150150
} finally {
151151
gzipOutputStream.close();
152152
}
153-
httpsURLConnection.connect();
154153

155154
int httpResponseCode = httpsURLConnection.getResponseCode();
156155
switch (httpResponseCode) {

0 commit comments

Comments
 (0)