From 2d00e02d1d5d3c291b4774466ebe3f34deaf9e8b Mon Sep 17 00:00:00 2001
From: Neil Alexander <neilalexander@users.noreply.github.com>
Date: Fri, 21 Aug 2020 10:22:28 +0100
Subject: [PATCH] Document sqlutil.Writer interface

---
 internal/sqlutil/sql.go              |  4 ---
 internal/sqlutil/writer.go           | 43 ++++++++++++++++++++++++++++
 internal/sqlutil/writer_dummy.go     |  6 ++++
 internal/sqlutil/writer_exclusive.go |  1 +
 4 files changed, 50 insertions(+), 4 deletions(-)
 create mode 100644 internal/sqlutil/writer.go

diff --git a/internal/sqlutil/sql.go b/internal/sqlutil/sql.go
index 75ba8e54c..d296c4180 100644
--- a/internal/sqlutil/sql.go
+++ b/internal/sqlutil/sql.go
@@ -103,7 +103,3 @@ func SQLiteDriverName() string {
 	}
 	return "sqlite3"
 }
-
-type Writer interface {
-	Do(db *sql.DB, txn *sql.Tx, f func(txn *sql.Tx) error) error
-}
diff --git a/internal/sqlutil/writer.go b/internal/sqlutil/writer.go
new file mode 100644
index 000000000..6e5ff5705
--- /dev/null
+++ b/internal/sqlutil/writer.go
@@ -0,0 +1,43 @@
+package sqlutil
+
+import "database/sql"
+
+// The Writer interface is designed to solve the problem of how
+// to handle database writes for database engines that don't allow
+// concurrent writes, e.g. SQLite.
+//
+// The interface has a single Do function which takes an optional
+// database parameter, an optional transaction parameter and a
+// required function parameter. The Writer will call the function
+// provided when it is safe to do so, optionally providing a
+// transaction to use.
+//
+// Depending on the combination of parameters provided, the Writer
+// will behave in one of three ways:
+//
+// 1. `db` provided, `txn` provided:
+//
+// The Writer will call f() when it is safe to do so. The supplied
+// "txn" will ALWAYS be passed through to f(). Use this when you
+// already have a transaction open.
+//
+// 2. `db` provided, `txn` not provided (nil):
+//
+// The Writer will open a new transaction on the provided database
+// and then will call f() when it is safe to do so. The new
+// transaction will ALWAYS be passed through to f(). Use this if
+// you plan to perform more than one SQL query within f().
+//
+// 3. `db` not provided (nil), `txn` not provided (nil):
+//
+// The Writer will call f() when it is safe to do so, but will
+// not make any attempt to open a new database transaction or to
+// pass through an existing one. The "txn" parameter within f()
+// will ALWAYS be nil in this mode. This is useful if you just
+// want to perform a single query on an already-prepared statement
+// without the overhead of opening a new transaction to do it in.
+type Writer interface {
+	// Queue up one or more database write operations within the
+	// provided function to be executed when it is safe to do so.
+	Do(db *sql.DB, txn *sql.Tx, f func(txn *sql.Tx) error) error
+}
diff --git a/internal/sqlutil/writer_dummy.go b/internal/sqlutil/writer_dummy.go
index 043d5075a..f426c2bc3 100644
--- a/internal/sqlutil/writer_dummy.go
+++ b/internal/sqlutil/writer_dummy.go
@@ -4,9 +4,15 @@ import (
 	"database/sql"
 )
 
+// DummyWriter implements sqlutil.Writer.
+// The DummyWriter is designed to allow reuse of the sqlutil.Writer
+// interface but, unlike ExclusiveWriter, it will not guarantee
+// writer exclusivity. This is fine in PostgreSQL where overlapping
+// transactions and writes are acceptable.
 type DummyWriter struct {
 }
 
+// NewDummyWriter returns a new dummy writer.
 func NewDummyWriter() Writer {
 	return &DummyWriter{}
 }
diff --git a/internal/sqlutil/writer_exclusive.go b/internal/sqlutil/writer_exclusive.go
index 75eeba3d8..002bc32cf 100644
--- a/internal/sqlutil/writer_exclusive.go
+++ b/internal/sqlutil/writer_exclusive.go
@@ -7,6 +7,7 @@ import (
 	"go.uber.org/atomic"
 )
 
+// ExclusiveWriter implements sqlutil.Writer.
 // ExclusiveWriter allows queuing database writes so that you don't
 // contend on database locks in, e.g. SQLite. Only one task will run
 // at a time on a given ExclusiveWriter.