schema: fix conversion of :implicit message into :optional not perfor…

…med with oneof field
s-expresso · Jan 1, 2025 · 90b8764 · 90b8764
1 parent 285f550
commit 90b8764
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 11 deletions.
diff --git a/src/clojobuf/schema.cljc b/src/clojobuf/schema.cljc
@@ -275,14 +275,18 @@
     (sp/transform [sp/ALL-WITH-META
                    (sp/nthpath 1)
                    ; visit all elements of message (:map) type
-                   (sp/if-path #(= :map (first %)) sp/ALL-WITH-META)
+                   (sp/cond-path #(= :map (first %)) sp/ALL-WITH-META
+                                 #(and (= :and (-> % first)) ; if message contains oneof, top level is [:and [:map ...] ...]
+                                       (= :map (-> % second first))) [(sp/nthpath 1) sp/ALL-WITH-META])
+
                    ; only visit implicit message field
                    (sp/if-path vector? sp/STAY)                                        ; filter out non fields
                    (sp/if-path #(-> % last vector?) sp/STAY)                           ; filter out primitive fields
                    (sp/if-path #(= 3 (count %)) sp/STAY)                               ; filter out required fields which are w/o property
 
-                   (sp/if-path #(= :implicit (get (second %) :presence)) sp/STAY)            ; filter out non implicit fields
+                   (sp/if-path #(= :implicit (get (second %) :presence)) sp/STAY)      ; filter out non implicit fields
                    (sp/if-path #(= :ref (-> % last first)) sp/STAY)                    ; filter out non :ref fields
                    (sp/if-path #(not= :enum (-> % last last vschemas first)) sp/STAY)] ; filter out if referenced type is :enum (other possibiilities :map & :and)
                   update-implicit-property
                   vschemas)))
+
diff --git a/test/clojobuf/schema_malli_test.cljc b/test/clojobuf/schema_malli_test.cljc
@@ -387,32 +387,74 @@
                            [:field :int32]]}]
     (is (= (vschemas-update-msg-field-presence
             (merge base {:my.ns/MsgA [:map {:closed true}
-                                      [:msg_val {:optional true :presence :implicit}[:ref :my.ns/MsgB]] ; :implicit
-                                      [:enum_val [:ref :my.ns/Enum]]]}))
+                                      [:msg_val {:optional true :presence :implicit} [:ref :my.ns/MsgB]] ; :implicit
+                                      [:enum_val {:optional true :presence :implicit} [:ref :my.ns/Enum]]]}))
            (merge base {:my.ns/MsgA [:map {:closed true}
                                      [:msg_val {:optional true :presence :optional} [:ref :my.ns/MsgB]] ; becomes :optional
-                                     [:enum_val [:ref :my.ns/Enum]]]})))
+                                     [:enum_val {:optional true :presence :implicit} [:ref :my.ns/Enum]]]})))
 
     (is (= (vschemas-update-msg-field-presence
             (merge base {:my.ns/MsgA [:map {:closed true}
                                       [:msg_val {:optional true :presence :optional}[:ref :my.ns/MsgB]] ; opitonal
-                                      [:enum_val [:ref :my.ns/Enum]]]}))
+                                      [:enum_val {:optional true :presence :optional} [:ref :my.ns/Enum]]]}))
            (merge base {:my.ns/MsgA [:map {:closed true}
                                      [:msg_val {:optional true :presence :optional} [:ref :my.ns/MsgB]]  ; no change
-                                     [:enum_val [:ref :my.ns/Enum]]]}))) 
+                                     [:enum_val {:optional true :presence :optional} [:ref :my.ns/Enum]]]}))) 
 
     (is (= (vschemas-update-msg-field-presence
             (merge base {:my.ns/MsgA [:map {:closed true}
                                       [:msg_val {:optional false :presence :required} [:ref :my.ns/MsgB]] ; required
-                                      [:enum_val [:ref :my.ns/Enum]]]}))
+                                      [:enum_val {:optional false :presence :required} [:ref :my.ns/Enum]]]}))
            (merge base {:my.ns/MsgA [:map {:closed true}
                                      [:msg_val {:optional false :presence :required} [:ref :my.ns/MsgB]]  ; no change
-                                     [:enum_val [:ref :my.ns/Enum]]]})))
+                                     [:enum_val {:optional false :presence :required} [:ref :my.ns/Enum]]]})))
 
     (is (= (vschemas-update-msg-field-presence
             (merge base {:my.ns/MsgA [:map {:closed true}
                                       [:msg_val {} [:ref :my.ns/MsgB]]                ; empty property
-                                      [:enum_val [:ref :my.ns/Enum]]]}))
+                                      [:enum_val {} [:ref :my.ns/Enum]]]}))
            (merge base {:my.ns/MsgA [:map {:closed true}
                                      [:msg_val {} [:ref :my.ns/MsgB]]                 ; no change
-                                     [:enum_val [:ref :my.ns/Enum]]]})))))
+                                     [:enum_val {} [:ref :my.ns/Enum]]]})))))
+
+(deftest test-update-msg-with-oneof-field-presence
+  (is (= (vschemas-update-msg-field-presence {:my.ns/Enum [:enum :ZERO :ONE]
+                                              :my.ns/MsgA [:and
+                                                           [:map
+                                                            {:closed true}
+                                                            [:either
+                                                             {:optional true :presence :oneof}
+                                                             [:enum
+                                                              :int32_val
+                                                              :int64_val]]
+                                                            [:int32_val {:optional true :presence :oneof-field} :int32]
+                                                            [:int64_val {:optional true :presence :oneof-field} :int64]
+                                                            [:msg_val {:optional true :presence :implicit} [:ref :my.ns/MsgB]] ; implicit
+                                                            [:enum_val {:optional true :presence :implicit} [:ref :my.ns/Enum]]]
+                                                           [:oneof
+                                                            :either
+                                                            [:int32_val
+                                                             :int64_val]]]
+                                              :my.ns/MsgB [:map
+                                                           {:closed true}
+                                                           [:field :int32]]})
+         {:my.ns/Enum [:enum :ZERO :ONE]
+          :my.ns/MsgA [:and
+                       [:map
+                        {:closed true}
+                        [:either
+                         {:optional true :presence :oneof}
+                         [:enum
+                          :int32_val
+                          :int64_val]]
+                        [:int32_val {:optional true :presence :oneof-field} :int32]
+                        [:int64_val {:optional true :presence :oneof-field} :int64]
+                        [:msg_val {:optional true :presence :optional} [:ref :my.ns/MsgB]] ; becomes: optional
+                        [:enum_val {:optional true :presence :implicit} [:ref :my.ns/Enum]]]
+                       [:oneof
+                        :either
+                        [:int32_val
+                         :int64_val]]]
+          :my.ns/MsgB [:map
+                       {:closed true}
+                       [:field :int32]]})))