From 0f4c16e2e20b273c916ec27aa5cde878a328a525 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Sun, 8 Jan 2023 14:14:44 -0500 Subject: [PATCH 1/2] Fix coverage gap in FFIErrors, and make DB interface consistent Signed-off-by: Peter Broadhurst --- internal/contracts/manager.go | 6 +- internal/contracts/manager_test.go | 4 +- internal/database/sqlcommon/ffi_errors_sql.go | 24 ++- .../database/sqlcommon/ffi_errors_sql_test.go | 177 ++++++++++++++++++ mocks/databasemocks/plugin.go | 29 ++- pkg/database/plugin.go | 2 +- 6 files changed, 218 insertions(+), 24 deletions(-) create mode 100644 internal/database/sqlcommon/ffi_errors_sql_test.go diff --git a/internal/contracts/manager.go b/internal/contracts/manager.go index fe7af44890..21e46df4d2 100644 --- a/internal/contracts/manager.go +++ b/internal/contracts/manager.go @@ -169,7 +169,8 @@ func (cm *contractManager) getFFIChildren(ctx context.Context, ffi *fftypes.FFI) event.Signature = cm.blockchain.GenerateEventSignature(ctx, &event.FFIEventDefinition) } - ffi.Errors, err = cm.database.GetFFIErrors(ctx, cm.namespace, ffi.ID) + fb := database.FFIErrorQueryFactory.NewFilter(ctx) + ffi.Errors, _, err = cm.database.GetFFIErrors(ctx, cm.namespace, fb.Eq("interface", ffi.ID)) if err != nil { return err } @@ -326,7 +327,8 @@ func (cm *contractManager) resolveInvokeContractRequest(ctx context.Context, req if err != nil || req.Method == nil { return i18n.NewError(ctx, coremsgs.MsgContractMethodResolveError, err) } - req.Errors, err = cm.database.GetFFIErrors(ctx, cm.namespace, req.Interface) + fb := database.FFIErrorQueryFactory.NewFilter(ctx) + req.Errors, _, err = cm.database.GetFFIErrors(ctx, cm.namespace, fb.Eq("interface", req.Interface)) if err != nil { return i18n.NewError(ctx, coremsgs.MsgContractErrorsResolveError, err) } diff --git a/internal/contracts/manager_test.go b/internal/contracts/manager_test.go index 638b40fb44..897433528b 100644 --- a/internal/contracts/manager_test.go +++ b/internal/contracts/manager_test.go @@ -1350,7 +1350,7 @@ func TestGetFFIByIDWithChildrenErrorsFail(t *testing.T) { {ID: fftypes.NewUUID(), Name: "method1"}, }, nil, nil) mdb.On("GetFFIEvents", mock.Anything, "ns1", mock.Anything).Return([]*fftypes.FFIEvent{}, nil, nil) - mdb.On("GetFFIErrors", mock.Anything, "ns1", mock.Anything).Return(nil, fmt.Errorf("pop")) + mdb.On("GetFFIErrors", mock.Anything, "ns1", mock.Anything).Return(nil, nil, fmt.Errorf("pop")) _, err := cm.GetFFIByIDWithChildren(context.Background(), cid) @@ -1775,7 +1775,7 @@ func TestInvokeContractErrorsFail(t *testing.T) { mim.On("NormalizeSigningKey", mock.Anything, "", identity.KeyNormalizationBlockchainPlugin).Return("key-resolved", nil) mdb.On("GetFFIMethod", mock.Anything, "ns1", req.Interface, req.MethodPath).Return(&fftypes.FFIMethod{Name: "set"}, nil) - mdb.On("GetFFIErrors", mock.Anything, "ns1", req.Interface).Return(nil, fmt.Errorf("pop")) + mdb.On("GetFFIErrors", mock.Anything, "ns1", mock.Anything).Return(nil, nil, fmt.Errorf("pop")) _, err := cm.InvokeContract(context.Background(), req, false) diff --git a/internal/database/sqlcommon/ffi_errors_sql.go b/internal/database/sqlcommon/ffi_errors_sql.go index 09ba5fb9af..364b903226 100644 --- a/internal/database/sqlcommon/ffi_errors_sql.go +++ b/internal/database/sqlcommon/ffi_errors_sql.go @@ -21,6 +21,7 @@ import ( "database/sql" sq "github.com/Masterminds/squirrel" + "github.com/hyperledger/firefly-common/pkg/ffapi" "github.com/hyperledger/firefly-common/pkg/fftypes" "github.com/hyperledger/firefly-common/pkg/i18n" "github.com/hyperledger/firefly/internal/coremsgs" @@ -38,6 +39,9 @@ var ( "description", "params", } + ffiErrorFilterFieldMap = map[string]string{ + "interface": "interface_id", + } ) const ffierrorsTable = "ffierrors" @@ -112,25 +116,27 @@ func (s *SQLCommon) ffiErrorResult(ctx context.Context, row *sql.Rows) (*fftypes return &errorDef, nil } -func (s *SQLCommon) GetFFIErrors(ctx context.Context, namespace string, interfaceID *fftypes.UUID) (errors []*fftypes.FFIError, err error) { - rows, _, err := s.Query(ctx, ffierrorsTable, - sq.Select(ffiErrorsColumns...). - From(ffierrorsTable). - Where(sq.Eq{"namespace": namespace, "interface_id": interfaceID}), - ) +func (s *SQLCommon) GetFFIErrors(ctx context.Context, namespace string, filter ffapi.Filter) (errors []*fftypes.FFIError, res *ffapi.FilterResult, err error) { + query, fop, fi, err := s.FilterSelect(ctx, "", sq.Select(ffiErrorsColumns...).From(ffierrorsTable), + filter, ffiErrorFilterFieldMap, []interface{}{"sequence"}, sq.Eq{"namespace": namespace}) + if err != nil { + return nil, nil, err + } + + rows, tx, err := s.Query(ctx, ffierrorsTable, query) if err != nil { - return nil, err + return nil, nil, err } defer rows.Close() for rows.Next() { ci, err := s.ffiErrorResult(ctx, rows) if err != nil { - return nil, err + return nil, nil, err } errors = append(errors, ci) } - return errors, err + return errors, s.QueryRes(ctx, ffierrorsTable, tx, fop, fi), err } diff --git a/internal/database/sqlcommon/ffi_errors_sql_test.go b/internal/database/sqlcommon/ffi_errors_sql_test.go new file mode 100644 index 0000000000..75def26082 --- /dev/null +++ b/internal/database/sqlcommon/ffi_errors_sql_test.go @@ -0,0 +1,177 @@ +// Copyright © 2021 Kaleido, Inc. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sqlcommon + +import ( + "context" + "encoding/json" + "fmt" + "testing" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/hyperledger/firefly-common/pkg/fftypes" + "github.com/hyperledger/firefly/pkg/core" + "github.com/hyperledger/firefly/pkg/database" + "github.com/stretchr/testify/assert" +) + +func TestFFIErrorsE2EWithDB(t *testing.T) { + + s, cleanup := newSQLiteTestProvider(t) + defer cleanup() + ctx := context.Background() + + // Create a new ffiErr entry + interfaceID := fftypes.NewUUID() + ffiErrID := fftypes.NewUUID() + ffiErr := &fftypes.FFIError{ + ID: ffiErrID, + Interface: interfaceID, + Namespace: "ns", + Pathname: "Changed_1", + FFIErrorDefinition: fftypes.FFIErrorDefinition{ + Name: "Changed", + Description: "Things changed", + Params: fftypes.FFIParams{ + { + Name: "value", + Schema: fftypes.JSONAnyPtr(`{"type": "integer"}`), + }, + }, + }, + } + + s.callbacks.On("UUIDCollectionNSEvent", database.CollectionFFIErrors, core.ChangeEventTypeCreated, "ns", ffiErrID).Return() + s.callbacks.On("UUIDCollectionNSEvent", database.CollectionFFIErrors, core.ChangeEventTypeUpdated, "ns", ffiErrID).Return() + + err := s.UpsertFFIError(ctx, ffiErr) + assert.NoError(t, err) + + // Query back the ffiErr (by query filter) + fb := database.FFIErrorQueryFactory.NewFilter(ctx) + filter := fb.And( + fb.Eq("name", ffiErr.Name), + ) + errors, res, err := s.GetFFIErrors(ctx, "ns", filter.Count(true)) + assert.NoError(t, err) + assert.Equal(t, 1, len(errors)) + assert.Equal(t, int64(1), *res.TotalCount) + ffiErrJson, _ := json.Marshal(&ffiErr) + ffiErrReadJson, _ := json.Marshal(errors[0]) + assert.Equal(t, string(ffiErrJson), string(ffiErrReadJson)) + + // Update ffiErr + ffiErr.Params = fftypes.FFIParams{} + err = s.UpsertFFIError(ctx, ffiErr) + assert.NoError(t, err) + + s.callbacks.AssertExpectations(t) +} + +func TestFFIErrorDBFailBeginTransaction(t *testing.T) { + s, mock := newMockProvider().init() + mock.ExpectBegin().WillReturnError(fmt.Errorf("pop")) + err := s.UpsertFFIError(context.Background(), &fftypes.FFIError{}) + assert.Regexp(t, "FF00175", err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestFFIErrorDBFailSelect(t *testing.T) { + s, mock := newMockProvider().init() + mock.ExpectBegin() + mock.ExpectQuery("SELECT .*").WillReturnError(fmt.Errorf("pop")) + err := s.UpsertFFIError(context.Background(), &fftypes.FFIError{}) + assert.Regexp(t, "pop", err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestFFIErrorDBFailInsert(t *testing.T) { + rows := sqlmock.NewRows([]string{"id", "namespace", "name", "version"}) + s, mock := newMockProvider().init() + mock.ExpectBegin() + mock.ExpectQuery("SELECT .*").WillReturnRows(rows) + ffiErr := &fftypes.FFIError{ + ID: fftypes.NewUUID(), + } + err := s.UpsertFFIError(context.Background(), ffiErr) + assert.Regexp(t, "FF00177", err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestFFIErrorDBFailUpdate(t *testing.T) { + rows := sqlmock.NewRows([]string{"id", "namespace", "name", "version"}). + AddRow("7e2c001c-e270-4fd7-9e82-9dacee843dc2", "ns1", "math", "v1.0.0") + s, mock := newMockProvider().init() + mock.ExpectBegin() + mock.ExpectQuery("SELECT .*").WillReturnRows(rows) + mock.ExpectQuery("UPDATE .*").WillReturnError(fmt.Errorf("pop")) + ffiErr := &fftypes.FFIError{ + ID: fftypes.NewUUID(), + } + err := s.UpsertFFIError(context.Background(), ffiErr) + assert.Regexp(t, "pop", err) +} + +func TestGetFFIErrors(t *testing.T) { + // filter := database.FFIErrorQueryFactory.NewFilter(context.Background()).In("", []driver.Value{}) + + fb := database.FFIErrorQueryFactory.NewFilter(context.Background()) + filter := fb.And( + fb.Eq("name", "sum"), + ) + s, mock := newMockProvider().init() + rows := sqlmock.NewRows(ffiErrorsColumns). + AddRow(fftypes.NewUUID().String(), fftypes.NewUUID().String(), "ns1", "sum", "sum", "", []byte(`[]`)) + mock.ExpectQuery("SELECT .*").WillReturnRows(rows) + _, _, err := s.GetFFIErrors(context.Background(), "ns1", filter) + assert.NoError(t, err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetFFIErrorsFilterSelectFail(t *testing.T) { + fb := database.FFIErrorQueryFactory.NewFilter(context.Background()) + s, _ := newMockProvider().init() + _, _, err := s.GetFFIErrors(context.Background(), "ns1", fb.And(fb.Eq("id", map[bool]bool{true: false}))) + assert.Error(t, err) +} + +func TestGetFFIErrorsQueryFail(t *testing.T) { + fb := database.FFIErrorQueryFactory.NewFilter(context.Background()) + filter := fb.And( + fb.Eq("id", fftypes.NewUUID()), + ) + s, mock := newMockProvider().init() + mock.ExpectQuery("SELECT .*").WillReturnError(fmt.Errorf("pop")) + _, _, err := s.GetFFIErrors(context.Background(), "ns1", filter) + assert.Regexp(t, "pop", err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +func TestGetFFIErrorsQueryResultFail(t *testing.T) { + fb := database.FFIErrorQueryFactory.NewFilter(context.Background()) + filter := fb.And( + fb.Eq("id", fftypes.NewUUID()), + ) + s, mock := newMockProvider().init() + rows := sqlmock.NewRows([]string{"id", "namespace", "name", "version"}). + AddRow("7e2c001c-e270-4fd7-9e82-9dacee843dc2", "ns1", "math", "v1.0.0"). + AddRow("7e2c001c-e270-4fd7-9e82-9dacee843dc2", nil, "math", "v1.0.0") + mock.ExpectQuery("SELECT .*").WillReturnRows(rows) + _, _, err := s.GetFFIErrors(context.Background(), "ns1", filter) + assert.Regexp(t, "FF10121", err) + assert.NoError(t, mock.ExpectationsWereMet()) +} diff --git a/mocks/databasemocks/plugin.go b/mocks/databasemocks/plugin.go index 4b1fea3d11..c2796cc7b8 100644 --- a/mocks/databasemocks/plugin.go +++ b/mocks/databasemocks/plugin.go @@ -811,27 +811,36 @@ func (_m *Plugin) GetFFIByID(ctx context.Context, namespace string, id *fftypes. return r0, r1 } -// GetFFIErrors provides a mock function with given fields: ctx, namespace, interfaceID -func (_m *Plugin) GetFFIErrors(ctx context.Context, namespace string, interfaceID *fftypes.UUID) ([]*fftypes.FFIError, error) { - ret := _m.Called(ctx, namespace, interfaceID) +// GetFFIErrors provides a mock function with given fields: ctx, namespace, filter +func (_m *Plugin) GetFFIErrors(ctx context.Context, namespace string, filter ffapi.Filter) ([]*fftypes.FFIError, *ffapi.FilterResult, error) { + ret := _m.Called(ctx, namespace, filter) var r0 []*fftypes.FFIError - if rf, ok := ret.Get(0).(func(context.Context, string, *fftypes.UUID) []*fftypes.FFIError); ok { - r0 = rf(ctx, namespace, interfaceID) + if rf, ok := ret.Get(0).(func(context.Context, string, ffapi.Filter) []*fftypes.FFIError); ok { + r0 = rf(ctx, namespace, filter) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*fftypes.FFIError) } } - var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, *fftypes.UUID) error); ok { - r1 = rf(ctx, namespace, interfaceID) + var r1 *ffapi.FilterResult + if rf, ok := ret.Get(1).(func(context.Context, string, ffapi.Filter) *ffapi.FilterResult); ok { + r1 = rf(ctx, namespace, filter) } else { - r1 = ret.Error(1) + if ret.Get(1) != nil { + r1 = ret.Get(1).(*ffapi.FilterResult) + } } - return r0, r1 + var r2 error + if rf, ok := ret.Get(2).(func(context.Context, string, ffapi.Filter) error); ok { + r2 = rf(ctx, namespace, filter) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 } // GetFFIEvent provides a mock function with given fields: ctx, namespace, interfaceID, pathName diff --git a/pkg/database/plugin.go b/pkg/database/plugin.go index a24ca27b35..9c16822bf1 100644 --- a/pkg/database/plugin.go +++ b/pkg/database/plugin.go @@ -450,7 +450,7 @@ type iFFIErrorCollection interface { UpsertFFIError(ctx context.Context, method *fftypes.FFIError) error // GetFFIErrors - Get FFI error - GetFFIErrors(ctx context.Context, namespace string, interfaceID *fftypes.UUID) (errors []*fftypes.FFIError, err error) + GetFFIErrors(ctx context.Context, namespace string, filter ffapi.Filter) (errors []*fftypes.FFIError, res *ffapi.FilterResult, err error) } type iContractAPICollection interface { From abcfcd99b5277e6169d5e1ee638cdc3fcfe40908 Mon Sep 17 00:00:00 2001 From: Peter Broadhurst Date: Sun, 8 Jan 2023 14:26:26 -0500 Subject: [PATCH 2/2] Fix metrics coverage Signed-off-by: Peter Broadhurst --- internal/metrics/metrics_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 19184c59a5..7e8c685f46 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -171,6 +171,17 @@ func TestTransferConfirmedMintBurn(t *testing.T) { assert.Equal(t, len(mm.timeMap), 0) } +func TestBlockchainContractDeployment(t *testing.T) { + mm, cancel := newTestMetricsManager(t) + defer cancel() + mm.timeMap[tokenLocalID.String()] = time.Now() + mm.BlockchainContractDeployment() + m, err := BlockchainTransactionsCounter.GetMetricWith(prometheus.Labels{LocationLabelName: "", MethodNameLabelName: ""}) + assert.NoError(t, err) + v := testutil.ToFloat64(m) + assert.Equal(t, float64(1), v) +} + func TestBlockchainTransaction(t *testing.T) { mm, cancel := newTestMetricsManager(t) defer cancel()