From e390198e73f14a9c4e94edcdec6eed8ef5cf45fd Mon Sep 17 00:00:00 2001 From: josroseboom Date: Wed, 5 Feb 2025 15:39:06 +0100 Subject: [PATCH 1/3] RESOURCE_EXHAUSTED is a possible error code as well, given https://developers.google.com/instance-id/reference/server --- .../firebase/messaging/TopicManagementResponse.java | 1 + .../firebase/messaging/InstanceIdClientImplTest.java | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java index bbf4c944a..d9017d34a 100644 --- a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java +++ b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java @@ -41,6 +41,7 @@ public class TopicManagementResponse { .put("NOT_FOUND", "registration-token-not-registered") .put("INTERNAL", "internal-error") .put("TOO_MANY_TOPICS", "too-many-topics") + .put("RESOURCE_EXHAUSTED", "resource-exhausted") .build(); private final int successCount; diff --git a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java index d0151bb94..caf9b0a23 100644 --- a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java +++ b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java @@ -413,6 +413,17 @@ public void testTopicManagementResponseErrorToString() { assertEquals(expected, topicManagementResponse.getErrors().toString()); } + @Test + public void testTopicManagementResponseErrorResourceExhausted() { + GenericJson json = new GenericJson().set("error", "RESOURCE_EXHAUSTED"); + ImmutableList jsonList = ImmutableList.of(json); + + TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); + + String expected = "[Error{index=0, reason=resource-exhausted}]"; + assertEquals(expected, topicManagementResponse.getErrors().toString()); + } + private static InstanceIdClientImpl initInstanceIdClient( final MockLowLevelHttpResponse mockResponse, final HttpResponseInterceptor interceptor) { From 814bc6debf5cc090fdb26ddb647435628c43a8d6 Mon Sep 17 00:00:00 2001 From: josroseboom Date: Wed, 5 Feb 2025 15:43:45 +0100 Subject: [PATCH 2/3] If the reason is not in the predefined ERROR_CODES it would be more helpful to use the reason String that came in as an parameter than UNKNOWN_ERROR. UNKNOWN_ERROR could be used for null or empty Strings. --- .../messaging/TopicManagementResponse.java | 7 +++-- .../messaging/InstanceIdClientImplTest.java | 27 +++++++++++++++++-- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java index d9017d34a..4dd00d0f4 100644 --- a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java +++ b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java @@ -102,8 +102,11 @@ public static class Error { private Error(int index, String reason) { this.index = index; - this.reason = ERROR_CODES.containsKey(reason) - ? ERROR_CODES.get(reason) : UNKNOWN_ERROR; + if (reason == null || reason.trim().isEmpty()) { + this.reason = UNKNOWN_ERROR; + } else { + this.reason = ERROR_CODES.getOrDefault(reason, reason); + } } /** diff --git a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java index caf9b0a23..36826621b 100644 --- a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java +++ b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java @@ -404,7 +404,30 @@ public void testTopicManagementResponseWithEmptyList() { @Test public void testTopicManagementResponseErrorToString() { - GenericJson json = new GenericJson().set("error", "test error"); + GenericJson json = new GenericJson().set("error", "INVALID_ARGUMENT"); + ImmutableList jsonList = ImmutableList.of(json); + + TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); + + String expected = "[Error{index=0, reason=invalid-argument}]"; + assertEquals(expected, topicManagementResponse.getErrors().toString()); + } + + @Test + public void testTopicManagementResponseErrorNotInErrorCodes() { + String myError = "myError"; + GenericJson json = new GenericJson().set("error", myError); + ImmutableList jsonList = ImmutableList.of(json); + + TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); + + String expected = "[Error{index=0, reason=" + myError + "}]"; + assertEquals(expected, topicManagementResponse.getErrors().toString()); + } + + @Test + public void testTopicManagementResponseErrorUnknown() { + GenericJson json = new GenericJson().set("error", ""); ImmutableList jsonList = ImmutableList.of(json); TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); @@ -443,7 +466,7 @@ private void checkTopicManagementRequest( assertEquals(1, result.getFailureCount()); assertEquals(1, result.getErrors().size()); assertEquals(1, result.getErrors().get(0).getIndex()); - assertEquals("unknown-error", result.getErrors().get(0).getReason()); + assertEquals("error_reason", result.getErrors().get(0).getReason()); ByteArrayOutputStream out = new ByteArrayOutputStream(); request.getContent().writeTo(out); From 869b553639ef2f40a14e25d22f31e48f14cc0448 Mon Sep 17 00:00:00 2001 From: josroseboom Date: Wed, 19 Nov 2025 10:45:18 +0100 Subject: [PATCH 3/3] For consistency with the current format of the error reasons, logic is added to map them from UPPER_SNAKE_CASE to lower-kebab-case. Entries from the ERROR_CODES that can be computed this way are removed from the map. Extra tests are added for those 3 cases. --- .../messaging/TopicManagementResponse.java | 5 +---- .../messaging/InstanceIdClientImplTest.java | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java index 4dd00d0f4..9f75f5871 100644 --- a/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java +++ b/src/main/java/com/google/firebase/messaging/TopicManagementResponse.java @@ -37,11 +37,8 @@ public class TopicManagementResponse { // Server error codes as defined in https://developers.google.com/instance-id/reference/server // TODO: Should we handle other error codes here (e.g. PERMISSION_DENIED)? private static final Map ERROR_CODES = ImmutableMap.builder() - .put("INVALID_ARGUMENT", "invalid-argument") .put("NOT_FOUND", "registration-token-not-registered") .put("INTERNAL", "internal-error") - .put("TOO_MANY_TOPICS", "too-many-topics") - .put("RESOURCE_EXHAUSTED", "resource-exhausted") .build(); private final int successCount; @@ -105,7 +102,7 @@ private Error(int index, String reason) { if (reason == null || reason.trim().isEmpty()) { this.reason = UNKNOWN_ERROR; } else { - this.reason = ERROR_CODES.getOrDefault(reason, reason); + this.reason = ERROR_CODES.getOrDefault(reason, reason.toLowerCase().replace('_', '-')); } } diff --git a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java index 36826621b..a6f1c7543 100644 --- a/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java +++ b/src/test/java/com/google/firebase/messaging/InstanceIdClientImplTest.java @@ -415,13 +415,13 @@ public void testTopicManagementResponseErrorToString() { @Test public void testTopicManagementResponseErrorNotInErrorCodes() { - String myError = "myError"; + String myError = "MY_ERROR"; GenericJson json = new GenericJson().set("error", myError); ImmutableList jsonList = ImmutableList.of(json); TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); - String expected = "[Error{index=0, reason=" + myError + "}]"; + String expected = "[Error{index=0, reason=my-error}]"; assertEquals(expected, topicManagementResponse.getErrors().toString()); } @@ -447,6 +447,18 @@ public void testTopicManagementResponseErrorResourceExhausted() { assertEquals(expected, topicManagementResponse.getErrors().toString()); } + @Test + public void testTopicManagementResponseErrorTooManyTopics() { + GenericJson json = new GenericJson().set("error", "TOO_MANY_TOPICS"); + ImmutableList jsonList = ImmutableList.of(json); + + TopicManagementResponse topicManagementResponse = new TopicManagementResponse(jsonList); + + String expected = "[Error{index=0, reason=too-many-topics}]"; + assertEquals(expected, topicManagementResponse.getErrors().toString()); + } + + private static InstanceIdClientImpl initInstanceIdClient( final MockLowLevelHttpResponse mockResponse, final HttpResponseInterceptor interceptor) { @@ -466,7 +478,7 @@ private void checkTopicManagementRequest( assertEquals(1, result.getFailureCount()); assertEquals(1, result.getErrors().size()); assertEquals(1, result.getErrors().get(0).getIndex()); - assertEquals("error_reason", result.getErrors().get(0).getReason()); + assertEquals("error-reason", result.getErrors().get(0).getReason()); ByteArrayOutputStream out = new ByteArrayOutputStream(); request.getContent().writeTo(out);