Merge pull request #141 from nilenso/type-check-args-specs

Verify encoding inconsistencies post de/serialization of args

deps.edn

@@ -10,7 +10,8 @@
  :paths   ["src"]
 
  :aliases {:test       {:extra-paths ["test"]
-                        :extra-deps  {io.github.cognitect-labs/test-runner {:git/tag "v0.5.0" :git/sha "b3fd0d2"}}
+                        :extra-deps  {io.github.cognitect-labs/test-runner {:git/tag "v0.5.0" :git/sha "b3fd0d2"}
+                                      cnuernber/dtype-next                 {:mvn/version "10.102"}}
                         :exec-fn     test-runner/test-and-shutdown}
            :repl       {:extra-deps {vvvvalvalval/scope-capture  {:mvn/version "0.3.2"}
                                      org.clojure/tools.namespace {:mvn/version "1.3.0"}}}

src/goose/specs.clj

@@ -12,7 +12,8 @@
     [clojure.string :as str]
     [taoensso.carmine.connections :refer [IConnectionPool]])
   (:import
-    (java.time Instant ZoneId)))
+    (java.time Instant ZoneId)
+    (java.util Arrays)))
 
 ;;; ========== Qualified Function Symbols ==============
 (s/def ::fn-sym (s/and qualified-symbol? resolve #(fn? @(resolve %))))
@@ -157,7 +158,12 @@
 
 ;;; ============== Client ==============
 (s/def ::args-serializable?
-  #(try (= % (u/decode (u/encode %)))
+  ;; Serializability of args is determined by consistency in encoding.
+  ;; If this spec fails, serialize your custom data type as follows:
+  ;; https://github.com/nilenso/goose/wiki/Serializing-Custom-data-types
+  #(try (let [encoding (u/encode %)
+              re-encoding (u/encode (u/decode encoding))]
+          (Arrays/equals ^"[B" encoding ^"[B" re-encoding))
         (catch Exception _ false)))
 (s/def ::instant #(instance? Instant %))
 (s/def ::client-opts (s/keys :req-un [::broker ::queue ::retry-opts]))

test/goose/specs_test.clj

@@ -8,20 +8,40 @@
     [goose.metrics.statsd :as statsd]
     [goose.specs :as specs]
     [goose.test-utils :as tu]
+    [goose.utils :as u]
     [goose.worker :as w]
 
+    [clojure.spec.alpha :as s]
     [clojure.test :refer [deftest is are]])
   (:import
     (clojure.lang ExceptionInfo)
-    (java.time Instant)))
+    (java.time Instant)
+    (java.util HashMap)
+    (tech.v3.datatype FastStruct)))
 
 (defn single-arity-fn [_] "dummy")
 (def now (Instant/now))
 
+(deftest args-encoding-consistency-test
+  ;; Not extending nippy serialization for custom data-types can lead to unexpected bugs.
+  ;; For instance, a de/serialized FastStruct object's value will be equal to original.
+  ;; However, the de/serialized object's type gets implicitly altered to PersistentHashMap.
+  ;; This type change leads to inconsistent encoding, leading to BUG #141.
+  ;; TODO: Reproduce value-equality & type-mismatch bug without dependency on a 3rd party library.
+  (let [;; Encoding inconsistency happens only when count of keys in FastStruct are greater than 8.
+        slots (HashMap. {:a 0 :b 1 :c 2 :d 3 :e 4 :f 5 :g 6 :h 7 :i 8})
+        vals [1 2 3 4 5 6 7 8 9]
+        arg (FastStruct. slots vals)]
+    ;; Test value-equality post de/serialization.
+    (is (= arg (u/decode (u/encode arg))))
+    ;; Expect encoding inconsistency post de/serialization.
+    (is (false? (s/valid? ::specs/args-serializable? arg)))))
+
 (deftest specs-test
   (specs/instrument)
   (are [sut]
     (is
+      ;; When specs are instrumented, expect exceptions for incorrect parameters.
       (thrown-with-msg?
         ExceptionInfo
         #"Call to goose.* did not conform to spec."