Repository: hyperledger/firefly
Pull request #1458 (2024-02-01 15:27:28)

fix: Merge ID into PATCH calls for identity if one is not provided
ref: https://github.com/hyperledger/firefly/issues/1450

When calling the PATCH identity API, if an ID is not provided, we grab the ID from the existing identity and use this. This effectively means that a PATCH operation should always have an ID in the payload, which should stop the downstream DX connectors from crashing when they attempt to peer on a restart.


Patch:
diff --git a/internal/networkmap/update_identity.go b/internal/networkmap/update_identity.go
index a7435182e..d675c8e75 100644
--- a/internal/networkmap/update_identity.go
+++ b/internal/networkmap/update_identity.go
@@ -1,4 +1,4 @@
-// Copyright © 2022 Kaleido, Inc.
+// Copyright © 2024 Kaleido, Inc.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -44,6 +44,12 @@ func (nm *networkMap) updateIdentityID(ctx context.Context, id *fftypes.UUID, dt
 		return nil, i18n.NewError(ctx, coremsgs.Msg404NoResult)
 	}
 
+	// We can't sparse merge the generic JSON fields, but we need to propagate the ID
+	if dto.IdentityProfile.Profile.GetString("id") == "" {
+		existingID := identity.IdentityProfile.Profile.GetString("id")
+		dto.IdentityProfile.Profile["id"] = existingID
+	}
+
 	var updateSigner *core.SignerRef
 
 	if nm.multiparty != nil {


Test patch:
diff --git a/internal/networkmap/update_identity_test.go b/internal/networkmap/update_identity_test.go
index 20f0ce921..08507082b 100644
--- a/internal/networkmap/update_identity_test.go
+++ b/internal/networkmap/update_identity_test.go
@@ -192,3 +192,79 @@ func TestUpdateIdentityProfileBadID(t *testing.T) {
 	_, err := nm.UpdateIdentity(nm.ctx, "badness", &core.IdentityUpdateDTO{}, true)
 	assert.Regexp(t, "FF00138", err)
 }
+
+func TestUpdateIdentityProfileExistingIdNotSpecified(t *testing.T) {
+
+	nm, cancel := newTestNetworkmap(t)
+	defer cancel()
+
+	identity := testOrg("org1")
+	identity.Profile["id"] = "member_0/node_aaaaaa"
+
+	mim := nm.identity.(*identitymanagermocks.Manager)
+	mim.On("CachedIdentityLookupByID", nm.ctx, identity.ID).Return(identity, nil)
+	signerRef := &core.SignerRef{Key: "0x12345"}
+	mim.On("ResolveIdentitySigner", nm.ctx, identity).Return(signerRef, nil)
+
+	mds := nm.defsender.(*definitionsmocks.Sender)
+
+	mds.On("UpdateIdentity", nm.ctx,
+		mock.AnythingOfType("*core.Identity"),
+		mock.MatchedBy(func(iden *core.IdentityUpdate) bool {
+			return iden.Updates.Profile.GetString("id") == "member_0/node_aaaaaa"
+		}),
+		mock.MatchedBy(func(sr *core.SignerRef) bool {
+			return sr.Key == "0x12345"
+		}),
+		true).Return(nil)
+
+	org, err := nm.UpdateIdentity(nm.ctx, identity.ID.String(), &core.IdentityUpdateDTO{
+		IdentityProfile: core.IdentityProfile{
+			Description: "new desc",
+			Profile:     fftypes.JSONObject{"new": "profile"},
+		},
+	}, true)
+	assert.NoError(t, err)
+	assert.NotNil(t, org)
+
+	mim.AssertExpectations(t)
+	mds.AssertExpectations(t)
+}
+
+func TestUpdateIdentityProfileOverwrite(t *testing.T) {
+
+	nm, cancel := newTestNetworkmap(t)
+	defer cancel()
+
+	identity := testOrg("org1")
+	identity.Profile["id"] = "member_0/node_aaaaaa"
+
+	mim := nm.identity.(*identitymanagermocks.Manager)
+	mim.On("CachedIdentityLookupByID", nm.ctx, identity.ID).Return(identity, nil)
+	signerRef := &core.SignerRef{Key: "0x12345"}
+	mim.On("ResolveIdentitySigner", nm.ctx, identity).Return(signerRef, nil)
+
+	mds := nm.defsender.(*definitionsmocks.Sender)
+
+	mds.On("UpdateIdentity", nm.ctx,
+		mock.AnythingOfType("*core.Identity"),
+		mock.MatchedBy(func(iden *core.IdentityUpdate) bool {
+			return iden.Updates.Profile.GetString("id") == "member_0/node_bbbbbb"
+		}),
+		mock.MatchedBy(func(sr *core.SignerRef) bool {
+			return sr.Key == "0x12345"
+		}),
+		true).Return(nil)
+
+	org, err := nm.UpdateIdentity(nm.ctx, identity.ID.String(), &core.IdentityUpdateDTO{
+		IdentityProfile: core.IdentityProfile{
+			Description: "new desc",
+			Profile:     fftypes.JSONObject{"new": "profile", "id": "member_0/node_bbbbbb"},
+		},
+	}, true)
+	assert.NoError(t, err)
+	assert.NotNil(t, org)
+
+	mim.AssertExpectations(t)
+	mds.AssertExpectations(t)
+}