Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace gson with jackson #974

Closed
g1geordie opened this issue Oct 20, 2022 · 7 comments · Fixed by #1156 or #1160
Closed

Replace gson with jackson #974

g1geordie opened this issue Oct 20, 2022 · 7 comments · Fixed by #1156 or #1160

Comments

@g1geordie
Copy link
Contributor

g1geordie commented Oct 20, 2022

    > 因為現在執行 gson 沒有問題 有需要在這個時候就把它換成 jackson 嗎

如果會涉及大量變更的話,就在另外一隻PR處理。總之,我們盡可能在整個專案只用一種json工具,如果這邊確定要用jackson的話,那就要後面發PR把gson取代掉

Originally posted by @chia7712 in #848 (comment)

...
說明

  • gson : 在反序列化時不需要 default constructor
  • jackson : 所有反序列化都需要有 default constructor

特殊問題
在兩個框架 在反序列化 Optional opt; 的時候
在 json 中沒有 key(opt) 是不會進入反序列話的流程 將導致 opt = null
所以需要初始化Optioanl

取代為jackson 的原因 :

  • gson 不支援 name order 不支援 optional
  • jackson 原生支援以上

todo

  1. 確定反序列化沒問題
  2. 撰寫test 確定他和他的nested class 都有default constructor
  3. 移除gson dependency
@g1geordie
Copy link
Contributor Author

@chia7712
在這個ticket 中我們希望可以 check 所有request 的class 都有implement default constructor
想請問

  1. 我們 request 怎麼不像 response 一樣直接綁定成物件 要分 key 各別去取得呀
  2. 因為目前 request 沒有共通介面。我們是不是要建立一個共通的介面 好讓 test 去識別要檢查哪些class 的default constuctor 呢

@chia7712
Copy link
Contributor

chia7712 commented Nov 7, 2022

我們 request 怎麼不像 response 一樣直接綁定成物件 要分 key 各別去取得呀

當時是要方便List這個型別裡面可以混合各種型別,所以並沒有要求大家綁定成物件。現在使用jackson應該也會碰到類似的問題,如果沒有方便的解法,我會建議改變APIs的規格,要求使用array時綁定的物件都要是同一個類型,也就是下列request視為非法的:

{
  "requests": [
    {
      "a": "b"
    },
    {
      "a": 123
    },
    
  ]
}

因為目前 request 沒有共通介面。我們是不是要建立一個共通的介面 好讓 test 去識別要檢查哪些class 的default constuctor 呢

yep,可以宣告RequestResponse,並且透過測試來驗證大家都有 default constructor

@g1geordie
Copy link
Contributor Author

g1geordie commented Nov 21, 2022

根據需求 由於MR 太大 過於容易有衝突 將改為以下幾隻MR

Impl RecordHandler

add json dependency ,
add new <T> T request(TypeRef<T> typeRef) to channel which will replace postReqest

Impl TopicHandler

參考recordHandler 並將 PostRequest 改成使用 Request 物件形式拿取JSON

Impl ThrottleHandler

參考recordHandler 並將 PostRequest 改成使用 Request 物件形式拿取JSON

Impl ReassignmentHandler

參考recordHandler 並將 PostRequest 改成使用 Request 物件形式拿取JSON

Impl QuotaHandler

參考recordHandler 並將 PostRequest 改成使用 Request 物件形式拿取JSON

Impl BalancerHandler

參考recordHandler 並將 PostRequest 改成使用 Request 物件形式拿取JSON (Post and Put)

Change Gson to JsonConverter

將 new Gson() 改成 JsonConverter.defaultConverter()
remove PostReuqest
remove gson dependency

@chia7712
Copy link
Contributor

@g1geordie 感謝,可否把這些題目填寫在描述裡面?看看其他人有沒有機會一起幫忙,在你先完成基礎第一支基礎設施後

@g1geordie
Copy link
Contributor Author

了解 已經補上敘述 要實作的人
因該可以參考 #1128

@chia7712
Copy link
Contributor

了解 已經補上敘述 要實作的人

主要是 connector client 這個方便的工具還沒合併,需要這個來幫助我們控制 kafka connector

@chia7712
Copy link
Contributor

chia7712 commented Dec 7, 2022

重新開啟此議題,因為尚未完全更新完

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants