Merge #543

543: Simplify task fetching to avoid timeout in strapi v3 r=bidoubiwa a=bidoubiwa

Tests in cypress were failing because of a weird tasks listener. 

I simplified it by removing the complex system to listen to tasks and replacing it with a setInterval running until no tasks are left enqueued or processing.





Co-authored-by: Charlotte Vermandel <[email protected]>
Co-authored-by: cvermand <[email protected]>

Author: meili-bors[bot]

admin/src/components/Collections.js

@@ -21,92 +21,18 @@ import { headers } from '../utils/collection-header'
  */
 const Collections = ({ updateCredentials }) => {
   const [collectionsList, setCollectionsList] = useState([]) // All Collections
-  const [upToDateCollections, setUpToDateCollection] = useState(false) // Boolean that informs if collections have been updated.
   const [needReload, setNeedReload] = useState(false) // Boolean to inform that reload is requested.
-  const [collectionInWaitMode, setCollectionInWaitMode] = useState([]) // Collections that are waiting for their indexation to complete.
-  const [collectionTaskUids, setCollectionTaskUids] = useState({}) // List of collection's enqueued task uids.
+  const [realTimeReports, setRealTimeReports] = useState(false) // List of collection's enqueued task uids.
+  const [refetchIndex, setRefetchIndex] = useState(true)
 
-  // Trigger a task uids fetcher to find enqueued task of the indexed collections.
-  useEffect(() => {
-    findTaskUids()
-  }, [])
+  const refetchCollection = () =>
+    setRefetchIndex(prevRefetchIndex => !prevRefetchIndex)
 
   // Adds a listener that informs if collections have been updated.
   useEffect(() => {
-    setUpToDateCollection(false)
+    refetchCollection()
   }, [updateCredentials])
 
-  // Adds a listener that updates collections informations on updates
-  useEffect(() => {
-    if (!upToDateCollections) fetchCollections()
-  }, [upToDateCollections, updateCredentials])
-
-  // Trigger a watch if a collection has enqueued task uid's.
-  useEffect(() => {
-    for (const collection in collectionTaskUids) {
-      if (collectionTaskUids[collection].length > 0) {
-        watchTasks({ collection })
-      }
-    }
-  }, [collectionTaskUids])
-
-  /**
-   * Find all enqueued task uid's of the indexed collections.
-   * It is triggered on load.
-   */
-  const findTaskUids = async () => {
-    const response = await request(`/${pluginId}/collection/tasks`, {
-      method: 'GET',
-    })
-
-    if (response.error) errorNotifications(response)
-    setCollectionTaskUids(response.taskUids)
-  }
-
-  /**
-   * Watches a collection (if not already)
-   * For a maximum of 5 enqueued tasks in Meilisearch.
-   *
-   * @param {string} collection - Collection name.
-   */
-  const watchTasks = async ({ collection }) => {
-    // If collection has pending tasks
-    const taskUids = collectionTaskUids[collection]
-
-    if (!collectionInWaitMode.includes(collection) && taskUids?.length > 0) {
-      addIndexedStatus
-      setCollectionInWaitMode(prev => [...prev, collection])
-
-      const taskUidsIdsChunk = taskUids.splice(0, 1)
-      const response = await request(
-        `/${pluginId}/collection/${collection}/tasks/batch`,
-        {
-          method: 'POST',
-          body: { taskUids: taskUidsIdsChunk },
-        }
-      )
-
-      if (response.error) errorNotifications(response)
-
-      const { tasksStatus } = response
-
-      tasksStatus.map(task => {
-        if (task.status === 'failed') {
-          task.error.message = `Some documents could not be added: \n${task.error.message}`
-          errorNotifications(task.error)
-        }
-      })
-
-      setCollectionInWaitMode(prev => prev.filter(col => col !== collection))
-      setCollectionTaskUids(prev => ({
-        ...prev,
-        [collection]: taskUids,
-      }))
-
-      setUpToDateCollection(false) // Ask for collections to be updated.
-    }
-  }
-
   /**
    * Add a collection to Meilisearch
    *
@@ -122,14 +48,7 @@ const Collections = ({ updateCredentials }) => {
 
     createResponseNotification(response, `${collection} is created!`)
 
-    if (!response.error) {
-      setCollectionTaskUids(prev => ({
-        ...prev,
-        [collection]: response.taskUids,
-      }))
-    }
-
-    setUpToDateCollection(false) // Ask for collections to be updated.
+    refetchCollection()
   }
 
   /**
@@ -147,14 +66,7 @@ const Collections = ({ updateCredentials }) => {
 
     createResponseNotification(response, `${collection} update started!`)
 
-    if (!response.error) {
-      setCollectionTaskUids(prev => ({
-        ...prev,
-        [collection]: response.taskUids,
-      }))
-    }
-
-    setUpToDateCollection(false) // Ask for collections to be updated.
+    refetchCollection()
   }
 
   /**
@@ -172,7 +84,7 @@ const Collections = ({ updateCredentials }) => {
       `${collection} collection is removed from Meilisearch!`
     )
 
-    setUpToDateCollection(false) // Ask for collections to be updated.
+    refetchCollection()
   }
 
   /**
@@ -200,12 +112,9 @@ const Collections = ({ updateCredentials }) => {
 
     if (error) errorNotifications(res)
     else {
-      // Start watching collections that have pending tasks
-      collections.map(col => {
-        if (col.isIndexing) {
-          watchTasks({ collection: col.collection })
-        }
-      })
+      const isIndexing = collections.find(col => col.isIndexing === true)
+
+      setRealTimeReports(isIndexing)
 
       // Transform collections information to verbose string.
       const renderedCols = collections.map(col => transformCollections(col))
@@ -217,10 +126,26 @@ const Collections = ({ updateCredentials }) => {
 
       setNeedReload(reloading) // A reload is required for a collection to be listened or de-listened
       setCollectionsList(renderedCols) // Store all `Strapi collections
-      setUpToDateCollection(true) // Collection information is up to date
     }
   }
 
+  // Start refreshing the collections when a collection is being indexed
+  useEffect(() => {
+    let interval
+    if (realTimeReports) {
+      interval = setInterval(() => {
+        refetchCollection()
+      }, 1000)
+    } else {
+      clearInterval(interval)
+    }
+    return () => clearInterval(interval)
+  }, [realTimeReports])
+
+  useEffect(() => {
+    fetchCollections()
+  }, [refetchIndex])
+
   return (
     <div className="col-md-12">
       <Wrapper>

config/routes.json

@@ -32,22 +32,6 @@
         "policies": ["isAdmin"]
       }
     },
-    {
-      "method": "POST",
-      "path": "/collection/:collection/tasks/batch",
-      "handler": "meilisearch.waitForTasks",
-      "config": {
-        "policies": ["isAdmin"]
-      }
-    },
-    {
-      "method": "GET",
-      "path": "/collection/tasks",
-      "handler": "meilisearch.getTaskUids",
-      "config": {
-        "policies": ["isAdmin"]
-      }
-    },
     {
       "method": "POST",
       "path": "/collections/:collection",

connectors/meilisearch/index.js

@@ -66,49 +66,6 @@ module.exports = async ({ storeConnector, collectionConnector }) => {
       await client.index(indexUid).deleteDocuments(documentsIds)
     },
 
-    /**
-     * Wait for an task to be processed in Meilisearch.
-     *
-     * @param  {string} collection - Collection name.
-     * @param  {number} taskUid - Task identifier.
-     *
-     * @returns {{Record<string, string>}} - Task body returned by Meilisearch API.
-     */
-    waitForTask: async function ({ collection, taskUid }) {
-      try {
-        const client = MeiliSearch({ apiKey, host })
-        const indexUid = collectionConnector.getIndexName(collection)
-        const task = await client
-          .index(indexUid)
-          .waitForTask(taskUid, { intervalMs: 5000 })
-
-        return task
-      } catch (e) {
-        console.error(e)
-        return 0
-      }
-    },
-
-    /**
-     * Wait for a batch of tasks uids to be processed.
-     *
-     * @param  {string} collection - Collection name.
-     * @param  {number[]} taskUids - Array of tasks identifiers.
-     *
-     * @returns { Record<string, string>[] } - List of all tasks returned by Meilisearch API.
-     */
-    waitForTasks: async function ({ collection, taskUids }) {
-      const tasks = []
-      for (const taskUid of taskUids) {
-        const status = await this.waitForTask({
-          collection,
-          taskUid,
-        })
-        tasks.push(status)
-      }
-      return tasks
-    },
-
     /**
      * Get indexes with a safe guard in case of error.
      *
@@ -117,6 +74,7 @@ module.exports = async ({ storeConnector, collectionConnector }) => {
     getIndexes: async function () {
       try {
         const client = MeiliSearch({ apiKey, host })
+
         const { results } = await client.getIndexes()
         return results
       } catch (e) {
@@ -141,32 +99,6 @@ module.exports = async ({ storeConnector, collectionConnector }) => {
       }
     },
 
-    /**
-     * Get enqueued tasks ids of indexed collections.
-     *
-     * @returns { { string: number[] } } - Collections with their respective task uids
-     */
-    getTaskUids: async function () {
-      const indexes = await this.getIndexes()
-      const indexUids = indexes.map(index => index.uid)
-      const collections = collectionConnector.allEligbleCollections()
-      const client = MeiliSearch({ apiKey, host })
-
-      const collectionTaskUids = {}
-      for (const collection of collections) {
-        const indexUid = collectionConnector.getIndexName(collection)
-        if (indexUids.includes(indexUid)) {
-          const { results: tasks } = await client.index(indexUid).getTasks()
-
-          const enqueueded = tasks
-            .filter(task => task.status === 'enqueued')
-            .map(task => task.uid)
-          collectionTaskUids[collection] = enqueueded
-        }
-      }
-      return collectionTaskUids
-    },
-
     /**
      * Information about collections in Meilisearch.
      *

controllers/meilisearch.js

@@ -130,37 +130,9 @@ async function addCollection(ctx) {
   const connector = await createConnector()
   const { collection } = ctx.params
   const taskUids = await connector.addCollectionInMeiliSearch(collection)
-  return { message: 'Index created', taskUids }
-}
-
-/**
- * Wait for one collection to be completely indexed in Meilisearch.
- *
- * @param  {object} ctx - Http request object.
- *
- * @returns { numberOfDocumentsIndexed: number }
- */
-async function waitForTasks(ctx) {
-  const connector = await createConnector()
-  const { collection } = ctx.params
-  const { taskUids } = ctx.request.body
-  const tasksStatus = await connector.waitForTasks({
-    taskUids,
-    collection,
-  })
-  return { tasksStatus }
-}
+  console.log({ taskUids })
 
-/**
- * Wait for one collection to be completely indexed in Meilisearch.
- *
- * @returns { taskUids: number[] }
- */
-async function getTaskUids() {
-  const connector = await createConnector()
-  const taskUids = await connector.getTaskUids()
-
-  return { taskUids }
+  return { message: 'Index created', taskUids }
 }
 
 /**
@@ -180,7 +152,5 @@ module.exports = {
   addCredentials: async ctx => ctxWrapper(ctx, addCredentials),
   removeCollection: async ctx => ctxWrapper(ctx, removeCollection),
   updateCollections: async ctx => ctxWrapper(ctx, updateCollections),
-  waitForTasks: async ctx => ctxWrapper(ctx, waitForTasks),
-  getTaskUids: async ctx => ctxWrapper(ctx, getTaskUids),
   reload: async ctx => ctxWrapper(ctx, reload),
 }