Skip to content

Commit 52d259d

Browse files
diwu-aretediwu-arete
authored
FirebaseSegmentation SDK changes (#673)
* Implement Firebase segmentation SDK device local cache * [Firebase Segmentation] Add custom installation id cache layer and tests for it. * Add test for updating cache * Switch to use SQLiteOpenHelper * Switch to use SharedPreferences from SQLite. * Change the cache class to be singleton * Wrap shared pref commit in a async task. * Address comments * Google format fix * Replace some deprecated code. * Package refactor * nit * nit * Add the state machine of updating custom installation id in the local cache and update to Firebase Segmentation backend. CL also contains unit tests. (The http client is not implemented yet.) * minor format fix * Address comments #1 * Http client in Firebase Segmentation SDK to call backend service. * Revert unintentional change * Fix connected device test * Fix connected device test * 1. Add a few annotations to make java code Kotlin friendly 2. Some fixes for the http request format * Fix java format * Fix API version * Change the segmentation API implementation to synchronous and put the entire synchronous code block in async task. * Fix a async getResult race issue. * OkHttpClient -> HttpsUrlConnection * Use gzip for compressing content and fix ourput stream memory leak risk. * Addressed a few comments * FirebaseSegmentation SDK 1. Clean up http client response code. 2. When updateCustomInstallationId is called, on non-retryable server errors, the SDK should clean up the local cache. Instead, for retryable errors, SDK can keep the local cache for retrying update later.
1 parent 7f30aea commit 52d259d

File tree

4 files changed

+66
-17
lines changed

4 files changed

+66
-17
lines changed

‎firebase-segmentation/src/androidTest/java/com/google/firebase/segmentation/FirebaseSegmentationInstrumentedTest.java

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ public void testUpdateCustomInstallationId_CacheOk_BackendOk() throws Exception
127127
}
128128

129129
@Test
130-
public void testUpdateCustomInstallationId_CacheOk_BackendError() throws InterruptedException {
130+
public void testUpdateCustomInstallationId_CacheOk_BackendError_Retryable()
131+
throws InterruptedException {
131132
FirebaseSegmentation firebaseSegmentation =
132133
new FirebaseSegmentation(
133134
firebaseApp, firebaseInstanceId, actualCache, backendClientReturnsError);
@@ -150,6 +151,31 @@ public void testUpdateCustomInstallationId_CacheOk_BackendError() throws Interru
150151
.isEqualTo(CustomInstallationIdCache.CacheStatus.PENDING_UPDATE);
151152
}
152153

154+
@Test
155+
public void testUpdateCustomInstallationId_CacheOk_BackendError_NotRetryable()
156+
throws InterruptedException {
157+
when(backendClientReturnsError.updateCustomInstallationId(
158+
anyLong(), anyString(), anyString(), anyString(), anyString()))
159+
.thenReturn(SegmentationServiceClient.Code.CONFLICT);
160+
FirebaseSegmentation firebaseSegmentation =
161+
new FirebaseSegmentation(
162+
firebaseApp, firebaseInstanceId, actualCache, backendClientReturnsError);
163+
164+
// Expect exception
165+
try {
166+
Tasks.await(firebaseSegmentation.setCustomInstallationId(CUSTOM_INSTALLATION_ID));
167+
fail();
168+
} catch (ExecutionException expected) {
169+
Throwable cause = expected.getCause();
170+
assertThat(cause).isInstanceOf(SetCustomInstallationIdException.class);
171+
assertThat(((SetCustomInstallationIdException) cause).getStatus())
172+
.isEqualTo(SetCustomInstallationIdException.Status.DUPLICATED_CUSTOM_INSTALLATION_ID);
173+
}
174+
175+
CustomInstallationIdCacheEntryValue entryValue = actualCache.readCacheEntryValue();
176+
assertThat(entryValue).isNull();
177+
}
178+
153179
@Test
154180
public void testUpdateCustomInstallationId_CacheError_BackendOk() throws InterruptedException {
155181
FirebaseSegmentation firebaseSegmentation =

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

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ private Void updateCustomInstallationId(String customInstallationId)
128128
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
129129
} catch (ExecutionException | InterruptedException e) {
130130
throw new SetCustomInstallationIdException(
131-
"Failed to get Firebase instance id", Status.CLIENT_ERROR);
131+
Status.CLIENT_ERROR, "Failed to get Firebase instance id");
132132
}
133133

134134
boolean firstUpdateCacheResult =
@@ -140,7 +140,7 @@ private Void updateCustomInstallationId(String customInstallationId)
140140

141141
if (!firstUpdateCacheResult) {
142142
throw new SetCustomInstallationIdException(
143-
"Failed to update client side cache", Status.CLIENT_ERROR);
143+
Status.CLIENT_ERROR, "Failed to update client side cache");
144144
}
145145

146146
// Start requesting backend when first cache updae is done.
@@ -164,19 +164,30 @@ private Void updateCustomInstallationId(String customInstallationId)
164164
instanceIdResult.getId(),
165165
CustomInstallationIdCache.CacheStatus.SYNCED));
166166
break;
167-
case HTTP_CLIENT_ERROR:
168-
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR);
167+
case UNAUTHORIZED:
168+
localCache.clear();
169+
throw new SetCustomInstallationIdException(
170+
Status.CLIENT_ERROR, "Instance id token is invalid.");
169171
case CONFLICT:
170-
throw new SetCustomInstallationIdException(Status.DUPLICATED_CUSTOM_INSTALLATION_ID);
172+
localCache.clear();
173+
throw new SetCustomInstallationIdException(
174+
Status.DUPLICATED_CUSTOM_INSTALLATION_ID,
175+
"The custom installation id is used by another Firebase installation in your project.");
176+
case HTTP_CLIENT_ERROR:
177+
localCache.clear();
178+
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR, "Http client error(4xx)");
179+
case NETWORK_ERROR:
180+
case SERVER_ERROR:
171181
default:
182+
// These are considered retryable errors, so not to clean up the cache.
172183
throw new SetCustomInstallationIdException(Status.BACKEND_ERROR);
173184
}
174185

175186
if (finalUpdateCacheResult) {
176187
return null;
177188
} else {
178189
throw new SetCustomInstallationIdException(
179-
"Failed to update client side cache", Status.CLIENT_ERROR);
190+
Status.CLIENT_ERROR, "Failed to update client side cache");
180191
}
181192
}
182193

@@ -204,7 +215,7 @@ private Void clearCustomInstallationId() throws SetCustomInstallationIdException
204215
instanceIdResult = Tasks.await(firebaseInstanceId.getInstanceId());
205216
} catch (ExecutionException | InterruptedException e) {
206217
throw new SetCustomInstallationIdException(
207-
"Failed to get Firebase instance id", Status.CLIENT_ERROR);
218+
Status.CLIENT_ERROR, "Failed to get Firebase instance id");
208219
}
209220

210221
boolean firstUpdateCacheResult =
@@ -214,7 +225,7 @@ private Void clearCustomInstallationId() throws SetCustomInstallationIdException
214225

215226
if (!firstUpdateCacheResult) {
216227
throw new SetCustomInstallationIdException(
217-
"Failed to update client side cache", Status.CLIENT_ERROR);
228+
Status.CLIENT_ERROR, "Failed to update client side cache");
218229
}
219230

220231
String iid = instanceIdResult.getId();
@@ -231,17 +242,23 @@ private Void clearCustomInstallationId() throws SetCustomInstallationIdException
231242
case OK:
232243
finalUpdateCacheResult = localCache.clear();
233244
break;
245+
case UNAUTHORIZED:
246+
throw new SetCustomInstallationIdException(
247+
Status.CLIENT_ERROR, "Instance id token is invalid.");
234248
case HTTP_CLIENT_ERROR:
235-
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR);
249+
throw new SetCustomInstallationIdException(Status.CLIENT_ERROR, "Http client error(4xx)");
250+
case NETWORK_ERROR:
251+
case SERVER_ERROR:
236252
default:
253+
// These are considered retryable errors, so not to clean up the cache.
237254
throw new SetCustomInstallationIdException(Status.BACKEND_ERROR);
238255
}
239256

240257
if (finalUpdateCacheResult) {
241258
return null;
242259
} else {
243260
throw new SetCustomInstallationIdException(
244-
"Failed to update client side cache", Status.CLIENT_ERROR);
261+
Status.CLIENT_ERROR, "Failed to update client side cache");
245262
}
246263
}
247264
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ public enum Status {
4545
this.status = status;
4646
}
4747

48-
SetCustomInstallationIdException(@NonNull String message, @NonNull Status status) {
48+
SetCustomInstallationIdException(@NonNull Status status, @NonNull String message) {
4949
super(message);
5050
this.status = status;
5151
}
5252

5353
SetCustomInstallationIdException(
54-
@NonNull String message, @NonNull Status status, Throwable cause) {
54+
@NonNull Status status, @NonNull String message, Throwable cause) {
5555
super(message, cause);
5656
this.status = status;
5757
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ public class SegmentationServiceClient {
4141
public enum Code {
4242
OK,
4343

44-
HTTP_CLIENT_ERROR,
45-
4644
CONFLICT,
4745

46+
UNAUTHORIZED,
47+
4848
NETWORK_ERROR,
4949

50-
SERVER_ERROR,
50+
HTTP_CLIENT_ERROR,
5151

52-
UNAUTHORIZED,
52+
SERVER_ERROR,
5353
}
5454

5555
@NonNull
@@ -100,6 +100,9 @@ public Code updateCustomInstallationId(
100100
case 409:
101101
return Code.CONFLICT;
102102
default:
103+
if (httpResponseCode / 100 == 4) {
104+
return Code.HTTP_CLIENT_ERROR;
105+
}
103106
return Code.SERVER_ERROR;
104107
}
105108
} catch (IOException e) {
@@ -158,6 +161,9 @@ public Code clearCustomInstallationId(
158161
case 401:
159162
return Code.UNAUTHORIZED;
160163
default:
164+
if (httpResponseCode / 100 == 4) {
165+
return Code.HTTP_CLIENT_ERROR;
166+
}
161167
return Code.SERVER_ERROR;
162168
}
163169
} catch (IOException e) {

0 commit comments

Comments
 (0)