diff --git a/docs/postgresql.md b/docs/postgresql.md index 4a79591..2b83740 100644 --- a/docs/postgresql.md +++ b/docs/postgresql.md @@ -21,9 +21,9 @@ supplies the PostgreSQL driver. ## Connection Ownership and Transactions `JdbcQueryExecutor` has two construction paths. Both share the same positional -parameter binding, row mapping, single-row and multi-row result handling, -affected-row counting, and exception translation, and both accept the same -generated repositories without regeneration: +parameter binding, row mapping, cardinality enforcement, affected-row counting, +and exception translation, and both accept the same generated repositories +without regeneration: | Construction | Connection ownership | | --- | --- | @@ -39,14 +39,38 @@ savepoints, and no isolation configuration. The [Quickstart](quickstart.md) runs that pattern end to end. In both paths the `PreparedStatement` and any `ResultSet` opened for an -operation are closed before that operation returns, on success and on failure. +operation are closed before that operation returns, on success, on a cardinality +failure, and on any other failure. + +Every generated method passes the generated repository's class name and its +query name to the executor, so every runtime failure names the query the +application called. Every `SQLException` raised while acquiring a connection, preparing a statement, binding parameters, executing, reading results, or closing a DataSource-acquired connection is translated into `dev.sqlcj.runtime.QueryExecutionException` with -the message `Failed to execute query` and the `SQLException` as its cause. A -failed operation on a caller-owned connection leaves the connection open, so the -application decides whether to continue or roll back. +the message `Failed to execute query '' in ` and the +`SQLException` as its cause: + +```text +Failed to execute query 'GetAuthor' in AuthorRepository +``` + +A row count an annotation does not allow raises +`dev.sqlcj.runtime.QueryCardinalityException`, a subclass of +`QueryExecutionException` that carries only a message: + +```text +Query 'GetAuthor' in AuthorRepository returned no row; expected exactly one +Query 'GetAuthor' in AuthorRepository returned more than one row; expected exactly one +Query 'FindAuthor' in AuthorRepository returned more than one row; expected at most one +``` + +A cardinality check runs after the statement has executed, so a returning write +that fails it has already changed the database. + +A failed operation on a caller-owned connection leaves the connection open, so +the application decides whether to continue or roll back. The runtime is blocking and synchronous. An executor built on a caller-owned connection inherits that connection's confinement to a single thread at a time. @@ -130,8 +154,18 @@ input and is not carried into the model at all. ## Nulls Every generated method parameter and every generated result component uses a -reference type, so each of them can represent SQL `NULL`. `Optional` and custom -nullable wrappers are not used. +reference type, so each of them can represent SQL `NULL`. A result component may +be `null` exactly when its column is modeled nullable by the schema snapshot. + +Row absence is a different thing from a null component, and the two are never +mixed: + +- absence is expressed only by the query's annotation: `:optional` returns + `Optional.empty()`, and `:one` fails with + `dev.sqlcj.runtime.QueryCardinalityException`, +- `Optional` is used only as the return type of an `:optional` query. No record + component and no method parameter is wrapped in `Optional`, and no custom + nullable wrapper is used. Null handling is uniform and independent of the column type: @@ -233,6 +267,9 @@ Nulls: - `PostgresIntegrationTest.shouldBindAndReadNullValuesThroughGeneratedCode` binds null arguments and reads null results through generated code against PostgreSQL. +- `PostgresIntegrationTest.shouldEnforceResultCardinalitiesAgainstPostgres` + proves that row absence is reported by `:optional` and `:one` rather than by a + null result. - `DefaultSchemaParserTest.shouldParseColumns`, `DefaultSchemaParserTest.shouldParseNullabilityOfAddedColumnTypes`, and `DefaultSchemaParserTest.shouldParseSerialColumnAsNotNullable` cover parsed @@ -240,13 +277,15 @@ Nulls: Connection ownership and transactions: -- `JdbcQueryExecutorTest` covers both construction paths for `query`, - `queryMany`, and `execute`, including +- `JdbcQueryExecutorTest` covers both construction paths for `queryOne`, + `queryOptional`, `queryMany`, and `execute`, including `shouldCloseAcquiredConnectionForEachDataSourceOperation`, `shouldCloseAcquiredConnectionWhenDataSourceOperationFails`, + `shouldCloseAcquiredConnectionWhenCardinalityFails`, `shouldLeaveCallerOwnedConnectionOpenAndItsTransactionStateUnchanged`, `shouldCloseStatementsAndResultSetsOfCallerOwnedConnection`, - `shouldCloseStatementWhenExecutionFailsOnCallerOwnedConnection`, and + `shouldCloseStatementWhenExecutionFailsOnCallerOwnedConnection`, + `shouldCloseStatementWhenCardinalityFailsOnCallerOwnedConnection`, and `shouldWrapSqlExceptionForCallerOwnedConnection`. - `PostgresIntegrationTest.shouldCommitGeneratedOperationsOnCallerOwnedConnection` and diff --git a/docs/queries.md b/docs/queries.md index 324fed7..a9409cc 100644 --- a/docs/queries.md +++ b/docs/queries.md @@ -38,23 +38,37 @@ WHERE id = $1; ## Annotations and Cardinality -sqlcj accepts three annotations. Any other annotation is rejected. +sqlcj accepts four annotations. Any other annotation is rejected. -| Annotation | Accepted statements | Generated return type | Result when no row matches | -| --- | --- | --- | --- | -| `:one` | `SELECT`, or a write with `RETURNING` | the generated result record | `null` | -| `:many` | `SELECT`, or a write with `RETURNING` | `List<`result record`>` | an empty list | -| `:exec` | `INSERT`, `UPDATE`, `DELETE` without `RETURNING` | `int` affected-row count | `0` | +| Annotation | Accepted statements | Generated return type | Result when no row matches | Result when several rows match | +| --- | --- | --- | --- | --- | +| `:one` | `SELECT`, or a write with `RETURNING` | the generated result record | `QueryCardinalityException` | `QueryCardinalityException` | +| `:optional` | `SELECT`, or a write with `RETURNING` | `Optional<`result record`>` | `Optional.empty()` | `QueryCardinalityException` | +| `:many` | `SELECT`, or a write with `RETURNING` | `List<`result record`>` | an empty list | every row | +| `:exec` | `INSERT`, `UPDATE`, `DELETE` without `RETURNING` | `int` affected-row count | `0` | the affected-row count | The annotation and the statement must agree: -- a `SELECT` must be `:one` or `:many`, +- a `SELECT` must be `:one`, `:optional`, or `:many`, - a write without `RETURNING` must be `:exec`, -- a write with `RETURNING` must be `:one` or `:many`. +- a write with `RETURNING` must be `:one`, `:optional`, or `:many`. -`:one` reads the first row of the result set and does not read any further row, -so a query that matches several rows returns the first one rather than failing. -`:many` returns every row in the order the database produced it. +The three result annotations differ only in how many rows they accept: + +- `:one` returns exactly one row. It raises + `dev.sqlcj.runtime.QueryCardinalityException` when the query returned no row + and when it returned more than one. +- `:optional` returns at most one row, as `Optional.empty()` or + `Optional.of(row)`. It raises the same exception when the query returned more + than one row. +- `:many` returns every row in the order the database produced it, and an empty + list when there was none. + +A cardinality check runs after the statement has executed, so a returning write +that fails its check has already changed the database. The application controls +whether that change is kept, by running the write on a caller-owned connection +and rolling back. The exception messages are listed in +[PostgreSQL Support](postgresql.md#connection-ownership-and-transactions). ## Reads @@ -141,8 +155,8 @@ WHERE id = $1; ### `RETURNING` -A supported `INSERT`, `UPDATE`, or `DELETE` declared `:one` or `:many` may end -with a `RETURNING` clause that lists either: +A supported `INSERT`, `UPDATE`, or `DELETE` declared `:one`, `:optional`, or +`:many` may end with a `RETURNING` clause that lists either: - unaliased direct columns of the target table, in the order they are declared, or @@ -189,6 +203,8 @@ in its `SET` clause, so the generated method takes `(id, bio)` and binds ```java public int updateAuthorBio(Long id, String bio) { return executor.execute( + "AuthorRepository", + "UpdateAuthorBio", """ UPDATE authors SET bio = ? @@ -215,13 +231,13 @@ that repository; sqlcj never generates a class per query. constructor taking that executor. - Methods appear in query-source order. A method name is the lower camel form of the query name, as in `get_author` and `GetAuthor` to `getAuthor`. -- A `:one` or `:many` query that returns one complete table row — a +- A `:one`, `:optional`, or `:many` query that returns one complete table row — a single-source `SELECT *` or `SELECT .*`, or a write whose `RETURNING` clause is exactly `*` — returns the repository's nested `Row` record. That record and its private `RowMapper` field are generated once per table, after the constructor, in the order the queries first use them, and every query returning that row shares them. -- Every other `:one` or `:many` query generates a nested `public record` named +- Every other `:one`, `:optional`, or `:many` query generates a nested `public record` named `Result` in upper camel case, whose components follow the selected-column order and are named after each column's projection alias or column name, plus a private `RowMapper` field that reads each column by its @@ -229,13 +245,16 @@ that repository; sqlcj never generates a class per query. every column, a wildcard combined with another projection item, a wildcard in a join, and a `RETURNING` column list. - A `:exec` query generates no result record and returns `int`. +- Every generated method passes the repository class name and the query name to + the executor, as the first two arguments of its call, so a runtime failure + names the query the application called. - Generated source imports only `dev.sqlcj.runtime.QueryExecutor`, - `dev.sqlcj.runtime.RowMapper`, `java.util.List`, and the JDK types of the - mapped columns, so the runtime artifact is the only sqlcj dependency a - consumer needs. + `dev.sqlcj.runtime.RowMapper`, `java.util.List`, `java.util.Optional` when the + entry declares an `:optional` query, and the JDK types of the mapped columns, + so the runtime artifact is the only sqlcj dependency a consumer needs. The `Author` entry of the [Quickstart](quickstart.md), which declares -`CreateAuthor`, `GetAuthor`, `ListAuthors`, `UpdateAuthorBio`, and +`CreateAuthor`, `GetAuthor`, `FindAuthor`, `ListAuthors`, `UpdateAuthorBio`, and `DeleteAuthor`, generates one `AuthorRepository`: ```java @@ -244,6 +263,7 @@ package com.example.app.db; import dev.sqlcj.runtime.QueryExecutor; import dev.sqlcj.runtime.RowMapper; import java.util.List; +import java.util.Optional; import java.time.LocalDateTime; /** @@ -281,7 +301,9 @@ public final class AuthorRepository { * Type: ONE */ public AuthorsRow createAuthor(String name, String bio) { - return executor.query( + return executor.queryOne( + "AuthorRepository", + "CreateAuthor", """ INSERT INTO authors (name, bio) VALUES (?, ?) @@ -291,14 +313,34 @@ public final class AuthorRepository { ); } - // getAuthor, listAuthors, updateAuthorBio, and deleteAuthor follow here. + // getAuthor follows here, in query-source order. + + /** + * Query: FindAuthor + * Table: authors + * Type: OPTIONAL + */ + public Optional findAuthor(Long id) { + return executor.queryOptional( + "AuthorRepository", + "FindAuthor", + """ + SELECT * + FROM authors + WHERE id = ?;""", + java.util.Arrays.asList(id), + authorsRowMapper + ); + } + + // listAuthors, updateAuthorBio, and deleteAuthor follow here. } ``` -`CreateAuthor`, `GetAuthor`, and `ListAuthors` each return one complete -`authors` row, so all three use the one `AuthorsRow` record and the one -`authorsRowMapper` field. `getAuthor` returns `AuthorsRow` and `listAuthors` -returns `List`. +`CreateAuthor`, `GetAuthor`, `FindAuthor`, and `ListAuthors` each return one +complete `authors` row, so all four use the one `AuthorsRow` record and the one +`authorsRowMapper` field. `getAuthor` returns `AuthorsRow`, `findAuthor` returns +`Optional`, and `listAuthors` returns `List`. The application constructs the repository once per execution context: @@ -306,6 +348,8 @@ The application constructs the repository once per execution context: AuthorRepository authors = new AuthorRepository(new JdbcQueryExecutor(dataSource)); AuthorRepository.AuthorsRow author = authors.getAuthor(1L); + +Optional found = authors.findAuthor(2L); ``` A query name, projection alias, column name, and parameter name becomes a @@ -351,7 +395,7 @@ name, and its header line. - An aliased, computed, qualified-wildcard, or unknown `RETURNING` item; `RETURNING` on `:exec`; and `ON CONFLICT`, `UPDATE ... FROM`, `DELETE ... USING`, or a common table expression in a returning write. -- Any annotation other than `:one`, `:many`, and `:exec`. +- Any annotation other than `:one`, `:optional`, `:many`, and `:exec`. ### Not analyzed @@ -382,20 +426,39 @@ Query sources and annotations: `DefaultQueryParserTest.parsesMultipleQueries`, `DefaultQueryParserTest.preservesQueryOrder`, `DefaultQueryParserTest.parsesHeaderLineOfEachQuery`, + `DefaultQueryParserTest.parsesOptionalQueryType`, + `DefaultQueryParserTest.parsesExecQueryType`, `DefaultQueryParserTest.rejectsInvalidHeader`, `DefaultQueryParserTest.rejectsInvalidQueryType`, `DefaultQueryParserTest.rejectsDuplicateQueryNames`, and `DefaultQueryParserTest.rejectsQueryWithoutSql` cover the file format. - `QueryAnalyzerTest.shouldRejectSelectWithoutResultQueryType`, + `QueryAnalyzerTest.shouldAnalyzeSelectDeclaredAsOptional`, `QueryAnalyzerTest.shouldRejectWriteWithoutExecQueryType`, `QueryAnalyzerTest.shouldRejectReturningWriteDeclaredAsExec`, and - `QueryAnalyzerTest.shouldAnalyzeReturningWriteForBothResultQueryTypes` cover + `QueryAnalyzerTest.shouldAnalyzeReturningWriteForEveryResultQueryType` cover annotation and statement agreement. -- `JdbcQueryExecutorTest.shouldReturnNullWhenQueryFindsNoRows`, +- `JdbcQueryExecutorTest.shouldReturnTheRowWhenQueryOneFindsExactlyOneRow`, + `JdbcQueryExecutorTest.shouldFailWhenQueryOneFindsNoRow`, + `JdbcQueryExecutorTest.shouldFailWhenQueryOneFindsMoreThanOneRow`, + `JdbcQueryExecutorTest.shouldReturnTheRowWhenQueryOptionalFindsOneRow`, + `JdbcQueryExecutorTest.shouldReturnEmptyOptionalWhenQueryOptionalFindsNoRow`, + `JdbcQueryExecutorTest.shouldFailWhenQueryOptionalFindsMoreThanOneRow`, `JdbcQueryExecutorTest.shouldReturnEmptyListWhenQueryManyFindsNoRows`, `JdbcQueryExecutorTest.shouldReturnAllRowsForQueryMany`, and `JdbcQueryExecutorTest.shouldReturnAffectedRowCountForExecute` cover the - cardinality results. + cardinality results and their exact messages. +- `PostgresIntegrationTest.shouldEnforceResultCardinalitiesAgainstPostgres` + executes `:one`, `:optional`, and `:many` over zero, one, and two matching + rows of a non-unique column against PostgreSQL 16, through both executor + construction paths. +- `JavaCodeGeneratorTest.shouldGenerateOptionalExecutionForOptionalQuery`, + `JavaCodeGeneratorTest.shouldShareOneRowRecordBetweenOptionalAndOtherFullRowQueries`, + `JavaCodeGeneratorTest.shouldNotImportOptionalWithoutAnOptionalQuery`, + `JavaCodeGeneratorTest.shouldPassRepositoryAndQueryIdentityToEveryExecutorCall`, + and + `JavaCodeGeneratorNamingTest.shouldPassExactRepositoryAndQueryNameToTheExecutor` + cover the generated calls, return types, imports, and identity literals. Reads: @@ -443,6 +506,7 @@ Writes and `RETURNING`: - `PostgresIntegrationTest.shouldExecuteGeneratedWriteAgainstPostgres`, `PostgresIntegrationTest.shouldExecuteGeneratedInsertReturningAgainstPostgres`, `PostgresIntegrationTest.shouldExecuteGeneratedUpdateReturningAgainstPostgres`, + whose no-row returning update fails its `:one` cardinality check, and `PostgresIntegrationTest.shouldExecuteGeneratedDeleteReturningAgainstPostgres` execute them against PostgreSQL 16. diff --git a/docs/quickstart.md b/docs/quickstart.md index ff78d58..d258d05 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -187,6 +187,11 @@ SELECT * FROM authors WHERE id = $1; +-- name: FindAuthor :optional +SELECT * +FROM authors +WHERE id = $1; + -- name: ListAuthors :many SELECT * FROM authors @@ -203,14 +208,19 @@ FROM authors WHERE id = $1; ``` -All five queries become methods of the one generated `AuthorRepository`: -`createAuthor`, `getAuthor`, `listAuthors`, `updateAuthorBio`, and -`deleteAuthor`. `CreateAuthor`, `GetAuthor`, and `ListAuthors` each return one -complete `authors` row, so all three share the nested record +All six queries become methods of the one generated `AuthorRepository`: +`createAuthor`, `getAuthor`, `findAuthor`, `listAuthors`, `updateAuthorBio`, and +`deleteAuthor`. `CreateAuthor`, `GetAuthor`, `FindAuthor`, and `ListAuthors` +each return one complete `authors` row, so all four share the nested record `AuthorRepository.AuthorsRow`, generated once from the schema's column order. A query with its own result shape, such as a partial projection or a `RETURNING` column list, generates a nested `AuthorRepository.Result` record -instead. The full query contract is documented in [Queries](queries.md). +instead. + +`GetAuthor` and `FindAuthor` read the same row by the same key and differ only +in cardinality: `getAuthor` returns `AuthorsRow` and requires exactly one row, +while `findAuthor` returns `Optional` and accepts none. The full +query contract is documented in [Queries](queries.md). ## 8. `src/main/java/com/example/app/App.java` @@ -219,12 +229,14 @@ package com.example.app; import com.example.app.db.AuthorRepository; import dev.sqlcj.runtime.JdbcQueryExecutor; +import dev.sqlcj.runtime.QueryCardinalityException; import dev.sqlcj.runtime.QueryExecutor; import org.postgresql.ds.PGSimpleDataSource; import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; +import java.util.Optional; public final class App { @@ -251,7 +263,15 @@ public final class App { System.out.println("listed: " + author.id() + " " + author.name()); } - System.out.println("missing row: " + authors.getAuthor(-1L)); + Optional missing = authors.findAuthor(-1L); + + System.out.println("missing row: " + missing.isPresent()); + + try { + authors.getAuthor(-1L); + } catch (QueryCardinalityException e) { + System.out.println("missing row rejected: " + e.getMessage()); + } try (Connection connection = dataSource.getConnection()) { connection.setAutoCommit(false); @@ -278,7 +298,7 @@ public final class App { connection.rollback(); - System.out.println("rolled back: " + authors.getAuthor(discarded.id())); + System.out.println("rolled back: " + authors.findAuthor(discarded.id()).isPresent()); } System.out.println("deleted rows: " + authors.deleteAuthor(created.id())); @@ -352,9 +372,10 @@ created: 1 Ada Lovelace read: Ada Lovelace / First programmer updated rows: 1 listed: 1 Ada Lovelace -missing row: null +missing row: false +missing row rejected: Query 'GetAuthor' in AuthorRepository returned no row; expected exactly one committed: Compiler pioneer -rolled back: null +rolled back: false deleted rows: 1 ``` @@ -364,7 +385,12 @@ That output is the whole MVP contract in one run: keeps the types and order of its named query. - `CreateAuthor` is a `:one` write whose `RETURNING` clause reads back the database-generated `BIGSERIAL` identifier as a typed `Long`. -- `GetAuthor` is a `:one` read, and returns `null` when no row matches. +- `GetAuthor` is a `:one` read: it returns the row when exactly one matches, and + fails with `dev.sqlcj.runtime.QueryCardinalityException` naming the query and + the repository when none matches or several do. +- `FindAuthor` is an `:optional` read of the same row, and returns + `Optional.empty()` when no row matches, which is how the sample checks for a + missing, rolled back, or deleted author. - `ListAuthors` is a `:many` read, and returns an empty list when no row matches. - `UpdateAuthorBio` and `DeleteAuthor` are `:exec` writes and return their diff --git a/examples/maven-postgresql/sql/queries.sql b/examples/maven-postgresql/sql/queries.sql index 80ae52e..3f1ee3e 100644 --- a/examples/maven-postgresql/sql/queries.sql +++ b/examples/maven-postgresql/sql/queries.sql @@ -8,6 +8,11 @@ SELECT * FROM authors WHERE id = $1; +-- name: FindAuthor :optional +SELECT * +FROM authors +WHERE id = $1; + -- name: ListAuthors :many SELECT * FROM authors diff --git a/examples/maven-postgresql/src/main/java/com/example/app/App.java b/examples/maven-postgresql/src/main/java/com/example/app/App.java index 85f6e37..4dd4048 100644 --- a/examples/maven-postgresql/src/main/java/com/example/app/App.java +++ b/examples/maven-postgresql/src/main/java/com/example/app/App.java @@ -2,6 +2,7 @@ import com.example.app.db.AuthorRepository; import dev.sqlcj.runtime.JdbcQueryExecutor; +import dev.sqlcj.runtime.QueryCardinalityException; import dev.sqlcj.runtime.QueryExecutor; import org.postgresql.ds.PGSimpleDataSource; @@ -10,21 +11,27 @@ import java.sql.SQLException; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Runs the documented sqlcj workflow against PostgreSQL through generated - * code: create, read, list, update, a missing-row read, a committed - * transaction, a rolled back transaction, and delete. + * code: create, read, optional read, list, update, a missing-row read, a + * committed transaction, a rolled back transaction, and delete. * - *

All five named queries are methods of one generated + *

All six named queries are methods of one generated * {@link AuthorRepository}. The repository is constructed once per execution * context: once from the {@code DataSource}-backed executor, and once more per * transaction from a caller-owned connection. * - *

Create, read, and list each return one complete {@code authors} row, so - * all three share the repository's single + *

Create, read, optional read, and list each return one complete + * {@code authors} row, so all four share the repository's single * {@link AuthorRepository.AuthorsRow} record. * + *

Row absence is expressed by the {@code :optional} {@code FindAuthor} + * query, which returns an empty {@link Optional}, while the {@code :one} + * {@code GetAuthor} query requires exactly one row and raises a + * {@link QueryCardinalityException} when none matches. + * *

Every step is checked, so the process exits non-zero as soon as one * generated operation returns an unexpected result. * @@ -61,6 +68,13 @@ public static void main(String[] args) throws SQLException { System.out.println("read: " + read.name() + " / " + read.bio()); + Optional found = authors.findAuthor(created.id()); + + check(found.isPresent(), "FindAuthor returned no row for the created author"); + checkEquals("Ada Lovelace", found.get().name(), "FindAuthor name"); + + System.out.println("found: " + found.get().name()); + List listed = authors.listAuthors(); checkEquals(1, listed.size(), "ListAuthors row count after create"); @@ -76,9 +90,20 @@ public static void main(String[] args) throws SQLException { System.out.println("updated rows: " + updatedRows); - check(authors.getAuthor(-1L) == null, "GetAuthor must return null for a missing row"); + check( + authors.findAuthor(-1L).isEmpty(), + "FindAuthor must return an empty result for a missing row" + ); + + System.out.println("missing row: empty"); - System.out.println("missing row: null"); + try { + authors.getAuthor(-1L); + + throw new IllegalStateException("GetAuthor must fail for a missing row"); + } catch (QueryCardinalityException e) { + System.out.println("missing row rejected: " + e.getMessage()); + } Long committedId = writeAndCommit(dataSource); @@ -92,14 +117,20 @@ public static void main(String[] args) throws SQLException { Long discardedId = writeAndRollback(dataSource); - check(authors.getAuthor(discardedId) == null, "the rolled back author must not be readable"); + check( + authors.findAuthor(discardedId).isEmpty(), + "the rolled back author must not be readable" + ); - System.out.println("rolled back: null"); + System.out.println("rolled back: empty"); int deletedRows = authors.deleteAuthor(created.id()); checkEquals(1, deletedRows, "DeleteAuthor affected rows"); - check(authors.getAuthor(created.id()) == null, "the deleted author must not be readable"); + check( + authors.findAuthor(created.id()).isEmpty(), + "the deleted author must not be readable" + ); System.out.println("deleted rows: " + deletedRows); diff --git a/scripts/verify-release.sh b/scripts/verify-release.sh index 5efff6f..97828f7 100755 --- a/scripts/verify-release.sh +++ b/scripts/verify-release.sh @@ -123,7 +123,7 @@ generated_count=$(find "${generated_root}" -type f -name '*.java' | wc -l) [ "${generated_count}" -eq 1 ] \ || fail "expected 1 generated source but found ${generated_count}" -for method in createAuthor getAuthor listAuthors updateAuthorBio deleteAuthor; do +for method in createAuthor getAuthor findAuthor listAuthors updateAuthorBio deleteAuthor; do grep -q " ${method}(" "${generated_package}/AuthorRepository.java" \ || fail "the generated repository is missing the ${method} method" done diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java index 15afd87..6afeadd 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/analysis/QueryAnalyzer.java @@ -208,9 +208,9 @@ private QueryModel analyzeDelete(Query query, ParsedSql parsedSql, Delete delete } private void requireResultQueryType(Query query) { - if (query.type() != QueryType.ONE && query.type() != QueryType.MANY) { + if (!producesResult(query)) { throw new UnsupportedOperationException( - "SELECT queries must be declared as :one or :many" + "SELECT queries must be declared as :one, :optional, or :many" ); } } @@ -231,13 +231,20 @@ private void requireWriteQueryType(Query query, ReturningClause returningClause) return; } - if (query.type() != QueryType.ONE && query.type() != QueryType.MANY) { + if (!producesResult(query)) { throw new UnsupportedOperationException( - "Write queries with RETURNING must be declared as :one or :many" + "Write queries with RETURNING must be declared as :one, :optional, or :many" ); } } + /** The annotations that declare a row-producing result shape. */ + private boolean producesResult(Query query) { + return query.type() == QueryType.ONE + || query.type() == QueryType.OPTIONAL + || query.type() == QueryType.MANY; + } + /** * Resolves the returned columns in declared order against the single write * target, expanding a bare {@code *} in schema column order. diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaCodeGenerator.java b/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaCodeGenerator.java index e172ef8..c3d2d4e 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaCodeGenerator.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaCodeGenerator.java @@ -98,6 +98,10 @@ private String generateImports(QueryGroupModel group) { imports.add("java.util.List"); } + if (group.queries().stream().anyMatch(this::isOptional)) { + imports.add("java.util.Optional"); + } + for (QueryModel query : group.queries()) { query.columns().stream() .map(QueryColumn::type) @@ -119,9 +123,14 @@ private String generateImports(QueryGroupModel group) { private boolean hasResult(QueryModel query) { return query.type() == QueryType.ONE + || query.type() == QueryType.OPTIONAL || query.type() == QueryType.MANY; } + private boolean isOptional(QueryModel query) { + return query.type() == QueryType.OPTIONAL; + } + private boolean isExec(QueryModel query) { return query.type() == QueryType.EXEC; } @@ -213,7 +222,10 @@ private String generateClass(QueryGroupModel group, JavaNames names) { ); } - members.add(generateJavaDoc(query) + generateMethod(query, queryNames)); + members.add( + generateJavaDoc(query) + + generateMethod(query, names.repositoryClassName(), queryNames) + ); } return """ @@ -286,18 +298,28 @@ private String generateMethodParameters(QueryModel query, JavaNames.QueryNames n .collect(Collectors.joining(", ")); } - private String generateMethod(QueryModel query, JavaNames.QueryNames names) { + private String generateMethod( + QueryModel query, + String repositoryClassName, + JavaNames.QueryNames names + ) { return switch (query.type()) { - case ONE, MANY -> generateQueryMethod(query, names); - case EXEC -> generateExecMethod(query, names); + case ONE, OPTIONAL, MANY -> generateQueryMethod(query, repositoryClassName, names); + case EXEC -> generateExecMethod(query, repositoryClassName, names); case EXEC_RESULT, BATCH_EXEC, BATCH_MANY, BATCH_ONE -> generateUnsupportedMethod(query, names); }; } - private String generateQueryMethod(QueryModel query, JavaNames.QueryNames names) { + private String generateQueryMethod( + QueryModel query, + String repositoryClassName, + JavaNames.QueryNames names + ) { return """ public %s %s(%s) { return executor.%s( + %s, + %s, %s, %s, %s @@ -309,16 +331,24 @@ private String generateQueryMethod(QueryModel query, JavaNames.QueryNames names) names.methodName(), generateMethodParameters(query, names), generateExecutorMethod(query), + generateStringLiteral(repositoryClassName), + generateStringLiteral(query.name()), generateSql(query), generateParameterList(query, names), names.rowMapperName() ); } - private String generateExecMethod(QueryModel query, JavaNames.QueryNames names) { + private String generateExecMethod( + QueryModel query, + String repositoryClassName, + JavaNames.QueryNames names + ) { return """ public %s %s(%s) { return executor.execute( + %s, + %s, %s, %s ); @@ -328,6 +358,8 @@ private String generateExecMethod(QueryModel query, JavaNames.QueryNames names) generateReturnType(query, names), names.methodName(), generateMethodParameters(query, names), + generateStringLiteral(repositoryClassName), + generateStringLiteral(query.name()), generateSql(query), generateParameterList(query, names) ); @@ -348,7 +380,8 @@ private String generateUnsupportedMethod(QueryModel query, JavaNames.QueryNames private String generateExecutorMethod(QueryModel query) { return switch (query.type()) { - case ONE -> "query"; + case ONE -> "queryOne"; + case OPTIONAL -> "queryOptional"; case MANY -> "queryMany"; default -> throw new IllegalStateException( "Query type does not produce a result: " + query.type() @@ -356,6 +389,18 @@ private String generateExecutorMethod(QueryModel query) { }; } + /** + * Renders the repository and query identity an executor call reports in its + * failures as a Java string literal. + */ + private String generateStringLiteral(String value) { + return "\"" + + value + .replace("\\", "\\\\") + .replace("\"", "\\\"") + + "\""; + } + private String generateSql(QueryModel query) { return "\"\"\"\n" + escapeTextBlock(query.executableSql()) @@ -447,6 +492,7 @@ private Map generateParameterNamesByIndex( private String generateReturnType(QueryModel query, JavaNames.QueryNames names) { return switch (query.type()) { case ONE -> names.resultTypeName(); + case OPTIONAL -> "Optional<" + names.resultTypeName() + ">"; case MANY -> "List<" + names.resultTypeName() + ">"; case EXEC -> "int"; case EXEC_RESULT -> "void"; diff --git a/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaNames.java b/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaNames.java index f0fb99b..cee21bc 100644 --- a/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaNames.java +++ b/sqlcj-cli/src/main/java/dev/sqlcj/generator/JavaNames.java @@ -283,7 +283,9 @@ private static Set queryRowMapperNames(QueryGroupModel group, List ONE; + case ":optional" -> OPTIONAL; case ":many" -> MANY; case ":exec" -> EXEC; case ":execresult" -> EXEC_RESULT; diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java index d9eb05f..a5d826f 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/analysis/QueryAnalyzerTest.java @@ -1719,6 +1719,34 @@ void shouldRejectNamedParameterInInList() { ); } + @Test + void shouldAnalyzeSelectDeclaredAsOptional() { + String sql = "SELECT id, name FROM users WHERE id = $1"; + + QueryModel model = analyzer.analyze( + new Query("FindUser", QueryType.OPTIONAL, sql), + parser.parse(sql), + schema + ); + + assertEquals("FindUser", model.name()); + assertEquals(QueryType.OPTIONAL, model.type()); + assertEquals("users", model.table()); + + assertEquals( + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true) + ), + model.columns() + ); + + assertEquals( + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + model.parameters() + ); + } + @Test void shouldRejectSelectWithoutResultQueryType() { String sql = "SELECT id FROM users WHERE id = $1"; @@ -1732,7 +1760,7 @@ void shouldRejectSelectWithoutResultQueryType() { ); assertEquals( - "SELECT queries must be declared as :one or :many", + "SELECT queries must be declared as :one, :optional, or :many", exception.getMessage() ); } @@ -1755,7 +1783,7 @@ void shouldRejectReturningWriteDeclaredAsExec(String sql) { ); assertEquals( - "Write queries with RETURNING must be declared as :one or :many", + "Write queries with RETURNING must be declared as :one, :optional, or :many", exception.getMessage() ); } @@ -1768,8 +1796,8 @@ void shouldRejectReturningWriteDeclaredAsExec(String sql) { "DELETE FROM users WHERE id = $1 RETURNING id" } ) - void shouldAnalyzeReturningWriteForBothResultQueryTypes(String sql) { - for (QueryType type : List.of(QueryType.ONE, QueryType.MANY)) { + void shouldAnalyzeReturningWriteForEveryResultQueryType(String sql) { + for (QueryType type : List.of(QueryType.ONE, QueryType.OPTIONAL, QueryType.MANY)) { QueryModel model = analyzer.analyze( new Query("WriteUser", type, sql), parser.parse(sql), diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java index bf7ab49..62e4073 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/PostgresIntegrationTest.java @@ -4,6 +4,7 @@ import dev.sqlcj.config.JavaConfig; import dev.sqlcj.config.SqlConfig; import dev.sqlcj.runtime.JdbcQueryExecutor; +import dev.sqlcj.runtime.QueryCardinalityException; import dev.sqlcj.runtime.QueryExecutor; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -20,6 +21,7 @@ import javax.tools.JavaCompiler; import javax.tools.ToolProvider; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.RecordComponent; import java.math.BigDecimal; @@ -36,6 +38,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.stream.Stream; @@ -45,6 +48,7 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -139,6 +143,28 @@ INSERT INTO users (id, code, name) ORDER BY id; """; + /** + * Queries used by the cardinality test: the same predicate on the + * non-unique {@code code} column declared with each result annotation. + */ + private static final String CARDINALITY_QUERIES = """ + -- name: GetUserByCode :one + SELECT id, name + FROM users + WHERE code = $1; + + -- name: FindUserByCode :optional + SELECT id, name + FROM users + WHERE code = $1; + + -- name: ListUsersByCode :many + SELECT id, name + FROM users + WHERE code = $1 + ORDER BY id; + """; + private static final UUID EXTERNAL_ID = UUID.fromString("3f2504e0-4f89-11d3-9a0c-0305e82c3301"); private static final OffsetDateTime UPDATED_AT = OffsetDateTime.of( @@ -645,7 +671,8 @@ INSERT INTO users (id, code, name) /** * Covers a returning update: the repeated and out-of-order placeholders are * bound in textual order, the returned columns keep their declared order, - * and a no-row update produces the {@code :one} null result. + * and a no-row update fails the {@code :one} cardinality check after the + * statement itself executed. */ @Test void shouldExecuteGeneratedUpdateReturningAgainstPostgres() throws Exception { @@ -688,7 +715,10 @@ INSERT INTO users (id, code, name, bio) assertEquals("Renamed", component(result, "name")); assertNull(component(result, "score")); - assertNull(method.invoke(repository, 404L, "Missing", 42)); + assertEquals( + "Query 'RenameUser' in UsersRepository returned no row; expected exactly one", + cardinalityFailure(method, repository, 404L, "Missing", 42).getMessage() + ); } } @@ -818,6 +848,109 @@ INSERT INTO users (id, code, name, active) } } + /** + * Covers the three result annotations over zero, one, and two matching rows + * of a non-unique column, through both executor construction paths: a + * {@code :one} query requires exactly one row, {@code :optional} accepts + * none or one, and {@code :many} accepts any number. A failed cardinality + * check leaves the caller-owned connection open. + */ + @Test + void shouldEnforceResultCardinalitiesAgainstPostgres() throws Exception { + Path classesDirectory = generateAndCompile(CARDINALITY_QUERIES); + + execute(""" + INSERT INTO users (id, code, name) + VALUES + (1, 42, 'Alice'), + (2, 7, 'Bob'), + (3, 7, 'Carol') + """); + + try ( + URLClassLoader classLoader = classLoader(classesDirectory); + Connection connection = dataSource.getConnection() + ) { + assertCardinalityContract(newRepository(classLoader)); + + assertCardinalityContract( + newRepository(classLoader, new JdbcQueryExecutor(connection)) + ); + + assertFalse(connection.isClosed()); + } + } + + /** + * Asserts the documented contract of the three cardinality annotations on + * one generated repository instance. + */ + private void assertCardinalityContract(Object repository) throws Exception { + Method one = repository.getClass().getMethod("getUserByCode", Integer.class); + Method optional = repository.getClass().getMethod("findUserByCode", Integer.class); + Method many = repository.getClass().getMethod("listUsersByCode", Integer.class); + + Object single = one.invoke(repository, 42); + + assertEquals(1L, component(single, "id")); + assertEquals("Alice", component(single, "name")); + + assertEquals( + "Query 'GetUserByCode' in UsersRepository returned no row; expected exactly one", + cardinalityFailure(one, repository, 1).getMessage() + ); + + assertEquals( + "Query 'GetUserByCode' in UsersRepository returned more than one row; expected exactly one", + cardinalityFailure(one, repository, 7).getMessage() + ); + + Optional found = assertInstanceOf( + Optional.class, + optional.invoke(repository, 42) + ); + + assertEquals(1L, component(found.orElseThrow(), "id")); + + assertTrue( + assertInstanceOf(Optional.class, optional.invoke(repository, 1)).isEmpty() + ); + + assertEquals( + "Query 'FindUserByCode' in UsersRepository returned more than one row; expected at most one", + cardinalityFailure(optional, repository, 7).getMessage() + ); + + assertTrue( + assertInstanceOf(List.class, many.invoke(repository, 1)).isEmpty() + ); + + List listed = assertInstanceOf(List.class, many.invoke(repository, 7)); + + assertEquals(2, listed.size()); + assertEquals(2L, component(listed.get(0), "id")); + assertEquals("Bob", component(listed.get(0), "name")); + assertEquals(3L, component(listed.get(1), "id")); + assertEquals("Carol", component(listed.get(1), "name")); + } + + /** + * Invokes a generated method that must fail its cardinality check and + * returns the runtime exception it raised. + */ + private QueryCardinalityException cardinalityFailure( + Method method, + Object repository, + Object... arguments + ) { + InvocationTargetException failure = assertThrows( + InvocationTargetException.class, + () -> method.invoke(repository, arguments) + ); + + return assertInstanceOf(QueryCardinalityException.class, failure.getCause()); + } + /** * Runs three generated operations on one caller-owned connection with * auto-commit disabled: an affected-row write, a returning write, and a diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java index df47ba2..41248ff 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/compiler/SqlcjCompilerIntegrationTest.java @@ -4,6 +4,7 @@ import dev.sqlcj.config.JavaConfig; import dev.sqlcj.config.SqlConfig; import dev.sqlcj.runtime.JdbcQueryExecutor; +import dev.sqlcj.runtime.QueryCardinalityException; import dev.sqlcj.runtime.QueryExecutor; import org.h2.jdbcx.JdbcDataSource; import org.junit.jupiter.api.Test; @@ -13,6 +14,7 @@ import javax.tools.ToolProvider; import java.io.IOException; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.math.BigDecimal; import java.net.URL; @@ -29,7 +31,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -684,7 +685,20 @@ void shouldExecuteGeneratedQueryWithRepeatedPlaceholder() throws Exception { assertEquals(1L, getRecordComponent(result, "id")); assertEquals("Alice", getRecordComponent(result, "name")); - assertNull(method.invoke(generatedQuery, 2L, "Alice")); + InvocationTargetException failure = assertThrows( + InvocationTargetException.class, + () -> method.invoke(generatedQuery, 2L, "Alice") + ); + + QueryCardinalityException cardinality = assertInstanceOf( + QueryCardinalityException.class, + failure.getCause() + ); + + assertEquals( + "Query 'FindUser' in UsersRepository returned no row; expected exactly one", + cardinality.getMessage() + ); } } @@ -740,7 +754,7 @@ void shouldExecuteGeneratedQueryWithProtectedPlaceholderText() throws Exception void shouldExecuteGeneratedQueryWithoutParameters() throws Exception { Path classesDirectory = generateAndCompile( """ - -- name: GetFirstUser :one + -- name: ListAllUsers :many SELECT id, name FROM users ORDER BY id; @@ -751,7 +765,7 @@ void shouldExecuteGeneratedQueryWithoutParameters() throws Exception { assertTrue( source.contains( - "public GetFirstUserResult getFirstUser()" + "public List listAllUsers()" ) ); @@ -770,14 +784,19 @@ void shouldExecuteGeneratedQueryWithoutParameters() throws Exception { .getConstructor(QueryExecutor.class) .newInstance(executor); - Object result = generatedClass - .getMethod("getFirstUser") - .invoke(generatedQuery); + List results = assertInstanceOf( + List.class, + generatedClass + .getMethod("listAllUsers") + .invoke(generatedQuery) + ); - assertNotNull(result); + assertEquals(2, results.size()); - assertEquals(1L, getRecordComponent(result, "id")); - assertEquals("Alice", getRecordComponent(result, "name")); + assertEquals(1L, getRecordComponent(results.get(0), "id")); + assertEquals("Alice", getRecordComponent(results.get(0), "name")); + assertEquals(2L, getRecordComponent(results.get(1), "id")); + assertEquals("Bob", getRecordComponent(results.get(1), "name")); } } @@ -825,7 +844,9 @@ INSERT INTO users (id, name, active) assertEquals( "Carol", - executor.query( + executor.queryOne( + "UsersRepository", + "GetUserName", "SELECT name FROM users WHERE id = ?", List.of(3L), resultSet -> resultSet.getString("name") @@ -876,7 +897,9 @@ void shouldExecuteGeneratedUpdateWithOutOfOrderPlaceholders() throws Exception { assertEquals( "Alicia", - executor.query( + executor.queryOne( + "UsersRepository", + "GetUserName", "SELECT name FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getString("name") @@ -919,12 +942,14 @@ void shouldExecuteGeneratedDelete() throws Exception { assertEquals(1, affected); - assertNull( - executor.query( + assertTrue( + executor.queryOptional( + "UsersRepository", + "FindUserName", "SELECT name FROM users WHERE id = ?", List.of(2L), resultSet -> resultSet.getString("name") - ) + ).isEmpty() ); } } @@ -1759,7 +1784,7 @@ INSERT INTO users (id, name) assertTrue(repository.contains("Boolean active")); assertTrue(repository.contains("public UsersRow insertUser(Long id, String name)")); assertTrue(repository.contains("private static final RowMapper usersRowMapper")); - assertTrue(repository.contains("return executor.query(")); + assertTrue(repository.contains("return executor.queryOne(")); assertTrue(repository.contains("VALUES (?, ?)")); assertTrue(repository.contains("RETURNING *")); diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorNamingTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorNamingTest.java index a49bc02..748b2eb 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorNamingTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorNamingTest.java @@ -20,6 +20,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import java.util.Optional; import java.util.stream.Stream; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -253,11 +254,49 @@ void shouldExecuteExactSqlForTextBlockDelimiterAndTrailingWhitespace() throws Ex assertEquals(sql, executedSql(file, "listUsers")); } + /** + * A query name containing a quote and a backslash is passed to the runtime + * as its exact SQL spelling, so the generated identity literal escapes both + * characters. + */ + @Test + void shouldPassExactRepositoryAndQueryNameToTheExecutor() throws Exception { + GeneratedFile file = generate( + query( + "List\"Users\\", + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of() + ) + ); + + assertTrue(file.content().contains("\"UsersRepository\"")); + assertTrue(file.content().contains("\"List\\\"Users\\\\\"")); + + assertCompiles(file); + + RecordingQueryExecutor executor = execute(file, "listUsers"); + + assertEquals("UsersRepository", executor.repository); + assertEquals("List\"Users\\", executor.query); + } + /** * Compiles the generated class, runs its query method against a recording * executor, and returns the SQL the generated code passed to the runtime. */ private String executedSql(GeneratedFile file, String methodName, Object... arguments) throws Exception { + return execute(file, methodName, arguments).sql; + } + + /** + * Compiles the generated class, runs its query method against a recording + * executor, and returns that executor. + */ + private RecordingQueryExecutor execute( + GeneratedFile file, + String methodName, + Object... arguments + ) throws Exception { Path classesDirectory = tempDir.resolve("classes"); try ( @@ -283,34 +322,69 @@ private String executedSql(GeneratedFile file, String methodName, Object... argu method.invoke(instance, arguments); - return executor.sql; + return executor; } } private static final class RecordingQueryExecutor implements QueryExecutor { + private String repository; + + private String query; + private String sql; @Override - public T query(String sql, List parameters, RowMapper mapper) { - this.sql = sql; + public T queryOne( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + record(repository, query, sql); return null; } @Override - public List queryMany(String sql, List parameters, RowMapper mapper) { - this.sql = sql; + public Optional queryOptional( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + record(repository, query, sql); + + return Optional.empty(); + } + + @Override + public List queryMany( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + record(repository, query, sql); return List.of(); } @Override - public int execute(String sql, List parameters) { - this.sql = sql; + public int execute(String repository, String query, String sql, List parameters) { + record(repository, query, sql); return 0; } + + private void record(String repository, String query, String sql) { + this.repository = repository; + this.query = query; + this.sql = sql; + } } /** Generates the repository of a single-query group. */ diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorTest.java index 8f3405c..7b84b20 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/generator/JavaCodeGeneratorTest.java @@ -735,7 +735,7 @@ void shouldGenerateQueryExecutionMethodBody() { String source = generate(query).content(); - assertTrue(source.contains("return executor.query(")); + assertTrue(source.contains("return executor.queryOne(")); assertTrue(source.contains("java.util.Arrays.asList(id)")); assertTrue(source.contains("getUserRowMapper")); assertFalse(source.contains("UnsupportedOperationException")); @@ -1179,7 +1179,7 @@ void shouldGenerateExecutionForOneQuery() { assertTrue( source.contains(""" - return executor.query( + return executor.queryOne( """) ); @@ -1207,7 +1207,7 @@ void shouldGenerateExecutionForOneQuery() { assertTrue( source.contains(""" - return executor.query( + return executor.queryOne( """) ); @@ -1270,7 +1270,7 @@ void shouldGenerateQueryManyExecutionForManyQuery() { assertFalse( source.contains( - "return executor.query(" + "return executor.queryOne(" ) ); } @@ -1721,6 +1721,236 @@ void shouldShareOneRowRecordAcrossFullRowQueriesOfOneTable() throws IOException assertCompiles(file); } + @Test + void shouldGenerateOptionalExecutionForOptionalQuery() throws IOException { + QueryModel query = new QueryModel( + "FindUser", + QueryType.OPTIONAL, + "users", + """ + SELECT id, name + FROM users + WHERE id = ? + """, + List.of(1), + List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true) + ), + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + null + ); + + GeneratedFile file = generate(query); + + String source = file.content(); + + assertTrue(source.contains("import java.util.Optional;")); + assertTrue(source.contains("public record FindUserResult(")); + assertTrue( + source.contains("private static final RowMapper findUserRowMapper =") + ); + assertTrue(source.contains("public Optional findUser(Long id)")); + assertTrue(source.contains("return executor.queryOptional(")); + assertTrue(source.contains("java.util.Arrays.asList(id)")); + assertTrue(source.contains("Type: OPTIONAL")); + + assertFalse(source.contains("return executor.queryOne(")); + assertFalse(source.contains("UnsupportedOperationException")); + + assertCompiles(file); + } + + @Test + void shouldShareOneRowRecordBetweenOptionalAndOtherFullRowQueries() throws IOException { + List columns = List.of( + new QueryColumn("id", ColumnType.BIGINT, false), + new QueryColumn("name", ColumnType.VARCHAR, true) + ); + + QueryModel getUser = new QueryModel( + "GetUser", + QueryType.ONE, + "users", + "SELECT * FROM users WHERE id = ?", + List.of(1), + columns, + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + "users" + ); + + QueryModel findUser = new QueryModel( + "FindUser", + QueryType.OPTIONAL, + "users", + "SELECT * FROM users WHERE id = ?", + List.of(1), + columns, + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + "users" + ); + + GeneratedFile file = codeGenerator.generate( + new QueryGroupModel( + GROUP, + List.of(getUser, findUser) + ) + ); + + String source = file.content(); + + assertEquals( + 1, + source.lines() + .filter(line -> line.equals(" public record UsersRow(")) + .count() + ); + + assertEquals( + 1, + source.lines() + .filter(line -> line.contains("private static final RowMapper usersRowMapper")) + .count() + ); + + assertTrue(source.contains("public UsersRow getUser(Long id)")); + assertTrue(source.contains("public Optional findUser(Long id)")); + + assertCompiles(file); + } + + @Test + void shouldNotImportOptionalWithoutAnOptionalQuery() { + QueryModel getUser = new QueryModel( + "GetUser", + QueryType.ONE, + "users", + SQL, + List.of(), + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of(), + null + ); + + QueryModel listUsers = new QueryModel( + "ListUsers", + QueryType.MANY, + "users", + SQL, + List.of(), + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of(), + null + ); + + String source = codeGenerator + .generate( + new QueryGroupModel( + GROUP, + List.of(getUser, listUsers) + ) + ) + .content(); + + assertTrue(source.contains("import java.util.List;")); + assertFalse(source.contains("import java.util.Optional;")); + } + + /** + * Every executor call names the generated repository and the query it was + * generated from, so a runtime failure identifies the method the + * application called. + */ + @Test + void shouldPassRepositoryAndQueryIdentityToEveryExecutorCall() throws IOException { + QueryModel getUser = new QueryModel( + "GetUser", + QueryType.ONE, + "users", + "SELECT id FROM users WHERE id = ?", + List.of(1), + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + null + ); + + QueryModel findUser = new QueryModel( + "FindUser", + QueryType.OPTIONAL, + "users", + "SELECT id FROM users WHERE id = ?", + List.of(1), + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + null + ); + + QueryModel listUsers = new QueryModel( + "ListUsers", + QueryType.MANY, + "users", + "SELECT id FROM users", + List.of(), + List.of(new QueryColumn("id", ColumnType.BIGINT, false)), + List.of(), + null + ); + + QueryModel deleteUser = new QueryModel( + "DeleteUser", + QueryType.EXEC, + "users", + "DELETE FROM users WHERE id = ?", + List.of(1), + List.of(), + List.of(new QueryParameter(1, "id", ColumnType.BIGINT)), + null + ); + + GeneratedFile file = codeGenerator.generate( + new QueryGroupModel( + GROUP, + List.of(getUser, findUser, listUsers, deleteUser) + ) + ); + + String source = file.content(); + + assertTrue( + source.contains(""" + return executor.queryOne( + "UsersRepository", + "GetUser", + """) + ); + + assertTrue( + source.contains(""" + return executor.queryOptional( + "UsersRepository", + "FindUser", + """) + ); + + assertTrue( + source.contains(""" + return executor.queryMany( + "UsersRepository", + "ListUsers", + """) + ); + + assertTrue( + source.contains(""" + return executor.execute( + "UsersRepository", + "DeleteUser", + """) + ); + + assertCompiles(file); + } + @Test void shouldGenerateOneRowRecordPerRowTable() throws IOException { QueryModel getUser = new QueryModel( diff --git a/sqlcj-cli/src/test/java/dev/sqlcj/parser/DefaultQueryParserTest.java b/sqlcj-cli/src/test/java/dev/sqlcj/parser/DefaultQueryParserTest.java index a2473d9..b347a17 100644 --- a/sqlcj-cli/src/test/java/dev/sqlcj/parser/DefaultQueryParserTest.java +++ b/sqlcj-cli/src/test/java/dev/sqlcj/parser/DefaultQueryParserTest.java @@ -62,6 +62,22 @@ void parsesMultipleQueries() { assertEquals(QueryType.MANY, second.type()); } + @Test + void parsesOptionalQueryType() { + String source = """ + -- name: FindUser :optional + SELECT * + FROM users + WHERE id = $1; + """; + + List queries = parser.parse(source); + + assertEquals(1, queries.size()); + assertEquals("FindUser", queries.getFirst().name()); + assertEquals(QueryType.OPTIONAL, queries.getFirst().type()); + } + @Test void parsesExecQueryType() { String source = """ diff --git a/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/JdbcQueryExecutor.java b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/JdbcQueryExecutor.java index ae3a18c..3d0a1bf 100644 --- a/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/JdbcQueryExecutor.java +++ b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/JdbcQueryExecutor.java @@ -8,15 +8,16 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Optional; /** * Executes generated queries over JDBC. * *

An executor is constructed either with a {@link DataSource} or with a * caller-owned {@link Connection}. Both construction paths share the same - * positional parameter binding, row mapping, single-row and multi-row result - * handling, affected-row counting, and exception translation. Only connection - * ownership differs: + * positional parameter binding, row mapping, cardinality enforcement, + * affected-row counting, and exception translation. Only connection ownership + * differs: * *

    *
  • {@link #JdbcQueryExecutor(DataSource)} obtains one connection per @@ -31,15 +32,23 @@ * commit or rollback.
    • *
* + *

{@link #queryOne} reads the first row, maps it, and then advances the + * result set once more to prove that there is no second row. + * {@link #queryOptional} does the same but reports no row as + * {@link Optional#empty()}. A row count an annotation does not allow raises a + * {@link QueryCardinalityException} naming the query and its repository; the + * statement has already executed, so a returning write that fails the check has + * already changed the database. + * *

In both paths the {@link PreparedStatement} and any {@link ResultSet} - * opened for an operation are closed before that operation returns, on success - * and on failure. + * opened for an operation are closed before that operation returns, on success, + * on a cardinality failure, and on any other failure. * *

Every {@link SQLException} raised while acquiring a connection, preparing * a statement, binding parameters, executing, reading results, or closing a * DataSource-acquired connection is translated into a * {@link QueryExecutionException} with the message {@code "Failed to execute - * query"} and the {@code SQLException} as its cause. + * query '' in "} and the {@code SQLException} as its cause. * *

This executor is blocking and synchronous. An instance constructed with a * caller-owned connection inherits that connection's confinement to a single @@ -71,21 +80,82 @@ public JdbcQueryExecutor(Connection connection) { } @Override - public T query(String sql, List parameters, RowMapper mapper) { - return onStatement(sql, parameters, statement -> { + public T queryOne( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + return onStatement(repository, query, sql, parameters, statement -> { try (ResultSet resultSet = statement.executeQuery()) { if (!resultSet.next()) { - return null; + throw new QueryCardinalityException( + cardinalityMessage( + repository, + query, + "returned no row; expected exactly one" + ) + ); } - return mapper.map(resultSet); + T row = mapper.map(resultSet); + + if (resultSet.next()) { + throw new QueryCardinalityException( + cardinalityMessage( + repository, + query, + "returned more than one row; expected exactly one" + ) + ); + } + + return row; } }); } @Override - public List queryMany(String sql, List parameters, RowMapper mapper) { - return onStatement(sql, parameters, statement -> { + public Optional queryOptional( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + return onStatement(repository, query, sql, parameters, statement -> { + try (ResultSet resultSet = statement.executeQuery()) { + if (!resultSet.next()) { + return Optional.empty(); + } + + T row = mapper.map(resultSet); + + if (resultSet.next()) { + throw new QueryCardinalityException( + cardinalityMessage( + repository, + query, + "returned more than one row; expected at most one" + ) + ); + } + + return Optional.of(row); + } + }); + } + + @Override + public List queryMany( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ) { + return onStatement(repository, query, sql, parameters, statement -> { try (ResultSet resultSet = statement.executeQuery()) { List results = new ArrayList<>(); @@ -99,8 +169,8 @@ public List queryMany(String sql, List parameters, RowMapper mapper } @Override - public int execute(String sql, List parameters) { - return onStatement(sql, parameters, PreparedStatement::executeUpdate); + public int execute(String repository, String query, String sql, List parameters) { + return onStatement(repository, query, sql, parameters, PreparedStatement::executeUpdate); } /** @@ -109,19 +179,21 @@ public int execute(String sql, List parameters) { * the connection only when this executor acquired it. */ private T onStatement( + String repository, + String query, String sql, List parameters, StatementOperation operation ) { if (connection != null) { - return onConnection(connection, sql, parameters, operation); + return onConnection(connection, repository, query, sql, parameters, operation); } try (Connection acquired = Objects.requireNonNull(dataSource).getConnection()) { - return onConnection(acquired, sql, parameters, operation); + return onConnection(acquired, repository, query, sql, parameters, operation); } catch (SQLException e) { throw new QueryExecutionException( - "Failed to execute query", + failureMessage(repository, query), e ); } @@ -129,6 +201,8 @@ private T onStatement( private T onConnection( Connection target, + String repository, + String query, String sql, List parameters, StatementOperation operation @@ -139,7 +213,7 @@ private T onConnection( return operation.run(statement); } catch (SQLException e) { throw new QueryExecutionException( - "Failed to execute query", + failureMessage(repository, query), e ); } @@ -151,6 +225,14 @@ private void bindParameters(PreparedStatement statement, List parameters) thr } } + private static String failureMessage(String repository, String query) { + return "Failed to execute query '%s' in %s".formatted(query, repository); + } + + private static String cardinalityMessage(String repository, String query, String outcome) { + return "Query '%s' in %s %s".formatted(query, repository, outcome); + } + private interface StatementOperation { T run(PreparedStatement statement) throws SQLException; diff --git a/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryCardinalityException.java b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryCardinalityException.java new file mode 100644 index 0000000..106fc34 --- /dev/null +++ b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryCardinalityException.java @@ -0,0 +1,17 @@ +package dev.sqlcj.runtime; + +/** + * Thrown when a query returned a number of rows its annotation does not allow: + * a {@code :one} query that returned no row or more than one row, or a + * {@code :optional} query that returned more than one row. + * + *

The message names the query and the generated repository it belongs to. + * The statement itself executed successfully, so a returning write that fails + * this check has already changed the database. + */ +public final class QueryCardinalityException extends QueryExecutionException { + + public QueryCardinalityException(String message) { + super(message, null); + } +} diff --git a/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryExecutor.java b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryExecutor.java index 8004646..bf5aed8 100644 --- a/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryExecutor.java +++ b/sqlcj-runtime/src/main/java/dev/sqlcj/runtime/QueryExecutor.java @@ -1,15 +1,59 @@ package dev.sqlcj.runtime; import java.util.List; +import java.util.Optional; +/** + * Executes the SQL of one generated repository method. + * + *

Every operation is given the generated repository's class name and the + * name of the query it was generated from, so an execution or cardinality + * failure names the query the application called. + */ public interface QueryExecutor { - T query(String sql, List parameters, RowMapper mapper); + /** + * Executes a query that must return exactly one row and returns that row. + * + * @throws QueryCardinalityException if no row or more than one row is + * returned + */ + T queryOne( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ); - List queryMany(String sql, List parameters, RowMapper mapper); + /** + * Executes a query that may return at most one row and returns that row if + * there is one, or an empty {@link Optional} if there is none. + * + * @throws QueryCardinalityException if more than one row is returned + */ + Optional queryOptional( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ); + + /** + * Executes a query and returns every row it produced, in the order the + * database produced it, or an empty list if it produced none. + */ + List queryMany( + String repository, + String query, + String sql, + List parameters, + RowMapper mapper + ); /** * Executes a write statement and returns its affected-row count. */ - int execute(String sql, List parameters); + int execute(String repository, String query, String sql, List parameters); } diff --git a/sqlcj-runtime/src/test/java/dev/sqlcj/runtime/JdbcQueryExecutorTest.java b/sqlcj-runtime/src/test/java/dev/sqlcj/runtime/JdbcQueryExecutorTest.java index 34ab300..2873255 100644 --- a/sqlcj-runtime/src/test/java/dev/sqlcj/runtime/JdbcQueryExecutorTest.java +++ b/sqlcj-runtime/src/test/java/dev/sqlcj/runtime/JdbcQueryExecutorTest.java @@ -18,6 +18,7 @@ import java.time.LocalDateTime; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -29,6 +30,9 @@ class JdbcQueryExecutorTest { + /** The generated repository every operation of these tests reports. */ + private static final String REPOSITORY = "UsersRepository"; + private JdbcDataSource dataSource; private JdbcQueryExecutor executor; @@ -73,7 +77,9 @@ balance DECIMAL(10, 2) @Test void shouldBindSingleParameter() { - Long result = executor.query( + Long result = executor.queryOne( + REPOSITORY, + "GetUser", "SELECT id FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getLong("id") @@ -84,7 +90,9 @@ void shouldBindSingleParameter() { @Test void shouldBindParametersInOrder() { - String result = executor.query( + String result = executor.queryOne( + REPOSITORY, + "GetUser", """ SELECT name FROM users @@ -100,7 +108,9 @@ void shouldBindParametersInOrder() { @Test void shouldBindSupportedJavaTypes() { - String result = executor.query( + String result = executor.queryOne( + REPOSITORY, + "GetUser", """ SELECT name FROM users @@ -122,6 +132,8 @@ void shouldBindSupportedJavaTypes() { @Test void shouldReturnAllRowsForQueryMany() { List results = executor.queryMany( + REPOSITORY, + "ListUsers", """ SELECT id, name FROM users @@ -147,6 +159,8 @@ void shouldReturnAllRowsForQueryMany() { @Test void shouldReturnEmptyListWhenQueryManyFindsNoRows() { List results = executor.queryMany( + REPOSITORY, + "ListUsers", """ SELECT id, name FROM users @@ -163,26 +177,113 @@ void shouldReturnEmptyListWhenQueryManyFindsNoRows() { } @Test - void shouldReturnNullWhenQueryFindsNoRows() { - User result = executor.query( - """ - SELECT id, name - FROM users - WHERE id = ? - """, + void shouldReturnTheRowWhenQueryOneFindsExactlyOneRow() { + User result = executor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id, name FROM users WHERE id = ?", + List.of(1L), + userMapper() + ); + + assertEquals(new User(1L, "Alice"), result); + } + + @Test + void shouldFailWhenQueryOneFindsNoRow() { + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> executor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id, name FROM users WHERE id = ?", + List.of(999L), + userMapper() + ) + ); + + assertEquals( + "Query 'GetUser' in UsersRepository returned no row; expected exactly one", + exception.getMessage() + ); + + assertNull(exception.getCause()); + assertInstanceOf(QueryExecutionException.class, exception); + } + + @Test + void shouldFailWhenQueryOneFindsMoreThanOneRow() { + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> executor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id, name FROM users WHERE active = ? ORDER BY id", + List.of(true), + userMapper() + ) + ); + + assertEquals( + "Query 'GetUser' in UsersRepository returned more than one row; expected exactly one", + exception.getMessage() + ); + + assertNull(exception.getCause()); + } + + @Test + void shouldReturnTheRowWhenQueryOptionalFindsOneRow() { + Optional result = executor.queryOptional( + REPOSITORY, + "FindUser", + "SELECT id, name FROM users WHERE id = ?", + List.of(2L), + userMapper() + ); + + assertEquals(Optional.of(new User(2L, "Bob")), result); + } + + @Test + void shouldReturnEmptyOptionalWhenQueryOptionalFindsNoRow() { + Optional result = executor.queryOptional( + REPOSITORY, + "FindUser", + "SELECT id, name FROM users WHERE id = ?", List.of(999L), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") + userMapper() + ); + + assertTrue(result.isEmpty()); + } + + @Test + void shouldFailWhenQueryOptionalFindsMoreThanOneRow() { + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> executor.queryOptional( + REPOSITORY, + "FindUser", + "SELECT id, name FROM users WHERE active = ? ORDER BY id", + List.of(true), + userMapper() ) ); - assertNull(result); + assertEquals( + "Query 'FindUser' in UsersRepository returned more than one row; expected at most one", + exception.getMessage() + ); + + assertNull(exception.getCause()); } @Test void shouldReturnAffectedRowCountForExecute() { int affected = executor.execute( + REPOSITORY, + "UpdateUserName", """ UPDATE users SET name = ? @@ -193,7 +294,9 @@ void shouldReturnAffectedRowCountForExecute() { assertEquals(1, affected); - String name = executor.query( + String name = executor.queryOne( + REPOSITORY, + "GetUserName", "SELECT name FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getString("name") @@ -207,12 +310,18 @@ void shouldWrapSqlExceptionForExecute() { QueryExecutionException exception = assertThrows( QueryExecutionException.class, () -> executor.execute( + REPOSITORY, + "DeleteUsers", "DELETE FROM missing_table", List.of() ) ); - assertEquals("Failed to execute query", exception.getMessage()); + assertEquals( + "Failed to execute query 'DeleteUsers' in UsersRepository", + exception.getMessage() + ); + assertInstanceOf(SQLException.class, exception.getCause()); } @@ -220,14 +329,20 @@ void shouldWrapSqlExceptionForExecute() { void shouldWrapSqlException() { QueryExecutionException exception = assertThrows( QueryExecutionException.class, - () -> executor.query( + () -> executor.queryOne( + REPOSITORY, + "GetUser", "SELECT * FROM missing_table", List.of(), resultSet -> resultSet.getLong("id") ) ); - assertEquals("Failed to execute query", exception.getMessage()); + assertEquals( + "Failed to execute query 'GetUser' in UsersRepository", + exception.getMessage() + ); + assertInstanceOf(SQLException.class, exception.getCause()); } @@ -236,7 +351,9 @@ void shouldQueryThroughCallerOwnedConnection() throws SQLException { try (Connection connection = dataSource.getConnection()) { JdbcQueryExecutor connectionExecutor = new JdbcQueryExecutor(connection); - User result = connectionExecutor.query( + User result = connectionExecutor.queryOne( + REPOSITORY, + "GetUser", """ SELECT id, name FROM users @@ -244,24 +361,38 @@ void shouldQueryThroughCallerOwnedConnection() throws SQLException { AND active = ? """, List.of(1L, true), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() ); assertEquals(new User(1L, "Alice"), result); - assertNull( - connectionExecutor.query( + assertTrue( + connectionExecutor.queryOptional( + REPOSITORY, + "FindUser", "SELECT id, name FROM users WHERE id = ?", List.of(999L), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() + ).isEmpty() + ); + + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> connectionExecutor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id, name FROM users WHERE id = ?", + List.of(999L), + userMapper() ) ); + + assertEquals( + "Query 'GetUser' in UsersRepository returned no row; expected exactly one", + exception.getMessage() + ); + + assertFalse(connection.isClosed()); } } @@ -271,6 +402,8 @@ void shouldQueryManyThroughCallerOwnedConnection() throws SQLException { JdbcQueryExecutor connectionExecutor = new JdbcQueryExecutor(connection); List results = connectionExecutor.queryMany( + REPOSITORY, + "ListUsers", """ SELECT id, name FROM users @@ -278,10 +411,7 @@ void shouldQueryManyThroughCallerOwnedConnection() throws SQLException { ORDER BY id """, List.of(true), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() ); assertEquals( @@ -294,12 +424,11 @@ void shouldQueryManyThroughCallerOwnedConnection() throws SQLException { assertTrue( connectionExecutor.queryMany( + REPOSITORY, + "ListUsers", "SELECT id, name FROM users WHERE id = ?", List.of(999L), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() ).isEmpty() ); } @@ -311,6 +440,8 @@ void shouldReturnAffectedRowCountForExecuteThroughCallerOwnedConnection() throws JdbcQueryExecutor connectionExecutor = new JdbcQueryExecutor(connection); int affected = connectionExecutor.execute( + REPOSITORY, + "RenameActiveUsers", """ UPDATE users SET name = ? @@ -323,7 +454,9 @@ void shouldReturnAffectedRowCountForExecuteThroughCallerOwnedConnection() throws assertEquals( "Renamed", - connectionExecutor.query( + connectionExecutor.queryOne( + REPOSITORY, + "GetUserName", "SELECT name FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getString("name") @@ -348,11 +481,15 @@ void shouldLeaveCallerOwnedConnectionOpenAndItsTransactionStateUnchanged() throw JdbcQueryExecutor connectionExecutor = new JdbcQueryExecutor(connection); connectionExecutor.execute( + REPOSITORY, + "InsertUser", "INSERT INTO users (id, name, active) VALUES (?, ?, ?)", List.of(4L, "Dora", true) ); connectionExecutor.execute( + REPOSITORY, + "UpdateUserName", "UPDATE users SET name = ? WHERE id = ?", List.of("Alicia", 1L) ); @@ -363,6 +500,8 @@ void shouldLeaveCallerOwnedConnectionOpenAndItsTransactionStateUnchanged() throw new User(4L, "Dora") ), connectionExecutor.queryMany( + REPOSITORY, + "ListUsers", """ SELECT id, name FROM users @@ -370,10 +509,7 @@ WHERE id IN (?, ?) ORDER BY id """, List.of(1L, 4L), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() ) ); @@ -386,6 +522,8 @@ WHERE id IN (?, ?) assertEquals( List.of(new User(1L, "Alice")), connectionExecutor.queryMany( + REPOSITORY, + "ListUsers", """ SELECT id, name FROM users @@ -393,10 +531,7 @@ WHERE id IN (?, ?) ORDER BY id """, List.of(1L, 4L), - resultSet -> new User( - resultSet.getLong("id"), - resultSet.getString("name") - ) + userMapper() ) ); } @@ -411,25 +546,39 @@ void shouldCloseStatementsAndResultSetsOfCallerOwnedConnection() throws SQLExcep tracker.track(Connection.class, connection) ); - connectionExecutor.query( + connectionExecutor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id FROM users WHERE id = ?", + List.of(1L), + resultSet -> resultSet.getLong("id") + ); + + connectionExecutor.queryOptional( + REPOSITORY, + "FindUser", "SELECT id FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getLong("id") ); connectionExecutor.queryMany( + REPOSITORY, + "ListUsers", "SELECT id FROM users", List.of(), resultSet -> resultSet.getLong("id") ); connectionExecutor.execute( + REPOSITORY, + "UpdateUserName", "UPDATE users SET name = ? WHERE id = ?", List.of("Alicia", 1L) ); - assertEquals(3, tracker.statements.size()); - assertEquals(2, tracker.resultSets.size()); + assertEquals(4, tracker.statements.size()); + assertEquals(3, tracker.resultSets.size()); tracker.assertAllStatementsAndResultSetsClosed(); assertFalse(connection.isClosed()); @@ -447,14 +596,20 @@ void shouldCloseStatementWhenExecutionFailsOnCallerOwnedConnection() throws SQLE QueryExecutionException exception = assertThrows( QueryExecutionException.class, - () -> connectionExecutor.query( + () -> connectionExecutor.queryOne( + REPOSITORY, + "GetUser", "SELECT id FROM users WHERE id = ?", List.of(), resultSet -> resultSet.getLong("id") ) ); - assertEquals("Failed to execute query", exception.getMessage()); + assertEquals( + "Failed to execute query 'GetUser' in UsersRepository", + exception.getMessage() + ); + assertInstanceOf(SQLException.class, exception.getCause()); assertEquals(1, tracker.statements.size()); @@ -464,6 +619,44 @@ void shouldCloseStatementWhenExecutionFailsOnCallerOwnedConnection() throws SQLE } } + /** + * A cardinality failure is raised after the statement has executed, so the + * statement and its result set are still closed and the caller-owned + * connection is left open. + */ + @Test + void shouldCloseStatementWhenCardinalityFailsOnCallerOwnedConnection() throws SQLException { + JdbcResourceTracker tracker = new JdbcResourceTracker(); + + try (Connection connection = dataSource.getConnection()) { + JdbcQueryExecutor connectionExecutor = new JdbcQueryExecutor( + tracker.track(Connection.class, connection) + ); + + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> connectionExecutor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id FROM users WHERE active = ?", + List.of(true), + resultSet -> resultSet.getLong("id") + ) + ); + + assertEquals( + "Query 'GetUser' in UsersRepository returned more than one row; expected exactly one", + exception.getMessage() + ); + + assertEquals(1, tracker.statements.size()); + assertEquals(1, tracker.resultSets.size()); + tracker.assertAllStatementsAndResultSetsClosed(); + + assertFalse(connection.isClosed()); + } + } + @Test void shouldWrapSqlExceptionForCallerOwnedConnection() throws SQLException { try (Connection connection = dataSource.getConnection()) { @@ -471,25 +664,37 @@ void shouldWrapSqlExceptionForCallerOwnedConnection() throws SQLException { QueryExecutionException queryException = assertThrows( QueryExecutionException.class, - () -> connectionExecutor.query( + () -> connectionExecutor.queryOne( + REPOSITORY, + "GetUser", "SELECT * FROM missing_table", List.of(), resultSet -> resultSet.getLong("id") ) ); - assertEquals("Failed to execute query", queryException.getMessage()); + assertEquals( + "Failed to execute query 'GetUser' in UsersRepository", + queryException.getMessage() + ); + assertInstanceOf(SQLException.class, queryException.getCause()); QueryExecutionException executeException = assertThrows( QueryExecutionException.class, () -> connectionExecutor.execute( + REPOSITORY, + "DeleteUsers", "DELETE FROM missing_table", List.of() ) ); - assertEquals("Failed to execute query", executeException.getMessage()); + assertEquals( + "Failed to execute query 'DeleteUsers' in UsersRepository", + executeException.getMessage() + ); + assertInstanceOf(SQLException.class, executeException.getCause()); assertFalse(connection.isClosed()); @@ -504,26 +709,40 @@ void shouldCloseAcquiredConnectionForEachDataSourceOperation() throws SQLExcepti tracker.track(DataSource.class, dataSource) ); - dataSourceExecutor.query( + dataSourceExecutor.queryOne( + REPOSITORY, + "GetUser", + "SELECT id FROM users WHERE id = ?", + List.of(1L), + resultSet -> resultSet.getLong("id") + ); + + dataSourceExecutor.queryOptional( + REPOSITORY, + "FindUser", "SELECT id FROM users WHERE id = ?", List.of(1L), resultSet -> resultSet.getLong("id") ); dataSourceExecutor.queryMany( + REPOSITORY, + "ListUsers", "SELECT id FROM users", List.of(), resultSet -> resultSet.getLong("id") ); dataSourceExecutor.execute( + REPOSITORY, + "UpdateUserName", "UPDATE users SET name = ? WHERE id = ?", List.of("Alicia", 1L) ); - assertEquals(3, tracker.connections.size()); - assertEquals(3, tracker.statements.size()); - assertEquals(2, tracker.resultSets.size()); + assertEquals(4, tracker.connections.size()); + assertEquals(4, tracker.statements.size()); + assertEquals(3, tracker.resultSets.size()); tracker.assertAllConnectionsClosed(); tracker.assertAllStatementsAndResultSetsClosed(); @@ -539,14 +758,20 @@ void shouldCloseAcquiredConnectionWhenDataSourceOperationFails() throws SQLExcep QueryExecutionException exception = assertThrows( QueryExecutionException.class, - () -> dataSourceExecutor.query( + () -> dataSourceExecutor.queryOne( + REPOSITORY, + "GetUser", "SELECT id FROM users WHERE id = ?", List.of(), resultSet -> resultSet.getLong("id") ) ); - assertEquals("Failed to execute query", exception.getMessage()); + assertEquals( + "Failed to execute query 'GetUser' in UsersRepository", + exception.getMessage() + ); + assertInstanceOf(SQLException.class, exception.getCause()); assertEquals(1, tracker.connections.size()); @@ -556,6 +781,49 @@ void shouldCloseAcquiredConnectionWhenDataSourceOperationFails() throws SQLExcep tracker.assertAllStatementsAndResultSetsClosed(); } + /** + * A cardinality failure on the DataSource path closes the acquired + * connection together with the statement and its result set. + */ + @Test + void shouldCloseAcquiredConnectionWhenCardinalityFails() throws SQLException { + JdbcResourceTracker tracker = new JdbcResourceTracker(); + + JdbcQueryExecutor dataSourceExecutor = new JdbcQueryExecutor( + tracker.track(DataSource.class, dataSource) + ); + + QueryCardinalityException exception = assertThrows( + QueryCardinalityException.class, + () -> dataSourceExecutor.queryOptional( + REPOSITORY, + "FindUser", + "SELECT id FROM users WHERE active = ?", + List.of(true), + resultSet -> resultSet.getLong("id") + ) + ); + + assertEquals( + "Query 'FindUser' in UsersRepository returned more than one row; expected at most one", + exception.getMessage() + ); + + assertEquals(1, tracker.connections.size()); + assertEquals(1, tracker.statements.size()); + assertEquals(1, tracker.resultSets.size()); + + tracker.assertAllConnectionsClosed(); + tracker.assertAllStatementsAndResultSetsClosed(); + } + + private RowMapper userMapper() { + return resultSet -> new User( + resultSet.getLong("id"), + resultSet.getString("name") + ); + } + private record User( long id, String name