search

deckarep / golang-set

A simple, battle-tested and generic set type for the Go language. Trusted by Docker, 1Password, Ethereum and Hashicorp.
Other
4k stars 272 forks source link

fix: Deserialize using a generic approach. #121

Closed fujie-xiyou closed 11 months ago

fujie-xiyou commented 1 year ago

Deserialize using generics. The previous deserialization method had issues in generic scenarios.

fujie-xiyou commented 1 year ago

There is currently a known issue where the threadUnsafeSet type was changed to a non-pointer type in version 2.2.0. As a result, when using the json.Unmarshal function to deserialize the threadUnsafeSettype, an error occurs and it cannot correctly enter the UnmarshalJSONmethod of threadUnsafeSet. However, directly calling the UnmarshalJSON method can successfully deserialize it.

fujie-xiyou commented 1 year ago

One more question, currently threadSafeSet and threadUnsafeSet structs are not exported. When a struct includes a set.Set type, json.Unmarshal cannot be used to deserialize the struct. Could you please let me know if it's possible to make threadSafeSet and threadUnsafeSet exportable so that they can be used in this scenario?

deckarep commented 11 months ago

Deserialize using generics. The previous deserialization method had issues in generic scenarios.

Can you be more specific what the issue was? I personally don't use the Marshal/Unmarshal too much but to my knowledge the tests were all previously passing.

deckarep commented 11 months ago

One more question, currently threadSafeSet and threadUnsafeSet structs are not exported. When a struct includes a set.Set type, json.Unmarshal cannot be used to deserialize the struct. Could you please let me know if it's possible to make threadSafeSet and threadUnsafeSet exportable so that they can be used in this scenario?

I'd rather ideally rather not export them because the Set interface is the type user's should be working with. Please provide a code example of what doesn't work.

deckarep commented 11 months ago

There is currently a known issue where the threadUnsafeSet type was changed to a non-pointer type in version 2.2.0. As a result, when using the json.Unmarshal function to deserialize the threadUnsafeSettype, an error occurs and it cannot correctly enter the UnmarshalJSONmethod of threadUnsafeSet. However, directly calling the UnmarshalJSON method can successfully deserialize it.

Is this what your PR fixes? Or is this a separate issue. I would be more inclined to expedite this work if we can get a issue/PR created per issue that you are having.

fujie-xiyou commented 11 months ago

Deserialize using generics. The previous deserialization method had issues in generic scenarios.

Can you be more specific what the issue was? I personally don't use the Marshal/Unmarshal too much but to my knowledge the tests were all previously passing.

The current test for Marshal/Unmarshal only includes tests for the string type. When the element type of the set is int64, you can refer to this code snippet. In reality, it cannot be deserialized successfully as expected. This pull request fixes the issue. With this correction, deserialization of int64 elements works correctly.