Fix data propagation for collections in xargs (#112)

* Fix data propagation for collections in xargs

* Fixup tests for xargs behavior

src/yetibot/core/commands/collections.clj

@@ -167,24 +167,34 @@
      :result/data-collection data-collection}
     (if-let [itms (or (ensure-items-collection opts)
                       (ensure-items-collection data-collection))]
-      (let [cat (-> (str args " " (first opts))
+      (let [indexed-itms (map-indexed (comp identity vector) itms)
+            cat (-> (str args " " (first opts))
                     command-execution-info :matched-sub-cmd meta :yb/cat)
+            ;; do not propagate top-level data or data-collection args to the
+            ;; cmd that xargs is running for each item
+            params (dissoc cmd-params :data :data-collection)
             cmd-runner (if (contains? cat :async) 'map 'pmap)
             _ (debug "xargs using cmd-runner:" cmd-runner "for command"
                      (pr-str args))
             xargs-results
             ((resolve cmd-runner)
-             (fn [item]
+             (fn [[idx item]]
                (try
-                 (let [cmd-result
-                       (apply handle-cmd
-                              ;; item could be a collection, such as when xargs
-                              ;; is used on nested collections, e.g.:
-                              ;; repeat 5 jargon | xargs words | xargs head
-                              (if (coll? item)
-                                [args (merge cmd-params {:raw item :opts item})]
-                                [(psuedo-format args item)
-                                 (merge cmd-params {:raw item :opts nil})]))
+                 (let [params-with-data (if data-collection
+                                          (assoc params
+                                                 :data (nth data-collection
+                                                            idx)))
+                       _ (info "data for" idx (nth data-collection idx))
+                       cmd-result
+                       (apply
+                        handle-cmd
+                        ;; item could be a collection, such as when xargs is
+                        ;; used on nested collections, e.g.:
+                        ;; repeat 5 jargon | xargs words | xargs head
+                        (if (coll? item)
+                          [args (merge params-with-data {:raw item :opts item})]
+                          [(psuedo-format args item)
+                           (merge params-with-data {:raw item :opts nil})]))
 
                        _ (debug "xargs cmd-result" (pr-str cmd-result))]
                    (if (map? cmd-result)
@@ -197,7 +207,7 @@
                    (error "Exception in xargs cmd-runner:" cmd-runner
                           (format-exception-log ex))
                    ex)))
-             itms)]
+             indexed-itms)]
         {:result/value (map :result/value xargs-results)
          :result/data-collection (map :result/data-collection xargs-results)
          :result/data (map :result/data xargs-results)})

src/yetibot/core/commands/render.clj

@@ -44,10 +44,10 @@
 
    See https://github.com/yogthos/Selmer for docs on templating."
   [{data :data match :match raw :raw :as args}]
-  (info "render-cmd" match raw)
+  (info "render-cmd" {:data data :match match :raw raw})
   (try
     (let [template (if (and raw (not (coll? raw)))
-                     (string/replace match raw "") ;; remove piped args
+                     (string/replace match (str " " raw) "") ;; remove piped args
                      match)
           rendered (if (sequential? data)
                      (map (partial render template) data)

test/yetibot/core/test/commands/collections.clj

@@ -1,10 +1,12 @@
 (ns yetibot.core.test.commands.collections
   (:require
-    [yetibot.core.commands.collections :refer :all]
-    yetibot.core.commands.about
-    [taoensso.timbre :refer [info]]
-    [yetibot.core.util.command-info :refer [command-execution-info]]
-    [clojure.test :refer :all]))
+   [yetibot.core.commands.collections :refer :all]
+   yetibot.core.commands.render
+   yetibot.core.commands.about
+   yetibot.core.commands.echo
+   [taoensso.timbre :refer [info]]
+   [yetibot.core.util.command-info :refer [command-execution-info]]
+   [clojure.test :refer :all]))
 
 (deftest random-test
   (testing "Random with no args"
@@ -217,30 +219,35 @@
   (testing "xargs still works on simple commands that don't return a map"
     (is (= ["value is red" "value is green" "value is blue"]
            (-> (command-execution-info
-                            ;; only matches "red" and
-                            ;; "green"
+                 ;; only matches "red" and "green"
                 "xargs echo value is" params)
                :result
                :result/value))))
 
   (testing "xargs accumulates and propagates data when it exists"
-    (is (= #:result{:value ["red" "green" "blue"],
-                    :data-collection
-                    [[{"red" "red"} {"green" "green"} {"blue" "blue"}]
-                     [{"red" "red"} {"green" "green"} {"blue" "blue"}]
-                     [{"red" "red"} {"green" "green"} {"blue" "blue"}]],
-                    :data
-                    [{:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                      :count 3}
-                     {:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                      :count 3}
-                     {:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                      :count 3}]}
-           (-> (command-execution-info
+    (is (=
+         (-> (command-execution-info
                  ;; only matches "red" and
                  ;; "green"
-                "xargs trim" params)
-               :result))))
+              "xargs trim" params)
+             :result)
+         #:result{:value ["red" "green" "blue"],
+                  :data-collection [nil nil nil],
+                  :data [{"red" "red"} {"green" "green"} {"blue" "blue"}]})))
+
+  (testing
+   "xargs should properly propagate data for each item when data-collection is
+    present"
+    (is
+     (= (-> (command-execution-info
+             "xargs render {{name}}"
+             {:data [{:name "foo"} {:name "bar"} {:name "qux"}]
+              :data-collection [{:name "foo"} {:name "bar"} {:name "qux"}]
+              :opts ["foo" "bar" "qux"]
+              :run-command? true})
+            :result
+            :result/value)
+        ["foo" "bar" "qux"])))
 
   (testing "xargs falls back to data if opts not passed in"
     (is
@@ -251,14 +258,5 @@
            (-> params (dissoc :opts)))
           :result)
       #:result{:value [["red"] ["green"] ["blue"]],
-               :data-collection
-               [[{"red" "red"} {"green" "green"} {"blue" "blue"}]
-                [{"red" "red"} {"green" "green"} {"blue" "blue"}]
-                [{"red" "red"} {"green" "green"} {"blue" "blue"}]],
-               :data
-               [{:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                 :count 3}
-                {:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                 :count 3}
-                {:items [{"red" "red"} {"green" "green"} {"blue" "blue"}],
-                 :count 3}]}))))
+               :data-collection [nil nil nil],
+               :data [{"red" "red"} {"green" "green"} {"blue" "blue"}]}))))