diff --git a/android/src/main/java/com/onesignal/rnonesignalandroid/RNOneSignal.java b/android/src/main/java/com/onesignal/rnonesignalandroid/RNOneSignal.java index a9b093de..9967e2d7 100644 --- a/android/src/main/java/com/onesignal/rnonesignalandroid/RNOneSignal.java +++ b/android/src/main/java/com/onesignal/rnonesignalandroid/RNOneSignal.java @@ -598,6 +598,10 @@ public void addOutcomeWithValue(String name, double value) { @Override public void login(String externalUserId) { + if (externalUserId == null || externalUserId.isEmpty()) { + Logging.error("login called with a null or empty externalUserId", null); + return; + } OneSignal.login(externalUserId); } diff --git a/ios/RCTOneSignal/RCTOneSignalEventEmitter.mm b/ios/RCTOneSignal/RCTOneSignalEventEmitter.mm index 4273ac11..180c35eb 100644 --- a/ios/RCTOneSignal/RCTOneSignalEventEmitter.mm +++ b/ios/RCTOneSignal/RCTOneSignalEventEmitter.mm @@ -138,6 +138,11 @@ + (void)sendEventWithName:(NSString *)name withBody:(NSDictionary *)body { } RCT_EXPORT_METHOD(login : (NSString *)externalId) { + if (externalId == nil || [externalId length] == 0) { + [OneSignalLog onesignalLog:ONE_S_LL_ERROR + message:@"login called with a nil or empty externalId"]; + return; + } [OneSignal login:externalId]; } diff --git a/src/helpers.test.ts b/src/helpers.test.ts index 8d904ebf..a71394f2 100644 --- a/src/helpers.test.ts +++ b/src/helpers.test.ts @@ -5,6 +5,7 @@ import { IOS_NULL_SENTINEL } from './constants/internal'; import { encodeNullsForIOS, isNativeModuleLoaded, + isMissing, isObjectSerializable, isValidCallback, } from './helpers'; @@ -66,6 +67,23 @@ describe('helpers', () => { }); }); + describe('isMissing', () => { + test.each([ + { description: 'a non-empty string', value: 'id', expected: false }, + { description: 'a whitespace string', value: ' ', expected: false }, + { description: 'an empty string', value: '', expected: true }, + { description: 'null', value: null, expected: true }, + { description: 'undefined', value: undefined, expected: true }, + { description: 'a number', value: 1, expected: true }, + { description: 'a boolean', value: true, expected: true }, + ])( + 'should return $expected for $description', + ({ value, expected }: { description: string; value: unknown; expected: boolean }) => { + expect(isMissing(value, 'login: externalId')).toBe(expected); + }, + ); + }); + describe('isObjectSerializable', () => { test.each([ { description: 'an empty object', value: {} }, diff --git a/src/helpers.ts b/src/helpers.ts index ca6e0f41..cd52f7e6 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -18,6 +18,12 @@ export function isNativeModuleLoaded(module: object | null | undefined): boolean return true; } +export function isMissing(value: unknown, api: string): boolean { + if (typeof value === 'string' && value.length > 0) return false; + console.error(`OneSignal: ${api} is required`); + return true; +} + /** * Returns true if the value is a JSON-serializable object. */ diff --git a/src/index.test.ts b/src/index.test.ts index 99bc865f..eb8b1c48 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -112,6 +112,18 @@ describe('OneSignal', () => { expect(mockRNOneSignal.initialize).not.toHaveBeenCalled(); }); + test('should not initialize if appId is null', () => { + OneSignal.initialize(null as unknown as string); + expect(mockRNOneSignal.initialize).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: initialize: appId is required'); + }); + + test('should not initialize if appId is empty', () => { + OneSignal.initialize(''); + expect(mockRNOneSignal.initialize).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: initialize: appId is required'); + }); + test('should keep a permission event that arrives before the startup read resolves', async () => { let resolveStartupRead: ((granted: boolean) => void) | undefined; vi.mocked(mockRNOneSignal.hasNotificationPermission).mockReturnValueOnce( @@ -180,6 +192,18 @@ describe('OneSignal', () => { OneSignal.login('external-123'); expect(mockRNOneSignal.login).not.toHaveBeenCalled(); }); + + test('should not login if externalId is null', () => { + OneSignal.login(null as unknown as string); + expect(mockRNOneSignal.login).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: login: externalId is required'); + }); + + test('should not login if externalId is empty', () => { + OneSignal.login(''); + expect(mockRNOneSignal.login).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: login: externalId is required'); + }); }); describe('logout', () => { @@ -676,6 +700,16 @@ describe('OneSignal', () => { expect(mockRNOneSignal.setLanguage).toHaveBeenCalledWith('en'); }); + test('forwards an empty language so native can reset', () => { + OneSignal.User.setLanguage(''); + expect(mockRNOneSignal.setLanguage).toHaveBeenCalledWith(''); + }); + + test('does not set a null language', () => { + OneSignal.User.setLanguage(null as unknown as string); + expect(mockRNOneSignal.setLanguage).not.toHaveBeenCalled(); + }); + test('should not set language if native module is not loaded', () => { isNativeLoadedSpy.mockReturnValue(false); OneSignal.User.setLanguage('en'); @@ -748,6 +782,18 @@ describe('OneSignal', () => { OneSignal.User.addEmail(EMAIL); expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled(); }); + + test('should not add email if email is null', () => { + OneSignal.User.addEmail(null as unknown as string); + expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: addEmail: email is required'); + }); + + test('should not add email if email is empty', () => { + OneSignal.User.addEmail(''); + expect(mockRNOneSignal.addEmail).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalledWith('OneSignal: addEmail: email is required'); + }); }); describe('removeEmail', () => { @@ -1306,16 +1352,16 @@ describe('OneSignal', () => { expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('key', 'value'); }); - test('should log error but still call native method if key is missing', () => { + test('should not add trigger if key is missing', () => { OneSignal.InAppMessages.addTrigger('', 'value'); expect(errorSpy).toHaveBeenCalled(); - expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('', 'value'); + expect(mockRNOneSignal.addTrigger).not.toHaveBeenCalled(); }); - test('should log error but still call native method if value is null', () => { + test('should not add trigger if value is null', () => { OneSignal.InAppMessages.addTrigger('key', null as unknown as string); expect(errorSpy).toHaveBeenCalled(); - expect(mockRNOneSignal.addTrigger).toHaveBeenCalledWith('key', null); + expect(mockRNOneSignal.addTrigger).not.toHaveBeenCalled(); }); test('should not add trigger if native module is not loaded', () => { diff --git a/src/index.ts b/src/index.ts index 212299d1..ea49125d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,6 +18,7 @@ import NotificationWillDisplayEvent from './events/NotificationWillDisplayEvent' import { encodeNullsForIOS, isNativeModuleLoaded, + isMissing, isObjectSerializable, isValidCallback, } from './helpers'; @@ -107,6 +108,7 @@ export namespace OneSignal { /** Initializes the OneSignal SDK. This should be called during startup of the application. */ export function initialize(appId: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(appId, 'initialize: appId')) return; RNOneSignal.initialize(appId); @@ -121,6 +123,7 @@ export namespace OneSignal { */ export function login(externalId: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(externalId, 'login: externalId')) return; RNOneSignal.login(externalId); } @@ -445,9 +448,13 @@ export namespace OneSignal { return RNOneSignal.getExternalId(); } - /** Explicitly set a 2-character language code for the user. */ + /** Explicitly set a 2-character language code for the user. Empty string resets to the device language. */ export function setLanguage(language: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (typeof language !== 'string') { + console.error('OneSignal: setLanguage: language is required'); + return; + } RNOneSignal.setLanguage(language); } @@ -455,6 +462,7 @@ export namespace OneSignal { /** Set an alias for the current user. If this alias label already exists on this user, it will be overwritten with the new alias id. */ export function addAlias(label: string, id: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(label, 'addAlias: label') || isMissing(id, 'addAlias: id')) return; RNOneSignal.addAlias(label, id); } @@ -469,6 +477,7 @@ export namespace OneSignal { /** Remove an alias from the current user. */ export function removeAlias(label: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(label, 'removeAlias: label')) return; RNOneSignal.removeAlias(label); } @@ -483,6 +492,7 @@ export namespace OneSignal { /** Add a new email subscription to the current user. */ export function addEmail(email: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(email, 'addEmail: email')) return; RNOneSignal.addEmail(email); } @@ -493,6 +503,7 @@ export namespace OneSignal { */ export function removeEmail(email: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(email, 'removeEmail: email')) return; RNOneSignal.removeEmail(email); } @@ -500,6 +511,7 @@ export namespace OneSignal { /** Add a new SMS subscription to the current user. */ export function addSms(smsNumber: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(smsNumber, 'addSms: smsNumber')) return; RNOneSignal.addSms(smsNumber); } @@ -510,6 +522,7 @@ export namespace OneSignal { */ export function removeSms(smsNumber: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(smsNumber, 'removeSms: smsNumber')) return; RNOneSignal.removeSms(smsNumber); } @@ -521,8 +534,9 @@ export namespace OneSignal { export function addTag(key: string, value: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; - if (!key || value === undefined || value === null) { - console.error('OneSignal: addTag: must include a key and a value'); + if (isMissing(key, 'addTag: key')) return; + if (value == null) { + console.error('OneSignal: addTag: value is required'); return; } @@ -543,6 +557,7 @@ export namespace OneSignal { /** Remove the data tag with the provided key from the current user. */ export function removeTag(key: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(key, 'removeTag: key')) return; RNOneSignal.removeTag(key); } @@ -787,9 +802,11 @@ export namespace OneSignal { export function addTrigger(key: string, value: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; - // value can be assigned to `false` so we cannot just check `!value` - if (!key || value == null) { - console.error('OneSignal: addTrigger: must include a key and a value'); + // false is a valid trigger value, so reject only null/undefined for value. + if (isMissing(key, 'addTrigger: key')) return; + if (value == null) { + console.error('OneSignal: addTrigger: value is required'); + return; } RNOneSignal.addTrigger(key, value); @@ -808,6 +825,7 @@ export namespace OneSignal { /** Remove the trigger with the provided key from the current user. */ export function removeTrigger(key: string) { if (!isNativeModuleLoaded(RNOneSignal)) return; + if (isMissing(key, 'removeTrigger: key')) return; RNOneSignal.removeTrigger(key); }