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

Introducing ExtendedBatchGetLatestValuesGraceful #546

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 57 additions & 0 deletions mocks/pkg/contractreader/extended.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 61 additions & 0 deletions pkg/contractreader/extended.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,21 @@ type Extended interface {
ctx context.Context,
request ExtendedBatchGetLatestValuesRequest,
) (types.BatchGetLatestValuesResult, error)

// ExtendedBatchGetLatestValuesGraceful performs the same operations as ExtendedBatchGetLatestValues
// but handles ErrNoBindings gracefully by:
// 1. Skipping contracts with no bindings instead of returning an error
// 2. Returning partial results for contracts that do have bindings
// 3. Including information about which contracts were skipped due to no bindings
ExtendedBatchGetLatestValuesGraceful(
Comment on lines +88 to +93
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using ExtendedBatchGetLatestValues anywhere? Maybe we can add this functionality to the existing function instead of creating a new one.

ctx context.Context,
request ExtendedBatchGetLatestValuesRequest,
) (BatchGetLatestValuesGracefulResult, error)
}

type BatchGetLatestValuesGracefulResult struct {
Results types.BatchGetLatestValuesResult
SkippedNoBinds []string // List of contract names that were skipped due to no bindings
}

type ExtendedBatchGetLatestValuesRequest map[string]types.ContractBatch
Expand Down Expand Up @@ -292,6 +307,52 @@ func (e *extendedContractReader) Bind(ctx context.Context, allBindings []types.B
return nil
}

func (e *extendedContractReader) ExtendedBatchGetLatestValuesGraceful(
ctx context.Context,
request ExtendedBatchGetLatestValuesRequest,
) (BatchGetLatestValuesGracefulResult, error) {
// Convert the request from contract names to BoundContracts
convertedRequest := make(types.BatchGetLatestValuesRequest)
var skippedContracts []string

for contractName, batch := range request {
// Get the binding for this contract name
binding, err := e.getOneBinding(contractName)
if err != nil {
if errors.Is(err, ErrNoBindings) {
// Track skipped contracts but continue processing
skippedContracts = append(skippedContracts, contractName)
continue
}
// Return error for any other binding issues
return BatchGetLatestValuesGracefulResult{}, fmt.Errorf(
"BatchGetLatestValuesGraceful: failed to get binding for contract %s: %w", contractName, err)
}

// Use the resolved binding for the request
convertedRequest[binding.Binding] = batch
}

// If we have no valid bindings after filtering, return empty result
if len(convertedRequest) == 0 {
return BatchGetLatestValuesGracefulResult{
Results: make(types.BatchGetLatestValuesResult),
SkippedNoBinds: skippedContracts,
}, nil
}

// Call the underlying BatchGetLatestValues with the converted request
results, err := e.BatchGetLatestValues(ctx, convertedRequest)
if err != nil {
return BatchGetLatestValuesGracefulResult{}, err
}

return BatchGetLatestValuesGracefulResult{
Results: results,
SkippedNoBinds: skippedContracts,
}, nil
}

func (e *extendedContractReader) GetBindings(contractName string) []ExtendedBoundContract {
e.mu.RLock()
defer e.mu.RUnlock()
Expand Down
146 changes: 146 additions & 0 deletions pkg/contractreader/extended_unit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,152 @@ func TestExtendedBatchGetLatestValues(t *testing.T) {
}
}

func TestExtendedBatchGetLatestValuesGraceful(t *testing.T) {
tests := []struct {
name string
bindings map[string][]ExtendedBoundContract
request ExtendedBatchGetLatestValuesRequest
mockResponse types.BatchGetLatestValuesResult
expectedError error
expectedResult BatchGetLatestValuesGracefulResult
}{
{
name: "all contracts valid",
bindings: map[string][]ExtendedBoundContract{
"contract1": {
{Binding: types.BoundContract{Name: "contract1", Address: "0x123"}},
},
"contract2": {
{Binding: types.BoundContract{Name: "contract2", Address: "0x456"}},
},
},
request: ExtendedBatchGetLatestValuesRequest{
"contract1": {
{ReadName: "read1", Params: "params1", ReturnVal: "return1"},
},
"contract2": {
{ReadName: "read2", Params: "params2", ReturnVal: "return2"},
},
},
mockResponse: types.BatchGetLatestValuesResult{
types.BoundContract{Name: "contract1", Address: "0x123"}: {
{ReadName: "read1"},
},
types.BoundContract{Name: "contract2", Address: "0x456"}: {
{ReadName: "read2"},
},
},
expectedError: nil,
expectedResult: BatchGetLatestValuesGracefulResult{
Results: types.BatchGetLatestValuesResult{
types.BoundContract{Name: "contract1", Address: "0x123"}: {
{ReadName: "read1"},
},
types.BoundContract{Name: "contract2", Address: "0x456"}: {
{ReadName: "read2"},
},
},
SkippedNoBinds: nil,
},
},
{
name: "some contracts skipped",
bindings: map[string][]ExtendedBoundContract{
"contract1": {
{Binding: types.BoundContract{Name: "contract1", Address: "0x123"}},
},
},
request: ExtendedBatchGetLatestValuesRequest{
"contract1": {
{ReadName: "read1", Params: "params1", ReturnVal: "return1"},
},
"nonexistent": {
{ReadName: "read2", Params: "params2", ReturnVal: "return2"},
},
},
mockResponse: types.BatchGetLatestValuesResult{
types.BoundContract{Name: "contract1", Address: "0x123"}: {
{ReadName: "read1"},
},
},
expectedError: nil,
expectedResult: BatchGetLatestValuesGracefulResult{
Results: types.BatchGetLatestValuesResult{
types.BoundContract{Name: "contract1", Address: "0x123"}: {
{ReadName: "read1"},
},
},
SkippedNoBinds: []string{"nonexistent"},
},
},
{
name: "all contracts skipped",
bindings: map[string][]ExtendedBoundContract{
"contract1": {
{Binding: types.BoundContract{Name: "contract1", Address: "0x123"}},
},
},
request: ExtendedBatchGetLatestValuesRequest{
"nonexistent1": {
{ReadName: "read1", Params: "params1", ReturnVal: "return1"},
},
"nonexistent2": {
{ReadName: "read2", Params: "params2", ReturnVal: "return2"},
},
},
mockResponse: nil,
expectedError: nil,
expectedResult: BatchGetLatestValuesGracefulResult{
Results: types.BatchGetLatestValuesResult{},
SkippedNoBinds: []string{"nonexistent1", "nonexistent2"},
},
},
{
name: "error on too many bindings",
bindings: map[string][]ExtendedBoundContract{
"contract1": {
{Binding: types.BoundContract{Name: "contract1", Address: "0x123"}},
{Binding: types.BoundContract{Name: "contract1", Address: "0x456"}},
},
},
request: ExtendedBatchGetLatestValuesRequest{
"contract1": {
{ReadName: "read1", Params: "params1", ReturnVal: "return1"},
},
},
mockResponse: nil,
expectedError: ErrTooManyBindings,
expectedResult: BatchGetLatestValuesGracefulResult{},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

mockReader := &mockContractReader{
BatchGetLatestValuesResponse: tt.mockResponse,
}

extendedReader := &extendedContractReader{
reader: mockReader,
contractBindingsByName: tt.bindings,
mu: &sync.RWMutex{},
}

result, err := extendedReader.ExtendedBatchGetLatestValuesGraceful(context.Background(), tt.request)

if tt.expectedError != nil {
assert.ErrorIs(t, err, tt.expectedError)
} else {
assert.NoError(t, err)
assert.Equal(t, tt.expectedResult.Results, result.Results)
assert.ElementsMatch(t, tt.expectedResult.SkippedNoBinds, result.SkippedNoBinds)
}
})
}
}

// mockContractReader implements ContractReaderFacade for testing
type mockContractReader struct {
ContractReaderFacade
Expand Down
Loading