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.

Sign up for GitHub

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

Jump to bottom

api: supports /regions/key by hex key #8262

Merged
merged 8 commits into from
Jun 13, 2024
Merged

api: supports /regions/key by hex key #8262

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b173483
add hex
HuSharp Jun 6, 2024
616a245
extract common func
HuSharp Jun 6, 2024
509c5fb
address comment
HuSharp Jun 6, 2024
ea750a2
make static happy
HuSharp Jun 6, 2024
3f515a6
change function to bytes and add more comment
HuSharp Jun 13, 2024
9377fd5
add invalid key test
HuSharp Jun 13, 2024
7ec5d7c
Merge branch 'master' into support_scan_regions_hex
ti-chi-bot[bot] Jun 13, 2024
364b209
Merge branch 'master' into support_scan_regions_hex
ti-chi-bot[bot] Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/core/region_tree.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func (t *regionTree) find(item *regionItem) *regionItem {
// until f return false
func (t *regionTree) scanRange(startKey []byte, f func(*RegionInfo) bool) {
region := &RegionInfo{meta: &metapb.Region{StartKey: startKey}}
// find if there is a region with key range [s, d), s < startKey < d
// find if there is a region with key range [s, d), s <= startKey < d
fn := func(item *regionItem) bool {
r := item
return f(r.RegionInfo)
Expand Down
16 changes: 16 additions & 0 deletions pkg/utils/apiutil/apiutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,22 @@
return returned, rawKey, nil
}

// ParseHexKeys decodes hexadecimal keys to bytes if the format is "hex".
func ParseHexKeys(format string, keys ...*string) error {
Copy link
Member

Choose a reason for hiding this comment

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

Returning parsed key to user is better than returning result by input parameter implicitly.

Suggested change
func ParseHexKeys(format string, keys ...*string) error {
func ParseHexKeys(format string, keys []string) ([]string, error) {

Copy link
Member Author

Choose a reason for hiding this comment

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

want to reduce redundant code :(

Copy link
Member

@okJiang okJiang Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know what you want... But this suggestion aims to avoid to changing data in the input implicitly, and each update operation, I think, should re-assign to the old value. Let's see what other reviewers think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps there is no need to provide a function. Because of the multiple arguments and the error check which will make code more redundant.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @JmPotato

Copy link
Member

Choose a reason for hiding this comment

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

This involves converting between []byte and string. I prefer a more performant approach; a benchmark may help.

HuSharp marked this conversation as resolved.
Show resolved Hide resolved
if format != "hex" {
return nil
}

for _, key := range keys {
keyBytes, err := hex.DecodeString(*key)
if err != nil {
return err

Check warning on line 348 in pkg/utils/apiutil/apiutil.go

View check run for this annotation

Codecov / codecov/patch

pkg/utils/apiutil/apiutil.go#L348 

Added line #L348 was not covered by tests
}
*key = string(keyBytes)
}
return nil
}

// ReadJSON reads a JSON data from r and then closes it.
// An error due to invalid json will be returned as a JSONError
func ReadJSON(r io.ReadCloser, data any) error {
Expand Down
21 changes: 12 additions & 9 deletions server/api/region.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import (
"container/heap"
"encoding/hex"
"fmt"
"net/http"
"net/url"
Expand Down Expand Up @@ -93,14 +92,10 @@
return
}
// decode hex if query has params with hex format
formatStr := r.URL.Query().Get("format")
if formatStr == "hex" {
keyBytes, err := hex.DecodeString(key)
if err != nil {
h.rd.JSON(w, http.StatusBadRequest, err.Error())
return
}
key = string(keyBytes)
err = apiutil.ParseHexKeys(r.URL.Query().Get("format"), &key)
if err != nil {
h.rd.JSON(w, http.StatusBadRequest, err.Error())
return

Check warning on line 98 in server/api/region.go

View check run for this annotation

Codecov / codecov/patch

server/api/region.go#L97-L98 

Added lines #L97 - L98 were not covered by tests
}

regionInfo := rc.GetRegionByKey([]byte(key))
Expand Down Expand Up @@ -176,6 +171,14 @@
rc := getCluster(r)
startKey := r.URL.Query().Get("key")
endKey := r.URL.Query().Get("end_key")

// decode hex if query has params with hex format
err := apiutil.ParseHexKeys(r.URL.Query().Get("format"), &startKey, &endKey)
if err != nil {
h.rd.JSON(w, http.StatusBadRequest, err.Error())
return

Check warning on line 179 in server/api/region.go

View check run for this annotation

Codecov / codecov/patch

server/api/region.go#L178-L179 

Added lines #L178 - L179 were not covered by tests
}

limit, err := h.AdjustLimit(r.URL.Query().Get("limit"))
if err != nil {
h.rd.JSON(w, http.StatusBadRequest, err.Error())
Expand Down
29 changes: 29 additions & 0 deletions server/api/region_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,35 @@ func (suite *getRegionTestSuite) TestScanRegionByKeys() {
for i, v := range regionIDs {
re.Equal(regions.Regions[i].ID, v)
}
url = fmt.Sprintf("%s/regions/key?key=%s&format=hex", suite.urlPrefix, hex.EncodeToString([]byte("b")))
regionIDs = []uint64{3, 4, 5, 99}
regions = &response.RegionsInfo{}
err = tu.ReadGetJSON(re, testDialClient, url, regions)
re.NoError(err)
re.Len(regionIDs, regions.Count)
for i, v := range regionIDs {
re.Equal(regions.Regions[i].ID, v)
}
url = fmt.Sprintf("%s/regions/key?key=%s&end_key=%s&format=hex",
suite.urlPrefix, hex.EncodeToString([]byte("b")), hex.EncodeToString([]byte("g")))
regionIDs = []uint64{3, 4}
regions = &response.RegionsInfo{}
err = tu.ReadGetJSON(re, testDialClient, url, regions)
re.NoError(err)
re.Len(regionIDs, regions.Count)
for i, v := range regionIDs {
re.Equal(regions.Regions[i].ID, v)
}
url = fmt.Sprintf("%s/regions/key?key=%s&end_key=%s&format=hex",
suite.urlPrefix, hex.EncodeToString([]byte("b")), hex.EncodeToString([]byte{0xFF, 0xFF, 0xCC}))
regionIDs = []uint64{3, 4, 5, 99}
regions = &response.RegionsInfo{}
err = tu.ReadGetJSON(re, testDialClient, url, regions)
re.NoError(err)
re.Len(regionIDs, regions.Count)
for i, v := range regionIDs {
re.Equal(regions.Regions[i].ID, v)
}
}

// Start a new test suite to prevent from being interfered by other tests.
Expand Down