Skip to content

Commit a6885b5

Browse files
author
Luis Sanchez
committed
[FAB-8103] final HasMore should return false
final GET_STATE_BY_RANGE response now indicates that there are no more results when the total number of responses are a multiple of maxResultLimit (the maximum number of responses that can be returned in a single GET_STATE_BY_RANGE response). Change-Id: Ifd482f6f0951ac69698a5cd9f2c610ef84654bf7 Signed-off-by: Luis Sanchez <sanchezl@us.ibm.com>
1 parent 6860525 commit a6885b5

File tree

2 files changed

+174
-45
lines changed

2 files changed

+174
-45
lines changed

core/chaincode/handler.go

Lines changed: 65 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,18 @@ type transactionContext struct {
4747
responseNotifier chan *pb.ChaincodeMessage
4848

4949
// tracks open iterators used for range queries
50-
queryIteratorMap map[string]commonledger.ResultsIterator
50+
queryIteratorMap map[string]commonledger.ResultsIterator
51+
pendingQueryResults map[string]*pendingQueryResult
5152

5253
txsimulator ledger.TxSimulator
5354
historyQueryExecutor ledger.HistoryQueryExecutor
5455
}
5556

57+
type pendingQueryResult struct {
58+
batch []*pb.QueryResultBytes
59+
count int
60+
}
61+
5662
type nextStateInfo struct {
5763
msg *pb.ChaincodeMessage
5864
sendToCC bool
@@ -181,7 +187,8 @@ func (handler *Handler) createTxContext(ctxt context.Context, chainID string, tx
181187
}
182188
txctx := &transactionContext{chainID: chainID, signedProp: signedProp,
183189
proposal: prop, responseNotifier: make(chan *pb.ChaincodeMessage, 1),
184-
queryIteratorMap: make(map[string]commonledger.ResultsIterator)}
190+
queryIteratorMap: make(map[string]commonledger.ResultsIterator),
191+
pendingQueryResults: make(map[string]*pendingQueryResult)}
185192
handler.txCtxs[txCtxID] = txctx
186193
txctx.txsimulator = getTxSimulator(ctxt)
187194
txctx.historyQueryExecutor = getHistoryQueryExecutor(ctxt)
@@ -205,11 +212,12 @@ func (handler *Handler) deleteTxContext(chainID, txid string) {
205212
}
206213
}
207214

208-
func (handler *Handler) putQueryIterator(txContext *transactionContext, queryID string,
215+
func (handler *Handler) initializeQueryContext(txContext *transactionContext, queryID string,
209216
queryIterator commonledger.ResultsIterator) {
210217
handler.Lock()
211218
defer handler.Unlock()
212219
txContext.queryIteratorMap[queryID] = queryIterator
220+
txContext.pendingQueryResults[queryID] = &pendingQueryResult{batch: make([]*pb.QueryResultBytes, 0)}
213221
}
214222

215223
func (handler *Handler) getQueryIterator(txContext *transactionContext, queryID string) commonledger.ResultsIterator {
@@ -218,10 +226,12 @@ func (handler *Handler) getQueryIterator(txContext *transactionContext, queryID
218226
return txContext.queryIteratorMap[queryID]
219227
}
220228

221-
func (handler *Handler) deleteQueryIterator(txContext *transactionContext, queryID string) {
229+
func (handler *Handler) cleanupQueryContext(txContext *transactionContext, queryID string) {
222230
handler.Lock()
223231
defer handler.Unlock()
232+
txContext.queryIteratorMap[queryID].Close()
224233
delete(txContext.queryIteratorMap, queryID)
234+
delete(txContext.pendingQueryResults, queryID)
225235
}
226236

227237
// Check if the transactor is allow to call this chaincode on this channel
@@ -710,8 +720,7 @@ func (handler *Handler) handleGetStateByRange(msg *pb.ChaincodeMessage) {
710720

711721
errHandler := func(err error, iter commonledger.ResultsIterator, errFmt string, errArgs ...interface{}) {
712722
if iter != nil {
713-
iter.Close()
714-
handler.deleteQueryIterator(txContext, iterID)
723+
handler.cleanupQueryContext(txContext, iterID)
715724
}
716725
payload := []byte(err.Error())
717726
chaincodeLogger.Errorf(errFmt, errArgs...)
@@ -730,7 +739,8 @@ func (handler *Handler) handleGetStateByRange(msg *pb.ChaincodeMessage) {
730739
return
731740
}
732741

733-
handler.putQueryIterator(txContext, iterID, rangeIter)
742+
handler.initializeQueryContext(txContext, iterID, rangeIter)
743+
734744
var payload *pb.QueryResponse
735745
payload, err = getQueryResponse(handler, txContext, rangeIter, iterID)
736746
if err != nil {
@@ -755,39 +765,52 @@ const maxResultLimit = 100
755765
//getQueryResponse takes an iterator and fetch state to construct QueryResponse
756766
func getQueryResponse(handler *Handler, txContext *transactionContext, iter commonledger.ResultsIterator,
757767
iterID string) (*pb.QueryResponse, error) {
758-
759-
var err error
760-
var queryResult commonledger.QueryResult
761-
var queryResultsBytes []*pb.QueryResultBytes
762-
763-
for i := 0; i < maxResultLimit; i++ {
764-
queryResult, err = iter.Next()
765-
if err != nil {
768+
pendingQueryResults := txContext.pendingQueryResults[iterID]
769+
for {
770+
queryResult, err := iter.Next()
771+
switch {
772+
case err != nil:
766773
chaincodeLogger.Errorf("Failed to get query result from iterator")
767-
break
768-
}
769-
if queryResult == nil {
770-
break
771-
}
772-
var resultBytes []byte
773-
resultBytes, err = proto.Marshal(queryResult.(proto.Message))
774-
if err != nil {
775-
chaincodeLogger.Errorf("Failed to get encode query result as bytes")
776-
break
774+
handler.cleanupQueryContext(txContext, iterID)
775+
return nil, err
776+
case queryResult == nil:
777+
// nil response from iterator indicates end of query results
778+
batch := pendingQueryResults.cut()
779+
handler.cleanupQueryContext(txContext, iterID)
780+
return &pb.QueryResponse{Results: batch, HasMore: false, Id: iterID}, nil
781+
case pendingQueryResults.count == maxResultLimit:
782+
// max number of results queued up, cut batch, then add current result to pending batch
783+
batch := pendingQueryResults.cut()
784+
if err := pendingQueryResults.add(queryResult); err != nil {
785+
handler.cleanupQueryContext(txContext, iterID)
786+
return nil, err
787+
}
788+
return &pb.QueryResponse{Results: batch, HasMore: true, Id: iterID}, nil
789+
default:
790+
if err := pendingQueryResults.add(queryResult); err != nil {
791+
handler.cleanupQueryContext(txContext, iterID)
792+
return nil, err
793+
}
777794
}
778-
779-
qresultBytes := pb.QueryResultBytes{ResultBytes: resultBytes}
780-
queryResultsBytes = append(queryResultsBytes, &qresultBytes)
781795
}
796+
}
782797

783-
if queryResult == nil || err != nil {
784-
iter.Close()
785-
handler.deleteQueryIterator(txContext, iterID)
786-
if err != nil {
787-
return nil, err
788-
}
798+
func (p *pendingQueryResult) cut() []*pb.QueryResultBytes {
799+
batch := p.batch
800+
p.batch = nil
801+
p.count = 0
802+
return batch
803+
}
804+
805+
func (p *pendingQueryResult) add(queryResult commonledger.QueryResult) error {
806+
queryResultBytes, err := proto.Marshal(queryResult.(proto.Message))
807+
if err != nil {
808+
chaincodeLogger.Errorf("Failed to get encode query result as bytes")
809+
return err
789810
}
790-
return &pb.QueryResponse{Results: queryResultsBytes, HasMore: queryResult != nil, Id: iterID}, nil
811+
p.batch = append(p.batch, &pb.QueryResultBytes{ResultBytes: queryResultBytes})
812+
p.count = len(p.batch)
813+
return nil
791814
}
792815

793816
// afterQueryStateNext handles a QUERY_STATE_NEXT request from the chaincode.
@@ -831,8 +854,7 @@ func (handler *Handler) handleQueryStateNext(msg *pb.ChaincodeMessage) {
831854

832855
errHandler := func(payload []byte, iter commonledger.ResultsIterator, errFmt string, errArgs ...interface{}) {
833856
if iter != nil {
834-
iter.Close()
835-
handler.deleteQueryIterator(txContext, queryStateNext.Id)
857+
handler.cleanupQueryContext(txContext, queryStateNext.Id)
836858
}
837859
chaincodeLogger.Errorf(errFmt, errArgs...)
838860
serialSendMsg = &pb.ChaincodeMessage{Type: pb.ChaincodeMessage_ERROR, Payload: payload, Txid: msg.Txid, ChannelId: msg.ChannelId}
@@ -931,8 +953,7 @@ func (handler *Handler) handleQueryStateClose(msg *pb.ChaincodeMessage) {
931953

932954
iter := handler.getQueryIterator(txContext, queryStateClose.Id)
933955
if iter != nil {
934-
iter.Close()
935-
handler.deleteQueryIterator(txContext, queryStateClose.Id)
956+
handler.cleanupQueryContext(txContext, queryStateClose.Id)
936957
}
937958

938959
payload := &pb.QueryResponse{HasMore: false, Id: queryStateClose.Id}
@@ -989,8 +1010,7 @@ func (handler *Handler) handleGetQueryResult(msg *pb.ChaincodeMessage) {
9891010

9901011
errHandler := func(payload []byte, iter commonledger.ResultsIterator, errFmt string, errArgs ...interface{}) {
9911012
if iter != nil {
992-
iter.Close()
993-
handler.deleteQueryIterator(txContext, iterID)
1013+
handler.cleanupQueryContext(txContext, iterID)
9941014
}
9951015
chaincodeLogger.Errorf(errFmt, errArgs...)
9961016
serialSendMsg = &pb.ChaincodeMessage{Type: pb.ChaincodeMessage_ERROR, Payload: payload, Txid: msg.Txid, ChannelId: msg.ChannelId}
@@ -1025,7 +1045,8 @@ func (handler *Handler) handleGetQueryResult(msg *pb.ChaincodeMessage) {
10251045
return
10261046
}
10271047

1028-
handler.putQueryIterator(txContext, iterID, executeIter)
1048+
handler.initializeQueryContext(txContext, iterID, executeIter)
1049+
10291050
var payload *pb.QueryResponse
10301051
payload, err = getQueryResponse(handler, txContext, executeIter, iterID)
10311052
if err != nil {
@@ -1087,8 +1108,7 @@ func (handler *Handler) handleGetHistoryForKey(msg *pb.ChaincodeMessage) {
10871108

10881109
errHandler := func(payload []byte, iter commonledger.ResultsIterator, errFmt string, errArgs ...interface{}) {
10891110
if iter != nil {
1090-
iter.Close()
1091-
handler.deleteQueryIterator(txContext, iterID)
1111+
handler.cleanupQueryContext(txContext, iterID)
10921112
}
10931113
chaincodeLogger.Errorf(errFmt, errArgs...)
10941114
serialSendMsg = &pb.ChaincodeMessage{Type: pb.ChaincodeMessage_ERROR, Payload: payload, Txid: msg.Txid, ChannelId: msg.ChannelId}
@@ -1115,7 +1135,7 @@ func (handler *Handler) handleGetHistoryForKey(msg *pb.ChaincodeMessage) {
11151135
return
11161136
}
11171137

1118-
handler.putQueryIterator(txContext, iterID, historyIter)
1138+
handler.initializeQueryContext(txContext, iterID, historyIter)
11191139

11201140
var payload *pb.QueryResponse
11211141
payload, err = getQueryResponse(handler, txContext, historyIter, iterID)

core/chaincode/handler_test.go

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
Copyright IBM Corp. All Rights Reserved.
3+
4+
SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
package chaincode
8+
9+
import (
10+
"fmt"
11+
"math"
12+
"testing"
13+
14+
"github.com/hyperledger/fabric/common/ledger"
15+
"github.com/hyperledger/fabric/protos/ledger/queryresult"
16+
"github.com/stretchr/testify/assert"
17+
"github.com/stretchr/testify/mock"
18+
)
19+
20+
func TestGetQueryResponse(t *testing.T) {
21+
22+
queryResult := &queryresult.KV{
23+
Key: "key",
24+
Namespace: "namespace",
25+
Value: []byte("value"),
26+
}
27+
28+
// test various boundry cases around maxResultLimit
29+
testCases := []struct {
30+
expectedResultCount int
31+
expectedHasMoreCount int
32+
}{
33+
{0, 0},
34+
{1, 0},
35+
{10, 0},
36+
{maxResultLimit - 2, 0},
37+
{maxResultLimit - 1, 0},
38+
{maxResultLimit, 0},
39+
{maxResultLimit + 1, 1},
40+
{maxResultLimit + 2, 1},
41+
{int(math.Floor(maxResultLimit * 1.5)), 1},
42+
{maxResultLimit * 2, 1},
43+
{10*maxResultLimit - 2, 9},
44+
{10*maxResultLimit - 1, 9},
45+
{10 * maxResultLimit, 9},
46+
{10*maxResultLimit + 1, 10},
47+
{10*maxResultLimit + 2, 10},
48+
}
49+
50+
for _, tc := range testCases {
51+
handler := &Handler{}
52+
transactionContext := &transactionContext{
53+
queryIteratorMap: make(map[string]ledger.ResultsIterator),
54+
pendingQueryResults: make(map[string]*pendingQueryResult),
55+
}
56+
queryID := "test"
57+
t.Run(fmt.Sprintf("%d", tc.expectedResultCount), func(t *testing.T) {
58+
resultsIterator := &MockResultsIterator{}
59+
handler.initializeQueryContext(transactionContext, queryID, resultsIterator)
60+
if tc.expectedResultCount > 0 {
61+
resultsIterator.On("Next").Return(queryResult, nil).Times(tc.expectedResultCount)
62+
}
63+
resultsIterator.On("Next").Return(nil, nil).Once()
64+
resultsIterator.On("Close").Return().Once()
65+
totalResultCount := 0
66+
for hasMoreCount := 0; hasMoreCount <= tc.expectedHasMoreCount; hasMoreCount++ {
67+
queryResponse, _ := getQueryResponse(handler, transactionContext, resultsIterator, queryID)
68+
assert.NotNil(t, queryResponse.GetResults())
69+
if queryResponse.GetHasMore() {
70+
t.Logf("Got %d results and more are expected.", len(queryResponse.GetResults()))
71+
} else {
72+
t.Logf("Got %d results and no more are expected.", len(queryResponse.GetResults()))
73+
}
74+
75+
switch {
76+
case hasMoreCount < tc.expectedHasMoreCount:
77+
// max limit sized batch retrieved, more expected
78+
assert.True(t, queryResponse.GetHasMore())
79+
assert.Len(t, queryResponse.GetResults(), maxResultLimit)
80+
default:
81+
// remainder retrieved, no more expected
82+
assert.Len(t, queryResponse.GetResults(), tc.expectedResultCount-totalResultCount)
83+
assert.False(t, queryResponse.GetHasMore())
84+
85+
}
86+
87+
totalResultCount += len(queryResponse.GetResults())
88+
}
89+
resultsIterator.AssertExpectations(t)
90+
})
91+
}
92+
93+
}
94+
95+
type MockResultsIterator struct {
96+
mock.Mock
97+
}
98+
99+
func (m *MockResultsIterator) Next() (ledger.QueryResult, error) {
100+
args := m.Called()
101+
if args.Get(0) == nil {
102+
return nil, args.Error(1)
103+
}
104+
return args.Get(0).(ledger.QueryResult), args.Error(1)
105+
}
106+
107+
func (m *MockResultsIterator) Close() {
108+
m.Called()
109+
}

0 commit comments

Comments
 (0)