oneof: use malli.core/-simple-schema to replace :fn and fix wrong one…

…of target encoding

src/clojobuf/core.cljc

@@ -38,13 +38,17 @@
   (-> (m/explain (dot-qualify msg-id) msg (second registry))
       (me/humanize)))
 
+(defn ->complete-malli-schema
+  "Add value schema to a composite registry. Return the new registry."
+  [schema] (into vschemas-pb-types schema))
+
 (defn ->malli-registry
   "Use `input`, which is expected to be (second (protoc ... :malli-composite-registry false)), to build
    a malli registry and returns it."
-  [input]
+  [schema]
   {:registry (mr/composite-registry
               m/default-registry
-              (into vschemas-pb-types input))})
+              (->complete-malli-schema schema))})
 
 ; protoc needs file access which is not available to cljs browser runtime
 ; * for cljs browser runtime, use clojobuf.macro/protoc-macro

src/clojobuf/encode.cljc

@@ -5,6 +5,10 @@
 
 (defn oneof? [field-def] (= (first field-def) :oneof))
 
+(defn oneof-target?
+  "field-def example: [14 :fixed32 :oneof nil]"
+  [field-def] (= (nth field-def 2) :oneof))
+
 (defn encode-prifield
   "Encode a primitive field
    field-schema: e.g. [12 :string :optional nil]"
@@ -103,11 +107,14 @@
                 (encode-prifield writer proto2|3 target-field-schema (msg v))))
 
             (msg|enum? field-schema)
-            (encode-msgfield|enumfield writer codec-registry field-schema v)
+            (when-not (oneof-target? field-schema)
+              (encode-msgfield|enumfield writer codec-registry field-schema v))
 
             (map?? field-schema)
             (encode-mapfield writer codec-registry field-schema v)
 
-            :else (encode-prifield writer proto2|3 field-schema v)))
+            :else
+            (when-not (oneof-target? field-schema)
+              (encode-prifield writer proto2|3 field-schema v))))
         (recur (inc idx))))
     (->bytes writer)))

src/clojobuf/schema.cljc

@@ -1,7 +1,8 @@
 (ns clojobuf.schema
   (:require [clojobuf.constant :refer [sint32-max sint32-min sint53-max sint53-min sint64-max sint64-min uint32-max uint32-min uint64-max uint64-min]]
             [clojobuf.util :refer [dot-qualify]]
-            [clojure.set :refer [map-invert]]))
+            [clojure.set :refer [map-invert]]
+            [malli.core :as m]))
 
 (defn- qualify-name [package name]
   (keyword (if (empty? package) name (str package "/" name))))
@@ -69,6 +70,22 @@
                :oneof (bxform-oneof msg-child)
                nil)))
 
+(def OneOf
+  (m/-simple-schema
+   {:type `OneOf
+    :compile (fn [_properties [oneof targets] _options]
+               (when-not (and (keyword? oneof)
+                              (every? keyword? targets)) 
+                 (m/-fail! ::invalid-children {:oneof oneof :targets targets}))
+               {:pred (fn [kvs] (if-let [target (kvs oneof)]
+                                  (and (contains? kvs target)
+                                       (= 1 (count (clojure.set/intersection (set targets) (set (keys kvs))))))
+                                  (= 0 (count (clojure.set/intersection (set targets) (set (keys kvs)))))))
+                :min 2 ;; at least 1 child
+                :max 2 ;; at most 1 child
+                :type-properties  {:error/fn (fn [_error _reg] {oneof (str "oneof condition not met: only this field's target can be set but not the other targets" )})
+                                   :error/path [oneof]}})}))
+
 ; -------------------- msg validator -----------------------------
 (def vschemas-pb-types {:int32     [:int {:min sint32-min, :max sint32-max}]
                         :sint32    [:int {:min sint32-min, :max sint32-max}]
@@ -87,7 +104,8 @@
                                                                                   (<= v uint64-max)))]]
                                       :cljs [:int {:min uint64-min, :max uint64-max}])
                         :bytes     #?(:clj 'bytes?
-                                      :cljs [:fn (fn [v] (= js/Uint8Array (type v)))])})
+                                      :cljs [:fn (fn [v] (= js/Uint8Array (type v)))])
+                        :oneof     OneOf})
 
 (defn- get-malli-type [typ]
   (cond
@@ -117,16 +135,14 @@
    [ [[:either [:enum :name :msg]]
       [:name string?]
       [:msg [:ref :Msg]]]
-     [:fn '(fn [kvs] (if-let [oneof-target (kvs :either)]
-                     (contains? kvs oneof-target)
-                     true))] ]"
+     [:oneof :either [:name :msg]] ]"
   [[_ name & forms]]
-  [(into
-    [[(keyword name) {:optional true} (into [:enum] (map #(keyword (nth % 2))) forms)]]
-    (map vxform-oneof-field forms))
-   [:fn `(fn [kvs#] (if-let [oneof-target# (kvs# ~(keyword name))]
-                     (contains? kvs# oneof-target#)
-                     true))]])
+  (let [oneof-key (keyword name)
+        targets (map #(keyword (nth % 2)) forms)]
+    [(into
+      [[oneof-key {:optional true} (into [:enum] targets)]]
+      (map vxform-oneof-field forms))
+     [:oneof oneof-key (into [] targets)]]))
 
 (defn- vld-msg-children-processor
   "msg-children are child nodes within message; each can be :field, :mapField, :oneof or :option."

test/clojobuf/core_test.cljc

@@ -52,6 +52,11 @@
   (rt :my.ns.oneof/Either {:either :packed_msg, :packed_msg {}})
   (rt :my.ns.oneof/Either {:either :packed_msg, :packed_msg {:int32_val [1]}}))
 
+(deftest test-codec-oneof-failure
+  (is (= (find-fault registry :my.ns.oneof/Either {:either :int32_val, :int64_val 0})
+         [{:either "oneof condition not met: only this field's target can be set but not the other targets"}]))
+  (is (= (find-fault registry :my.ns.oneof/Either {:int64_val 0})
+         [{:either "oneof condition not met: only this field's target can be set but not the other targets"}])))
 
 (deftest test-codec-singular
   (rt :my.ns.singular/Singular {:int32_val    -1})
@@ -189,4 +194,3 @@
   (rt :my.ns.extension/Extendable {:int32_val 1, :int64_val 2})
   (rt :my.ns.extension/Extendable {:my.ns.extension/Msg1.double_val 1.0})
   (rt :my.ns.extension/Extendable {:my.ns.extension/string_val "abcd"}))
-

test/clojobuf/schema_malli_test.cljc

@@ -122,7 +122,7 @@
           ; TODO compare dynamic function at end of form (currently bypassed with drop-last)
           ]))
   (is (true?  (m/validate [:ref :my.ns.oneof/Either] {:either :string_val, :string_val "abc"} registry)))
-  (is (true?  (m/validate [:ref :my.ns.oneof/Either] {:string_val "abc"} registry)))
+  (is (false?  (m/validate [:ref :my.ns.oneof/Either] {:string_val "abc"} registry)))
   (is (false?  (m/validate [:ref :my.ns.oneof/Either] {:either :string_val} registry)))
   (is (false?  (m/validate [:ref :my.ns.oneof/Either] {:either :string_val, :uint32_val 1} registry)))
   (is (false?  (m/validate [:ref :my.ns.oneof/Either] {:a :b} registry))))