From fc62c67eae3c7b4cf52bc0a095c3e4235bed9baa Mon Sep 17 00:00:00 2001 From: Nicko Guyer Date: Tue, 22 Feb 2022 11:36:37 -0500 Subject: [PATCH] Fix empty swagger URLs on contract API creation Signed-off-by: Nicko Guyer --- .../apiserver/route_post_new_contract_api.go | 2 +- .../route_post_new_contract_api_test.go | 4 +-- internal/apiserver/route_put_contract_api.go | 2 +- .../apiserver/route_put_contract_api_test.go | 4 +-- internal/contracts/manager.go | 5 ++-- internal/contracts/manager_test.go | 25 +++++++++++-------- mocks/contractmocks/manager.go | 14 +++++------ 7 files changed, 30 insertions(+), 26 deletions(-) diff --git a/internal/apiserver/route_post_new_contract_api.go b/internal/apiserver/route_post_new_contract_api.go index e1f0fe6334..505895c072 100644 --- a/internal/apiserver/route_post_new_contract_api.go +++ b/internal/apiserver/route_post_new_contract_api.go @@ -45,6 +45,6 @@ var postNewContractAPI = &oapispec.Route{ JSONHandler: func(r *oapispec.APIRequest) (output interface{}, err error) { waitConfirm := strings.EqualFold(r.QP["confirm"], "true") r.SuccessStatus = syncRetcode(waitConfirm) - return getOr(r.Ctx).Contracts().BroadcastContractAPI(r.Ctx, r.PP["ns"], r.Input.(*fftypes.ContractAPI), waitConfirm) + return getOr(r.Ctx).Contracts().BroadcastContractAPI(r.Ctx, r.APIBaseURL, r.PP["ns"], r.Input.(*fftypes.ContractAPI), waitConfirm) }, } diff --git a/internal/apiserver/route_post_new_contract_api_test.go b/internal/apiserver/route_post_new_contract_api_test.go index 2389d22de2..86276b83d9 100644 --- a/internal/apiserver/route_post_new_contract_api_test.go +++ b/internal/apiserver/route_post_new_contract_api_test.go @@ -39,7 +39,7 @@ func TestPostNewContractAPI(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - mcm.On("BroadcastContractAPI", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), false). + mcm.On("BroadcastContractAPI", mock.Anything, mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), false). Return(&fftypes.ContractAPI{}, nil) r.ServeHTTP(res, req) @@ -57,7 +57,7 @@ func TestPostNewContractAPISync(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - mcm.On("BroadcastContractAPI", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), true). + mcm.On("BroadcastContractAPI", mock.Anything, mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), true). Return(&fftypes.ContractAPI{}, nil) r.ServeHTTP(res, req) diff --git a/internal/apiserver/route_put_contract_api.go b/internal/apiserver/route_put_contract_api.go index 1d74921cb4..af0d7ba904 100644 --- a/internal/apiserver/route_put_contract_api.go +++ b/internal/apiserver/route_put_contract_api.go @@ -50,7 +50,7 @@ var putContractAPI = &oapispec.Route{ api.ID, err = fftypes.ParseUUID(r.Ctx, r.PP["id"]) var res interface{} if err == nil { - res, err = getOr(r.Ctx).Contracts().BroadcastContractAPI(r.Ctx, r.PP["ns"], api, waitConfirm) + res, err = getOr(r.Ctx).Contracts().BroadcastContractAPI(r.Ctx, r.APIBaseURL, r.PP["ns"], api, waitConfirm) } return res, err }, diff --git a/internal/apiserver/route_put_contract_api_test.go b/internal/apiserver/route_put_contract_api_test.go index 5809cf6fbe..03612debf6 100644 --- a/internal/apiserver/route_put_contract_api_test.go +++ b/internal/apiserver/route_put_contract_api_test.go @@ -39,7 +39,7 @@ func TestPutContractAPI(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - mcm.On("BroadcastContractAPI", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), false). + mcm.On("BroadcastContractAPI", mock.Anything, mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), false). Return(&fftypes.ContractAPI{}, nil) r.ServeHTTP(res, req) @@ -57,7 +57,7 @@ func TestPutContractAPISync(t *testing.T) { req.Header.Set("Content-Type", "application/json; charset=utf-8") res := httptest.NewRecorder() - mcm.On("BroadcastContractAPI", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), true). + mcm.On("BroadcastContractAPI", mock.Anything, mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), true). Return(&fftypes.ContractAPI{}, nil) r.ServeHTTP(res, req) diff --git a/internal/contracts/manager.go b/internal/contracts/manager.go index 6a3aff5946..81fc3eb771 100644 --- a/internal/contracts/manager.go +++ b/internal/contracts/manager.go @@ -43,7 +43,7 @@ type Manager interface { InvokeContractAPI(ctx context.Context, ns, apiName, methodPath string, req *fftypes.ContractCallRequest) (interface{}, error) GetContractAPI(ctx context.Context, httpServerURL, ns, apiName string) (*fftypes.ContractAPI, error) GetContractAPIs(ctx context.Context, httpServerURL, ns string, filter database.AndFilter) ([]*fftypes.ContractAPI, *database.FilterResult, error) - BroadcastContractAPI(ctx context.Context, ns string, api *fftypes.ContractAPI, waitConfirm bool) (output *fftypes.ContractAPI, err error) + BroadcastContractAPI(ctx context.Context, httpServerURL, ns string, api *fftypes.ContractAPI, waitConfirm bool) (output *fftypes.ContractAPI, err error) SubscribeContract(ctx context.Context, ns, eventPath string, req *fftypes.ContractSubscribeRequest) (*fftypes.ContractSubscription, error) SubscribeContractAPI(ctx context.Context, ns, apiName, eventPath string, req *fftypes.ContractSubscribeRequest) (*fftypes.ContractSubscription, error) @@ -311,7 +311,7 @@ func (cm *contractManager) resolveFFIReference(ctx context.Context, ns string, r } } -func (cm *contractManager) BroadcastContractAPI(ctx context.Context, ns string, api *fftypes.ContractAPI, waitConfirm bool) (output *fftypes.ContractAPI, err error) { +func (cm *contractManager) BroadcastContractAPI(ctx context.Context, httpServerURL, ns string, api *fftypes.ContractAPI, waitConfirm bool) (output *fftypes.ContractAPI, err error) { api.ID = fftypes.NewUUID() api.Namespace = ns @@ -337,6 +337,7 @@ func (cm *contractManager) BroadcastContractAPI(ctx context.Context, ns string, return nil, err } api.Message = msg.Header.ID + cm.addContractURLs(httpServerURL, api) return api, nil } diff --git a/internal/contracts/manager_test.go b/internal/contracts/manager_test.go index 9ccb94259c..785bd04eaa 100644 --- a/internal/contracts/manager_test.go +++ b/internal/contracts/manager_test.go @@ -1501,8 +1501,11 @@ func TestBroadcastContractAPI(t *testing.T) { mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(&fftypes.FFI{}, nil) mbm.On("BroadcastDefinitionAsNode", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), fftypes.SystemTagDefineContractAPI, false).Return(msg, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + api, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.NoError(t, err) + assert.NotNil(t, api) + assert.NotEmpty(t, api.URLs.OpenAPI) + assert.NotEmpty(t, api.URLs.UI) } func TestBroadcastContractAPIExisting(t *testing.T) { @@ -1539,7 +1542,7 @@ func TestBroadcastContractAPIExisting(t *testing.T) { mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(existing, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(&fftypes.FFI{}, nil) mbm.On("BroadcastDefinitionAsNode", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), fftypes.SystemTagDefineContractAPI, false).Return(msg, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.NoError(t, err) } @@ -1577,7 +1580,7 @@ func TestBroadcastContractAPICannotChangeLocation(t *testing.T) { mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(existing, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(&fftypes.FFI{}, nil) mbm.On("BroadcastDefinition", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), fftypes.SystemTagDefineContractAPI, false).Return(msg, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "FF10316", err) } @@ -1606,7 +1609,7 @@ func TestBroadcastContractAPIInterfaceName(t *testing.T) { mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFI", mock.Anything, "ns1", "my-ffi", "1").Return(&fftypes.FFI{ID: interfaceID}, nil) mbm.On("BroadcastDefinitionAsNode", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), fftypes.SystemTagDefineContractAPI, false).Return(msg, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.NoError(t, err) assert.Equal(t, *interfaceID, *api.Interface.ID) } @@ -1629,7 +1632,7 @@ func TestBroadcastContractAPIFail(t *testing.T) { mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(&fftypes.FFI{}, nil) mbm.On("BroadcastDefinitionAsNode", mock.Anything, "ns1", mock.AnythingOfType("*fftypes.ContractAPI"), fftypes.SystemTagDefineContractAPI, false).Return(nil, fmt.Errorf("pop")) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "pop", err) } @@ -1645,7 +1648,7 @@ func TestBroadcastContractAPINoInterface(t *testing.T) { Name: "banana", } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "FF10303", err) } @@ -1665,7 +1668,7 @@ func TestBroadcastContractAPIInterfaceIDFail(t *testing.T) { } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(nil, fmt.Errorf("pop")) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.EqualError(t, err, "pop") } @@ -1685,7 +1688,7 @@ func TestBroadcastContractAPIInterfaceIDNotFound(t *testing.T) { } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFIByID", mock.Anything, api.Interface.ID).Return(nil, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "FF10303.*"+api.Interface.ID.String(), err) } @@ -1706,7 +1709,7 @@ func TestBroadcastContractAPIInterfaceNameFail(t *testing.T) { } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFI", mock.Anything, "ns1", "my-ffi", "1").Return(nil, fmt.Errorf("pop")) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.EqualError(t, err, "pop") } @@ -1727,7 +1730,7 @@ func TestBroadcastContractAPIInterfaceNameNotFound(t *testing.T) { } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) mdb.On("GetFFI", mock.Anything, "ns1", "my-ffi", "1").Return(nil, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "FF10303.*my-ffi", err) } @@ -1746,7 +1749,7 @@ func TestBroadcastContractAPIInterfaceNoVersion(t *testing.T) { }, } mdb.On("GetContractAPIByName", mock.Anything, api.Namespace, api.Name).Return(nil, nil) - _, err := cm.BroadcastContractAPI(context.Background(), "ns1", api, false) + _, err := cm.BroadcastContractAPI(context.Background(), "http://localhost/api", "ns1", api, false) assert.Regexp(t, "FF10303.*my-ffi", err) } diff --git a/mocks/contractmocks/manager.go b/mocks/contractmocks/manager.go index 546d058fcf..1bf29b0685 100644 --- a/mocks/contractmocks/manager.go +++ b/mocks/contractmocks/manager.go @@ -40,13 +40,13 @@ func (_m *Manager) AddContractSubscription(ctx context.Context, ns string, sub * return r0, r1 } -// BroadcastContractAPI provides a mock function with given fields: ctx, ns, api, waitConfirm -func (_m *Manager) BroadcastContractAPI(ctx context.Context, ns string, api *fftypes.ContractAPI, waitConfirm bool) (*fftypes.ContractAPI, error) { - ret := _m.Called(ctx, ns, api, waitConfirm) +// BroadcastContractAPI provides a mock function with given fields: ctx, httpServerURL, ns, api, waitConfirm +func (_m *Manager) BroadcastContractAPI(ctx context.Context, httpServerURL string, ns string, api *fftypes.ContractAPI, waitConfirm bool) (*fftypes.ContractAPI, error) { + ret := _m.Called(ctx, httpServerURL, ns, api, waitConfirm) var r0 *fftypes.ContractAPI - if rf, ok := ret.Get(0).(func(context.Context, string, *fftypes.ContractAPI, bool) *fftypes.ContractAPI); ok { - r0 = rf(ctx, ns, api, waitConfirm) + if rf, ok := ret.Get(0).(func(context.Context, string, string, *fftypes.ContractAPI, bool) *fftypes.ContractAPI); ok { + r0 = rf(ctx, httpServerURL, ns, api, waitConfirm) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*fftypes.ContractAPI) @@ -54,8 +54,8 @@ func (_m *Manager) BroadcastContractAPI(ctx context.Context, ns string, api *fft } var r1 error - if rf, ok := ret.Get(1).(func(context.Context, string, *fftypes.ContractAPI, bool) error); ok { - r1 = rf(ctx, ns, api, waitConfirm) + if rf, ok := ret.Get(1).(func(context.Context, string, string, *fftypes.ContractAPI, bool) error); ok { + r1 = rf(ctx, httpServerURL, ns, api, waitConfirm) } else { r1 = ret.Error(1) }