Merge pull request hyperledger#1395 from kaleido-io/fix-api-update-bu…

…g-for-unpublished

handle update for unpublished API

internal/database/sqlcommon/contractapis_sql.go

@@ -135,6 +135,7 @@ func (s *SQLCommon) InsertOrGetContractAPI(ctx context.Context, api *core.Contra
  if insertErr == nil {
  return nil, s.CommitTx(ctx, tx, autoCommit)
  }
+ log.L(ctx).Debugf("Contract API insert failed due to err: %+v, retrieving the existing contract API", insertErr)
 
  // Do a select within the transaction to determine if the API already exists
  existing, queryErr := s.getContractAPIPred(ctx, api.Namespace+":"+api.Name, sq.And{

internal/definitions/handler_contracts.go

@@ -100,6 +100,7 @@ func (dh *definitionHandler) reconcilePublishedFFI(ctx context.Context, existing
 }
 
 func (dh *definitionHandler) persistContractAPI(ctx context.Context, httpServerURL string, api *core.ContractAPI, isAuthor bool) (retry bool, err error) {
+ l := log.L(ctx)
  for i := 1; ; i++ {
  if err := dh.contracts.ResolveContractAPI(ctx, httpServerURL, api); err != nil {
  return false, i18n.WrapError(ctx, err, coremsgs.MsgDefRejectedValidateFail, "contract API", api.ID)
@@ -108,21 +109,25 @@ func (dh *definitionHandler) persistContractAPI(ctx context.Context, httpServerU
  // Check if this conflicts with an existing API
  existing, err := dh.database.InsertOrGetContractAPI(ctx, api)
  if err != nil {
+ l.Errorf("Failed to InsertOrGetContractAPI due to err: %+v", err)
  return true, err
  }
 
  if existing == nil {
  // No conflict - new API was inserted successfully
+ l.Tracef("Successfully inserted the new contract API with ID %s", api.ID)
  break
  }
 
- if api.Published {
- if existing.ID.Equals(api.ID) {
- // ID conflict - check if this matches (or should overwrite) the existing record
- return dh.reconcilePublishedContractAPI(ctx, existing, api, isAuthor)
- }
+ if existing.ID.Equals(api.ID) {
+ // the matching record has the same ID, perform an update
+ l.Trace("Found an existing contract API with the same ID, reconciling the contract API")
+ // ID conflict - check if this matches (or should overwrite) the existing record
+ return dh.reconcileContractAPI(ctx, existing, api, isAuthor)
+ } else if api.Published {
 
  if existing.Name == api.Name {
+ l.Trace("Local name conflict, generating a unique name to retry")
  // Local name conflict - generate a unique name and try again
  api.Name = fmt.Sprintf("%s-%d", api.NetworkName, i)
  continue
@@ -137,15 +142,27 @@ func (dh *definitionHandler) persistContractAPI(ctx context.Context, httpServerU
  return false, nil
 }
 
-func (dh *definitionHandler) reconcilePublishedContractAPI(ctx context.Context, existing, api *core.ContractAPI, isAuthor bool) (retry bool, err error) {
- if existing.Message.Equals(api.Message) {
- // Message already recorded
- return false, nil
- }
+func (dh *definitionHandler) reconcileContractAPI(ctx context.Context, existing, api *core.ContractAPI, isAuthor bool) (retry bool, err error) {
+ l := log.L(ctx)
 
- if existing.Message == nil && isAuthor {
- // API was previously unpublished - if it was now published by this node, upsert the new version
- api.Name = existing.Name
+ if api.Published {
+ l.Trace("Reconciling a published API")
+ if existing.Message.Equals(api.Message) {
+ l.Trace("Reconciling a published API: message already recorded, no action required")
+ // Message already recorded
+ return false, nil
+ }
+
+ if existing.Message == nil && isAuthor {
+ // API was previously unpublished - if it was now published by this node, upsert the new version
+ api.Name = existing.Name
+ l.Tracef("Reconciling a published API: update API name from '%s' to the existing name '%s'", api.Name, existing.Name)
+ if err := dh.database.UpsertContractAPI(ctx, api, database.UpsertOptimizationExisting); err != nil {
+ return true, err
+ }
+ return false, nil
+ }
+ } else {
  if err := dh.database.UpsertContractAPI(ctx, api, database.UpsertOptimizationExisting); err != nil {
  return true, err
  }

internal/definitions/handler_contracts_test.go

@@ -603,6 +603,43 @@ func TestPersistContractAPIUpsertFail(t *testing.T) {
  assert.EqualError(t, err, "pop")
 }
 
+func TestPersistContractAPIUpsertNonPublished(t *testing.T) {
+ dh, _ := newTestDefinitionHandler(t)
+ defer dh.cleanup(t)
+
+ api := testContractAPI()
+ api.Published = false
+ api.Message = fftypes.NewUUID()
+ existing := &core.ContractAPI{
+ ID: api.ID,
+ }
+
+ dh.mdi.On("InsertOrGetContractAPI", mock.Anything, mock.Anything).Return(existing, nil)
+ dh.mcm.On("ResolveContractAPI", context.Background(), "", mock.Anything).Return(nil)
+ dh.mdi.On("UpsertContractAPI", context.Background(), api, database.UpsertOptimizationExisting).Return(nil)
+
+ _, err := dh.persistContractAPI(context.Background(), "", api, true)
+ assert.NoError(t, err)
+}
+
+func TestPersistContractAPIUpsertFailNonPublished(t *testing.T) {
+ dh, _ := newTestDefinitionHandler(t)
+ defer dh.cleanup(t)
+
+ api := testContractAPI()
+ api.Published = false
+ api.Message = fftypes.NewUUID()
+ existing := &core.ContractAPI{
+ ID: api.ID,
+ }
+
+ dh.mdi.On("InsertOrGetContractAPI", mock.Anything, mock.Anything).Return(existing, nil)
+ dh.mcm.On("ResolveContractAPI", context.Background(), "", mock.Anything).Return(nil)
+ dh.mdi.On("UpsertContractAPI", context.Background(), api, database.UpsertOptimizationExisting).Return(fmt.Errorf("pop"))
+
+ _, err := dh.persistContractAPI(context.Background(), "", api, true)
+ assert.EqualError(t, err, "pop")
+}
 func TestPersistContractAPIWrongMessage(t *testing.T) {
  dh, _ := newTestDefinitionHandler(t)
  defer dh.cleanup(t)

internal/definitions/sender_contracts_test.go

@@ -306,6 +306,27 @@ func TestDefineContractAPIPublishNonMultiparty(t *testing.T) {
  assert.Regexp(t, "FF10414", err)
 }
 
+func TestDefineContractAPINonMultipartyUpdate(t *testing.T) {
+ ds := newTestDefinitionSender(t)
+ defer ds.cleanup(t)
+ ds.multiparty = false
+ testUUID := fftypes.NewUUID()
+
+ url := "http://firefly"
+ api := &core.ContractAPI{
+ ID: testUUID,
+ Name: "banana",
+ Published: false,
+ }
+ ds.mcm.On("ResolveContractAPI", context.Background(), url, api).Return(nil)
+ ds.mdi.On("InsertOrGetContractAPI", mock.Anything, mock.Anything).Return(api, nil)
+ ds.mdi.On("UpsertContractAPI", mock.Anything, mock.Anything, mock.Anything).Return(nil)
+ ds.mdi.On("InsertEvent", mock.Anything, mock.Anything).Return(nil)
+
+ err := ds.DefineContractAPI(context.Background(), url, api, false)
+ assert.NoError(t, err)
+}
+
 func TestPublishFFI(t *testing.T) {
  ds := newTestDefinitionSender(t)
  defer ds.cleanup(t)