diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 46ee496..711d34e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,39 +30,36 @@ jobs: - name: Checkout source code uses: actions/checkout@v4 - - name: Cache choosenim - id: cache-choosenim - uses: actions/cache@v4 - with: - path: ~/.choosenim - key: ${{ runner.os }}-choosenim-stable - - - name: Cache nimble - id: cache-nimble - uses: actions/cache@v4 - with: - path: ~/.nimble - key: ${{ runner.os }}-nimble-stable - - name: Setup nim uses: jiro4989/setup-nim-action@v2 with: nim-version: ${{ matrix.nim }} - - name: Install dependencies + - name: Install Atlas + shell: bash + env: + ATLAS_INSTALL_DIR: ${{ runner.temp }}/atlas/bin run: | - nimble --useSystemNim install -d - nimble --useSystemNim install karax -y - nimble --useSystemNim install jester -y - nimble --useSystemNim install websocket -y + curl -fsSL --retry 3 \ + https://raw.githubusercontent.com/nim-lang/atlas/HEAD/install.sh | bash + echo "$ATLAS_INSTALL_DIR" >> "$GITHUB_PATH" + + - name: Cache dependencies + uses: actions/cache@v4 + with: + path: deps/ + key: atlas-${{ runner.os }}-${{ hashFiles('ormin.nimble') }} + + - name: Install dependencies + run: atlas install -uk --feature:examples - name: Run test sqlite - run: nimble --useSystemNim test + run: nim test - name: Run test postgres - run: nimble --useSystemNim test_postgres + run: nim test_postgres env: PGPASSWORD: test - name: Build examples - run: nimble buildexamples + run: nim buildexamples diff --git a/config.nims b/config.nims index 0377d94..83704b0 100644 --- a/config.nims +++ b/config.nims @@ -14,14 +14,12 @@ task test, "Run all test suite": buildimporterTask() cleanTask() - exec "nim c -f -r tests/tfeature" - exec "nim c -f -r tests/tcommon" - exec "nim c -f -r -d:release tests/tquery_types" - exec "nim c -f -r tests/tsqlite" - exec "nim c -f -r tests/tdb_utils" - exec "nim c -f -r tests/timportstatic" - exec "nim c -f -r tests/tpostgres_schema_import" - exec "nim c -f -r tests/tqualified_schema_queries" + # These suites share test.db and must execute serially. + exec("atlas-run tests --jobs:1 tests/tfeature.nim tests/tcommon.nim " & + "tests/tsqlite.nim tests/tdb_utils.nim tests/timportstatic.nim " & + "tests/ttransactions.nim tests/tpostgres_schema_import.nim " & + "tests/tqualified_schema_queries.nim") + exec "atlas-run tests --jobs:2 tests/tquery_types.nim -- -d:release" task setup_postgres, "Ensure local Postgres has test DB/user": # Use a simple script to avoid Nim/psql quoting pitfalls @@ -35,10 +33,11 @@ task test_postgres, "Run PostgreSQL test suite": exec "./tools/ormin_importer tests/forum_model_postgres.sql" exec "./tools/ormin_importer tests/model_postgre.sql" - exec "nim c -f -d:nimDebugDlOpen -r -d:postgre tests/tfeature" - exec "nim c -f -d:nimDebugDlOpen -r -d:postgre tests/tcommon" - exec "nim c -f -d:nimDebugDlOpen -r -d:release -d:postgre tests/tquery_types" - exec "nim c -f -d:nimDebugDlOpen -r -d:postgre tests/tpostgre" + # These suites share one PostgreSQL database and must execute serially. + exec("atlas-run tests --jobs:1 tests/tfeature.nim tests/tcommon.nim " & + "tests/tpostgre.nim -- -d:nimDebugDlOpen -d:postgre") + exec("atlas-run tests --jobs:2 tests/tquery_types.nim -- " & + "-d:nimDebugDlOpen -d:release -d:postgre") task buildexamples, "Build examples: chat and forum": buildimporterTask() diff --git a/ormin.nimble b/ormin.nimble index dcb6a15..7abfa5a 100644 --- a/ormin.nimble +++ b/ormin.nimble @@ -15,7 +15,7 @@ requires "db_connector >= 0.1.0" feature "examples": requires "websocket >= 0.2.2" requires "karax" - requires "jester" + requires "jester >= 0.6.0" import std/os when fileExists("config.nims"): diff --git a/ormin/ormin_sqlite.nim b/ormin/ormin_sqlite.nim index 762f18a..2126cf4 100644 --- a/ormin/ormin_sqlite.nim +++ b/ormin/ormin_sqlite.nim @@ -34,6 +34,11 @@ proc prepareStmt*(db: DbConn; q: string): PStmt = if prepare_v2(db, q, q.len.cint, result, nil) != SQLITE_OK: dbError(db) +proc finalizeStmt*(s: PStmt) {.raises: [].} = + ## Release a query's statement, including when bindings or result conversion raise. + ## stopQuery reports execution errors before this nonraising scope cleanup runs. + discard sqlite3.finalize(s) + template startBindings*(s: PStmt; n: int) = if clear_bindings(s) != SQLITE_OK: dbError(db) diff --git a/ormin/queries.nim b/ormin/queries.nim index 85a7fc6..f2426b9 100644 --- a/ormin/queries.nim +++ b/ormin/queries.nim @@ -165,24 +165,30 @@ type # Execute a non-row SQL statement strictly (errors on failure) template execNoRowsStrict*(sqlStmt: string) = - when defined(debugOrminTrace): - echo "[[Ormin Executing]]: ", q - let s {.gensym.} = prepareStmt(db, sqlStmt) - startQuery(db, s) - if stepQuery(db, s, false): - stopQuery(db, s) - else: - stopQuery(db, s) - dbError(db) + block: + when defined(debugOrminTrace): + echo "[[Ormin Executing]]: ", sqlStmt + let s {.gensym.} = prepareStmt(db, sqlStmt) + when dbBackend == DbBackend.sqlite: + defer: finalizeStmt(s) + startQuery(db, s) + if stepQuery(db, s, false): + stopQuery(db, s) + else: + stopQuery(db, s) + dbError(db) # Execute a non-row SQL statement, relying on startQuery to raise on failure template execNoRowsLoose(sqlStmt: string) = - when defined(debugOrminTrace): - echo "[[Ormin Executing]]: ", sqlStmt - let s {.gensym.} = prepareStmt(db, sqlStmt) - startQuery(db, s) - discard stepQuery(db, s, false) - stopQuery(db, s) + block: + when defined(debugOrminTrace): + echo "[[Ormin Executing]]: ", sqlStmt + let s {.gensym.} = prepareStmt(db, sqlStmt) + when dbBackend == DbBackend.sqlite: + defer: finalizeStmt(s) + startQuery(db, s) + discard stepQuery(db, s, false) + stopQuery(db, s) proc newQueryBuilder(): QueryBuilder {.compileTime.} = QueryBuilder(head: "", fromm: "", join: "", values: "", where: "", @@ -810,6 +816,11 @@ proc cond(n: NimNode; q: var string; params: var Params; else: macros.error "construct not supported in condition: " & treeRepr n, n +proc addStatementCleanup(body, statement: NimNode) = + when dbBackend == DbBackend.sqlite: + body.add quote do: + defer: finalizeStmt(`statement`) + proc generateRoutine(name: NimNode, q: QueryBuilder; sql: string; k: NimNodeKind): NimNode = let prepStmt = ident($name & "PrepStmt") @@ -818,6 +829,7 @@ proc generateRoutine(name: NimNode, q: QueryBuilder; let body = newStmtList() # Ensure the prepared statement is created before binding/starting the query body.add prepare + addStatementCleanup(body, prepStmt) var finalParams = newNimNode(nnkFormalParams) if q.retTypeIsJson: @@ -887,7 +899,7 @@ proc generateRoutine(name: NimNode, q: QueryBuilder; newEmptyNode(), # pragmas newEmptyNode(), body) - result = newStmtList(prepare, iter) + result = iter proc getColumnName(n: NimNode): string = case n.kind @@ -1607,9 +1619,10 @@ proc queryImpl(q: QueryBuilder; body: NimNode; attempt, produceJson: bool): NimN let prepStmt = genSym(nskLet) let res = genSym(nskVar) result = newTree( - if q.retType.len > 0: nnkStmtListExpr else: nnkStmtList, + nnkStmtList, newLetStmt(prepStmt, newCall(bindSym"prepareStmt", ident"db", newLit sql)) ) + addStatementCleanup(result, prepStmt) let rtyp = if q.retType.len > 1 or q.retType.len == 0: q.retType else: @@ -1724,6 +1737,7 @@ proc queryImpl(q: QueryBuilder; body: NimNode; attempt, produceJson: bool): NimN result.add newTree(nnkBlockStmt, newEmptyNode(), blk) if q.retType.len > 0: result.add res + result = newTree(nnkBlockStmt, newEmptyNode(), result) proc queryHookImpl(q: QueryBuilder; body: NimNode; attempt: bool; retType: NimNode): NimNode = expectKind body, nnkStmtList @@ -1742,9 +1756,10 @@ proc queryHookImpl(q: QueryBuilder; body: NimNode; attempt: bool; retType: NimNo let prepStmt = genSym(nskLet) let res = genSym(nskVar) result = newTree( - nnkStmtListExpr, + nnkStmtList, newLetStmt(prepStmt, newCall(bindSym"prepareStmt", ident"db", newLit sql)) ) + addStatementCleanup(result, prepStmt) if q.singleRow: result.add newTree(nnkVarSection, newIdentDefs(res, copyNimTree(retType), newEmptyNode())) else: @@ -1794,6 +1809,7 @@ proc queryHookImpl(q: QueryBuilder; body: NimNode; attempt: bool; retType: NimNo result.add newTree(nnkBlockStmt, newEmptyNode(), blk) result.add res + result = newTree(nnkBlockStmt, newEmptyNode(), result) macro query*(args: varargs[untyped]): untyped = if args.len == 1: diff --git a/tests/tsqlite.nim b/tests/tsqlite.nim index dc18454..cb0b76a 100644 --- a/tests/tsqlite.nim +++ b/tests/tsqlite.nim @@ -2,6 +2,7 @@ import unittest, db_connector/sqlite3, json, times, sequtils from db_connector/db_sqlite import exec, getValue import ormin import ormin/db_utils +import ./tstatement_lifetime importModel(DbBackend.sqlite, "model_sqlite") diff --git a/tests/tstatement_lifetime.nim b/tests/tstatement_lifetime.nim new file mode 100644 index 0000000..35e7df4 --- /dev/null +++ b/tests/tstatement_lifetime.nim @@ -0,0 +1,111 @@ +import std/assertions + +import db_connector/db_sqlite as sqliteDb except DbConn, open, close, dbError +import ormin +importModel(DbBackend.sqlite, "model_sqlite", includeStatic = true) + +proc newDatabase(): DbConn = + result = open(":memory:", "", "", "") + sqliteDb.exec(result, sql"CREATE TABLE tb_string(typstring TEXT NOT NULL UNIQUE)") + sqliteDb.exec(result, sql"INSERT INTO tb_string VALUES ('first'), ('second')") + +block repeated_queries_release_statements_before_connection_close: + let db = newDatabase() + for index in 0 ..< 100: + let rows = query: + select tb_string(typstring) + doAssert rows.len == 2 + let typed = query(string): + select tb_string(typstring) + doAssert typed.len == 2 + let missing = "missing" + let absent = tryQuery: + select tb_string(typstring) + where typstring == ?missing + limit 1 + discard absent + close(db) + +block unused_routine_declarations_do_not_prepare_statements: + let db = newDatabase() + createProc unusedProc: + select tb_string(typstring) + createIter unusedIterator: + select tb_string(typstring) + close(db) + +createIter strings: + select tb_string(typstring) + +createProc allStrings: + select tb_string(typstring) + +block generated_proc_and_complete_iteration_release_statements: + let db = newDatabase() + for index in 0 ..< 100: + doAssert allStrings(db).len == 2 + var count = 0 + for value in strings(db): + inc count + doAssert count == 2 + close(db) + +block iterator_break_releases_its_statement: + let db = newDatabase() + for value in strings(db): + doAssert value.len > 0 + break + close(db) + +proc returnFromIterator(db: DbConn): string = + for value in strings(db): + return value + +block caller_return_releases_iterator_statement: + let db = newDatabase() + doAssert returnFromIterator(db).len > 0 + close(db) + +block caller_exception_releases_iterator_statement: + let db = newDatabase() + doAssertRaises ValueError: + for value in strings(db): + raise newException(ValueError, value) + close(db) + +type + ReadFailure = distinct string + BindFailure = distinct string + +proc fromQueryHook*(dest: var ReadFailure, value: string) = + raise newException(ValueError, "result conversion failed") + +proc toQueryHook*(dest: var string, value: BindFailure) = + raise newException(ValueError, "parameter conversion failed") + +block result_hook_exception_releases_statement: + let db = newDatabase() + doAssertRaises ValueError: + discard query(ReadFailure): + select tb_string(typstring) + close(db) + +block binding_hook_exception_releases_statement: + let db = newDatabase() + let value = BindFailure("first") + doAssertRaises ValueError: + discard query: + select tb_string(typstring) + where typstring == ?value + close(db) + +block execution_error_preserves_error_and_releases_statement: + let db = newDatabase() + doAssertRaises DbError: + query: + insert tb_string(typstring = "first") + # The failed query must not poison a subsequent query or connection close. + let rows = query: + select tb_string(typstring) + doAssert rows.len == 2 + close(db)