refactor: make listener manually managed

Author: codebytere

README.md

@@ -34,6 +34,8 @@ Return Value Descriptions:
 * 'Denied' - The user explicitly denied access to contact data for the application.
 * 'Authorized' - The application is authorized to access contact data.
 
+Example Usage:
+
 ```js
 const authStatus = contacts.getAuthStatus()
 
@@ -73,7 +75,7 @@ The returned objects will take the following format:
 
 This method will return an empty array (`[]`) if access to Contacts has not been granted.
 
-Example:
+Example Usage:
 
 ```js
 const allContacts = contacts.getAllContacts()
@@ -123,7 +125,7 @@ The returned object will take the following format:
 
 This method will return an empty array (`[]`) if access to Contacts has not been granted.
 
-Example:
+Example Usage:
 
 ```js
 const contacts = contacts.getContactsByName('Appleseed')
@@ -160,7 +162,7 @@ Creates and save a new contact to the user's contacts database.
 
 This method will return `false` if access to Contacts has not been granted.
 
-Example:
+Example Usage:
 
 ```js
 const success = contacts.addNewContact({
@@ -188,7 +190,7 @@ However, you should take care to specify `name` to such a degree that you can be
 
 This method will return `false` if access to Contacts has not been granted.
 
-Example:
+Example Usage:
 
 ```js
 const name = 'Jonathan Appleseed'
@@ -215,7 +217,7 @@ You should take care to specify parameters to the `contact` object to such a deg
 
 This method will return `false` if access to Contacts has not been granted.
 
-Example:
+Example Usage:
 
 ```js
 // Change contact's nickname from Billy -> Will
@@ -232,11 +234,15 @@ console.log(`Contact was ${updated ? 'updated' : 'not updated'}.`)
 
 This module exposes an `EventEmitter`, which can be used to listen to potential changes to the `CNContactStore`. When a contact is changed either with methods contained in this module, or manually by a user, the `contact-changed` event will be emitted.
 
-Example:
+Owing to the underlying architecture of this module, the listener must be manually managed; before use you must initialize it with `listener.setup()` and when you are finished listening for events you must remove it with `listener.remove()`
+
+Example Usage:
 
 ```js
 const { listener, addNewContact } = require('node-mac-contacts')
 
+listener.setup()
+
 addNewContact({
   firstName: 'William',
   lastName: 'Grapeseed',
@@ -246,7 +252,8 @@ addNewContact({
   emailAddresses: ['billy@grapeseed.com'],
 })
 
-listener.on('contact-changed', () => {
+listener.once('contact-changed', () => {
   console.log('A contact was changed!')
+  listener.remove()
 })
 ```

contacts.mm

@@ -2,6 +2,7 @@
 #include <napi.h>
 
 Napi::ThreadSafeFunction ts_fn;
+id observer;
 
 /***** HELPERS *****/
 
@@ -508,12 +509,12 @@ CNAuthorizationStatus AuthStatus() {
   return Napi::Boolean::New(env, success);
 }
 
-Napi::Boolean HandleEmit(const Napi::CallbackInfo &info) {
+Napi::Boolean SetupListener(const Napi::CallbackInfo &info) {
   Napi::Env env = info.Env();
   ts_fn = Napi::ThreadSafeFunction::New(env, info[0].As<Napi::Function>(),
                                         "emitCallback", 0, 1);
 
-  [[NSNotificationCenter defaultCenter]
+  observer = [[NSNotificationCenter defaultCenter]
       addObserverForName:CNContactStoreDidChangeNotification
                   object:nil
                    queue:[NSOperationQueue mainQueue]
@@ -523,16 +524,32 @@ CNAuthorizationStatus AuthStatus() {
                   js_cb.Call({Napi::String::New(env, value)});
                 };
                 ts_fn.BlockingCall("contact-changed", callback);
-                ts_fn.Release();
               }];
 
   return Napi::Boolean::New(env, true);
 }
 
+Napi::Boolean RemoveListener(const Napi::CallbackInfo &info) {
+  Napi::Env env = info.Env();
+
+  if (!observer) {
+    Napi::Error::New(env, "No observers are currently listening")
+        .ThrowAsJavaScriptException();
+    return Napi::Boolean::New(env, false);
+  }
+
+  ts_fn.Release();
+  [[NSNotificationCenter defaultCenter] removeObserver:observer];
+
+  return Napi::Boolean::New(env, true);
+}
+
 // Initializes all functions exposed to JS.
 Napi::Object Init(Napi::Env env, Napi::Object exports) {
-  exports.Set(Napi::String::New(env, "handleEmit"),
-              Napi::Function::New(env, HandleEmit));
+  exports.Set(Napi::String::New(env, "setupListener"),
+              Napi::Function::New(env, SetupListener));
+  exports.Set(Napi::String::New(env, "removeListener"),
+              Napi::Function::New(env, RemoveListener));
   exports.Set(Napi::String::New(env, "getAuthStatus"),
               Napi::Function::New(env, GetAuthStatus));
   exports.Set(Napi::String::New(env, "getAllContacts"),

index.js

@@ -3,9 +3,16 @@ const contacts = require('bindings')('contacts.node')
 const { EventEmitter } = require('events')
 
 const listener = new EventEmitter()
-contacts.handleEmit(listener.emit.bind(listener))
 
-const validProperties = [
+listener.setup = () => {
+  contacts.setupListener(listener.emit.bind(listener))
+}
+
+listener.remove = () => {
+  contacts.removeListener()
+}
+
+const optionalProperties = [
   'jobTitle',
   'departmentName',
   'organizationName',
@@ -22,9 +29,9 @@ function getAllContacts(extraProperties = []) {
     throw new TypeError('extraProperties must be an array')
   }
 
-  if (!extraProperties.every((p) => validProperties.includes(p))) {
+  if (!extraProperties.every((p) => optionalProperties.includes(p))) {
     throw new TypeError(
-      `properties in extraProperties must be one of ${validProperties.join(
+      `properties in extraProperties must be one of ${optionalProperties.join(
         ', ',
       )}`,
     )
@@ -42,9 +49,9 @@ function getContactsByName(name, extraProperties = []) {
     throw new TypeError('extraProperties must be an array')
   }
 
-  if (!extraProperties.every((p) => validProperties.includes(p))) {
+  if (!extraProperties.every((p) => optionalProperties.includes(p))) {
     throw new TypeError(
-      `properties in extraProperties must be one of ${validProperties.join(
+      `properties in extraProperties must be one of ${optionalProperties.join(
         ', ',
       )}`,
     )

test/module.spec.js

@@ -23,29 +23,6 @@ describe('node-mac-contacts', () => {
     })
   })
 
-  describe('getContactsByName(name[, extraProperties])', () => {
-    it('should throw if name is not a string', () => {
-      expect(() => {
-        getContactsByName(12345)
-      }).to.throw(/name must be a string/)
-    })
-
-    it('should throw if extraProperties is not an array', () => {
-      expect(() => {
-        getContactsByName('jim-bob', 12345)
-      }).to.throw(/extraProperties must be an array/)
-    })
-
-    it('should throw if extraProperties contains invalid properties', () => {
-      const errorMessage =
-        'properties in extraProperties must be one of jobTitle, departmentName, organizationName, middleName, note, contactImage, contactThumbnailImage, instantMessageAddresses, socialProfiles'
-
-      expect(() => {
-        getContactsByName('jim-bob', ['bad-property'])
-      }).to.throw(errorMessage)
-    })
-  })
-
   describe('getAllContacts([extraProperties])', () => {
     it('should return an array', () => {
       const contacts = getAllContacts()
@@ -114,6 +91,59 @@ describe('node-mac-contacts', () => {
         addNewContact({ emailAddresses: 1 })
       }).to.throw(/emailAddresses must be an array/)
     })
+
+    it('should successfully add a contact', () => {
+      const success = addNewContact({
+        firstName: 'William',
+        lastName: 'Grapeseed',
+        nickname: 'Billy',
+        birthday: '1990-09-09',
+        phoneNumbers: ['+1234567890'],
+        emailAddresses: ['billy@grapeseed.com'],
+      })
+
+      expect(success).to.be.true
+    })
+  })
+
+  describe('getContactsByName(name[, extraProperties])', () => {
+    it('should throw if name is not a string', () => {
+      expect(() => {
+        getContactsByName(12345)
+      }).to.throw(/name must be a string/)
+    })
+
+    it('should throw if extraProperties is not an array', () => {
+      expect(() => {
+        getContactsByName('jim-bob', 12345)
+      }).to.throw(/extraProperties must be an array/)
+    })
+
+    it('should throw if extraProperties contains invalid properties', () => {
+      const errorMessage =
+        'properties in extraProperties must be one of jobTitle, departmentName, organizationName, middleName, note, contactImage, contactThumbnailImage, instantMessageAddresses, socialProfiles'
+
+      expect(() => {
+        getContactsByName('jim-bob', ['bad-property'])
+      }).to.throw(errorMessage)
+    })
+
+    it('should retrieve a contact by name predicates', () => {
+      addNewContact({
+        firstName: 'Sherlock',
+        lastName: 'Holmes',
+        nickname: 'Sherllock',
+        birthday: '1854-01-06',
+        phoneNumbers: ['+1234567890'],
+        emailAddresses: ['sherlock@holmes.com'],
+      })
+
+      const contacts = getContactsByName('Sherlock Holmes')
+      expect(contacts).to.be.an('array').of.length.gte(1)
+
+      const contact = contacts[0]
+      expect(contact.firstName).to.eql('Sherlock')
+    })
   })
 
   describe('deleteContact(name)', () => {
@@ -173,7 +203,15 @@ describe('node-mac-contacts', () => {
   })
 
   describe('listener', () => {
+    it('throws when trying to remove a nonexistent listener', () => {
+      expect(() => {
+        listener.remove()
+      }).to.throw(/No observers are currently listening/)
+    })
+
     it('emits an event when the contact is changed', (done) => {
+      listener.setup()
+
       addNewContact({
         firstName: 'William',
         lastName: 'Grapeseed',
@@ -184,6 +222,7 @@ describe('node-mac-contacts', () => {
       })
 
       listener.once('contact-changed', () => {
+        listener.remove()
         done()
       })
     })