fix wait_for for delete search (#1399)

This PR modifies current behaviour to use `wait_for_completion` instead of refresh to wait for the completion of the task. If it's set to `true` it returns the count of documents that match the request.

src/ctia/http/routes/crud.clj

@@ -424,7 +424,7 @@
                         (store/delete-search
                           query
                           identity-map
-                          (wait_for->refresh (:wait_for params))))
+                          {:wait_for_completion (boolean (:wait_for params))}))
                     (-> (get-store entity)
                         (store/query-string-count
                           query

src/ctia/stores/es/crud.clj

@@ -652,12 +652,19 @@ It returns the documents with full hits meta data including the real index in wh
    search-query :- SearchQuery
    ident
    es-params]
-  (let [query (make-search-query es-conn-state search-query ident)]
+  (let [query (make-search-query es-conn-state search-query ident)
+        opts (select-keys es-params [:wait_for_completion :refresh])
+        nb-deleted (ductile.doc/count-docs conn index query)]
+    (log/info (format "deleting %s documents in %s (%s)"
+                      nb-deleted
+                      index
+                      (pr-str query)))
     (:deleted
      (ductile.doc/delete-by-query conn
                                   [index]
                                   query
-                                  (prepare-opts es-conn-state es-params)))))
+                                  opts)
+     nb-deleted)))
 
 (s/defn handle-query-string-count :- (s/pred nat-int?)
   "ES count handler"

test/ctia/entity/incident_test.clj

@@ -114,6 +114,7 @@
                             {:app app
                              :patch-tests? true
                              :search-tests? true
+                             :delete-search-tests? true
                              :example (assoc new-incident-maximal
                                              :meta
                                              {:ai-generated-description true})

test/ctia/stores/es/crud_test.clj

@@ -839,7 +839,7 @@
                                            :user (:login ident-1)
                                            :tlp  "amber"))
                           search-metrics-entities)
-                check-fn (fn [{:keys [msg query matched ident deleted?]}]
+                check-fn (fn [{:keys [msg query matched ident deleted? wait_for_completion]}]
                            (create-fn es-conn-state
                                       data
                                       ident-1
@@ -850,33 +850,58 @@
                                      es-conn-state
                                      query
                                      ident
-                                     {}))
+                                     {:wait_for_completion wait_for_completion
+                                      :refresh "true"}))
                                  "the number of deleted entities shall be equal to the number of matched")
-                             (is (= (if deleted? 0 (count matched))
-                                    (count-fn es-conn-state query ident))
-                                 "only matched entities shall be deleted")))]
+                             (let [remaining (count-fn es-conn-state query ident)]
+                                (cond
+                                  (not deleted?)
+                                  (is (= remaining (count matched))
+                                      "only matched entities shall be deleted")
+
+                                  (true?  wait_for_completion)
+                                  (is (= remaining 0))
+
+                                  (false?  wait_for_completion)
+
+                                  (do (is (<= remaining (count matched)))
+                                      (Thread/sleep 1000)
+                                      (count-fn es-conn-state query ident))))))]
             (check-fn
              {:msg "queries that does not match anything shall not delete any data."
               :query {:filter-map {:title "DOES NOT MATCH ANYTHING"}}
               :matched []
               :ident ident-1
-              :deleted? false})
+              :deleted? false
+              :wait_for_completion true})
 
             (check-fn
              {:msg "user can only delete the data they have access to."
               :query {:full-text [{:query "title1"}]
                       :filter-map {:confidence "High"}}
               :matched []
               :ident ident-2
-              :deleted? false})
+              :deleted? false
+              :wait_for_completion true})
 
             (check-fn
              {:msg "matched entities must be properly deleted"
               :query {:full-text [{:query "title1"}]
                       :filter-map {:confidence "High"}}
               :matched high-t1-title1
               :ident ident-1
-              :deleted? true}))))))
+              :deleted? true
+              :wait_for_completion true})
+
+            (check-fn
+             {:msg "when wait_for_completion is false, we still return the number of matched entities"
+              :query {:full-text [{:query "title1"}]
+                      :filter-map {:confidence "High"}}
+              :matched high-t1-title1
+              :ident ident-1
+              :deleted? true
+              :wait_for_completion false})
+            )))))
 
 (deftest docs-with-indices-test
   (es-helpers/for-each-es-version

test/ctia/test_helpers/access_control.clj

@@ -93,7 +93,7 @@
                               :REALLY_DELETE_ALL_THESE_ENTITIES true
                               :wait_for true}
                :headers {"Authorization" "player-2-token"})
-
+        _ (Thread/sleep 1000)
         ;; re-count
         player-1-entity-count-2 (search-count "player-1-token")
         player-2-entity-count-2 (search-count "player-2-token")

test/ctia/test_helpers/crud.clj

@@ -184,16 +184,18 @@
                                        (DELETE app
                                                path
                                                :headers headers))
-                                     (check-refresh wait_for msg)))]
+                                     (let [expected (str "wait_for_completion=" (boolean wait_for))]
+                                       (is (some-> @es-params (string/includes? expected))
+                                           (format "%s (expected %s, actual: %s)" msg expected @es-params)))
+                                     (vreset! es-params nil)))]
           (test-delete-search true
-                              (str "Delete search should wait for index refresh when "
+                              (str "Delete search should wait for deletion completion when "
                                    "wait_for is true"))
           (test-delete-search false
-                              (str "Delete search should not wait for index refresh "
+                              (str "Delete search should not wait for deletion completion "
                                    "when wait_for is false"))
           (test-delete-search nil
-                              (str "Configured ctia.store.es.default.refresh value is "
-                                   "applied when wait_for is not specified")))))))
+                              (str "Delete search should not wait for deletion completion by default")))))))
 
 (defn entity-crud-test
   [{:keys [app
@@ -374,7 +376,7 @@
         (th.search/test-delete-search
          {:app        app
           :entity     entity-str
-          :bundle-key (keyword (string/replace plural #"-" "_"))
+          :bundle-key (keyword (string/replace (name plural) #"-" "_"))
           :example    example}))
       (when invalid-tests?
         (testing (format "POST invalid /ctia/%s :schema_version should be ignored" entity-str)

test/ctia/test_helpers/search.clj

@@ -32,11 +32,13 @@
 
 (defn delete-search
   [app entity query-params]
-  (let [delete-search-uri (format "ctia/%s/search" (name entity))]
-    (DELETE app
-            delete-search-uri
-            :headers {"Authorization" "45c1f5e3f05d0"}
-            :query-params query-params)))
+  (let [delete-search-uri (format "ctia/%s/search" (name entity))
+        res (DELETE app
+                delete-search-uri
+              :headers {"Authorization" "45c1f5e3f05d0"}
+              :query-params query-params)]
+    (Thread/sleep 1000)
+    res))
 
 (defn search-raw
   [app entity query-params]
@@ -341,15 +343,17 @@
 (defn test-delete-search
   [{:keys [app entity bundle-key example]}]
   (let [docs (->> (dissoc example :id)
-                  (repeat 100)
+                  (repeat 10)
                   (map #(assoc % :tlp (rand-nth ["green" "amber" "red"]))))
         {green-docs "green"
          amber-docs "amber"
          red-docs "red"} (group-by :tlp docs)
         count-fn #(:parsed-body (count-raw app entity %))
         delete-search-fn (fn [q confirm?]
                            (let [query (if (boolean? confirm?)
-                                         (assoc q :REALLY_DELETE_ALL_THESE_ENTITIES confirm?)
+                                         (into {:REALLY_DELETE_ALL_THESE_ENTITIES confirm?
+                                                :wait_for true}
+                                               q)
                                          q)]
                              (-> (delete-search app entity query)
                                  :body
@@ -382,7 +386,8 @@
              (count-fn filter-red))))
     (testing "delete with :REALLY_DELETE_ALL_THESE_ENTITIES set to true must really delete entities"
       (is (= (count green-docs)
-             (delete-search-fn filter-green true)))
+             (delete-search-fn (assoc filter-green :wait_for false)
+                               true)))
       (is (= (count amber-docs)
              (delete-search-fn filter-amber true)))
       (is (= (count red-docs)