Make column ordering explicit in createRecord return.

The Nim [Row][nim-row] implementation only supports positional
identification of columns. In other words, there is nothing to tell us
which column is in which position. Because of this, we always create SQL
statements which explicitly name the columns we wish to receive so that
we know the order of columns and can rebuild models appropriately.

`createRule` wasn't doing this but naively using `RETURNING *`. This
still works as long as the field ordering in the Nim model class match
the default column ordering returned by the database, but confuses
columns otherwise. This fixes that by specifying explicitly the column
ordering as we do in other places.

[nim-row]: https://nim-lang.org/docs/db_postgres.html#Row

fiber_orm.nimble

@@ -1,6 +1,6 @@
 # Package
 
-version       = "0.3.5"
+version       = "1.0.1"
 author        = "Jonathan Bernard"
 description   = "Lightweight Postgres ORM for Nim."
 license       = "GPL-3.0"

src/fiber_orm.nim

@@ -1,10 +1,13 @@
-import db_postgres, macros, options, sequtils, strutils, uuids
+import std/db_postgres, std/macros, std/options, std/sequtils, std/strutils
-import namespaced_logging
+import namespaced_logging, uuids
 
-from unicode import capitalize
+from std/unicode import capitalize
 
+import ./fiber_orm/pool
 import ./fiber_orm/util
+
 export
+  pool,
   util.columnNamesForModel,
   util.dbFormat,
   util.dbNameToIdent,
@@ -15,7 +18,11 @@ export
 
 type NotFoundError* = object of CatchableError
 
-let logNs = initLoggingNamespace(name = "fiber_orm", level = lvlNotice)
+var logNs {.threadvar.}: LoggingNamespace
+
+template log(): untyped =
+  if logNs.isNil: logNs = initLoggingNamespace(name = "fiber_orm", level = lvlNotice)
+  logNs
 
 proc newMutateClauses(): MutateClauses =
   return MutateClauses(
@@ -27,15 +34,13 @@ proc createRecord*[T](db: DbConn, rec: T): T =
   var mc = newMutateClauses()
   populateMutateClauses(rec, true, mc)
 
-  # Confusingly, getRow allows inserts and updates. We use it to get back the ID
-  # we want from the row.
   let sqlStmt =
     "INSERT INTO " & tableName(rec) &
     " (" & mc.columns.join(",") & ") " &
     " VALUES (" & mc.placeholders.join(",") & ") " &
-    " RETURNING *"
+    " RETURNING " & columnNamesForModel(rec).join(",")
 
-  logNs.debug "createRecord: [" & sqlStmt & "]"
+  log().debug "createRecord: [" & sqlStmt & "]"
   let newRow = db.getRow(sql(sqlStmt), mc.values)
 
   result = rowToModel(T, newRow)
@@ -50,19 +55,19 @@ proc updateRecord*[T](db: DbConn, rec: T): bool =
     " SET " & setClause &
     " WHERE id = ? "
 
-  logNs.debug "updateRecord: [" & sqlStmt & "] id: " & $rec.id
+  log().debug "updateRecord: [" & sqlStmt & "] id: " & $rec.id
   let numRowsUpdated = db.execAffectedRows(sql(sqlStmt), mc.values.concat(@[$rec.id]))
 
   return numRowsUpdated > 0;
 
 template deleteRecord*(db: DbConn, modelType: type, id: typed): untyped =
   let sqlStmt = "DELETE FROM " & tableName(modelType) & " WHERE id = ?"
-  logNs.debug "deleteRecord: [" & sqlStmt & "] id: " & $id
+  log().debug "deleteRecord: [" & sqlStmt & "] id: " & $id
   db.tryExec(sql(sqlStmt), $id)
 
 proc deleteRecord*[T](db: DbConn, rec: T): bool =
   let sqlStmt = "DELETE FROM " & tableName(rec) & " WHERE id = ?"
-  logNs.debug "deleteRecord: [" & sqlStmt & "] id: " & $rec.id
+  log().debug "deleteRecord: [" & sqlStmt & "] id: " & $rec.id
   return db.tryExec(sql(sqlStmt), $rec.id)
 
 template getRecord*(db: DbConn, modelType: type, id: typed): untyped =
@@ -71,7 +76,7 @@ template getRecord*(db: DbConn, modelType: type, id: typed): untyped =
     " FROM " & tableName(modelType) &
     " WHERE id = ?"
 
-  logNs.debug "getRecord: [" & sqlStmt & "] id: " & $id
+  log().debug "getRecord: [" & sqlStmt & "] id: " & $id
   let row = db.getRow(sql(sqlStmt), @[$id])
 
   if allIt(row, it.len == 0):
@@ -85,7 +90,7 @@ template findRecordsWhere*(db: DbConn, modelType: type, whereClause: string, val
     " FROM " & tableName(modelType) &
     " WHERE " & whereClause
 
-  logNs.debug "findRecordsWhere: [" & sqlStmt & "] values: (" & values.join(", ") & ")"
+  log().debug "findRecordsWhere: [" & sqlStmt & "] values: (" & values.join(", ") & ")"
   db.getAllRows(sql(sqlStmt), values).mapIt(rowToModel(modelType, it))
 
 template getAllRecords*(db: DbConn, modelType: type): untyped =
@@ -93,7 +98,7 @@ template getAllRecords*(db: DbConn, modelType: type): untyped =
     "SELECT " & columnNamesForModel(modelType).join(",") &
     " FROM " & tableName(modelType)
 
-  logNs.debug "getAllRecords: [" & sqlStmt & "]"
+  log().debug "getAllRecords: [" & sqlStmt & "]"
   db.getAllRows(sql(sqlStmt)).mapIt(rowToModel(modelType, it))
 
 template findRecordsBy*(db: DbConn, modelType: type, lookups: seq[tuple[field: string, value: string]]): untyped =
@@ -103,7 +108,7 @@ template findRecordsBy*(db: DbConn, modelType: type, lookups: seq[tuple[field: s
     " WHERE " & lookups.mapIt(it.field & " = ?").join(" AND ")
   let values = lookups.mapIt(it.value)
 
-  logNs.debug "findRecordsBy: [" & sqlStmt & "] values (" & values.join(", ") & ")"
+  log().debug "findRecordsBy: [" & sqlStmt & "] values (" & values.join(", ") & ")"
   db.getAllRows(sql(sqlStmt), values).mapIt(rowToModel(modelType, it))
 
 macro generateProcsForModels*(dbType: type, modelTypes: openarray[type]): untyped =
@@ -119,14 +124,27 @@ macro generateProcsForModels*(dbType: type, modelTypes: openarray[type]): untype
     let deleteName = ident("delete" & modelName)
     let idType = typeOfColumn(t, "id")
     result.add quote do:
-      proc `getName`*(db: `dbType`, id: `idType`): `t` = getRecord(db.conn, `t`, id)
+      proc `getName`*(db: `dbType`, id: `idType`): `t` =
-      proc `getAllName`*(db: `dbType`): seq[`t`] = getAllRecords(db.conn, `t`)
+        db.withConn: result = getRecord(conn, `t`, id)
+
+      proc `getAllName`*(db: `dbType`): seq[`t`] =
+        db.withConn: result = getAllRecords(conn, `t`)
+
       proc `findWhereName`*(db: `dbType`, whereClause: string, values: varargs[string, dbFormat]): seq[`t`] =
-        return findRecordsWhere(db.conn, `t`, whereClause, values)
+        db.withConn:
-      proc `createName`*(db: `dbType`, rec: `t`): `t` = createRecord(db.conn, rec)
+          result = findRecordsWhere(conn, `t`, whereClause, values)
-      proc `updateName`*(db: `dbType`, rec: `t`): bool = updateRecord(db.conn, rec)
+
-      proc `deleteName`*(db: `dbType`, rec: `t`): bool = deleteRecord(db.conn, rec)
+      proc `createName`*(db: `dbType`, rec: `t`): `t` =
-      proc `deleteName`*(db: `dbType`, id: `idType`): bool = deleteRecord(db.conn, `t`, id)
+        db.withConn: result = createRecord(conn, rec)
+
+      proc `updateName`*(db: `dbType`, rec: `t`): bool =
+        db.withConn: result = updateRecord(conn, rec)
+
+      proc `deleteName`*(db: `dbType`, rec: `t`): bool =
+        db.withConn: result = deleteRecord(conn, rec)
+
+      proc `deleteName`*(db: `dbType`, id: `idType`): bool =
+        db.withConn: result = deleteRecord(conn, `t`, id)
 
 macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyped =
   let fieldNames = fields[1].mapIt($it)
@@ -135,7 +153,7 @@ macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyp
   # Create proc skeleton
   result = quote do:
     proc `procName`*(db: `dbType`): seq[`modelType`] =
-      return findRecordsBy(db.conn, `modelType`)
+      db.withConn: result = findRecordsBy(conn, `modelType`)
 
   var callParams = quote do: @[]
 
@@ -145,10 +163,19 @@ macro generateLookup*(dbType: type, modelType: type, fields: seq[string]): untyp
     paramTuple.add(newColonExpr(ident("field"), newLit(identNameToDb(n))))
     paramTuple.add(newColonExpr(ident("value"), ident(n)))
 
+    # Add the parameter to the outer call (the generated proc)
+    # result[3] is ProcDef -> [3]: FormalParams
     result[3].add(newIdentDefs(ident(n), ident("string")))
+
+    # Build up the AST for the inner procedure call
     callParams[1].add(paramTuple)
 
-  result[6][0][0].add(callParams)
+  # Add the call params to the inner procedure call
+  # result[6][0][1][0][1] is
+  #   ProcDef -> [6]: StmtList (body) -> [0]: Call ->
+  #     [1]: StmtList (withConn body) -> [0]: Asgn (result =) ->
+  #     [1]: Call (inner findRecords invocation)
+  result[6][0][1][0][1].add(callParams)
 
 macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tuple[t: type, fields: seq[string]]]): untyped =
   result = newStmtList()
@@ -162,7 +189,7 @@ macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tup
     # Create proc skeleton
     let procDefAST = quote do:
       proc `procName`*(db: `dbType`): seq[`modelType`] =
-        return findRecordsBy(db.conn, `modelType`)
+        db.withConn: result = findRecordsBy(conn, `modelType`)
 
     var callParams = quote do: @[]
 
@@ -175,6 +202,28 @@ macro generateProcsForFieldLookups*(dbType: type, modelsAndFields: openarray[tup
       procDefAST[3].add(newIdentDefs(ident(n), ident("string")))
       callParams[1].add(paramTuple)
 
-    procDefAST[6][0][0].add(callParams)
+    procDefAST[6][0][1][0][1].add(callParams)
 
     result.add procDefAST
+
+proc initPool*(
+    connect: proc(): DbConn,
+    poolSize = 10,
+    hardCap = false,
+    healthCheckQuery = "SELECT 'true' AS alive"): DbConnPool =
+
+  initDbConnPool(DbConnPoolConfig(
+    connect: connect,
+    poolSize: poolSize,
+    hardCap: hardCap,
+    healthCheckQuery: healthCheckQuery))
+
+template inTransaction*(db: DbConnPool, body: untyped) =
+  pool.withConn(db):
+    conn.exec(sql"BEGIN TRANSACTION")
+    try:
+      body
+      conn.exec(sql"COMMIT")
+    except:
+      conn.exec(sql"ROLLBACK")
+      raise getCurrentException()

src/fiber_orm/pool.nim

@@ -0,0 +1,97 @@
+import std/db_postgres, std/sequtils, std/strutils, std/sugar
+
+import namespaced_logging
+
+
+type
+  DbConnPoolConfig* = object
+    connect*: () -> DbConn
+    poolSize*: int
+    hardCap*: bool
+    healthCheckQuery*: string
+
+  PooledDbConn = ref object
+    conn: DbConn
+    id: int
+    free: bool
+
+  DbConnPool* = ref object
+    conns: seq[PooledDbConn]
+    cfg: DbConnPoolConfig
+    lastId: int
+
+var logNs {.threadvar.}: LoggingNamespace
+
+template log(): untyped =
+  if logNs.isNil: logNs = initLoggingNamespace(name = "fiber_orm/pool", level = lvlNotice)
+  logNs
+
+proc initDbConnPool*(cfg: DbConnPoolConfig): DbConnPool =
+  log().debug("Initializing new pool (size: " & $cfg.poolSize)
+  result = DbConnPool(
+    conns: @[],
+    cfg: cfg)
+
+proc newConn(pool: DbConnPool): PooledDbConn =
+  log().debug("Creating a new connection to add to the pool.")
+  pool.lastId += 1
+  let conn = pool.cfg.connect()
+  result = PooledDbConn(
+    conn: conn,
+    id: pool.lastId,
+    free: true)
+  pool.conns.add(result)
+
+proc maintain(pool: DbConnPool): void =
+  log().debug("Maintaining pool. $# connections." % [$pool.conns.len])
+  pool.conns.keepIf(proc (pc: PooledDbConn): bool =
+    if not pc.free: return true
+
+    try:
+      discard getRow(pc.conn, sql(pool.cfg.healthCheckQuery), [])
+      return true
+    except:
+      try: pc.conn.close()  # try to close the connection
+      except: discard ""
+      return false
+  )
+  log().debug(
+    "Pruned dead connections. $# connections remaining." %
+    [$pool.conns.len])
+
+  let freeConns = pool.conns.filterIt(it.free)
+  if pool.conns.len > pool.cfg.poolSize and freeConns.len > 0:
+    let numToCull = min(freeConns.len, pool.conns.len - pool.cfg.poolSize)
+    let toCull = freeConns[0..numToCull]
+    pool.conns.keepIf((pc) => toCull.allIt(it.id != pc.id))
+    for culled in toCull:
+      try: culled.conn.close()
+      except: discard ""
+    log().debug(
+      "Trimming pool size. Culled $# free connections. $# connections remaining." %
+      [$toCull.len, $pool.conns.len])
+
+proc take*(pool: DbConnPool): tuple[id: int, conn: DbConn] =
+  pool.maintain
+  let freeConns = pool.conns.filterIt(it.free)
+
+  log().debug(
+    "Providing a new connection ($# currently free)." % [$freeConns.len])
+
+  let reserved =
+    if freeConns.len > 0: freeConns[0]
+    else: pool.newConn()
+
+  reserved.free = false
+  log().debug("Reserve connection $#" % [$reserved.id])
+  return (id: reserved.id, conn: reserved.conn)
+
+proc release*(pool: DbConnPool, connId: int): void =
+  log().debug("Reclaiming released connaction $#" % [$connId])
+  let foundConn = pool.conns.filterIt(it.id == connId)
+  if foundConn.len > 0: foundConn[0].free = true
+
+template withConn*(pool: DbConnPool, stmt: untyped): untyped =
+  let (connId, conn {.inject.}) = take(pool)
+  stmt
+  release(pool, connId)