pilagod / gorm-cursor-paginator

A paginator doing cursor-based pagination based on GORM
https://github.com/pilagod/gorm-cursor-paginator
MIT License
188 stars 44 forks source link

Fix slice encoding #64

Closed chrisroberts closed 2 months ago

chrisroberts commented 2 months ago

Encoding a field that is a slice will currently panic due to Elem() being called on a slice in util.ReflectValue. reflect.Value only allows for Elem() to be called on interface or pointer values and will panic otherwise. So this just removes slice from being included in the check to unwrap and adds in a test to validate that encoding the slice is successful.

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9752982979

Details


Totals Coverage Status
Change from base Build 9298275274: 0.0%
Covered Lines: 406
Relevant Lines: 419

💛 - Coveralls
pilagod commented 2 months ago

@chrisroberts Nice catch! Could you help me to add an integration test in cursor/encoding_test.go, by encoding and then decoding to ensure they work expectedly?

You can take other cases as examples:

https://github.com/pilagod/gorm-cursor-paginator/blob/49dddd09c77a2fe2b77325903ef6b3838130574e/cursor/encoding_test.go#L141-L165

But I am wondering that []byte should be also a slice, right? I am not sure why the []byte case could work perfectly, do you have any idea?

chrisroberts commented 2 months ago

@pilagod Yeah, no problem. Added in the extra tests.

As for the []byte case, it works because the value on the struct is not the byte slice directly, rather the value is the struct that contains the byte slice. So when the value is reflected, it's not seen as a slice.

coveralls commented 2 months ago

Pull Request Test Coverage Report for Build 9765782179

Details


Totals Coverage Status
Change from base Build 9298275274: 0.0%
Covered Lines: 406
Relevant Lines: 419

💛 - Coveralls
pilagod commented 2 months ago

You're right, I totally missed it is under another struct.

Thank you so much for this patch, it looks pretty good to me. I will notify you once there is a new release including this fix 💪

pilagod commented 2 months ago

I just released v2.6.1 to include this patch. Thanks again for the contribution. 🙌

chrisroberts commented 2 months ago

Awesome, thank you so much @pilagod!