From d962cfba49692e79d42a5e0834ecb1702340891c Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 7 Aug 2024 16:05:25 +0300 Subject: [PATCH 1/2] Fixed bug with option --- engine/access/rest/middleware/common_query_params.go | 6 +++--- .../rest/middleware/common_query_params_test.go | 2 +- engine/access/rest/util/select_filter.go | 12 ++++++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/engine/access/rest/middleware/common_query_params.go b/engine/access/rest/middleware/common_query_params.go index b81c828d91a..cfd9bf1bfde 100644 --- a/engine/access/rest/middleware/common_query_params.go +++ b/engine/access/rest/middleware/common_query_params.go @@ -8,7 +8,7 @@ import ( ) const ExpandQueryParam = "expand" -const selectQueryParam = "select" +const SelectQueryParam = "select" // commonQueryParamMiddleware generates a Middleware function that extracts the given query parameter from the request // and adds it to the request context as a key value pair with the key as the query param name. @@ -37,7 +37,7 @@ func QueryExpandable() mux.MiddlewareFunc { // QuerySelect middleware extracts out the 'select' query param field if present in the request func QuerySelect() mux.MiddlewareFunc { - return commonQueryParamMiddleware(selectQueryParam) + return commonQueryParamMiddleware(SelectQueryParam) } func getField(req *http.Request, key string) ([]string, bool) { @@ -54,5 +54,5 @@ func GetFieldsToExpand(req *http.Request) ([]string, bool) { } func GetFieldsToSelect(req *http.Request) ([]string, bool) { - return getField(req, selectQueryParam) + return getField(req, SelectQueryParam) } diff --git a/engine/access/rest/middleware/common_query_params_test.go b/engine/access/rest/middleware/common_query_params_test.go index c40a70e0783..a04882f7613 100644 --- a/engine/access/rest/middleware/common_query_params_test.go +++ b/engine/access/rest/middleware/common_query_params_test.go @@ -35,7 +35,7 @@ func TestCommonQueryParamMiddlewares(t *testing.T) { query.Add(ExpandQueryParam, strings.Join(expandList, ",")) } if len(selectList) > 0 { - query.Add(selectQueryParam, strings.Join(selectList, ",")) + query.Add(SelectQueryParam, strings.Join(selectList, ",")) } req.URL.RawQuery = query.Encode() diff --git a/engine/access/rest/util/select_filter.go b/engine/access/rest/util/select_filter.go index f63e5fa6814..4f7172a7ff5 100644 --- a/engine/access/rest/util/select_filter.go +++ b/engine/access/rest/util/select_filter.go @@ -25,10 +25,10 @@ func SelectFilter(object interface{}, selectKeys []string) (interface{}, error) } filter := sliceToMap(selectKeys) - switch itemAsType := (*outputMap).(type) { case []interface{}: - filterSlice(itemAsType, "", filter) + filteredSlice, _ := filterSlice(itemAsType, "", filter) + *outputMap = filteredSlice case map[string]interface{}: filterObject(itemAsType, "", filter) } @@ -40,6 +40,10 @@ func SelectFilter(object interface{}, selectKeys []string) (interface{}, error) func filterObject(jsonStruct map[string]interface{}, prefix string, filterMap map[string]bool) { for key, item := range jsonStruct { newPrefix := jsonPath(prefix, key) + // if the leaf object is the key, go to others + if filterMap[newPrefix] { + continue + } switch itemAsType := item.(type) { case []interface{}: // if the value of a key is a list, call filterSlice @@ -87,7 +91,7 @@ func filterSlice(jsonSlice []interface{}, prefix string, filterMap map[string]bo if len(itemAsType) == 0 { // since all elements of the slice are the same, if one sub-slice has been filtered out, we can safely // remove all sub-slices and return (instead of iterating all slice elements) - return nil, sliceType + return make([]interface{}, 0), sliceType } case map[string]interface{}: // if the slice has structs as elements, call filterObject @@ -96,7 +100,7 @@ func filterSlice(jsonSlice []interface{}, prefix string, filterMap map[string]bo if len(itemAsType) == 0 { // since all elements of the slice are the same, if one struct element has been filtered out, we can safely // remove all struct elements and return (instead of iterating all slice elements) - return nil, false + return make([]interface{}, 0), false } default: // if the elements are neither a slice nor a struct, then return the slice and true to indicate the slice has From 320071df02b6bdc8dfce0b1d9aa90806089f28f6 Mon Sep 17 00:00:00 2001 From: UlyanaAndrukhiv Date: Wed, 7 Aug 2024 16:06:09 +0300 Subject: [PATCH 2/2] Added more tests, refactored blocks tests --- engine/access/rest/routes/blocks_test.go | 178 ++++++++++++------ engine/access/rest/util/select_filter_test.go | 12 ++ 2 files changed, 137 insertions(+), 53 deletions(-) diff --git a/engine/access/rest/routes/blocks_test.go b/engine/access/rest/routes/blocks_test.go index 7facf06d423..94962e87bb6 100644 --- a/engine/access/rest/routes/blocks_test.go +++ b/engine/access/rest/routes/blocks_test.go @@ -44,6 +44,10 @@ func prepareTestVectors(t *testing.T, singleBlockCondensedResponse := expectedBlockResponsesExpanded(blocks[:1], executionResults[:1], false, flow.BlockStatusUnknown) multipleBlockCondensedResponse := expectedBlockResponsesExpanded(blocks, executionResults, false, flow.BlockStatusUnknown) + multipleBlockHeaderWithHeaderSelectedResponse := expectedBlockResponsesSelected(blocks, executionResults, flow.BlockStatusUnknown, []string{"header"}) + multipleBlockHeaderWithHeaderAndStatusSelectedResponse := expectedBlockResponsesSelected(blocks, executionResults, flow.BlockStatusUnknown, []string{"header", "block_status"}) + multipleBlockHeaderWithUnknownSelectedResponse := expectedBlockResponsesSelected(blocks, executionResults, flow.BlockStatusUnknown, []string{"unknown"}) + invalidID := unittest.IdentifierFixture().String() invalidHeight := fmt.Sprintf("%d", blkCnt+1) @@ -112,7 +116,7 @@ func prepareTestVectors(t *testing.T, }, { description: "Get block by both heights and start and end height", - request: requestURL(t, nil, heights[len(heights)-1], heights[0], true, heights...), + request: requestURL(t, nil, heights[len(heights)-1], heights[0], true, []string{}, heights...), expectedStatus: http.StatusBadRequest, expectedResponse: `{"code":400, "message": "can only provide either heights or start and end height range"}`, }, @@ -134,6 +138,24 @@ func prepareTestVectors(t *testing.T, expectedStatus: http.StatusBadRequest, expectedResponse: fmt.Sprintf(`{"code":400, "message": "at most %d IDs can be requested at a time"}`, request.MaxBlockRequestHeightRange), }, + { + description: "Get multiple blocks by IDs with header selected", + request: getByIDsCondensedWithSelectURL(t, blockIDs, []string{"header"}), + expectedStatus: http.StatusOK, + expectedResponse: multipleBlockHeaderWithHeaderSelectedResponse, + }, + { + description: "Get multiple blocks by IDs with header and block_status selected", + request: getByIDsCondensedWithSelectURL(t, blockIDs, []string{"header", "block_status"}), + expectedStatus: http.StatusOK, + expectedResponse: multipleBlockHeaderWithHeaderAndStatusSelectedResponse, + }, + { + description: "Get multiple blocks by IDs with unknown select object", + request: getByIDsCondensedWithSelectURL(t, blockIDs, []string{"unknown"}), + expectedStatus: http.StatusOK, + expectedResponse: multipleBlockHeaderWithUnknownSelectedResponse, + }, } return testVectors } @@ -154,7 +176,7 @@ func TestAccessGetBlocks(t *testing.T) { } } -func requestURL(t *testing.T, ids []string, start string, end string, expandResponse bool, heights ...string) *http.Request { +func requestURL(t *testing.T, ids []string, start string, end string, expandResponse bool, selectedFields []string, heights ...string) *http.Request { u, _ := url.Parse("/v1/blocks") q := u.Query() @@ -172,6 +194,11 @@ func requestURL(t *testing.T, ids []string, start string, end string, expandResp q.Add(heightQueryParam, heightsStr) } + if len(selectedFields) > 0 { + selectedStr := strings.Join(selectedFields, ",") + q.Add(middleware.SelectQueryParam, selectedStr) + } + if expandResponse { var expands []string expands = append(expands, ExpandableFieldPayload) @@ -188,19 +215,23 @@ func requestURL(t *testing.T, ids []string, start string, end string, expandResp } func getByIDsExpandedURL(t *testing.T, ids []string) *http.Request { - return requestURL(t, ids, "", "", true) + return requestURL(t, ids, "", "", true, []string{}) } func getByHeightsExpandedURL(t *testing.T, heights ...string) *http.Request { - return requestURL(t, nil, "", "", true, heights...) + return requestURL(t, nil, "", "", true, []string{}, heights...) } func getByStartEndHeightExpandedURL(t *testing.T, start, end string) *http.Request { - return requestURL(t, nil, start, end, true) + return requestURL(t, nil, start, end, true, []string{}) } func getByIDsCondensedURL(t *testing.T, ids []string) *http.Request { - return requestURL(t, ids, "", "", false) + return requestURL(t, ids, "", "", false, []string{}) +} + +func getByIDsCondensedWithSelectURL(t *testing.T, ids []string, selectedFields []string) *http.Request { + return requestURL(t, ids, "", "", false, selectedFields) } func generateMocks(backend *mock.API, count int) ([]string, []string, []*flow.Block, []*flow.ExecutionResult) { @@ -232,65 +263,106 @@ func generateMocks(backend *mock.API, count int) ([]string, []string, []*flow.Bl return blockIDs, heights, blocks, executionResults } -func expectedBlockResponsesExpanded(blocks []*flow.Block, execResult []*flow.ExecutionResult, expanded bool, status flow.BlockStatus) string { - blockResponses := make([]string, len(blocks)) +func expectedBlockResponsesExpanded( + blocks []*flow.Block, + execResult []*flow.ExecutionResult, + expanded bool, + status flow.BlockStatus, + selectedFields ...string, +) string { + blockResponses := make([]string, 0) for i, b := range blocks { - blockResponses[i] = expectedBlockResponse(b, execResult[i], expanded, status) + response := expectedBlockResponse(b, execResult[i], expanded, status, selectedFields...) + if response != "" { + blockResponses = append(blockResponses, response) + } } return fmt.Sprintf("[%s]", strings.Join(blockResponses, ",")) } -func expectedBlockResponse(block *flow.Block, execResult *flow.ExecutionResult, expanded bool, status flow.BlockStatus) string { +func expectedBlockResponsesSelected( + blocks []*flow.Block, + execResult []*flow.ExecutionResult, + status flow.BlockStatus, + selectedFields []string, +) string { + return expectedBlockResponsesExpanded(blocks, execResult, false, status, selectedFields...) +} + +func expectedBlockResponse( + block *flow.Block, + execResult *flow.ExecutionResult, + expanded bool, + status flow.BlockStatus, + selectedFields ...string, +) string { id := block.ID().String() execResultID := execResult.ID().String() - execLink := fmt.Sprintf("/v1/execution_results/%s", execResultID) blockLink := fmt.Sprintf("/v1/blocks/%s", id) payloadLink := fmt.Sprintf("/v1/blocks/%s/payload", id) - blockStatus := status.String() - + execLink := fmt.Sprintf("/v1/execution_results/%s", execResultID) timestamp := block.Header.Timestamp.Format(time.RFC3339Nano) + header := fmt.Sprintf(`"header": { + "id": "%s", + "parent_id": "%s", + "height": "%d", + "timestamp": "%s", + "parent_voter_signature": "%s" + }`, id, block.Header.ParentID.String(), block.Header.Height, timestamp, util.ToBase64(block.Header.ParentVoterSigData)) + + links := fmt.Sprintf(`"_links": { + "_self": "%s" + }`, blockLink) + + expandable := fmt.Sprintf(`"_expandable": { + "payload": "%s", + "execution_result": "%s" + }`, payloadLink, execLink) + + blockStatus := fmt.Sprintf(`"block_status": "%s"`, status.String()) + payload := `"payload": {"collection_guarantees": [],"block_seals": []}` + executionResult := fmt.Sprintf(`"execution_result": %s`, executionResultExpectedStr(execResult)) + + partsSet := make(map[string]string) + if expanded { - return fmt.Sprintf(` - { - "header": { - "id": "%s", - "parent_id": "%s", - "height": "%d", - "timestamp": "%s", - "parent_voter_signature": "%s" - }, - "payload": { - "collection_guarantees": [], - "block_seals": [] - }, - "execution_result": %s, - "_expandable": {}, - "_links": { - "_self": "%s" - }, - "block_status": "%s" - }`, id, block.Header.ParentID.String(), block.Header.Height, timestamp, - util.ToBase64(block.Header.ParentVoterSigData), executionResultExpectedStr(execResult), blockLink, blockStatus) + partsSet["header"] = header + partsSet["payload"] = payload + partsSet["executionResult"] = executionResult + partsSet["_expandable"] = `"_expandable": {}` + partsSet["_links"] = links + partsSet["block_status"] = blockStatus + } else { + partsSet["header"] = header + partsSet["_expandable"] = expandable + partsSet["_links"] = links + partsSet["block_status"] = blockStatus } - return fmt.Sprintf(` - { - "header": { - "id": "%s", - "parent_id": "%s", - "height": "%d", - "timestamp": "%s", - "parent_voter_signature": "%s" - }, - "_expandable": { - "payload": "%s", - "execution_result": "%s" - }, - "_links": { - "_self": "%s" - }, - "block_status": "%s" - }`, id, block.Header.ParentID.String(), block.Header.Height, timestamp, - util.ToBase64(block.Header.ParentVoterSigData), payloadLink, execLink, blockLink, blockStatus) + if len(selectedFields) > 0 { + // filter a json struct + // elements whose keys are not found in the filter map will be removed + selectedFieldSet := make(map[string]struct{}, len(selectedFields)) + for _, field := range selectedFields { + selectedFieldSet[field] = struct{}{} + } + + for key := range partsSet { + if _, found := selectedFieldSet[key]; !found { + delete(partsSet, key) + } + } + } + + // Iterate over the map and append the values to the slice + var values []string + for _, value := range partsSet { + values = append(values, value) + } + if len(values) == 0 { + return "" + } + + return fmt.Sprintf("{%s}", strings.Join(values, ",")) } diff --git a/engine/access/rest/util/select_filter_test.go b/engine/access/rest/util/select_filter_test.go index e810affece8..4e2e769bce4 100644 --- a/engine/access/rest/util/select_filter_test.go +++ b/engine/access/rest/util/select_filter_test.go @@ -52,6 +52,18 @@ func TestSelectFilter(t *testing.T) { keys: []string{"b.c"}, description: "single object with arrays as values", }, + { + input: `{ "a": 1, "b": {"c":2, "d":3}}`, + output: `{ "b": {"c":2, "d":3}}`, + keys: []string{"b"}, + description: "full single object with nested fields", + }, + { + input: `{ "a": 1, "b": {"c":2, "d":3}}`, + output: `{}`, + keys: []string{"e"}, + description: "unknown object", + }, } for _, tv := range testVectors {