From 2768ac0b314b175155954d256802a57b7087ca3f Mon Sep 17 00:00:00 2001 From: James Renken Date: Wed, 25 Sep 2024 15:49:07 -0700 Subject: [PATCH 1/3] Change ClearEmail to use direct UPDATE query --- sa/model.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sa/model.go b/sa/model.go index 5d0a15fd9bf..3f0c825f047 100644 --- a/sa/model.go +++ b/sa/model.go @@ -88,14 +88,18 @@ func ClearEmail(ctx context.Context, dbMap db.DatabaseMap, regID int64, email st return nil, nil } - currPb.Contact = newContacts - newModel, err := registrationPbToModel(currPb) - if err != nil { - return nil, err - } + // UPDATE the row with a direct database query, in order to avoid LockCol issues. + // + // TODO(#7716): Revert to use tx.Update() once LockCol has been removed. + _, err = dbMap.ExecContext(ctx, + "UPDATE registrations SET contact = ? WHERE id = ? LIMIT 1", + newContacts, + regID, + ) - return tx.Update(ctx, newModel) + return nil, err }) + if overallError != nil { return overallError } From 32d58476f0f165811f815479a383f4247712ce8b Mon Sep 17 00:00:00 2001 From: James Renken Date: Thu, 26 Sep 2024 13:58:21 -0700 Subject: [PATCH 2/3] Address feedback & fix JSON data type --- sa/model.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/sa/model.go b/sa/model.go index 3f0c825f047..548c63a8c07 100644 --- a/sa/model.go +++ b/sa/model.go @@ -88,16 +88,33 @@ func ClearEmail(ctx context.Context, dbMap db.DatabaseMap, regID int64, email st return nil, nil } + // We don't want to write literal JSON "null" strings into the database if the + // list of contact addresses is empty. Replace any possibly-`nil` slice with + // an empty JSON array. We don't need to check reg.ContactPresent, because + // we're going to write the whole object to the database anyway. + jsonContact := []byte("[]") + if len(newContacts) != 0 { + jsonContact, err = json.Marshal(newContacts) + if err != nil { + return nil, err + } + } + // UPDATE the row with a direct database query, in order to avoid LockCol issues. - // - // TODO(#7716): Revert to use tx.Update() once LockCol has been removed. - _, err = dbMap.ExecContext(ctx, + result, err := tx.ExecContext(ctx, "UPDATE registrations SET contact = ? WHERE id = ? LIMIT 1", - newContacts, + jsonContact, regID, ) + if err != nil { + return nil, err + } + rowsAffected, err := result.RowsAffected() + if err != nil || rowsAffected != 1 { + return nil, berrors.InternalServerError("no registration updated with new contact field") + } - return nil, err + return nil, nil }) if overallError != nil { From 625fd012077aae38ca67d7f86fce3af9ec98a655 Mon Sep 17 00:00:00 2001 From: James Renken Date: Thu, 26 Sep 2024 14:00:26 -0700 Subject: [PATCH 3/3] Remove newline for style --- sa/model.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sa/model.go b/sa/model.go index 548c63a8c07..fa3ce717a29 100644 --- a/sa/model.go +++ b/sa/model.go @@ -116,7 +116,6 @@ func ClearEmail(ctx context.Context, dbMap db.DatabaseMap, regID int64, email st return nil, nil }) - if overallError != nil { return overallError }