Make sure the contact/room/message isReady()-ed before `findAll()` return (#1545)

src/user/contact.spec.ts

+#!/usr/bin/env ts-node
+/**
+ *   Wechaty - https://github.com/chatie/wechaty
+ *
+ *   @copyright 2016-2018 Huan LI <zixia@zixia.net>
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ *
+ */
+// tslint:disable:no-shadowed-variable
+// tslint:disable:max-classes-per-file
+
+import test  from 'blue-tape'
+import sinon from 'sinon'
+
+import { PuppetMock } from 'wechaty-puppet-mock'
+
+import { Wechaty }    from '../wechaty'
+
+test('findAll()', async t => {
+  const EXPECTED_CONTACT_ID      = 'test-id'
+  const EXPECTED_CONTACT_NAME    = 'test-name'
+  const EXPECTED_CONTACT_ID_LIST = [EXPECTED_CONTACT_ID]
+
+  const sandbox = sinon.createSandbox()
+
+  const puppet = new PuppetMock()
+  const wechaty = new Wechaty({ puppet })
+
+  await wechaty.start()
+
+  sandbox.stub(puppet, 'contactSearch').resolves(EXPECTED_CONTACT_ID_LIST)
+  sandbox.stub(puppet, 'contactPayload').callsFake(async () => {
+    await new Promise(r => setImmediate(r))
+    return {
+      name: EXPECTED_CONTACT_NAME,
+    }
+  })
+
+  const contactList = await wechaty.Contact.findAll()
+  t.equal(contactList.length, 1, 'should find 1 contact')
+  t.equal(contactList[0].name(), EXPECTED_CONTACT_NAME, 'should get name from payload')
+
+  await wechaty.stop()
+})

src/user/contact.ts

@@ -200,15 +200,15 @@ export class Contact extends Accessory implements Sayable {
    * @example
    * const bot = new Wechaty()
    * await bot.start()
-   * const contactList = await bot.Contact.findAll()                    // get the contact list of the bot
-   * const contactList = await bot.Contact.findAll({name: 'ruirui'})    // find allof the contacts whose name is 'ruirui'
-   * const contactList = await bot.Contact.findAll({alias: 'lijiarui'}) // find all of the contacts whose alias is 'lijiarui'
+   * const contactList = await bot.Contact.findAll()                      // get the contact list of the bot
+   * const contactList = await bot.Contact.findAll({ name: 'ruirui' })    // find allof the contacts whose name is 'ruirui'
+   * const contactList = await bot.Contact.findAll({ alias: 'lijiarui' }) // find all of the contacts whose alias is 'lijiarui'
    */
   public static async findAll<T extends typeof Contact> (
     this  : T,
     query? : string | ContactQueryFilter,
   ): Promise<Array<T['prototype']>> {
-    log.verbose('Contact', 'findAll(%s)', JSON.stringify(query))
+    log.verbose('Contact', 'findAll(%s)', JSON.stringify(query) || '')
 
     if (query && Object.keys(query).length !== 1) {
       throw new Error('query only support one key. multi key support is not availble now.')
@@ -221,7 +221,7 @@ export class Contact extends Accessory implements Sayable {
       const BATCH_SIZE = 16
       let   batchIndex = 0
 
-      const invalidContactId: string[] = []
+      const invalidDict: { [id: string]: true } = {}
 
       while (batchIndex * BATCH_SIZE < contactList.length) {
         const batchContactList = contactList.slice(
@@ -230,20 +230,18 @@ export class Contact extends Accessory implements Sayable {
         )
         await Promise.all(
           batchContactList.map(
-            c => {
-              c.ready()
-              .catch(e => {
-                log.error('Contact', 'findAll() ready() exception: %s', e.message)
-                invalidContactId.push(c.id)
-              })
-            },
+            c => c.ready()
+                  .catch(e => {
+                    log.error('Contact', 'findAll() ready() exception: %s', e.message)
+                    invalidDict[c.id] = true
+                  }),
           ),
         )
 
         batchIndex++
       }
 
-      return contactList.filter(contact => !(contact.id in invalidContactId))
+      return contactList.filter(contact => !invalidDict[contact.id])
 
     } catch (e) {
       log.error('Contact', 'this.puppet.contactFindAll() rejected: %s', e.message)

src/user/message.ts

@@ -110,7 +110,7 @@ export class Message extends Accessory implements Sayable {
     this       : T,
     userQuery? : MessageUserQueryFilter,
   ): Promise<Array<T['prototype']>> {
-    log.verbose('Message', 'findAll(%s)', JSON.stringify(userQuery))
+    log.verbose('Message', 'findAll(%s)', JSON.stringify(userQuery) || '')
 
     let puppetQuery: undefined | MessageQueryFilter
 
@@ -124,22 +124,22 @@ export class Message extends Accessory implements Sayable {
       }
     }
 
+    const invalidDict: { [id: string]: true } = {}
+
     try {
       const MessageIdList = await this.puppet.messageSearch(puppetQuery)
       const messageList = MessageIdList.map(id => this.load(id))
       await Promise.all(
         messageList.map(
-          message => {
-            try {
-              return message.ready()
-            } catch (e) {
-              return {} as any
-            }
-          },
+          message => message.ready()
+                            .catch(e => {
+                              log.warn('Room', 'findAll() message.ready() rejection: %s', e)
+                              invalidDict[message.id] = true
+                            })
         ),
       )
 
-      return messageList
+      return messageList.filter(message => !invalidDict[message.id])
 
     } catch (e) {
       log.warn('Message', 'findAll() rejected: %s', e.message)

src/user/room.spec.ts

-// test('Room iterator for contact in it', async t => {
-//   // Mock
-//   const mockContactRoomRawPayload = (id: string) => {
-//     log.verbose('PuppeteerRoomTest', 'mockContactRawPayload(%s)', id)
-//     return new Promise(resolve => {
-//       if (id === ROOM_EXPECTED.id) {
-//         setImmediate(() => resolve(ROOM_RAW_PAYLOAD))
-//       } else if (id in CONTACT_RAW_PAYLOAD_DICT) {
-//         setImmediate(() => resolve(CONTACT_RAW_PAYLOAD_DICT[id]))
-//       } else {
-//         // ignore other ids
-//         setImmediate(() => resolve({ id }))
-//       }
-//     })
-//   }
-
-//   const sandbox = sinon.createSandbox()
-
-//   const puppet = new PuppetPuppeteer()
-
-//   sandbox.stub(puppet, 'contactRawPayload').callsFake(mockContactRoomRawPayload)
-//   sandbox.stub(puppet, 'roomRawPayload').callsFake(mockContactRoomRawPayload)
-
-//   const roomPayload = await puppet.roomPayload(ROOM_EXPECTED.id)
-
-//   const MEMBER_CONTACT_ID_LIST = ROOM_RAW_PAYLOAD.MemberList!.map(rawMember => rawMember.UserName)
-
-//   let n = 0
-//   for await (const memberContact of room) {
-//     t.ok(MEMBER_CONTACT_ID_LIST.includes(memberContact.id), 'should get one of the room member: ' + memberContact.id)
-//     n++
-//   }
-
-//   const memberList = await room.memberList()
-//   t.equal(n, memberList.length, 'should iterate all the members of the room')
-// })
+#!/usr/bin/env ts-node
+/**
+ *   Wechaty - https://github.com/chatie/wechaty
+ *
+ *   @copyright 2016-2018 Huan LI <zixia@zixia.net>
+ *
+ *   Licensed under the Apache License, Version 2.0 (the "License");
+ *   you may not use this file except in compliance with the License.
+ *   You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *   Unless required by applicable law or agreed to in writing, software
+ *   distributed under the License is distributed on an "AS IS" BASIS,
+ *   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *   See the License for the specific language governing permissions and
+ *   limitations under the License.
+ *
+ */
+// tslint:disable:no-shadowed-variable
+// tslint:disable:max-classes-per-file
+
+import test  from 'blue-tape'
+import sinon from 'sinon'
+
+import { PuppetMock } from 'wechaty-puppet-mock'
+
+import { Wechaty }    from '../wechaty'
+
+test('findAll()', async t => {
+  const EXPECTED_ROOM_ID      = 'test-id'
+  const EXPECTED_ROOM_TOPIC   = 'test-topic'
+  const EXPECTED_ROOM_ID_LIST = [EXPECTED_ROOM_ID]
+
+  const sandbox = sinon.createSandbox()
+
+  const puppet = new PuppetMock()
+  const wechaty = new Wechaty({ puppet })
+
+  await wechaty.start()
+
+  sandbox.stub(puppet, 'roomSearch').resolves(EXPECTED_ROOM_ID_LIST)
+  sandbox.stub(puppet, 'roomPayload').callsFake(async () => {
+    await new Promise(r => setImmediate(r))
+    return {
+      topic: EXPECTED_ROOM_TOPIC,
+    }
+  })
+
+  const roomList = await wechaty.Room.findAll()
+  t.equal(roomList.length, 1, 'should find 1 room')
+  t.equal(await roomList[0].topic(), EXPECTED_ROOM_TOPIC, 'should get topic from payload')
+
+  await wechaty.stop()
+})

src/user/room.ts

@@ -132,28 +132,28 @@ export class Room extends Accessory implements Sayable {
     this  : T,
     query : RoomQueryFilter = { topic: /.*/ },
   ): Promise<Array<T['prototype']>> {
-    log.verbose('Room', 'findAll(%s)', JSON.stringify(query))
+    log.verbose('Room', 'findAll(%s)', JSON.stringify(query) || '')
 
     if (!query.topic) {
       throw new Error('topicFilter not found')
     }
 
+    const invalidDict: { [id: string]: true } = {}
+
     try {
       const roomIdList = await this.puppet.roomSearch(query)
       const roomList = roomIdList.map(id => this.load(id))
       await Promise.all(
         roomList.map(
-          room => {
-            try {
-              return room.ready()
-            } catch (e) {
-              return {} as any
-            }
-          },
+          room => room.ready()
+                      .catch(e => {
+                        log.warn('Room', 'findAll() room.ready() rejection: %s', e)
+                        invalidDict[room.id] = true
+                      })
         ),
       )
 
-      return roomList
+      return roomList.filter(room => !invalidDict[room.id])
 
     } catch (e) {
       log.verbose('Room', 'findAll() rejected: %s', e.message)