fix linting and address reviews from @chadoh

packages/aragon-wrapper/src/identity/AddressBookIdentityProvider.js

@@ -1,56 +1,70 @@
 import AddressIdentityProvider from './AddressIdentityProvider'
-import { first, map } from 'rxjs/operators'
+import { from } from 'rxjs'
+import { concatAll, first, map, skipWhile, filter, flatMap, defaultIfEmpty, reduce } from 'rxjs/operators'
 import { getCacheKey } from '../utils/index'
+import { apmAppId } from '../utils/apps'
 
 const addressBookAppIds = [
-  '0x32ec8cc9f3136797e0ae30e7bf3740905b0417b81ff6d4a74f6100f9037425de'
-  // TODO Add in App Ids for rinkeby and mainnet appIds
+  apmAppId('address-book'),
+  apmAppId('address-book.open'),
+  apmAppId('tps-address-book.open'),
+  apmAppId('address-book-staging.open'),
+  apmAppId('address-book.hatch')
 ]
 /**
  * An identity provider for Address Book Entries
  *
  * @extends AddressIdentityProvider
  */
 export default class AddressBookIdentityProvider extends AddressIdentityProvider {
+  /**
+   * Create a new identity Provider that queries installed Address Book apps
+   * @param {Observable} apps apps Observable from the wrapper
+   * @param {Cache} cache the cache instance utilized by the wrapper
+   */
   constructor (apps, cache) {
     super()
     this.apps = apps
     this.cache = cache
   }
-  /**
-   * Optional initialization, if required by the provider
-   */
-  async init () {
-    // no-op
-  }
 
   /**
    * Resolve the identity metadata for an address
    * Should resolve to null if an identity does not exist
    * Will return the first successful resolution                                                                                                                                                                                                                                tity could not be found
    *
    * @param  {string} address Address to resolve
-   * @return {Promise} Resolved metadata or rejected error
+   * @return {Promise} Resolves with identity metadata or null if not found
    */
   async resolve (address) {
     address = address.toLowerCase()
-    const addressBookApps = await this.apps.pipe(
+    return this.apps.pipe(
       first(),
-      map(apps => apps.filter(app => addressBookAppIds.includes(app.appId)))
-    ).toPromise()
-
-    return addressBookApps.reduce(async (identity, app) => {
-      if (identity) {
-        return identity
+      concatAll(),
+      filter(app => addressBookAppIds.includes(app.appId)),
+      map(async app => {
+        const cacheKey = getCacheKey(app.proxyAddress, 'state')
+        const { entries = [] } = await this.cache.get(cacheKey)
+        const { data: entryData } = entries
+          .find(entry => entry.addr.toLowerCase() === address) || {}
+        return entryData || null
       }
-      const cacheKey = getCacheKey(app.proxyAddress, 'state')
-      const { entries = [] } = await this.cache.get(cacheKey)
-      const { data: entryData } = entries
-        .find(entry => entry.addr.toLowerCase() === address) || {}
-      return entryData || null
-    }, null)
+      ),
+      flatMap(pendingEntryData => from(pendingEntryData)),
+      skipWhile(entryData => !entryData),
+      defaultIfEmpty(null),
+      first()
+    ).toPromise()
   }
 
+  /**
+   * Search for matches in the installed address books.
+   *
+   * If the search term starts with '0x', addresses will be matched for instead.
+   *
+   * @param  {string} searchTerm Search term
+   * @return {Promise} Resolved with array of matches, each containing the address and name
+   */
   async search (searchTerm = '') {
     const isAddressSearch = searchTerm.substring(0, 2).toLowerCase() === '0x'
     const identities = await this.getAll()
@@ -68,23 +82,28 @@ export default class AddressBookIdentityProvider extends AddressIdentityProvider
 
   /**
    * get all identities from all installed address book instances
+   *
+   * @return {Promise} Resolved with an object of all identities when completed
    */
   async getAll () {
-    const addressBookApps = await this.apps.pipe(
+    return this.apps.pipe(
       first(),
-      map(apps => apps.filter(app => addressBookAppIds.includes(app.appId)))
+      concatAll(),
+      filter(app => addressBookAppIds.includes(app.appId)),
+      reduce(
+        async (allEntries, app) => {
+          const cacheKey = getCacheKey(app.proxyAddress, 'state')
+          const { entries = [] } = await this.cache.get(cacheKey)
+          const allEntriesResolved = await allEntries
+          const entriesObject = entries.reduce((obj, entry) => {
+            return { ...obj, [entry.addr.toLowerCase()]: entry.data }
+          }, {})
+          // ensure the entries retrieved from the first-installed address book aren't overwritten
+          return { ...entriesObject, ...allEntriesResolved }
+        },
+        Promise.resolve({})
+      )
     ).toPromise()
-
-    return addressBookApps.reduce(async (allEntries, app) => {
-      const cacheKey = getCacheKey(app.proxyAddress, 'state')
-      const { entries = [] } = await this.cache.get(cacheKey)
-      const allEntriesResolved = await allEntries
-      const entriesObject = entries.reduce((obj, entry) => {
-        return { ...obj, [entry.addr.toLowerCase()]: entry.data }
-      }, {})
-      // ensure the entries retrieved from the first-installed address book aren't overwritten
-      return { ...entriesObject, ...allEntriesResolved }
-    }, Promise.resolve({}))
   }
 
   /**

packages/aragon-wrapper/src/identity/AddressBookIdentityProvider.test.js

@@ -22,7 +22,7 @@ test.before(async t => {
 
   cache = new Cache('stubbedAddressBook')
   await cache.init()
-  cache.set('0x0.state', { entries: [{ addr: '0x3a', data: { name: 'testEntity' } }, { addr: '0x33', data: { name: 'testDude' } } ] })
+  cache.set('0x0.state', { entries: [{ addr: '0x3a', data: { name: 'testEntity' } }, { addr: '0x33', data: { name: 'testDude' } }] })
   cache.set('0x11.state', { entries: [{ addr: '0x3a', data: { name: 'testEntity2' } }] })
 })
 
@@ -52,21 +52,22 @@ test('getAll should return a combined Object containing all entries', async t =>
   const provider = t.context.addressBookIdentityProvider
   const allIdentities = await provider.getAll()
   t.deepEqual(allIdentities, {
-    '0x3a': { name: 'testEntity' }, 
+    '0x3a': { name: 'testEntity' },
     '0x33': { name: 'testDude' }
   })
 })
 
-test('search should return an aray of results of freely matching identities', async t => {
-  t.plan(3)
+test('search should return an array of results of freely matching identities', async t => {
   const provider = t.context.addressBookIdentityProvider
   let result = await provider.search('0x3a')
   t.deepEqual(result, [ { name: 'testEntity', address: '0x3a' } ])
 
   result = await provider.search('test')
-  t.deepEqual(result, [ { name: 'testEntity', address: '0x3a' },
-  { name: 'testDude', address: '0x33' } ])
+  t.deepEqual(result, [
+    { name: 'testEntity', address: '0x3a' },
+    { name: 'testDude', address: '0x33' }
+  ])
 
   result = await provider.search('testd')
-  t.deepEqual(result, [{ name: 'testDude', address: '0x33' } ])
+  t.deepEqual(result, [{ name: 'testDude', address: '0x33' }])
 })

packages/aragon-wrapper/src/index.js

@@ -15,7 +15,10 @@ import {
   startWith,
   switchMap,
   throttleTime,
-  withLatestFrom
+  withLatestFrom,
+  flatMap,
+  skipWhile,
+  defaultIfEmpty
 } from 'rxjs/operators'
 import Web3 from 'web3'
 import { isAddress } from 'web3-utils'
@@ -928,16 +931,19 @@ export default class Aragon {
    */
   resolveAddressIdentity (address) {
     const providerNames = [ 'local', 'addressBook' ] // TODO - get provider
-    return providerNames.reduce(async (identity, providerName, index) => {
-      if (await identity) {
-        return identity
-      }
-      const provider = this.identityProviderRegistrar.get(providerName)
-      if (provider && typeof provider.resolve === 'function') {
-        return provider.resolve(address)
-      }
-      return Promise.reject(new Error(`Provider (${providerName}) not installed`))
-    }, Promise.resolve(null))
+    return from(providerNames).pipe(
+      map(providerName => {
+        const provider = this.identityProviderRegistrar.get(providerName)
+        if (provider && typeof provider.resolve === 'function') {
+          return provider.resolve(address)
+        }
+        return Promise.reject(new Error(`Provider (${providerName}) not installed`))
+      }),
+      flatMap(unresolvedAddress => from(unresolvedAddress)),
+      skipWhile(entryData => !entryData),
+      defaultIfEmpty(null),
+      first()
+    ).toPromise()
   }
 
   /**
@@ -949,18 +955,18 @@ export default class Aragon {
   async searchIdentities (searchTerm) {
     const providerNames = [ 'local', 'addressBook' ] // TODO - get provider
     const resolvedResults = await Promise.all(
-      providerNames.map( (providerName) => {
+      providerNames.map(providerName => {
         const provider = this.identityProviderRegistrar.get(providerName)
         if (provider && typeof provider.search === 'function') {
           return provider.search(searchTerm)
         }
         return Promise.reject(new Error(`Provider (${providerName}) not installed`))
       })
     )
-    return  resolvedResults.reduce(
+    return resolvedResults.reduce(
       (combinedResults, providerResult) => {
-      return [ ...combinedResults, ...providerResult ]
-      }, 
+        return [ ...combinedResults, ...providerResult ]
+      },
       []
     )
   }

packages/aragon-wrapper/src/index.test.js

@@ -824,30 +824,30 @@ test('should search identities correctly', async (t) => {
     }
   ])
   await instance.cache.init()
-  await instance.cache.set('0x001.state', { 
+  await instance.cache.set('0x001.state', {
     entries: [
-      { addr: '0x789', data: { name: 'testEntity' } }, 
-      { addr: '0x456', data: { name: 'testDude' } } 
-    ] 
+      { addr: '0x789', data: { name: 'testEntity' } },
+      { addr: '0x456', data: { name: 'testDude' } }
+    ]
   })
   await instance.initIdentityProviders()
 
   // act
   await instance.modifyAddressIdentity('0x123', { name: 'testperson' })
   await instance.modifyAddressIdentity('0x456', { name: 'testDao' })
   // assert
-  //console.log(await instance.cache.get('0x001.state'))
   let result = await instance.searchIdentities('test')
-
-  t.deepEqual(result, [ { name: 'testperson', address: '0x123' },
-  { name: 'testDao', address: '0x456' },
-  { name: 'testEntity', address: '0x789' },
-  { name: 'testDude', address: '0x456' } ])
+
+  t.deepEqual(result, [
+    { name: 'testperson', address: '0x123' },
+    { name: 'testDao', address: '0x456' },
+    { name: 'testEntity', address: '0x789' },
+    { name: 'testDude', address: '0x456' } ])
 
   result = await instance.searchIdentities('0x456')
-  t.deepEqual(result, [ 
+  t.deepEqual(result, [
     { name: 'testDao', address: '0x456' },
-    { name: 'testDude', address: '0x456' } 
+    { name: 'testDude', address: '0x456' }
   ])
 })
 
@@ -867,23 +867,22 @@ test('should resolve identity correctly', async (t) => {
     }
   ])
   await instance.cache.init()
-  await instance.cache.set('0x001.state', { 
+  await instance.cache.set('0x001.state', {
     entries: [
-      { addr: '0x789', data: { name: 'testEntity' } }, 
-      { addr: '0x456', data: { name: 'testDude' } } 
-    ] 
+      { addr: '0x789', data: { name: 'testEntity' } },
+      { addr: '0x456', data: { name: 'testDude' } }
+    ]
   })
   await instance.initIdentityProviders()
 
   // act
   await instance.modifyAddressIdentity('0x123', { name: 'testperson' })
   await instance.modifyAddressIdentity('0x456', { name: 'testDao' })
   // assert
-  //console.log(await instance.cache.get('0x001.state'))
   let result = await instance.resolveAddressIdentity('0x456')
   console.log('wrapper result: ', result)
   t.is(result.name, 'testDao', 'should resolve to local label')
-  
+
   result = await instance.resolveAddressIdentity('0x789')
   t.is(result.name, 'testEntity', 'should resolve to address book entry')
 })