From b62e13a001abef079737774fb0589c55f31981d3 Mon Sep 17 00:00:00 2001 From: zeripath Date: Thu, 24 Sep 2020 23:36:26 +0100 Subject: [PATCH] Open transaction when adding Avatar email-hash pairs to the DB (#12577) (#12940) Backport #12577 When adding Avatar email-hash pairs we simply want the DB table to represent a Set. We don't care if the hash-pair is already present, so we just simply Insert and ignore the error. Unfortunately this seems to cause some DBs to log the duplicate insert to their logs - looking like a bug a in Gitea. Now, there is no standard way in SQL to say Insert but if there's an error ignore it. MySQL has INSERT IGNORE, PostgreSQL >= 9.5 has INSERT ... ON CONFLICT DO NOTHING, but I do not believe that SQLite or MSSQL have variants. This PR places the insert in a transaction which we are happy to fail if there is an error - hopefully this will stop the unnecessary logging. Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick Co-authored-by: techknowlogick --- models/avatar.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/models/avatar.go b/models/avatar.go index 311d71462..c9ba2961e 100644 --- a/models/avatar.go +++ b/models/avatar.go @@ -41,7 +41,18 @@ func AvatarLink(email string) string { Email: lowerEmail, Hash: sum, } - _, _ = x.Insert(emailHash) + // OK we're going to open a session just because I think that that might hide away any problems with postgres reporting errors + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + // we don't care about any DB problem just return the lowerEmail + return lowerEmail, nil + } + _, _ = sess.Insert(emailHash) + if err := sess.Commit(); err != nil { + // Seriously we don't care about any DB problems just return the lowerEmail - we expect the transaction to fail most of the time + return lowerEmail, nil + } return lowerEmail, nil }) return setting.AppSubURL + "/avatar/" + url.PathEscape(sum)