Making CriticalSound.name field required in typings (#426)

* Making CriticalSound.name field required in typings

* Disallowing non-empty strings in APNs sound config

* Test cases for empty string

Author: hiranya911

src/index.d.ts

@@ -473,7 +473,7 @@ declare namespace admin.messaging {
 
   type CriticalSound = {
     critical?: boolean;
-    name?: string;
+    name: string;
     volume?: number;
   }
 

src/messaging/messaging.ts

@@ -180,7 +180,7 @@ export interface Aps {
 
 export interface CriticalSound {
   critical?: boolean;
-  name?: string;
+  name: string;
   volume?: number;
 }
 
@@ -340,14 +340,19 @@ function validateAps(aps: Aps) {
 }
 
 function validateApsSound(sound: string | CriticalSound) {
-  if (typeof sound === 'undefined' || validator.isString(sound)) {
+  if (typeof sound === 'undefined' || validator.isNonEmptyString(sound)) {
     return;
   } else if (!validator.isNonNullObject(sound)) {
     throw new FirebaseMessagingError(
       MessagingClientErrorCode.INVALID_PAYLOAD,
-      'apns.payload.aps.sound must be a string or a non-null object');
+      'apns.payload.aps.sound must be a non-empty string or a non-null object');
   }
 
+  if (!validator.isNonEmptyString(sound.name)) {
+    throw new FirebaseMessagingError(
+      MessagingClientErrorCode.INVALID_PAYLOAD,
+      'apns.payload.aps.sound.name must be a non-empty string');
+  }
   const volume = sound.volume;
   if (typeof volume !== 'undefined') {
     if (!validator.isNumber(volume)) {

src/utils/validator.ts

@@ -100,7 +100,7 @@ export function isBase64String(value: any): boolean {
  * @param {any} value The value to validate.
  * @return {boolean} Whether the value is a non-empty string or not.
  */
-export function isNonEmptyString(value: any): boolean {
+export function isNonEmptyString(value: any): value is string {
   return isString(value) && value !== '';
 }
 

test/unit/messaging/messaging.spec.ts

@@ -1728,6 +1728,7 @@ describe('Messaging', () => {
             payload: {
               aps: {
                 sound: {
+                  name: 'default',
                   volume,
                 },
               },
@@ -1881,12 +1882,29 @@ describe('Messaging', () => {
       });
     });
 
-    const invalidApnsSounds: any[] = [null, [], true, 1.23];
+    const invalidApnsSounds: any[] = ['', null, [], true, 1.23];
     invalidApnsSounds.forEach((sound) => {
       it(`should throw given APNS payload with invalid aps sound: ${JSON.stringify(sound)}`, () => {
         expect(() => {
           messaging.send({apns: {payload: {aps: {sound}}}, token: 'token'});
-        }).to.throw('apns.payload.aps.sound must be a string or a non-null object');
+        }).to.throw('apns.payload.aps.sound must be a non-empty string or a non-null object');
+      });
+    });
+    invalidApnsSounds.forEach((name) => {
+      it(`should throw given invalid APNS critical sound name: ${name}`, () => {
+        const message: Message = {
+          condition: 'topic-name',
+          apns: {
+            payload: {
+              aps: {
+                sound: {name},
+              },
+            },
+          },
+        };
+        expect(() => {
+          messaging.send(message);
+        }).to.throw('apns.payload.aps.sound.name must be a non-empty string');
       });
     });
   });
@@ -2403,6 +2421,31 @@ describe('Messaging', () => {
           },
         },
       },
+      {
+        label: 'APNS critical sound name only',
+        req: {
+          apns: {
+            payload: {
+              aps: {
+                sound: {
+                  name: 'test.sound',
+                },
+              },
+            },
+          },
+        },
+        expectedReq: {
+          apns: {
+            payload: {
+              aps: {
+                sound: {
+                  name: 'test.sound',
+                },
+              },
+            },
+          },
+        },
+      },
       {
         label: 'APNS critical sound explicitly false',
         req: {