From c13d8893534ef723158a9bb9a8ea50257f4c2e7d Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 31 Aug 2020 18:36:01 -0400 Subject: [PATCH 1/4] Add initial classes for Remote Config API Add unit tests --- .../FirebaseRemoteConfigClient.java | 31 ++ .../FirebaseRemoteConfigClientImpl.java | 224 +++++++++++ .../FirebaseRemoteConfigException.java | 57 +++ .../remoteconfig/RemoteConfigErrorCode.java | 33 ++ .../remoteconfig/RemoteConfigTemplate.java | 33 ++ .../RemoteConfigServiceErrorResponse.java | 80 ++++ .../FirebaseRemoteConfigClientImplTest.java | 377 ++++++++++++++++++ 7 files changed, 835 insertions(+) create mode 100644 src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClient.java create mode 100644 src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java create mode 100644 src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigException.java create mode 100644 src/main/java/com/google/firebase/remoteconfig/RemoteConfigErrorCode.java create mode 100644 src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java create mode 100644 src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java create mode 100644 src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java diff --git a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClient.java b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClient.java new file mode 100644 index 000000000..9c844503d --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClient.java @@ -0,0 +1,31 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +/** + * An interface for managing Firebase Remote Config templates. + */ +interface FirebaseRemoteConfigClient { + + /** + * Gets the current active version of the Remote Config template. + * + * @return A {@link RemoteConfigTemplate}. + * @throws FirebaseRemoteConfigException If an error occurs while getting the template. + */ + RemoteConfigTemplate getTemplate() throws FirebaseRemoteConfigException; +} diff --git a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java new file mode 100644 index 000000000..53f02de96 --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java @@ -0,0 +1,224 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.api.client.http.HttpRequestFactory; +import com.google.api.client.http.HttpResponseInterceptor; +import com.google.api.client.json.JsonFactory; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import com.google.firebase.ErrorCode; +import com.google.firebase.FirebaseApp; +import com.google.firebase.FirebaseException; +import com.google.firebase.ImplFirebaseTrampolines; +import com.google.firebase.IncomingHttpResponse; +import com.google.firebase.internal.AbstractPlatformErrorHandler; +import com.google.firebase.internal.ApiClientUtils; +import com.google.firebase.internal.ErrorHandlingHttpClient; +import com.google.firebase.internal.HttpRequestInfo; +import com.google.firebase.internal.SdkUtils; +import com.google.firebase.remoteconfig.internal.RemoteConfigServiceErrorResponse; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/** + * A helper class for interacting with Firebase Remote Config service. + */ +final class FirebaseRemoteConfigClientImpl implements FirebaseRemoteConfigClient { + + private static final String RC_URL = "https://firebaseremoteconfig.googleapis.com/v1/projects/%s/remoteConfig"; + + private static final Map COMMON_HEADERS = + ImmutableMap.of( + "X-GOOG-API-FORMAT-VERSION", "2", + "X-Firebase-Client", "fire-admin-java/" + SdkUtils.getVersion(), + "Accept-Encoding", "gzip" + ); + + private final String rcSendUrl; + private final HttpRequestFactory requestFactory; + private final HttpRequestFactory childRequestFactory; + private final JsonFactory jsonFactory; + private final ErrorHandlingHttpClient httpClient; + + private FirebaseRemoteConfigClientImpl(Builder builder) { + checkArgument(!Strings.isNullOrEmpty(builder.projectId)); + this.rcSendUrl = String.format(RC_URL, builder.projectId); + this.requestFactory = checkNotNull(builder.requestFactory); + this.childRequestFactory = checkNotNull(builder.childRequestFactory); + this.jsonFactory = checkNotNull(builder.jsonFactory); + HttpResponseInterceptor responseInterceptor = builder.responseInterceptor; + RemoteConfigErrorHandler errorHandler = new RemoteConfigErrorHandler(this.jsonFactory); + this.httpClient = new ErrorHandlingHttpClient<>(requestFactory, jsonFactory, errorHandler) + .setInterceptor(responseInterceptor); + } + + @VisibleForTesting + String getRcSendUrl() { + return rcSendUrl; + } + + @VisibleForTesting + HttpRequestFactory getRequestFactory() { + return requestFactory; + } + + @VisibleForTesting + HttpRequestFactory getChildRequestFactory() { + return childRequestFactory; + } + + @VisibleForTesting + JsonFactory getJsonFactory() { + return jsonFactory; + } + + @Override + public RemoteConfigTemplate getTemplate() throws FirebaseRemoteConfigException { + HttpRequestInfo request = HttpRequestInfo.buildGetRequest(rcSendUrl) + .addAllHeaders(COMMON_HEADERS); + IncomingHttpResponse response = httpClient.send(request); + RemoteConfigTemplate parsed = httpClient.parse(response, RemoteConfigTemplate.class); + + List etagList = (List) response.getHeaders().get("etag"); + + if (etagList == null || etagList.isEmpty()) { + throw new FirebaseRemoteConfigException( + ErrorCode.INTERNAL, + "ETag header is not available in the server response.", null, null, + RemoteConfigErrorCode.INTERNAL); + } + + String etag = etagList.get(0); + + if (Strings.isNullOrEmpty(etag)) { + throw new FirebaseRemoteConfigException( + ErrorCode.INTERNAL, + "ETag header is not available in the server response.", null, null, + RemoteConfigErrorCode.INTERNAL); + } + + parsed.setETag(etag); + return parsed; + } + + static FirebaseRemoteConfigClientImpl fromApp(FirebaseApp app) { + String projectId = ImplFirebaseTrampolines.getProjectId(app); + checkArgument(!Strings.isNullOrEmpty(projectId), + "Project ID is required to access Remote Config service. Use a service " + + "account credential or set the project ID explicitly via FirebaseOptions. " + + "Alternatively you can also set the project ID via the GOOGLE_CLOUD_PROJECT " + + "environment variable."); + return FirebaseRemoteConfigClientImpl.builder() + .setProjectId(projectId) + .setRequestFactory(ApiClientUtils.newAuthorizedRequestFactory(app)) + .setChildRequestFactory(ApiClientUtils.newUnauthorizedRequestFactory(app)) + .setJsonFactory(app.getOptions().getJsonFactory()) + .build(); + } + + static Builder builder() { + return new Builder(); + } + + static final class Builder { + + private String projectId; + private HttpRequestFactory requestFactory; + private HttpRequestFactory childRequestFactory; + private JsonFactory jsonFactory; + private HttpResponseInterceptor responseInterceptor; + + private Builder() { } + + Builder setProjectId(String projectId) { + this.projectId = projectId; + return this; + } + + Builder setRequestFactory(HttpRequestFactory requestFactory) { + this.requestFactory = requestFactory; + return this; + } + + Builder setChildRequestFactory( + HttpRequestFactory childRequestFactory) { + this.childRequestFactory = childRequestFactory; + return this; + } + + Builder setJsonFactory(JsonFactory jsonFactory) { + this.jsonFactory = jsonFactory; + return this; + } + + Builder setResponseInterceptor( + HttpResponseInterceptor responseInterceptor) { + this.responseInterceptor = responseInterceptor; + return this; + } + + FirebaseRemoteConfigClientImpl build() { + return new FirebaseRemoteConfigClientImpl(this); + } + } + + private static class RemoteConfigErrorHandler + extends AbstractPlatformErrorHandler { + + private RemoteConfigErrorHandler(JsonFactory jsonFactory) { + super(jsonFactory); + } + + @Override + protected FirebaseRemoteConfigException createException(FirebaseException base) { + String response = getResponse(base); + RemoteConfigServiceErrorResponse parsed = safeParse(response); + return FirebaseRemoteConfigException.withRemoteConfigErrorCode( + base, parsed.getRemoteConfigErrorCode()); + } + + private String getResponse(FirebaseException base) { + if (base.getHttpResponse() == null) { + return null; + } + + return base.getHttpResponse().getContent(); + } + + private RemoteConfigServiceErrorResponse safeParse(String response) { + if (!Strings.isNullOrEmpty(response)) { + try { + return jsonFactory.createJsonParser(response) + .parseAndClose(RemoteConfigServiceErrorResponse.class); + } catch (IOException ignore) { + // Ignore any error that may occur while parsing the error response. The server + // may have responded with a non-json payload. + } + } + + return new RemoteConfigServiceErrorResponse(); + } + } +} diff --git a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigException.java b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigException.java new file mode 100644 index 000000000..66b066f38 --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigException.java @@ -0,0 +1,57 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +import com.google.firebase.ErrorCode; +import com.google.firebase.FirebaseException; +import com.google.firebase.IncomingHttpResponse; +import com.google.firebase.internal.NonNull; +import com.google.firebase.internal.Nullable; + +/** + * Generic exception related to Firebase Remote Config. Check the error code and message for more + * details. + */ +public final class FirebaseRemoteConfigException extends FirebaseException { + + private final RemoteConfigErrorCode errorCode; + + public FirebaseRemoteConfigException( + @NonNull ErrorCode errorCode, + @NonNull String message, + @Nullable Throwable cause, + @Nullable IncomingHttpResponse response, + @Nullable RemoteConfigErrorCode remoteConfigErrorCode) { + super(errorCode, message, cause, response); + this.errorCode = remoteConfigErrorCode; + } + + static FirebaseRemoteConfigException withRemoteConfigErrorCode( + FirebaseException base, @Nullable RemoteConfigErrorCode errorCode) { + return new FirebaseRemoteConfigException( + base.getErrorCode(), + base.getMessage(), + base.getCause(), + base.getHttpResponse(), + errorCode); + } + + @Nullable + public RemoteConfigErrorCode getRemoteConfigErrorCode() { + return errorCode; + } +} diff --git a/src/main/java/com/google/firebase/remoteconfig/RemoteConfigErrorCode.java b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigErrorCode.java new file mode 100644 index 000000000..6aa94027c --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigErrorCode.java @@ -0,0 +1,33 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +/** + * Error codes that can be raised by the Remote Config APIs. + */ +public enum RemoteConfigErrorCode { + + /** + * One or more arguments specified in the request were invalid. + */ + INVALID_ARGUMENT, + + /** + * Internal server error. + */ + INTERNAL, +} diff --git a/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java new file mode 100644 index 000000000..551e9802c --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java @@ -0,0 +1,33 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +import com.google.api.client.util.Key; + +public class RemoteConfigTemplate { + + @Key("etag") + private String etag; + + public String getETag() { + return this.etag; + } + + void setETag(String etag) { + this.etag = etag; + } +} diff --git a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java new file mode 100644 index 000000000..b140b8e12 --- /dev/null +++ b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java @@ -0,0 +1,80 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig.internal; + +import com.google.api.client.json.GenericJson; +import com.google.api.client.util.Key; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableMap; +import com.google.firebase.internal.Nullable; +import com.google.firebase.remoteconfig.RemoteConfigErrorCode; + +import java.util.Map; + +/** + * The DTO for parsing error responses from the Remote Config service. + * These error messages take the form, + * `"error": {"code": 123, "message": "[CODE]: Message Details", "status": "ERROR_STATUS"}` + */ +public class RemoteConfigServiceErrorResponse extends GenericJson { + + private static final Map RC_ERROR_CODES = + ImmutableMap.builder() + .put("INTERNAL", RemoteConfigErrorCode.INTERNAL) + .put("INVALID_ARGUMENT", RemoteConfigErrorCode.INVALID_ARGUMENT) + .build(); + + @Key("error") + private Map error; + + public String getStatus() { + if (error == null) { + return null; + } + + return (String) error.get("status"); + } + + @Nullable + public RemoteConfigErrorCode getRemoteConfigErrorCode() { + if (error == null) { + return null; + } + + String message = getErrorMessage(); + if (Strings.isNullOrEmpty(message)) { + return null; + } + + int separator = message.indexOf(':'); + if (separator != -1) { + String errorCode = message.substring(0, separator).replaceAll("\\[|\\]", ""); + return RC_ERROR_CODES.get(errorCode); + } + + return null; + } + + @Nullable + public String getErrorMessage() { + if (error == null) { + return null; + } + + return (String) error.get("message"); + } +} diff --git a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java new file mode 100644 index 000000000..e8ec40e6d --- /dev/null +++ b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java @@ -0,0 +1,377 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.firebase.remoteconfig; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +import com.google.api.client.googleapis.util.Utils; +import com.google.api.client.http.GenericUrl; +import com.google.api.client.http.HttpHeaders; +import com.google.api.client.http.HttpMethods; +import com.google.api.client.http.HttpRequest; +import com.google.api.client.http.HttpResponseException; +import com.google.api.client.http.HttpResponseInterceptor; +import com.google.api.client.http.HttpTransport; +import com.google.api.client.testing.http.MockHttpTransport; +import com.google.api.client.testing.http.MockLowLevelHttpResponse; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.firebase.ErrorCode; +import com.google.firebase.FirebaseApp; +import com.google.firebase.FirebaseOptions; +import com.google.firebase.OutgoingHttpRequest; +import com.google.firebase.auth.MockGoogleCredentials; +import com.google.firebase.internal.SdkUtils; +import com.google.firebase.testing.TestResponseInterceptor; +import com.google.firebase.testing.TestUtils; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; + +public class FirebaseRemoteConfigClientImplTest { + + private static final String TEST_RC_URL = + "https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig"; + + private static final List HTTP_ERRORS = ImmutableList.of(401, 404, 500); + + private static final Map HTTP_2_ERROR = ImmutableMap.of( + 401, ErrorCode.UNAUTHENTICATED, + 404, ErrorCode.NOT_FOUND, + 500, ErrorCode.INTERNAL); + + private static final String MOCK_TEMPLATE_RESPONSE = "{\"conditions\": [], \"parameters\": {}}"; + + private static final String TEST_ETAG = "etag-123456789012-1"; + + private MockLowLevelHttpResponse response; + private TestResponseInterceptor interceptor; + private FirebaseRemoteConfigClient client; + + @Before + public void setUp() { + response = new MockLowLevelHttpResponse(); + interceptor = new TestResponseInterceptor(); + client = initRemoteConfigClient(response, interceptor); + } + + @Test + public void testGetTemplate() throws Exception { + response.addHeader("etag", TEST_ETAG); + response.setContent(MOCK_TEMPLATE_RESPONSE); + RemoteConfigTemplate template = client.getTemplate(); + + assertEquals(TEST_ETAG, template.getETag()); + checkGetRequestHeader(interceptor.getLastRequest()); + } + + @Test + public void testGetTemplateWithInvalidEtags() { + // Empty ETag + response.addHeader("etag", ""); + response.setContent(MOCK_TEMPLATE_RESPONSE); + try { + client.getTemplate(); + fail("No error thrown for invalid ETag"); + } catch (FirebaseRemoteConfigException error) { + assertEquals(ErrorCode.INTERNAL, error.getErrorCode()); + assertEquals("ETag header is not available in the server response.", error.getMessage()); + assertEquals(RemoteConfigErrorCode.INTERNAL, error.getRemoteConfigErrorCode()); + } + checkGetRequestHeader(interceptor.getLastRequest()); + + // ETag does not exist + response.setContent(MOCK_TEMPLATE_RESPONSE); + try { + client.getTemplate(); + fail("No error thrown for invalid ETag"); + } catch (FirebaseRemoteConfigException error) { + assertEquals(ErrorCode.INTERNAL, error.getErrorCode()); + assertEquals("ETag header is not available in the server response.", error.getMessage()); + assertEquals(RemoteConfigErrorCode.INTERNAL, error.getRemoteConfigErrorCode()); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + + @Test + public void testGetTemplateHttpError() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent("{}"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + "Unexpected HTTP response with status: " + code + "\n{}"); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateTransportError() { + client = initClientWithFaultyTransport(); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + assertEquals(ErrorCode.UNKNOWN, error.getErrorCode()); + assertEquals("Unknown error while making a remote service call: transport error", + error.getMessage()); + assertTrue(error.getCause() instanceof IOException); + assertNull(error.getHttpResponse()); + assertNull(error.getRemoteConfigErrorCode()); + } + } + + @Test + public void testGetTemplateSuccessResponseWithUnexpectedPayload() { + response.setContent("not valid json"); + + try { + client.getTemplate(); + fail("No error thrown for malformed response"); + } catch (FirebaseRemoteConfigException error) { + assertEquals(ErrorCode.UNKNOWN, error.getErrorCode()); + assertTrue(error.getMessage().startsWith("Error while parsing HTTP response: ")); + assertNotNull(error.getCause()); + assertNotNull(error.getHttpResponse()); + assertNull(error.getRemoteConfigErrorCode()); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + + @Test + public void testGetTemplateErrorWithZeroContentResponse() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setZeroContent(); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + "Unexpected HTTP response with status: " + code + "\nnull"); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateErrorWithMalformedResponse() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent("not json"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + "Unexpected HTTP response with status: " + code + "\nnot json"); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateErrorWithDetails() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent( + "{\"error\": {\"status\": \"INVALID_ARGUMENT\", \"message\": \"test error\"}}"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateErrorWithCanonicalCode() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent( + "{\"error\": {\"status\": \"NOT_FOUND\", \"message\": \"test error\"}}"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, ErrorCode.NOT_FOUND); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateErrorWithRcError() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent( + "{\"error\": {\"status\": \"INVALID_ARGUMENT\", " + + "\"message\": \"[INVALID_ARGUMENT]: test error\"}}"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT, + RemoteConfigErrorCode.INVALID_ARGUMENT, "[INVALID_ARGUMENT]: test error"); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test + public void testGetTemplateErrorWithDetailsAndNoCode() { + for (int code : HTTP_ERRORS) { + response.setStatusCode(code).setContent( + "{\"error\": {\"status\": \"INVALID_ARGUMENT\", " + + "\"message\": \"test error\"}}"); + + try { + client.getTemplate(); + fail("No error thrown for HTTP error"); + } catch (FirebaseRemoteConfigException error) { + checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT); + } + checkGetRequestHeader(interceptor.getLastRequest()); + } + } + + @Test(expected = IllegalArgumentException.class) + public void testBuilderNullProjectId() { + fullyPopulatedBuilder().setProjectId(null).build(); + } + + @Test(expected = IllegalArgumentException.class) + public void testBuilderEmptyProjectId() { + fullyPopulatedBuilder().setProjectId("").build(); + } + + @Test(expected = NullPointerException.class) + public void testBuilderNullRequestFactory() { + fullyPopulatedBuilder().setRequestFactory(null).build(); + } + + @Test(expected = NullPointerException.class) + public void testBuilderNullChildRequestFactory() { + fullyPopulatedBuilder().setChildRequestFactory(null).build(); + } + + @Test + public void testFromApp() throws IOException { + FirebaseOptions options = FirebaseOptions.builder() + .setCredentials(new MockGoogleCredentials("test-token")) + .setProjectId("test-project") + .build(); + FirebaseApp app = FirebaseApp.initializeApp(options); + + try { + FirebaseRemoteConfigClientImpl client = FirebaseRemoteConfigClientImpl.fromApp(app); + + assertEquals(TEST_RC_URL, client.getRcSendUrl()); + assertSame(options.getJsonFactory(), client.getJsonFactory()); + + HttpRequest request = client.getRequestFactory().buildGetRequest( + new GenericUrl("https://example.com")); + assertEquals("Bearer test-token", request.getHeaders().getAuthorization()); + + request = client.getChildRequestFactory().buildGetRequest( + new GenericUrl("https://example.com")); + assertNull(request.getHeaders().getAuthorization()); + } finally { + app.delete(); + } + } + + private FirebaseRemoteConfigClientImpl initRemoteConfigClient( + MockLowLevelHttpResponse mockResponse, HttpResponseInterceptor interceptor) { + MockHttpTransport transport = new MockHttpTransport.Builder() + .setLowLevelHttpResponse(mockResponse) + .build(); + + return FirebaseRemoteConfigClientImpl.builder() + .setProjectId("test-project") + .setJsonFactory(Utils.getDefaultJsonFactory()) + .setRequestFactory(transport.createRequestFactory()) + .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()) + .setResponseInterceptor(interceptor) + .build(); + } + + private FirebaseRemoteConfigClientImpl initClientWithFaultyTransport() { + HttpTransport transport = TestUtils.createFaultyHttpTransport(); + return FirebaseRemoteConfigClientImpl.builder() + .setProjectId("test-project") + .setJsonFactory(Utils.getDefaultJsonFactory()) + .setRequestFactory(transport.createRequestFactory()) + .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()) + .build(); + } + + private FirebaseRemoteConfigClientImpl.Builder fullyPopulatedBuilder() { + return FirebaseRemoteConfigClientImpl.builder() + .setProjectId("test-project") + .setJsonFactory(Utils.getDefaultJsonFactory()) + .setRequestFactory(Utils.getDefaultTransport().createRequestFactory()) + .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()); + } + + private void checkGetRequestHeader(HttpRequest request) { + assertEquals("GET", request.getRequestMethod()); + assertEquals(TEST_RC_URL, request.getUrl().toString()); + HttpHeaders headers = request.getHeaders(); + assertEquals("2", headers.get("X-GOOG-API-FORMAT-VERSION")); + assertEquals("fire-admin-java/" + SdkUtils.getVersion(), headers.get("X-Firebase-Client")); + assertEquals("gzip", headers.getFirstHeaderStringValue("Accept-Encoding")); + } + + private void checkExceptionFromHttpResponse( + FirebaseRemoteConfigException error, + ErrorCode expectedCode) { + checkExceptionFromHttpResponse(error, expectedCode, null, "test error"); + } + + private void checkExceptionFromHttpResponse( + FirebaseRemoteConfigException error, + ErrorCode expectedCode, + RemoteConfigErrorCode expectedRemoteConfigCode, + String expectedMessage) { + assertEquals(expectedCode, error.getErrorCode()); + assertEquals(expectedMessage, error.getMessage()); + assertTrue(error.getCause() instanceof HttpResponseException); + assertEquals(expectedRemoteConfigCode, error.getRemoteConfigErrorCode()); + + assertNotNull(error.getHttpResponse()); + OutgoingHttpRequest request = error.getHttpResponse().getRequest(); + assertEquals(HttpMethods.GET, request.getMethod()); + assertTrue(request.getUrl().startsWith("https://firebaseremoteconfig.googleapis.com")); + } +} From 71151f80cadab869f73de197a8a4ee35278f1c1e Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Fri, 11 Sep 2020 16:09:11 -0400 Subject: [PATCH 2/4] PR fixes --- .../FirebaseRemoteConfigClientImpl.java | 51 +++----- .../remoteconfig/RemoteConfigTemplate.java | 2 +- .../RemoteConfigServiceErrorResponse.java | 13 +- .../FirebaseRemoteConfigClientImplTest.java | 114 ++++-------------- 4 files changed, 42 insertions(+), 138 deletions(-) diff --git a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java index 53f02de96..4817867d7 100644 --- a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java +++ b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java @@ -25,7 +25,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.ImmutableMap; -import com.google.firebase.ErrorCode; import com.google.firebase.FirebaseApp; import com.google.firebase.FirebaseException; import com.google.firebase.ImplFirebaseTrampolines; @@ -38,7 +37,6 @@ import com.google.firebase.remoteconfig.internal.RemoteConfigServiceErrorResponse; import java.io.IOException; -import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -47,26 +45,28 @@ */ final class FirebaseRemoteConfigClientImpl implements FirebaseRemoteConfigClient { - private static final String RC_URL = "https://firebaseremoteconfig.googleapis.com/v1/projects/%s/remoteConfig"; + private static final String REMOTE_CONFIG_URL = "https://firebaseremoteconfig.googleapis.com/v1/projects/%s/remoteConfig"; private static final Map COMMON_HEADERS = ImmutableMap.of( "X-GOOG-API-FORMAT-VERSION", "2", "X-Firebase-Client", "fire-admin-java/" + SdkUtils.getVersion(), + // There is a known issue in which the ETag is not properly returned in cases + // where the request does not specify a compression type. Currently, it is + // required to include the header `Accept-Encoding: gzip` or equivalent in all + // requests. https://firebase.google.com/docs/remote-config/use-config-rest#etag_usage_and_forced_updates "Accept-Encoding", "gzip" ); - private final String rcSendUrl; + private final String remoteConfigUrl; private final HttpRequestFactory requestFactory; - private final HttpRequestFactory childRequestFactory; private final JsonFactory jsonFactory; private final ErrorHandlingHttpClient httpClient; private FirebaseRemoteConfigClientImpl(Builder builder) { checkArgument(!Strings.isNullOrEmpty(builder.projectId)); - this.rcSendUrl = String.format(RC_URL, builder.projectId); + this.remoteConfigUrl = String.format(REMOTE_CONFIG_URL, builder.projectId); this.requestFactory = checkNotNull(builder.requestFactory); - this.childRequestFactory = checkNotNull(builder.childRequestFactory); this.jsonFactory = checkNotNull(builder.jsonFactory); HttpResponseInterceptor responseInterceptor = builder.responseInterceptor; RemoteConfigErrorHandler errorHandler = new RemoteConfigErrorHandler(this.jsonFactory); @@ -75,8 +75,8 @@ private FirebaseRemoteConfigClientImpl(Builder builder) { } @VisibleForTesting - String getRcSendUrl() { - return rcSendUrl; + String getRemoteConfigUrl() { + return remoteConfigUrl; } @VisibleForTesting @@ -84,11 +84,6 @@ HttpRequestFactory getRequestFactory() { return requestFactory; } - @VisibleForTesting - HttpRequestFactory getChildRequestFactory() { - return childRequestFactory; - } - @VisibleForTesting JsonFactory getJsonFactory() { return jsonFactory; @@ -96,28 +91,18 @@ JsonFactory getJsonFactory() { @Override public RemoteConfigTemplate getTemplate() throws FirebaseRemoteConfigException { - HttpRequestInfo request = HttpRequestInfo.buildGetRequest(rcSendUrl) + HttpRequestInfo request = HttpRequestInfo.buildGetRequest(remoteConfigUrl) .addAllHeaders(COMMON_HEADERS); IncomingHttpResponse response = httpClient.send(request); RemoteConfigTemplate parsed = httpClient.parse(response, RemoteConfigTemplate.class); List etagList = (List) response.getHeaders().get("etag"); - - if (etagList == null || etagList.isEmpty()) { - throw new FirebaseRemoteConfigException( - ErrorCode.INTERNAL, - "ETag header is not available in the server response.", null, null, - RemoteConfigErrorCode.INTERNAL); - } + checkArgument(!(etagList == null || etagList.isEmpty()), + "ETag header is not available in the server response."); String etag = etagList.get(0); - - if (Strings.isNullOrEmpty(etag)) { - throw new FirebaseRemoteConfigException( - ErrorCode.INTERNAL, - "ETag header is not available in the server response.", null, null, - RemoteConfigErrorCode.INTERNAL); - } + checkArgument(!Strings.isNullOrEmpty(etag), + "ETag header is not available in the server response."); parsed.setETag(etag); return parsed; @@ -133,7 +118,6 @@ static FirebaseRemoteConfigClientImpl fromApp(FirebaseApp app) { return FirebaseRemoteConfigClientImpl.builder() .setProjectId(projectId) .setRequestFactory(ApiClientUtils.newAuthorizedRequestFactory(app)) - .setChildRequestFactory(ApiClientUtils.newUnauthorizedRequestFactory(app)) .setJsonFactory(app.getOptions().getJsonFactory()) .build(); } @@ -146,7 +130,6 @@ static final class Builder { private String projectId; private HttpRequestFactory requestFactory; - private HttpRequestFactory childRequestFactory; private JsonFactory jsonFactory; private HttpResponseInterceptor responseInterceptor; @@ -162,12 +145,6 @@ Builder setRequestFactory(HttpRequestFactory requestFactory) { return this; } - Builder setChildRequestFactory( - HttpRequestFactory childRequestFactory) { - this.childRequestFactory = childRequestFactory; - return this; - } - Builder setJsonFactory(JsonFactory jsonFactory) { this.jsonFactory = jsonFactory; return this; diff --git a/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java index 551e9802c..e8028b000 100644 --- a/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java +++ b/src/main/java/com/google/firebase/remoteconfig/RemoteConfigTemplate.java @@ -18,7 +18,7 @@ import com.google.api.client.util.Key; -public class RemoteConfigTemplate { +public final class RemoteConfigTemplate { @Key("etag") private String etag; diff --git a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java index b140b8e12..fcf88cfea 100644 --- a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java +++ b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java @@ -30,7 +30,7 @@ * These error messages take the form, * `"error": {"code": 123, "message": "[CODE]: Message Details", "status": "ERROR_STATUS"}` */ -public class RemoteConfigServiceErrorResponse extends GenericJson { +public final class RemoteConfigServiceErrorResponse extends GenericJson { private static final Map RC_ERROR_CODES = ImmutableMap.builder() @@ -55,7 +55,7 @@ public RemoteConfigErrorCode getRemoteConfigErrorCode() { return null; } - String message = getErrorMessage(); + String message = (String) error.get("message"); if (Strings.isNullOrEmpty(message)) { return null; } @@ -68,13 +68,4 @@ public RemoteConfigErrorCode getRemoteConfigErrorCode() { return null; } - - @Nullable - public String getErrorMessage() { - if (error == null) { - return null; - } - - return (String) error.get("message"); - } } diff --git a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java index e8ec40e6d..7f23d3141 100644 --- a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java +++ b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java @@ -53,12 +53,12 @@ public class FirebaseRemoteConfigClientImplTest { - private static final String TEST_RC_URL = + private static final String TEST_REMOTE_CONFIG_URL = "https://firebaseremoteconfig.googleapis.com/v1/projects/test-project/remoteConfig"; - private static final List HTTP_ERRORS = ImmutableList.of(401, 404, 500); + private static final List HTTP_STATUS_CODES = ImmutableList.of(401, 404, 500); - private static final Map HTTP_2_ERROR = ImmutableMap.of( + private static final Map HTTP_STATUS_TO_ERROR_CODE = ImmutableMap.of( 401, ErrorCode.UNAUTHENTICATED, 404, ErrorCode.NOT_FOUND, 500, ErrorCode.INTERNAL); @@ -82,50 +82,37 @@ public void setUp() { public void testGetTemplate() throws Exception { response.addHeader("etag", TEST_ETAG); response.setContent(MOCK_TEMPLATE_RESPONSE); + RemoteConfigTemplate template = client.getTemplate(); assertEquals(TEST_ETAG, template.getETag()); checkGetRequestHeader(interceptor.getLastRequest()); } - @Test - public void testGetTemplateWithInvalidEtags() { + @Test(expected = IllegalArgumentException.class) + public void testGetTemplateWithInvalidEtags() throws FirebaseRemoteConfigException { + // ETag does not exist + response.setContent(MOCK_TEMPLATE_RESPONSE); + + client.getTemplate(); + // Empty ETag response.addHeader("etag", ""); response.setContent(MOCK_TEMPLATE_RESPONSE); - try { - client.getTemplate(); - fail("No error thrown for invalid ETag"); - } catch (FirebaseRemoteConfigException error) { - assertEquals(ErrorCode.INTERNAL, error.getErrorCode()); - assertEquals("ETag header is not available in the server response.", error.getMessage()); - assertEquals(RemoteConfigErrorCode.INTERNAL, error.getRemoteConfigErrorCode()); - } - checkGetRequestHeader(interceptor.getLastRequest()); - // ETag does not exist - response.setContent(MOCK_TEMPLATE_RESPONSE); - try { - client.getTemplate(); - fail("No error thrown for invalid ETag"); - } catch (FirebaseRemoteConfigException error) { - assertEquals(ErrorCode.INTERNAL, error.getErrorCode()); - assertEquals("ETag header is not available in the server response.", error.getMessage()); - assertEquals(RemoteConfigErrorCode.INTERNAL, error.getRemoteConfigErrorCode()); - } - checkGetRequestHeader(interceptor.getLastRequest()); + client.getTemplate(); } @Test public void testGetTemplateHttpError() { - for (int code : HTTP_ERRORS) { + for (int code : HTTP_STATUS_CODES) { response.setStatusCode(code).setContent("{}"); try { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + checkExceptionFromHttpResponse(error, HTTP_STATUS_TO_ERROR_CODE.get(code), null, "Unexpected HTTP response with status: " + code + "\n{}"); } checkGetRequestHeader(interceptor.getLastRequest()); @@ -168,14 +155,14 @@ public void testGetTemplateSuccessResponseWithUnexpectedPayload() { @Test public void testGetTemplateErrorWithZeroContentResponse() { - for (int code : HTTP_ERRORS) { + for (int code : HTTP_STATUS_CODES) { response.setStatusCode(code).setZeroContent(); try { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + checkExceptionFromHttpResponse(error, HTTP_STATUS_TO_ERROR_CODE.get(code), null, "Unexpected HTTP response with status: " + code + "\nnull"); } checkGetRequestHeader(interceptor.getLastRequest()); @@ -184,14 +171,14 @@ public void testGetTemplateErrorWithZeroContentResponse() { @Test public void testGetTemplateErrorWithMalformedResponse() { - for (int code : HTTP_ERRORS) { + for (int code : HTTP_STATUS_CODES) { response.setStatusCode(code).setContent("not json"); try { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, HTTP_2_ERROR.get(code), null, + checkExceptionFromHttpResponse(error, HTTP_STATUS_TO_ERROR_CODE.get(code), null, "Unexpected HTTP response with status: " + code + "\nnot json"); } checkGetRequestHeader(interceptor.getLastRequest()); @@ -200,7 +187,7 @@ public void testGetTemplateErrorWithMalformedResponse() { @Test public void testGetTemplateErrorWithDetails() { - for (int code : HTTP_ERRORS) { + for (int code : HTTP_STATUS_CODES) { response.setStatusCode(code).setContent( "{\"error\": {\"status\": \"INVALID_ARGUMENT\", \"message\": \"test error\"}}"); @@ -208,23 +195,7 @@ public void testGetTemplateErrorWithDetails() { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT); - } - checkGetRequestHeader(interceptor.getLastRequest()); - } - } - - @Test - public void testGetTemplateErrorWithCanonicalCode() { - for (int code : HTTP_ERRORS) { - response.setStatusCode(code).setContent( - "{\"error\": {\"status\": \"NOT_FOUND\", \"message\": \"test error\"}}"); - - try { - client.getTemplate(); - fail("No error thrown for HTTP error"); - } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, ErrorCode.NOT_FOUND); + checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT, null, "test error"); } checkGetRequestHeader(interceptor.getLastRequest()); } @@ -232,7 +203,7 @@ public void testGetTemplateErrorWithCanonicalCode() { @Test public void testGetTemplateErrorWithRcError() { - for (int code : HTTP_ERRORS) { + for (int code : HTTP_STATUS_CODES) { response.setStatusCode(code).setContent( "{\"error\": {\"status\": \"INVALID_ARGUMENT\", " + "\"message\": \"[INVALID_ARGUMENT]: test error\"}}"); @@ -248,23 +219,6 @@ public void testGetTemplateErrorWithRcError() { } } - @Test - public void testGetTemplateErrorWithDetailsAndNoCode() { - for (int code : HTTP_ERRORS) { - response.setStatusCode(code).setContent( - "{\"error\": {\"status\": \"INVALID_ARGUMENT\", " - + "\"message\": \"test error\"}}"); - - try { - client.getTemplate(); - fail("No error thrown for HTTP error"); - } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT); - } - checkGetRequestHeader(interceptor.getLastRequest()); - } - } - @Test(expected = IllegalArgumentException.class) public void testBuilderNullProjectId() { fullyPopulatedBuilder().setProjectId(null).build(); @@ -280,11 +234,6 @@ public void testBuilderNullRequestFactory() { fullyPopulatedBuilder().setRequestFactory(null).build(); } - @Test(expected = NullPointerException.class) - public void testBuilderNullChildRequestFactory() { - fullyPopulatedBuilder().setChildRequestFactory(null).build(); - } - @Test public void testFromApp() throws IOException { FirebaseOptions options = FirebaseOptions.builder() @@ -296,16 +245,12 @@ public void testFromApp() throws IOException { try { FirebaseRemoteConfigClientImpl client = FirebaseRemoteConfigClientImpl.fromApp(app); - assertEquals(TEST_RC_URL, client.getRcSendUrl()); + assertEquals(TEST_REMOTE_CONFIG_URL, client.getRemoteConfigUrl()); assertSame(options.getJsonFactory(), client.getJsonFactory()); HttpRequest request = client.getRequestFactory().buildGetRequest( new GenericUrl("https://example.com")); assertEquals("Bearer test-token", request.getHeaders().getAuthorization()); - - request = client.getChildRequestFactory().buildGetRequest( - new GenericUrl("https://example.com")); - assertNull(request.getHeaders().getAuthorization()); } finally { app.delete(); } @@ -321,7 +266,6 @@ private FirebaseRemoteConfigClientImpl initRemoteConfigClient( .setProjectId("test-project") .setJsonFactory(Utils.getDefaultJsonFactory()) .setRequestFactory(transport.createRequestFactory()) - .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()) .setResponseInterceptor(interceptor) .build(); } @@ -332,7 +276,6 @@ private FirebaseRemoteConfigClientImpl initClientWithFaultyTransport() { .setProjectId("test-project") .setJsonFactory(Utils.getDefaultJsonFactory()) .setRequestFactory(transport.createRequestFactory()) - .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()) .build(); } @@ -340,23 +283,16 @@ private FirebaseRemoteConfigClientImpl.Builder fullyPopulatedBuilder() { return FirebaseRemoteConfigClientImpl.builder() .setProjectId("test-project") .setJsonFactory(Utils.getDefaultJsonFactory()) - .setRequestFactory(Utils.getDefaultTransport().createRequestFactory()) - .setChildRequestFactory(Utils.getDefaultTransport().createRequestFactory()); + .setRequestFactory(Utils.getDefaultTransport().createRequestFactory()); } private void checkGetRequestHeader(HttpRequest request) { assertEquals("GET", request.getRequestMethod()); - assertEquals(TEST_RC_URL, request.getUrl().toString()); + assertEquals(TEST_REMOTE_CONFIG_URL, request.getUrl().toString()); HttpHeaders headers = request.getHeaders(); assertEquals("2", headers.get("X-GOOG-API-FORMAT-VERSION")); assertEquals("fire-admin-java/" + SdkUtils.getVersion(), headers.get("X-Firebase-Client")); - assertEquals("gzip", headers.getFirstHeaderStringValue("Accept-Encoding")); - } - - private void checkExceptionFromHttpResponse( - FirebaseRemoteConfigException error, - ErrorCode expectedCode) { - checkExceptionFromHttpResponse(error, expectedCode, null, "test error"); + assertEquals("gzip", headers.getAcceptEncoding()); } private void checkExceptionFromHttpResponse( From e6152cca2a89f8d2753d1a83978fa1cb7f0cac20 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Mon, 14 Sep 2020 14:12:04 -0400 Subject: [PATCH 3/4] Improve error handling --- .../FirebaseRemoteConfigClientImpl.java | 13 ++++++++----- .../RemoteConfigServiceErrorResponse.java | 16 +++++----------- .../FirebaseRemoteConfigClientImplTest.java | 3 +-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java index 4817867d7..9a7ef31da 100644 --- a/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java +++ b/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImpl.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import com.google.api.client.http.HttpRequestFactory; import com.google.api.client.http.HttpResponseInterceptor; @@ -49,7 +50,6 @@ final class FirebaseRemoteConfigClientImpl implements FirebaseRemoteConfigClient private static final Map COMMON_HEADERS = ImmutableMap.of( - "X-GOOG-API-FORMAT-VERSION", "2", "X-Firebase-Client", "fire-admin-java/" + SdkUtils.getVersion(), // There is a known issue in which the ETag is not properly returned in cases // where the request does not specify a compression type. Currently, it is @@ -95,17 +95,20 @@ public RemoteConfigTemplate getTemplate() throws FirebaseRemoteConfigException { .addAllHeaders(COMMON_HEADERS); IncomingHttpResponse response = httpClient.send(request); RemoteConfigTemplate parsed = httpClient.parse(response, RemoteConfigTemplate.class); + parsed.setETag(getETag(response)); + return parsed; + } + private String getETag(IncomingHttpResponse response) { List etagList = (List) response.getHeaders().get("etag"); - checkArgument(!(etagList == null || etagList.isEmpty()), + checkState(etagList != null && !etagList.isEmpty(), "ETag header is not available in the server response."); String etag = etagList.get(0); - checkArgument(!Strings.isNullOrEmpty(etag), + checkState(!Strings.isNullOrEmpty(etag), "ETag header is not available in the server response."); - parsed.setETag(etag); - return parsed; + return etag; } static FirebaseRemoteConfigClientImpl fromApp(FirebaseApp app) { diff --git a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java index fcf88cfea..e82aa4d88 100644 --- a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java +++ b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java @@ -24,6 +24,8 @@ import com.google.firebase.remoteconfig.RemoteConfigErrorCode; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * The DTO for parsing error responses from the Remote Config service. @@ -41,14 +43,6 @@ public final class RemoteConfigServiceErrorResponse extends GenericJson { @Key("error") private Map error; - public String getStatus() { - if (error == null) { - return null; - } - - return (String) error.get("status"); - } - @Nullable public RemoteConfigErrorCode getRemoteConfigErrorCode() { if (error == null) { @@ -60,9 +54,9 @@ public RemoteConfigErrorCode getRemoteConfigErrorCode() { return null; } - int separator = message.indexOf(':'); - if (separator != -1) { - String errorCode = message.substring(0, separator).replaceAll("\\[|\\]", ""); + Matcher errorMatcher = Pattern.compile("^\\[(\\w+)\\]:.*$").matcher(message); + if (errorMatcher.find()) { + String errorCode = errorMatcher.group(1); return RC_ERROR_CODES.get(errorCode); } diff --git a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java index 7f23d3141..8b6e94ce1 100644 --- a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java +++ b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java @@ -89,7 +89,7 @@ public void testGetTemplate() throws Exception { checkGetRequestHeader(interceptor.getLastRequest()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = IllegalStateException.class) public void testGetTemplateWithInvalidEtags() throws FirebaseRemoteConfigException { // ETag does not exist response.setContent(MOCK_TEMPLATE_RESPONSE); @@ -290,7 +290,6 @@ private void checkGetRequestHeader(HttpRequest request) { assertEquals("GET", request.getRequestMethod()); assertEquals(TEST_REMOTE_CONFIG_URL, request.getUrl().toString()); HttpHeaders headers = request.getHeaders(); - assertEquals("2", headers.get("X-GOOG-API-FORMAT-VERSION")); assertEquals("fire-admin-java/" + SdkUtils.getVersion(), headers.get("X-Firebase-Client")); assertEquals("gzip", headers.getAcceptEncoding()); } From 52319e96ef6b105e47a62ea2df22bb313b5fa8f1 Mon Sep 17 00:00:00 2001 From: Lahiru Maramba Date: Tue, 15 Sep 2020 11:37:44 -0400 Subject: [PATCH 4/4] Pre compute error matching pattern --- .../internal/RemoteConfigServiceErrorResponse.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java index e82aa4d88..0c517e14e 100644 --- a/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java +++ b/src/main/java/com/google/firebase/remoteconfig/internal/RemoteConfigServiceErrorResponse.java @@ -40,6 +40,8 @@ public final class RemoteConfigServiceErrorResponse extends GenericJson { .put("INVALID_ARGUMENT", RemoteConfigErrorCode.INVALID_ARGUMENT) .build(); + private static final Pattern RC_ERROR_CODE_PATTERN = Pattern.compile("^\\[(\\w+)\\]:.*$"); + @Key("error") private Map error; @@ -54,7 +56,7 @@ public RemoteConfigErrorCode getRemoteConfigErrorCode() { return null; } - Matcher errorMatcher = Pattern.compile("^\\[(\\w+)\\]:.*$").matcher(message); + Matcher errorMatcher = RC_ERROR_CODE_PATTERN.matcher(message); if (errorMatcher.find()) { String errorCode = errorMatcher.group(1); return RC_ERROR_CODES.get(errorCode);