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

feat: support for pointers to primitives in Scan() and HSet() #388

Merged
merged 4 commits into from
Oct 9, 2023

Conversation

numberinho
Copy link
Contributor

Easy way to implement support for pointers to primitives at the Scan() method. Uses built-in decoders by just passing the kind of the element of the pointer to the decoders map.

Verified

This commit was signed with the committer’s verified signature.
pietern Pieter Noordhuis
support pointers to primitives
@numberinho numberinho changed the title Update structmap.go Support for pointers to primitives in Scan() Oct 8, 2023
@numberinho numberinho changed the title Support for pointers to primitives in Scan() Support for pointers to primitives in Scan() and HSet() Oct 8, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 8, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (f0f37fc) 97.55% compared to head (c509a84) 97.55%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #388      +/-   ##
==========================================
- Coverage   97.55%   97.55%   -0.01%     
==========================================
  Files          76       76              
  Lines       30683    30693      +10     
==========================================
+ Hits        29934    29942       +8     
- Misses        627      629       +2     
  Partials      122      122              
Files Coverage Δ
rueidiscompat/structmap.go 96.82% <100.00%> (+0.27%) ⬆️
rueidiscompat/adapter.go 94.94% <0.00%> (-0.22%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rueian
Copy link
Collaborator

rueian commented Oct 8, 2023

Hi @numberinho, thank you for your contribution!

These changes look good to me. Could you also modify the testes here https://github.com/redis/rueidis/blob/main/rueidiscompat/hscan_test.go to cover these changes?

@rueian rueian added the feature label Oct 8, 2023
@numberinho
Copy link
Contributor Author

Hey @rueian, thank you for this awesome library and your super fast replies! I really appreciate your effort and work :)

I tried adding a test, but I'm not very used to the testing framework. Hopefully I implemented it in the right way.

Did you notice that I also added some lines to modify the HSet() - functioning to omit setting Go nil pointers to Redis in rueidiscompat/adapter.go ? How does this look to you?

@rueian
Copy link
Collaborator

rueian commented Oct 8, 2023

I tried adding a test, but I'm not very used to the testing framework. Hopefully I implemented it in the right way.

It is in the right way. The test is passed.

Did you notice that I also added some lines to modify the HSet() - functioning to omit setting Go nil pointers to Redis in rueidiscompat/adapter.go ? How does this look to you?

They also look good to me, but I notice that the test does not cover them.

Could you also edit another test here to cover them?

It("should HSet", func() {
hSet := adapter.HSet(ctx, "hash", "key", "hello")
Expect(hSet.Err()).NotTo(HaveOccurred())
Expect(hSet.Val()).To(Equal(int64(1)))
hGet := adapter.HGet(ctx, "hash", "key")
Expect(hGet.Err()).NotTo(HaveOccurred())
Expect(hGet.Val()).To(Equal("hello"))
// set struct
// MSet struct
type set struct {
Set1 string `redis:"set1"`
Set2 int16 `redis:"set2"`
Set3 time.Duration `redis:"set3"`
Set4 interface{} `redis:"set4"`
Set5 map[string]interface{} `redis:"-"`
Set6 string `redis:"set6,omitempty"`
}
hSet = adapter.HSet(ctx, "hash", &set{
Set1: "val1",
Set2: 1024,
Set3: 2 * time.Millisecond,
Set4: nil,
Set5: map[string]interface{}{"k1": 1},
})
Expect(hSet.Err()).NotTo(HaveOccurred())
Expect(hSet.Val()).To(Equal(int64(4)))
hMGet := adapter.HMGet(ctx, "hash", "set1", "set2", "set3", "set4", "set5", "set6")
Expect(hMGet.Err()).NotTo(HaveOccurred())
Expect(hMGet.Val()).To(Equal([]interface{}{
"val1",
"1024",
strconv.Itoa(int(2 * time.Millisecond.Nanoseconds())),
"",
nil,
nil,
}))
hSet = adapter.HSet(ctx, "hash2", &set{
Set1: "val2",
Set6: "val",
})
Expect(hSet.Err()).NotTo(HaveOccurred())
Expect(hSet.Val()).To(Equal(int64(5)))
hMGet = adapter.HMGet(ctx, "hash2", "set1", "set6")
Expect(hMGet.Err()).NotTo(HaveOccurred())
Expect(hMGet.Val()).To(Equal([]interface{}{
"val2",
"val",
}))
})

@rueian rueian changed the title Support for pointers to primitives in Scan() and HSet() feat: support for pointers to primitives in Scan() and HSet() Oct 9, 2023
@rueian rueian merged commit 78859ba into redis:main Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants