From 63027dc75d370ac38c0e5030bd72d68c7eeea3f9 Mon Sep 17 00:00:00 2001
From: Milton Reder <milt@yetanalytics.com>
Date: Wed, 17 Jan 2024 15:54:10 -0500
Subject: [PATCH] SQL-224 encode condition names

---
 src/main/lrsql/ops/util/reaction.clj  | 39 ++++++++++++++++++++-------
 src/main/lrsql/util/reaction.clj      | 29 +++++++++++++++++++-
 src/test/lrsql/util/reaction_test.clj | 14 ++++++++++
 3 files changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/main/lrsql/ops/util/reaction.clj b/src/main/lrsql/ops/util/reaction.clj
index 961608a12..85b9f72fe 100644
--- a/src/main/lrsql/ops/util/reaction.clj
+++ b/src/main/lrsql/ops/util/reaction.clj
@@ -23,7 +23,9 @@
 
 (defn- render-col
   [bk condition-name col]
-  (bp/-snip-col bk {:col (format "%s.%s" condition-name col)}))
+  (bp/-snip-col bk {:col (format "%s.%s"
+                                 (ru/encode-condition-name condition-name)
+                                 col)}))
 
 (s/fdef render-ref
   :args (s/cat :bk rs/reaction-backend?
@@ -46,7 +48,7 @@
     (render-col bk condition-name "registration")
     (bp/-snip-json-extract
      bk
-     {:col  (format "%s.payload" condition-name)
+     {:col  (format "%s.payload" (ru/encode-condition-name condition-name))
       :path path
       :datatype datatype})))
 
@@ -117,7 +119,9 @@
         ;; Clause special cases
         "contains"
         (bp/-snip-contains bk
-                           {:col   (format "%s.payload" condition-name)
+                           {:col   (format
+                                    "%s.payload"
+                                    (ru/encode-condition-name condition-name))
                             :path  path
                             :right right-snip
                             :datatype  (infer-type path val)})
@@ -150,8 +154,9 @@
                          [path ident-val] statement-identity]
                      (bp/-snip-clause
                       bk
-                      {:left  (render-ref bk condition-name path
-                                          :datatype (infer-type path ident-val))
+                      {:left  (render-ref
+                               bk condition-name path
+                               :datatype (infer-type path ident-val))
                        :op    "="
                        :right (bp/-snip-val bk {:val ident-val})})))}))
 
@@ -204,11 +209,12 @@
      bk
      {:select (mapv
                (fn [cn]
-                 [(format "%s.payload" cn) cn])
+                 (let [ecn (ru/encode-condition-name cn)]
+                   [(format "%s.payload" ecn) ecn]))
                condition-names)
       :from   (mapv
                (fn [cn]
-                 ["xapi_statement" cn])
+                 ["xapi_statement" (ru/encode-condition-name cn)])
                condition-names)
       :where
       (bp/-snip-and
@@ -236,9 +242,22 @@
 (defn query-reaction
   "For the given reaction input, return matching statements named for conditions."
   [bk tx input]
-  (bp/-query-reaction bk tx
-                      {:sql (query-reaction-sqlvec
-                             bk input)}))
+  (mapv
+   (fn [row]
+     (reduce-kv
+      (fn [m k v]
+        (assoc m
+               (-> k
+                   name
+                   ru/decode-condition-name
+                   keyword)
+               v))
+      {}
+      row))
+   (bp/-query-reaction
+    bk tx
+    {:sql (query-reaction-sqlvec
+           bk input)})))
 
 (s/fdef query-active-reactions
   :args (s/cat :bx rs/reaction-backend?
diff --git a/src/main/lrsql/util/reaction.clj b/src/main/lrsql/util/reaction.clj
index e0c6713be..c94050746 100644
--- a/src/main/lrsql/util/reaction.clj
+++ b/src/main/lrsql/util/reaction.clj
@@ -5,7 +5,9 @@
             [lrsql.spec.common :as cs]
             [lrsql.spec.reaction :as rs]
             [lrsql.spec.statement :as ss]
-            [xapi-schema.spec :as xs]))
+            [xapi-schema.spec :as xs]
+            [buddy.core.codecs :as bc]
+            [buddy.core.codecs.base64 :as b64]))
 
 (s/fdef path->string
   :args (s/cat :path ::rs/path
@@ -116,3 +118,28 @@
   (-> input
       (dissoc :reactionId)
       (assoc :reaction-id reactionId)))
+
+(s/fdef encode-condition-name
+  :args (s/cat :condition-name ::rs/condition-name)
+  :ret string?)
+
+(defn encode-condition-name
+  "Given a condition name string, encode it as hex."
+  [condition-name]
+  (-> condition-name
+      b64/encode
+      bc/bytes->hex
+      (->> (format "cond_%s")))) ;; add prefix to be valid SQL colname
+
+(s/fdef decode-condition-name
+  :args (s/cat :encoded-condition-name string?)
+  :ret ::rs/condition-name)
+
+(defn decode-condition-name
+  "Convert a condition name back to human-readable"
+  [encoded-condition-name]
+  (-> encoded-condition-name
+      (subs 5) ;; remove prefix
+      bc/hex->bytes
+      b64/decode
+      bc/bytes->str))
diff --git a/src/test/lrsql/util/reaction_test.clj b/src/test/lrsql/util/reaction_test.clj
index 278574235..82615fdbd 100644
--- a/src/test/lrsql/util/reaction_test.clj
+++ b/src/test/lrsql/util/reaction_test.clj
@@ -137,3 +137,17 @@
            {:data  {:type :lrsql.util.reaction/invalid-path
                     :path ["completed_a" "actor" "birthday"]}
             :cause "No value found at [\"completed_a\" \"actor\" \"birthday\"]"}))))
+
+(deftest encode-condition-name-test
+  (testing "unique"
+    (is (= (r/encode-condition-name "foo")
+           (r/encode-condition-name "foo")))
+    (is (not= (r/encode-condition-name "foo")
+              (r/encode-condition-name "bar"))))
+  (testing "alphanumeric"
+    (is (re-matches #"[a-z0-9\_]*" (r/encode-condition-name "foo"))))
+  (testing "reversible"
+    (is (-> "foo"
+            r/encode-condition-name
+            r/decode-condition-name
+            (= "foo")))))