Skip to content

Commit 4258052

Browse files
lfkellogglfkellogg
committed
Handle the case where a tester has access to 0 releases
1 parent 1986969 commit 4258052

File tree

4 files changed

+129
-95
lines changed

4 files changed

+129
-95
lines changed

‎firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/CheckForNewReleaseClient.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,11 @@ AppDistributionReleaseInternal getNewReleaseFromClient(
119119
firebaseAppDistributionTesterApiClient.fetchNewRelease(
120120
fid, appId, apiKey, authToken, firebaseApp.getApplicationContext());
121121

122+
if (retrievedNewRelease == null) {
123+
LogWrapper.getInstance().v(TAG + "Tester does not have access to any releases");
124+
return null;
125+
}
126+
122127
if (!canInstall(retrievedNewRelease)) {
123128
LogWrapper.getInstance().v(TAG + "New release has lower version code than current release");
124129
return null;

‎firebase-app-distribution/src/main/java/com/google/firebase/app/distribution/FirebaseAppDistributionTesterApiClient.java

Lines changed: 95 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,23 @@
1414

1515
package com.google.firebase.app.distribution;
1616

17-
import static com.google.firebase.app.distribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE;
18-
import static com.google.firebase.app.distribution.FirebaseAppDistributionException.Status.NETWORK_FAILURE;
19-
2017
import android.content.Context;
2118
import android.content.pm.PackageManager;
2219
import androidx.annotation.NonNull;
20+
import androidx.annotation.Nullable;
2321
import com.google.android.gms.common.util.AndroidUtilsLight;
2422
import com.google.android.gms.common.util.Hex;
23+
import com.google.firebase.app.distribution.Constants.ErrorMessages;
24+
import com.google.firebase.app.distribution.FirebaseAppDistributionException.Status;
2525
import java.io.BufferedInputStream;
2626
import java.io.ByteArrayOutputStream;
2727
import java.io.IOException;
2828
import java.io.InputStream;
2929
import java.net.MalformedURLException;
30+
import java.net.ProtocolException;
3031
import java.net.URL;
3132
import javax.net.ssl.HttpsURLConnection;
33+
import org.json.JSONArray;
3234
import org.json.JSONException;
3335
import org.json.JSONObject;
3436

@@ -55,26 +57,42 @@ class FirebaseAppDistributionTesterApiClient {
5557

5658
public static final int DEFAULT_BUFFER_SIZE = 8192;
5759

58-
public @NonNull AppDistributionReleaseInternal fetchNewRelease(
60+
/**
61+
* Fetches and returns the lastest release for the app that the tester has access to, or null if
62+
* the tester doesn't have access to any releases.
63+
*/
64+
@Nullable
65+
public AppDistributionReleaseInternal fetchNewRelease(
5966
@NonNull String fid,
6067
@NonNull String appId,
6168
@NonNull String apiKey,
6269
@NonNull String authToken,
6370
@NonNull Context context)
6471
throws FirebaseAppDistributionException {
72+
HttpsURLConnection connection = openHttpsUrlConnection(appId, fid, apiKey, authToken, context);
73+
String responseBody;
74+
try (BufferedInputStream inputStream = new BufferedInputStream(connection.getInputStream())) {
75+
responseBody = convertInputStreamToString(inputStream);
76+
} catch (IOException e) {
77+
throw getExceptionForHttpResponse(connection, e);
78+
} finally {
79+
connection.disconnect();
80+
}
81+
return parseNewRelease(responseBody);
82+
}
6583

66-
AppDistributionReleaseInternal newRelease;
67-
HttpsURLConnection connection = openHttpsUrlConnection(appId, fid);
84+
AppDistributionReleaseInternal parseNewRelease(String responseBody)
85+
throws FirebaseAppDistributionException {
6886
try {
69-
connection.setRequestMethod(REQUEST_METHOD);
70-
connection.setRequestProperty(API_KEY_HEADER, apiKey);
71-
connection.setRequestProperty(INSTALLATION_AUTH_HEADER, authToken);
72-
connection.addRequestProperty(X_ANDROID_PACKAGE_HEADER_KEY, context.getPackageName());
73-
connection.addRequestProperty(
74-
X_ANDROID_CERT_HEADER_KEY, getFingerprintHashForPackage(context));
75-
76-
InputStream inputStream = connection.getInputStream();
77-
JSONObject newReleaseJson = readFetchReleaseInputStream(inputStream);
87+
JSONObject responseJson = new JSONObject(responseBody);
88+
if (!responseJson.has("releases")) {
89+
return null;
90+
}
91+
JSONArray releasesJson = responseJson.getJSONArray("releases");
92+
if (releasesJson.length() == 0) {
93+
return null;
94+
}
95+
JSONObject newReleaseJson = releasesJson.getJSONObject(0);
7896
final String displayVersion = newReleaseJson.getString(DISPLAY_VERSION_JSON_KEY);
7997
final String buildVersion = newReleaseJson.getString(BUILD_VERSION_JSON_KEY);
8098
String releaseNotes = tryGetValue(newReleaseJson, RELEASE_NOTES_JSON_KEY);
@@ -88,59 +106,60 @@ class FirebaseAppDistributionTesterApiClient {
88106
? BinaryType.APK
89107
: BinaryType.AAB;
90108

91-
newRelease =
92-
AppDistributionReleaseInternal.builder()
93-
.setDisplayVersion(displayVersion)
94-
.setBuildVersion(buildVersion)
95-
.setReleaseNotes(releaseNotes)
96-
.setBinaryType(binaryType)
97-
.setIasArtifactId(iasArtifactId)
98-
.setCodeHash(codeHash)
99-
.setApkHash(apkHash)
100-
.setDownloadUrl(downloadUrl)
101-
.build();
102-
inputStream.close();
103-
104-
} catch (IOException | JSONException e) {
105-
if (e instanceof JSONException) {
106-
LogWrapper.getInstance().e(TAG + "Error parsing the new release.", e);
107-
throw new FirebaseAppDistributionException(
108-
Constants.ErrorMessages.JSON_PARSING_ERROR, NETWORK_FAILURE, e);
109-
}
110-
throw getExceptionForHttpResponse(connection);
111-
} finally {
112-
connection.disconnect();
109+
AppDistributionReleaseInternal newRelease = AppDistributionReleaseInternal.builder()
110+
.setDisplayVersion(displayVersion)
111+
.setBuildVersion(buildVersion)
112+
.setReleaseNotes(releaseNotes)
113+
.setBinaryType(binaryType)
114+
.setIasArtifactId(iasArtifactId)
115+
.setCodeHash(codeHash)
116+
.setApkHash(apkHash)
117+
.setDownloadUrl(downloadUrl)
118+
.build();
119+
120+
LogWrapper.getInstance().v("Zip hash for the new release " + newRelease.getApkHash());
121+
return newRelease;
122+
} catch (JSONException e) {
123+
LogWrapper.getInstance().e(TAG + "Error parsing the new release.", e);
124+
throw new FirebaseAppDistributionException(
125+
ErrorMessages.JSON_PARSING_ERROR, Status.UNKNOWN, e);
113126
}
114-
LogWrapper.getInstance().v("Zip hash for the new release " + newRelease.getApkHash());
115-
return newRelease;
116127
}
117128

118129
private FirebaseAppDistributionException getExceptionForHttpResponse(
119-
HttpsURLConnection connection) {
130+
HttpsURLConnection connection, Exception cause) {
131+
// TODO(lkellogg): this try-catch should be unnecessary because it will only throw an
132+
// IOException here if we couldn't connect to the server, in which case getInputStream() would
133+
// have already failed with the same exception. We also weirdly have to choose one of the two
134+
// thrown exceptions to set as the cause. We can avoid this by checking the response code
135+
// first, and then catching any unexpected exceptions when reading the input stream, essentially
136+
// combining the "default" case below with this try-catch.
137+
int responseCode;
120138
try {
121-
LogWrapper.getInstance().e(TAG + "Failed due to " + connection.getResponseCode());
122-
switch (connection.getResponseCode()) {
123-
case 401:
124-
return new FirebaseAppDistributionException(
125-
Constants.ErrorMessages.AUTHENTICATION_ERROR, AUTHENTICATION_FAILURE);
126-
case 403:
127-
case 400:
128-
return new FirebaseAppDistributionException(
129-
Constants.ErrorMessages.AUTHORIZATION_ERROR, AUTHENTICATION_FAILURE);
130-
case 404:
131-
return new FirebaseAppDistributionException(
132-
Constants.ErrorMessages.NOT_FOUND_ERROR, AUTHENTICATION_FAILURE);
133-
case 408:
134-
case 504:
135-
return new FirebaseAppDistributionException(
136-
Constants.ErrorMessages.TIMEOUT_ERROR, NETWORK_FAILURE);
137-
default:
138-
return new FirebaseAppDistributionException(
139-
Constants.ErrorMessages.NETWORK_ERROR, NETWORK_FAILURE);
140-
}
141-
} catch (IOException ex) {
139+
responseCode = connection.getResponseCode();
140+
} catch (IOException e) {
142141
return new FirebaseAppDistributionException(
143-
Constants.ErrorMessages.NETWORK_ERROR, NETWORK_FAILURE, ex);
142+
ErrorMessages.NETWORK_ERROR, Status.NETWORK_FAILURE, e);
143+
}
144+
LogWrapper.getInstance().e(TAG + "Failed due to " + responseCode);
145+
switch (responseCode) {
146+
case 401:
147+
return new FirebaseAppDistributionException(
148+
ErrorMessages.AUTHENTICATION_ERROR, Status.AUTHENTICATION_FAILURE, cause);
149+
case 403:
150+
case 400:
151+
return new FirebaseAppDistributionException(
152+
ErrorMessages.AUTHORIZATION_ERROR, Status.AUTHENTICATION_FAILURE, cause);
153+
case 404:
154+
return new FirebaseAppDistributionException(
155+
ErrorMessages.NOT_FOUND_ERROR, Status.AUTHENTICATION_FAILURE, cause);
156+
case 408:
157+
case 504:
158+
return new FirebaseAppDistributionException(
159+
ErrorMessages.TIMEOUT_ERROR, Status.NETWORK_FAILURE, cause);
160+
default:
161+
return new FirebaseAppDistributionException(
162+
ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN, cause);
144163
}
145164
}
146165

@@ -152,33 +171,27 @@ private String tryGetValue(JSONObject jsonObject, String key) {
152171
}
153172
}
154173

155-
private JSONObject readFetchReleaseInputStream(InputStream in)
156-
throws FirebaseAppDistributionException, IOException {
157-
JSONObject newRelease;
158-
InputStream jsonIn = new BufferedInputStream(in);
159-
String result = convertInputStreamToString(jsonIn);
160-
try {
161-
JSONObject json = new JSONObject(result);
162-
newRelease = json.getJSONArray("releases").getJSONObject(0);
163-
} catch (JSONException e) {
164-
throw new FirebaseAppDistributionException(
165-
Constants.ErrorMessages.JSON_PARSING_ERROR,
166-
FirebaseAppDistributionException.Status.UNKNOWN,
167-
e);
168-
}
169-
return newRelease;
170-
}
171-
172-
HttpsURLConnection openHttpsUrlConnection(String appId, String fid)
174+
HttpsURLConnection openHttpsUrlConnection(String appId, String fid, String apiKey,
175+
String authToken, Context context)
173176
throws FirebaseAppDistributionException {
174177
HttpsURLConnection httpsURLConnection;
175178
URL url = getReleasesEndpointUrl(appId, fid);
176179
try {
177180
httpsURLConnection = (HttpsURLConnection) url.openConnection();
178181
} catch (IOException e) {
179182
throw new FirebaseAppDistributionException(
180-
Constants.ErrorMessages.NETWORK_ERROR, NETWORK_FAILURE, e);
183+
ErrorMessages.NETWORK_ERROR, Status.NETWORK_FAILURE, e);
184+
}
185+
try {
186+
httpsURLConnection.setRequestMethod(REQUEST_METHOD);
187+
} catch (ProtocolException e) {
188+
throw new FirebaseAppDistributionException(ErrorMessages.UNKNOWN_ERROR, Status.UNKNOWN, e);
181189
}
190+
httpsURLConnection.setRequestProperty(API_KEY_HEADER, apiKey);
191+
httpsURLConnection.setRequestProperty(INSTALLATION_AUTH_HEADER, authToken);
192+
httpsURLConnection.addRequestProperty(X_ANDROID_PACKAGE_HEADER_KEY, context.getPackageName());
193+
httpsURLConnection.addRequestProperty(
194+
X_ANDROID_CERT_HEADER_KEY, getFingerprintHashForPackage(context));
182195
return httpsURLConnection;
183196
}
184197

@@ -188,7 +201,7 @@ private URL getReleasesEndpointUrl(String appId, String fid)
188201
return new URL(String.format(RELEASE_ENDPOINT_URL_FORMAT, appId, fid));
189202
} catch (MalformedURLException e) {
190203
throw new FirebaseAppDistributionException(
191-
Constants.ErrorMessages.UNKNOWN_ERROR,
204+
ErrorMessages.UNKNOWN_ERROR,
192205
FirebaseAppDistributionException.Status.UNKNOWN,
193206
e);
194207
}
@@ -206,8 +219,6 @@ private static String convertInputStreamToString(InputStream is) throws IOExcept
206219

207220
/**
208221
* Gets the Android package's SHA-1 fingerprint.
209-
*
210-
* @param context
211222
*/
212223
private String getFingerprintHashForPackage(Context context) {
213224
byte[] hash;

‎firebase-app-distribution/src/test/assets/testNoReleasesResponse.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎firebase-app-distribution/src/test/java/com/google/firebase/app/distribution/FirebaseAppDistributionTesterApiClientTest.java

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@
1616

1717
import static androidx.test.InstrumentationRegistry.getContext;
1818
import static org.junit.Assert.assertEquals;
19+
import static org.junit.Assert.assertNull;
1920
import static org.junit.Assert.assertThrows;
2021
import static org.mockito.Mockito.when;
2122

2223
import android.content.Context;
2324
import androidx.test.core.app.ApplicationProvider;
25+
import com.google.firebase.app.distribution.FirebaseAppDistributionException.Status;
2426
import java.io.ByteArrayInputStream;
2527
import java.io.IOException;
2628
import java.io.InputStream;
@@ -47,7 +49,8 @@ public class FirebaseAppDistributionTesterApiClientTest {
4749

4850
private FirebaseAppDistributionTesterApiClient firebaseAppDistributionTesterApiClient;
4951
private Context applicationContext;
50-
@Mock private HttpsURLConnection mockHttpsURLConnection;
52+
@Mock
53+
private HttpsURLConnection mockHttpsURLConnection;
5154

5255
@Before
5356
public void setup() throws Exception {
@@ -58,11 +61,12 @@ public void setup() throws Exception {
5861
firebaseAppDistributionTesterApiClient =
5962
Mockito.spy(new FirebaseAppDistributionTesterApiClient());
6063

64+
applicationContext = ApplicationProvider.getApplicationContext();
65+
6166
Mockito.doReturn(mockHttpsURLConnection)
6267
.when(firebaseAppDistributionTesterApiClient)
63-
.openHttpsUrlConnection(TEST_APP_ID_1, TEST_FID_1);
64-
65-
applicationContext = ApplicationProvider.getApplicationContext();
68+
.openHttpsUrlConnection(
69+
TEST_APP_ID_1, TEST_FID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext);
6670
}
6771

6872
@Test
@@ -112,7 +116,7 @@ public void fetchNewRelease_whenResponseFailsWith401_throwsError() throws Except
112116
firebaseAppDistributionTesterApiClient.fetchNewRelease(
113117
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
114118

115-
assertEquals(FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
119+
assertEquals(Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
116120
assertEquals("Failed to authenticate the tester", ex.getMessage());
117121
}
118122

@@ -128,7 +132,7 @@ public void fetchNewRelease_whenResponseFailsWith403_throwsError() throws Except
128132
firebaseAppDistributionTesterApiClient.fetchNewRelease(
129133
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
130134

131-
assertEquals(FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
135+
assertEquals(Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
132136
assertEquals("Failed to authorize the tester", ex.getMessage());
133137
}
134138

@@ -144,7 +148,7 @@ public void fetchNewRelease_whenResponseFailsWith404_throwsError() throws Except
144148
firebaseAppDistributionTesterApiClient.fetchNewRelease(
145149
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
146150

147-
assertEquals(FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
151+
assertEquals(Status.AUTHENTICATION_FAILURE, ex.getErrorCode());
148152
assertEquals("Tester or release not found", ex.getMessage());
149153
}
150154

@@ -160,7 +164,7 @@ public void fetchNewRelease_whenResponseFailsWith504_throwsError() throws Except
160164
firebaseAppDistributionTesterApiClient.fetchNewRelease(
161165
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
162166

163-
assertEquals(FirebaseAppDistributionException.Status.NETWORK_FAILURE, ex.getErrorCode());
167+
assertEquals(Status.NETWORK_FAILURE, ex.getErrorCode());
164168
assertEquals("Failed to fetch releases due to timeout", ex.getMessage());
165169
}
166170

@@ -176,8 +180,9 @@ public void fetchNewRelease_whenResponseFailsWithUnknownCode_throwsError() throw
176180
firebaseAppDistributionTesterApiClient.fetchNewRelease(
177181
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
178182

179-
assertEquals(FirebaseAppDistributionException.Status.NETWORK_FAILURE, ex.getErrorCode());
180-
assertEquals("Failed to fetch releases due to unknown network error", ex.getMessage());
183+
assertEquals(Status.UNKNOWN, ex.getErrorCode());
184+
assertEquals("Unknown Error", ex.getMessage());
185+
assertEquals(IOException.class, ex.getCause().getClass());
181186
}
182187

183188
@Test
@@ -192,11 +197,23 @@ public void fetchNewRelease_whenInvalidJson_throwsError() throws Exception {
192197
firebaseAppDistributionTesterApiClient.fetchNewRelease(
193198
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext));
194199

195-
assertEquals(FirebaseAppDistributionException.Status.UNKNOWN, ex.getErrorCode());
200+
assertEquals(Status.UNKNOWN, ex.getErrorCode());
196201
assertEquals("Error parsing service response", ex.getMessage());
197202
assert (ex.getCause() instanceof JSONException);
198203
}
199204

205+
@Test
206+
public void fetchNewRelease_whenNoReleases_returnsNull() throws Exception {
207+
JSONObject releaseJson = getTestJSON("testNoReleasesResponse.json");
208+
InputStream response =
209+
new ByteArrayInputStream(releaseJson.toString().getBytes(StandardCharsets.UTF_8));
210+
when(mockHttpsURLConnection.getInputStream()).thenReturn(response);
211+
AppDistributionReleaseInternal release =
212+
firebaseAppDistributionTesterApiClient.fetchNewRelease(
213+
TEST_FID_1, TEST_APP_ID_1, TEST_API_KEY, TEST_AUTH_TOKEN, applicationContext);
214+
assertNull(release);
215+
}
216+
200217
private JSONObject getTestJSON(String fileName) throws IOException, JSONException {
201218
final InputStream jsonInputStream = getContext().getResources().getAssets().open(fileName);
202219
final String testJsonString = streamToString(jsonInputStream);

0 commit comments

Comments
 (0)