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

Ftr/support register with no pojo object #243

Merged
merged 17 commits into from
Nov 27, 2020

Conversation

LaurenceLiZhixin
Copy link
Contributor

What this PR does:
Add not pojo object register support, to support dubbo-go-cli tool.
Before, we can't register an object that made by "reflect.New" because it's not impl pojo interface.
But now, we can register this object, as long as it contains field "JavaClassName"

Which issue(s) this PR fixes:

NONE
Special notes for your reviewer:
Create a function to deal with interface made by reflect, get JavaClassName field and register the object with javaClassName.

Does this PR introduce a user-facing change?:
NONE

@wongoo
Copy link
Contributor

wongoo commented Nov 13, 2020

@LaurenceLiZhixin I think the func can meet your requirement : func RegisterPOJOMapping(javaClassName string, o interface{}) int

@LaurenceLiZhixin
Copy link
Contributor Author

@LaurenceLiZhixin I think the func can meet your requirement : func RegisterPOJOMapping(javaClassName string, o interface{}) int

I think this function can register object, and I actually use this funciton in my code.
But if I want to encode a none POJO obj, hessian2 can't find out what the object is, because it can't identify the object without JavaClassName. That is what my codes do: find the JavaClassName field and use it to encode registered object.

@wongoo
Copy link
Contributor

wongoo commented Nov 16, 2020

@LaurenceLiZhixin thanks for submit this pr.

  1. it's not efficient to check whether it's exist a field named JavaClassName, u should call checkPOJORegistry first to check whether it's being registered or not.
  2. SHOULD NOT copy a whole method and just change a little logic in it, a better way is to create a new func encNamedPOJOObject(javaClassName string, v interface{}) , and the original func encPOJOObject calling the new func.

@codecov-io
Copy link

codecov-io commented Nov 20, 2020

Codecov Report

Merging #243 (acceb0a) into master (d93e48c) will decrease coverage by 0.14%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #243      +/-   ##
==========================================
- Coverage   66.24%   66.10%   -0.15%     
==========================================
  Files          25       25              
  Lines        2776     2785       +9     
==========================================
+ Hits         1839     1841       +2     
- Misses        711      716       +5     
- Partials      226      228       +2     
Impacted Files Coverage Δ
encode.go 84.04% <0.00%> (ø)
object.go 64.39% <36.36%> (-1.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d93e48c...acceb0a. Read the comment docs.

encode.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
encode.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
object.go Outdated Show resolved Hide resolved
Copy link
Contributor

@wongoo wongoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wongoo wongoo merged commit 07e9397 into apache:master Nov 27, 2020
zhaoyunxing92 pushed a commit that referenced this pull request Sep 4, 2021
* fix: add not pojo Object register support

* fix: change gomod

* fix: delete unused comment

* chore: rv unused import

* fix: change gomod name

* Fix: replace go versionon

* fix: change the way to encode no pojo interface

* fix: change the way to encode no pojo interface

* chore: cicd bugs

* chore: call cicd

* fix: go fmt file

* chore: add none pojo check classInfoList

* chore: delete pre check of none pojo obj

* fix: change perror

* fix: use loadPojoRegistry func

* chore: error msg fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants