clojusc / protobuf

A Clojure interface to Google's protocol buffers
https://clojusc.github.io/protobuf/
Eclipse Public License 1.0
72 stars 8 forks source link

mutating `protobuf.impl.flatland.core.FlatlandProtoBuf` #41

Open barkanido opened 5 years ago

barkanido commented 5 years ago

Hey folks, I am evaluating this and testing some simple scenarios of mutation. What I am trying to test is:

  1. creating a proto object
  2. mutate in some Clojure map-related API (that is allowed by the schema)
  3. serialize to bytes
  4. deserialize and see that the mutation actually worked:

now, given this schema:

syntax = "proto2";

package clojure_protobuf_poc.core.person;

option java_outer_classname = "PersonProto";

message Name {
  optional string name = 1;
  optional string surname = 2;
}

message Person {
  optional int32  id    = 1;
  optional Name name  = 2;
  optional string email = 3;
  repeated string likes = 4;
}

and this test code:

(ns clojure-protobuf-poc.core-test
  (:require [clojure.test :refer [testing deftest is]]
            [protobuf.core :as protobuf])
  (:import [clojure_protobuf_poc.core.person PersonProto$Name PersonProto$Person]
           [protobuf.impl.flatland.core FlatlandProtoBuf]))

(defn bytes->Person [person-bytes]
  (protobuf/bytes-> (protobuf/create PersonProto$Person) person-bytes))

(defn mutate->round-trip [pb f]
  (let [mutated (f pb)]
    (is (= FlatlandProtoBuf (type mutated)))
    (-> mutated 
        protobuf/->bytes 
        bytes->Person)))

(deftest mutation
  (let [alice-name (protobuf/create PersonProto$Name {:name "Alice" :surname "Cohen"})
        alice (protobuf/create PersonProto$Person {:id 108 :name alice-name :email "alice.cohen@proto.com"})]
    (println alice)
    (testing "no mutation"
      (let [alice-tag (mutate->round-trip alice identity)]
        (is (= alice-tag alice))))
    (testing "dissoc"
      (let [no-id (mutate->round-trip alice #(dissoc % :id))]
        (is (= nil (:id no-id)))))
    (testing "assoc-in"
      (let [new-surname (mutate->round-trip alice #(assoc-in % [:name :surname] "Levi"))]
        (is (= "Levi" (get-in new-surname [:name :surname])))))
    (testing "update-in"
      (let [no-surname (mutate->round-trip alice #(update-in % [:name] dissoc :surname))]
        (is (= nil (get-in no-surname [:name :surname]))))) ; fails-> we get "Cohen"
    (testing "assoc"
      (let [new-email1 (mutate->round-trip alice #(assoc % :email "new-email"))]
        (is (= "new-email" (:email new-email1)))))
    (testing "merge" 
      ; crashes cause after the merge we get  java.lang.IllegalArgumentException: No implementation of 
      ; method: :->bytes of protocol: #'protobuf.core/ProtoBufAPI 
      ; found for class: protobuf.PersistentProtocolBufferMap
      (let [new-email2 (mutate->round-trip alice #(merge % {:email "new-email"}))]
        (is (= "new-email" (:email new-email2)))))))

tests fail with:

FAIL in (mutation) (core_test.clj:31)
update-in
expected: nil
  actual: "Cohen"
    diff: + "Cohen"

lein test :only clojure-protobuf-poc.core-test/mutation

FAIL in (mutation) (core_test.clj:12)
merge
expected: protobuf.impl.flatland.core.FlatlandProtoBuf
  actual: protobuf.PersistentProtocolBufferMap
    diff: - protobuf.impl.flatland.core.FlatlandProtoBuf
          + protobuf.PersistentProtocolBufferMap

lein test :only clojure-protobuf-poc.core-test/mutation

ERROR in (mutation) (core_deftype.clj:583)
Uncaught exception, not in assertion.
expected: nil
  actual: java.lang.IllegalArgumentException: No implementation of method: :->bytes of protocol: #'protobuf.core/ProtoBufAPI found for class: protobuf.PersistentProtocolBufferMap
 at clojure.core$_cache_protocol_fn.invokeStatic (core_deftype.clj:583)
...
...

Can you explain why update-in and merge aren't supported and what I (probably) have done wrong? Thanks!

Maybe, put differently, it looks like PersistentProtocolBufferMap isn't 100% compatible with a Clojure map:

  1. Is that correct?
  2. If so, should I strive to convert it into a Clojure map? What is the fastest way to do so (recursively)?
fr33m0nk commented 4 years ago

@barkanido I had faced similar issue when the protobuf message was made of nested proto messages and if the nested message was being enriched with data and assoc-in was not yielding desired result.

To overcome this problem, I came up with following Clojure protocol which converts protobuf object to map:

(defn as-map [f o]
  (reduce (fn [m [k v]]
            (assoc m k (f v)))
          {} o))

(defprotocol ProtoBufToClojureMap
  (->clj-map [o]))

(extend-protocol ProtoBufToClojureMap
  protobuf.impl.flatland.core.FlatlandProtoBuf
  (->clj-map [o]
    (as-map ->clj-map o))

  protobuf.PersistentProtocolBufferMap
  (->clj-map [o]
    (as-map ->clj-map o))

  java.lang.Object
  (->clj-map [o] o))

Above protocol converts the protobuf map into a Clojure map, allowing you to do all sort of map operations

Usage is pretty straight forward as well:

(def alice-as-clj-map (->clj-map alice))

You can covert it back to protobuf object for publishing by using the standard library functions.

@oubiwann Please let me know if this seems fit enough for PR. This may sit in some utility name-space.

fr33m0nk commented 2 years ago

@oubiwann I have create a PR to resolve this Issue. This PR resolvesthe issue and :

Please review it in your time 😄 PS: As a well maintained Protobuf opertaional alternative, I would recommend using AppsFlyer/pronto though.