diff --git a/AGENTS.md b/AGENTS.md index 890d15b..07b3f86 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -78,6 +78,14 @@ CI will fail if files contain trailing whitespace, missing newline terminations, mise exec -- ./mvnw test ``` +### Property-Based Tests + +The unit lanes use jetCheck for the extracted pure seams and the connection transaction lifecycle. +A failure prints the minimized value and a `rechecking(...)` token. Use the token while debugging, +then add a named regression test for the minimized case; do not commit a fixed seed. See +[`docs/property-based-testing.md`](docs/property-based-testing.md) for the suite list, conventions, +and the xerial and Appendix B follow-ups. + ### Running Integration & Toxicity Tests Requires a running Docker or Podman daemon: ```bash diff --git a/docs/property-based-testing.md b/docs/property-based-testing.md new file mode 100644 index 0000000..7ee1a09 --- /dev/null +++ b/docs/property-based-testing.md @@ -0,0 +1,183 @@ +# Property-based testing + +The unit lanes use [jetCheck](https://github.com/JetBrains/jetCheck) for property-based tests. The +first set of properties is a pilot: it covers the seams the recent refactors extracted, and it stays +in the unit lane. The Appendix B profile and the xerial conformance suite remain the oracles for +type mapping and driver parity. + +## Library + +The dependency is pinned in the root `pom.xml` and declared test-scoped in `libsql-jdbc` and +`libsql-hrana-codec`: + +```xml + + org.jetbrains + jetCheck + 0.3.0 + test + +``` + +jetCheck is independent of the test framework, so the suites are ordinary JUnit 6 `@Test` methods. +It brings `org.jetbrains:annotations:13.0` onto the test classpath. It never enters a runtime or +shaded artifact. + +## Suites + +| Suite | Module | Properties | Iterations | +|---|---|---|---| +| `SqlScriptPropertyTest` | `libsql-jdbc` | statement conservation, never throws, no blank statements, split fixpoint, placeholder counting | 500 | +| `SqliteValueTextPropertyTest` | `libsql-jdbc` | REAL, LONG, DOUBLE and ISO date-time text round trips | 1 000 | +| `LibsqlUrlParserPropertyTest` | `libsql-jdbc` | only `SQLException` escapes parsing, metadata redaction idempotence | 500 | +| `SqlMetadataParsingPropertyTest` | `libsql-jdbc` | declared-type parsing and AUTOINCREMENT detection | 500 | +| `ConnectionLifecyclePropertyTest` | `libsql-jdbc` | model-based transaction and savepoint state machine using `ImperativeCommand` | 1 000 scenarios, up to 50 commands | +| `HranaJsonCodecPropertyTest` | `libsql-hrana-codec` | both codecs agree on encode and decode, 64-bit integers, base64 blobs | 1 000 | + +The suites add well under a second to the unit lane. They do not start a container. + +The statement splitter was the first target because a wrong split silently changes migration +behavior. The generated scripts combine text literals, quoted identifiers, line and block comments, +and trigger bodies that contain inner semicolons. The properties assert that: + +- `split` returns exactly the statements that were joined, in order; +- arbitrary text never makes `split` throw; +- every returned statement is non-blank; +- splitting any returned statement again returns that same statement; +- `?` inside quoted text, bracket identifiers, blob literals and comments is not a placeholder. + +The lifecycle suite is a small model, not a replay of named examples. The model tracks `closed`, +`autoCommit`, whether a transaction is active, whether a savepoint started it, and the live savepoint +names. jetCheck generates and shrinks the command sequence, and prints the command log when a +scenario fails. The fake client never rejects a statement, so a failure is a driver state-machine +error rather than a server error. + +## Running and reproducing + +Run the normal unit lane: + +```bash +mise exec -- ./mvnw test +``` + +Run one suite: + +```bash +mise exec -- ./mvnw -pl libsql-jdbc -Dtest=SqlScriptPropertyTest test +mise exec -- ./mvnw -pl libsql-hrana-codec -Dtest=HranaJsonCodecPropertyTest test +``` + +A failure is a `PropertyFalsified` exception. It reports the minimized value, the shrink statistics, +and a `rechecking(...)` token: + +```text +Falsified on DECIMAL(a) +Shrunk in 5 stages, by trying 5 examples + +To re-run the minimal failing case, run + PropertyChecker.customized().rechecking("6riN/RX6zqn4DQEBYQ==") + .forAll(...) +``` + +Use the token while debugging, then add a named regression test for the minimized value. Do not +commit `withSeed` calls: jetCheck deprecates them on purpose, because a seed becomes obsolete when +the test or the framework changes. For CI, run random seeds and keep the printed token in the +failure report. + +The iteration counts are per-suite constants so the unit-lane cost stays visible. A container-lane +property should use a much smaller count, since startup dominates. + +## Defects found + +### A non-numeric timeout escaped as `NumberFormatException` + +`LibsqlUrlParser.parse` is a JDBC boundary. Any input it accepts should produce a config or a +`SQLException`. The property generated a valid URL with an arbitrary `connectTimeout` value and +falsified on an empty value and on overflowing text. `resolveDuration` now wraps the parse failure in +`SQLException` with the parameter name. A named regression test covers both the URL and the +`Properties` path. + +A related gap remains: a negative or zero timeout still parses, but +`HttpClient.newBuilder().connectTimeout(...)` rejects it later. That is worth a follow-up property +and a decision on whether the URL parser or the transport should own validation. + +### Repeated user info was not redacted completely + +`metadataUrlIsIdempotent` falsified on: + +```text +jdbc:libsql:http://user:pass@user:pass@ +``` + +The old user-info pattern removed only the first `user:pass@` segment, so a second redaction +removed the next one and the first result still carried a credential. The pattern now consumes +through the last `@` before the path, query or fragment. Named tests cover repeated user info and an +`@` in the query string. + +## Comparison with QuickTheories + +A second implementation of the same suites used QuickTheories 0.26. Both libraries found the timeout +defect; jetCheck also found the repeated-user-info case. The comparison informed the choice of +jetCheck: + +| Dimension | QuickTheories 0.26 | jetCheck 0.3.0 | +|---|---|---| +| Last release | 2019 | 2026 | +| Stateful testing | No first-class support; generate a list and replay it yourself. | `ImperativeCommand` + `checkScenarios`, with command logs and sequence shrinking. | +| Shrinking | Automatic; prints several falsifying values and the seed. | Automatic; re-runs the minimal example and prints a `rechecking(...)` token. | +| Reproducibility | `QT_SEED`, `.withFixedSeed`, reported seed. | `rechecking` token and iteration seed; `withSeed` is deliberately deprecated. | +| Generators | `longs`, arrays, big integers, big decimals, dates, strings over all code points. | No `long` or array generator; char-based strings, so supplementary code points need a custom generator. Has `recursive`, `suchThat`, `noShrink`. | +| Assertions | `checkAssert(Consumer)` is a natural AssertJ fit. | Predicate API; assertions work but need `return true`. | +| JUnit integration | Failures are `AssertionError`, reported as failures. | Failures are `PropertyFalsified` (a `RuntimeException`), reported as errors. | +| Dependencies | Provided `jsr305`; no transitive runtime dependency. | Test-scope `org.jetbrains:annotations:13.0`. | + +The stateful support is the decisive difference for this repository. The connection, transaction +and savepoint work has more value in generated command sequences than in more primitive generators. + +## Conventions and limits + +- Property tests complement the named examples. They do not replace the Appendix B profile or the + xerial conformance suite. +- Use AssertJ inside the property and finish with `return true`. The assertion failure is caught and + shrunk by jetCheck. +- Keep generators described and deterministic. Do not depend on outside state. +- Avoid `suchThat` over sparse domains. Build the constrained generator instead. +- jetCheck's string generator is `char`-based. For arbitrary Unicode, generate code points and build + the string with `appendCodePoint`; do not let automatic shrinking split a surrogate pair. The codec + suite uses BMP ranges plus fixed supplementary constants. +- The two codecs produce equivalent, not byte-identical, JSON: the direct writer emits supplementary + characters as raw UTF-8, while Jackson escapes them as a surrogate pair. Compare a canonical token + stream, not bytes. +- `PropertyFalsified` is a `RuntimeException`. CI fails either way, but reports say "error" rather + than "failure". Decide once whether to wrap it in `AssertionError` or keep the jetCheck type. +- Keep container-lane iteration counts low. The unit suites are cheap; container startup is not. + +## Follow-ups + +1. **Xerial differential oracle.** Add a low-iteration property in the container lane. The test needs + both drivers on the classpath and a temporary SQLite file for xerial. For each generated value or + statement, run it through both drivers and compare column metadata, Java value classes, values, + and error types and messages. Appendix B wins over xerial, so the property needs a quarantine + list shaped like `XerialQuarantine`, keyed by property and generated case, with a reason. +2. **Appendix B matrix.** `JdbcValueConverter` is a pure source-type by target-type table. Encode the + table from `vendor/JDBC4-TYPE-MAPPING.md`, generate `(value, target type)` pairs, and assert + either the table's result or `SQLFeatureNotSupportedException`. This belongs in the unit lane and + is the highest-value next step after the differential oracle. +3. **Lifecycle failure injection.** Add commands for a stale baton, an expired stream, and a server + error on `BEGIN`, `COMMIT` or `ROLLBACK TO`. jetCheck's command log is the reason to model these + as `ImperativeCommand`s. +4. **Shared generator helpers.** If more suites need 64-bit values or code-point strings, move the + small helpers into a test utility rather than copying them. + +## Files + +- `pom.xml`, `libsql-jdbc/pom.xml`, `libsql-hrana-codec/pom.xml` — dependency management and test + scope. +- `libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlScriptPropertyTest.java` +- `libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqliteValueTextPropertyTest.java` +- `libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserPropertyTest.java` +- `libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlMetadataParsingPropertyTest.java` +- `libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/ConnectionLifecyclePropertyTest.java` +- `libsql-hrana-codec/src/test/java/io/github/magnusp/libsql/hrana/codec/HranaJsonCodecPropertyTest.java` +- `libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParser.java` — the two fixes the + properties found. diff --git a/libsql-hrana-codec/pom.xml b/libsql-hrana-codec/pom.xml index d40607b..bb6512a 100644 --- a/libsql-hrana-codec/pom.xml +++ b/libsql-hrana-codec/pom.xml @@ -38,6 +38,11 @@ assertj-core test + + org.jetbrains + jetCheck + test + diff --git a/libsql-hrana-codec/src/test/java/io/github/magnusp/libsql/hrana/codec/HranaJsonCodecPropertyTest.java b/libsql-hrana-codec/src/test/java/io/github/magnusp/libsql/hrana/codec/HranaJsonCodecPropertyTest.java new file mode 100644 index 0000000..4ac1d30 --- /dev/null +++ b/libsql-hrana-codec/src/test/java/io/github/magnusp/libsql/hrana/codec/HranaJsonCodecPropertyTest.java @@ -0,0 +1,307 @@ +package io.github.magnusp.libsql.hrana.codec; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.HexFormat; +import java.util.List; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import org.jetbrains.jetCheck.Generator; +import org.jetbrains.jetCheck.IntDistribution; +import org.jetbrains.jetCheck.PropertyChecker; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Differential properties for the two Hrana JSON implementations. + * + *

Both codecs speak the same protocol. The property is that a generated value produces the same + * JSON when either codec encodes it, and that both codecs read that JSON back to the same value. + * The generated domain includes the two precision guarantees from the module guide: every 64-bit + * integer is sent as a decimal string, and every blob is sent as base64. + */ +class HranaJsonCodecPropertyTest { + + private static final int ITERATIONS = 1_000; + + private final DirectHranaJsonCodec direct = new DirectHranaJsonCodec(); + private final JacksonHranaJsonCodec jackson = new JacksonHranaJsonCodec(); + + @Test + void bothCodecsEncodeTheSameValue() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(describedValues(), described -> { + String directJson = canonicalUnchecked(encodeUnchecked(direct, described.value())); + String jacksonJson = canonicalUnchecked(encodeUnchecked(jackson, described.value())); + if (!directJson.equals(jacksonJson)) { + throw new AssertionError( + "direct=" + escape(directJson) + " jackson=" + escape(jacksonJson)); + } + return true; + }); + } + + @Test + void bothCodecsDecodeTheSameResponse() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(describedValues(), described -> { + byte[] json = responseUnchecked(described.value()); + PipelineRespBody directResponse = decodeUnchecked(direct, json); + PipelineRespBody jacksonResponse = decodeUnchecked(jackson, json); + + assertThat(jacksonResponse).isEqualTo(directResponse); + assertThat(firstValue(directResponse)).isEqualTo(described.value()); + return true; + }); + } + + @Test + void integersKeepAllSixtyFourBitsInText() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(longs(), value -> { + String expected = "\"type\":\"integer\",\"value\":\"" + value + "\""; + assertThat(text(encodeUnchecked(direct, Value.of(value)))).contains(expected); + assertThat(text(encodeUnchecked(jackson, Value.of(value)))).contains(expected); + return true; + }); + } + + @Test + void blobsKeepTheirBytesInBase64() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(describedBlobs(), blob -> { + String expected = "\"type\":\"blob\",\"base64\":\"" + + Base64.getEncoder().encodeToString(blob.bytes()) + "\""; + assertThat(text(encodeUnchecked(direct, Value.of(blob.bytes())))).contains(expected); + assertThat(text(encodeUnchecked(jackson, Value.of(blob.bytes())))).contains(expected); + return true; + }); + } + + private record DescribedValue(String description, Value value) { + @Override + public String toString() { + return description; + } + } + + private record DescribedBlob(byte[] bytes) { + @Override + public String toString() { + return "blob " + HexFormat.of().formatHex(bytes); + } + } + + private static Generator describedValues() { + return values().map(value -> new DescribedValue(describe(value), value)); + } + + private static Generator values() { + return Generator.anyOf( + Generator.constant(Value.ofNull()), + longs().map(value -> Value.of(value.longValue())), + finiteDoubles().map(value -> Value.of(value.doubleValue())), + texts().map(Value::of), + describedBlobs().map(blob -> Value.of(blob.bytes()))); + } + + /** jetCheck has no built-in 64-bit generator, so compose one from two 32-bit draws. */ + private static Generator longs() { + return Generator.zipWith( + Generator.integers(), + Generator.integers(), + (high, low) -> ((long) high << 32) | (low & 0xffffffffL)); + } + + private static Generator finiteDoubles() { + return Generator.doubles().suchThat(Double::isFinite); + } + + private static Generator texts() { + return Generator.anyOf( + Generator.stringsOf( + IntDistribution.uniform(0, 40), Generator.charsInRange((char) 0, (char) 127)), + // Exclude the surrogate range: jetCheck shrinks strings at char boundaries, which + // can split a supplementary pair into a lone surrogate that is not well-formed + // Unicode. Supplementary text is covered by the constants below. + Generator.stringsOf( + IntDistribution.uniform(0, 20), Generator.charsInRange((char) 0x20, (char) 0xD7FF)), + Generator.stringsOf( + IntDistribution.uniform(0, 20), Generator.charsInRange((char) 0xE000, (char) 0xFFFF)), + Generator.constant("\u0000quoted \" text\n\t"), + Generator.constant("\uD840\uDC40 supplementary"), + Generator.constant("emoji \uD83D\uDE00")); + } + + private static Generator describedBlobs() { + return Generator.listsOf(IntDistribution.uniform(0, 32), Generator.integers(0, 255)) + .map(HranaJsonCodecPropertyTest::toBytes) + .map(DescribedBlob::new); + } + + private static byte[] toBytes(List values) { + byte[] bytes = new byte[values.size()]; + for (int i = 0; i < values.size(); i++) { + bytes[i] = (byte) (int) values.get(i); + } + return bytes; + } + + private static String describe(Value value) { + return switch (value) { + case Value.Null n -> "null"; + case Value.Integer i -> "integer " + i.value(); + case Value.Float f -> "float " + f.value(); + case Value.Text t -> "text \"" + escape(t.value()) + "\""; + case Value.Blob b -> "blob " + HexFormat.of().formatHex(b.bytes()); + }; + } + + private static String escape(String text) { + StringBuilder escaped = new StringBuilder(text.length()); + for (char c : text.toCharArray()) { + if (c >= 0x20 && c < 0x7f && c != '\\') { + escaped.append(c); + } else { + escaped.append(String.format("\\u%04x", (int) c)); + } + } + return escaped.toString(); + } + + private static PipelineRespBody decodeUnchecked(HranaJsonCodec codec, byte[] json) { + try { + return codec.deserializePipelineResponse(json); + } catch (IOException e) { + throw new AssertionError("could not decode " + text(json), e); + } + } + + private static byte[] encodeUnchecked(HranaJsonCodec codec, Value value) { + try { + return encode(codec, value); + } catch (IOException e) { + throw new AssertionError("could not encode " + value, e); + } + } + + private static String canonicalUnchecked(byte[] json) { + try { + return canonical(json); + } catch (IOException e) { + throw new AssertionError("could not read back " + text(json), e); + } + } + + private static byte[] responseUnchecked(Value value) { + try { + return response(value); + } catch (IOException e) { + throw new AssertionError("could not write a response for " + value, e); + } + } + + private static byte[] encode(HranaJsonCodec codec, Value value) throws IOException { + PipelineReqBody request = new PipelineReqBody( + null, List.of(new StreamRequest.Execute(new Stmt("select ?", List.of(value))))); + return codec.serializePipelineRequest(request); + } + + private static String text(byte[] json) { + return new String(json, StandardCharsets.UTF_8); + } + + /** + * Reduces JSON to a token stream, so formatting and string-escaping differences between the + * two writers do not appear as protocol differences. Field order still matters. + */ + private static String canonical(byte[] json) throws IOException { + StringBuilder canonical = new StringBuilder(); + try (JsonParser parser = new JsonFactory().createParser(json)) { + JsonToken token; + while ((token = parser.nextToken()) != null) { + canonical.append(token.name()).append('\u0000'); + if (token == JsonToken.FIELD_NAME + || token == JsonToken.VALUE_STRING + || token == JsonToken.VALUE_NUMBER_INT + || token == JsonToken.VALUE_NUMBER_FLOAT) { + canonical.append(parser.getText()); + } + canonical.append('\n'); + } + } + return canonical.toString(); + } + + private static byte[] response(Value value) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (JsonGenerator json = new JsonFactory().createGenerator(out)) { + json.writeStartObject(); + json.writeArrayFieldStart("results"); + json.writeStartObject(); + json.writeStringField("type", "ok"); + json.writeObjectFieldStart("response"); + json.writeStringField("type", "execute"); + json.writeObjectFieldStart("result"); + json.writeArrayFieldStart("cols"); + json.writeStartObject(); + json.writeStringField("name", "v"); + json.writeStringField("decltype", "TEXT"); + json.writeEndObject(); + json.writeEndArray(); + json.writeArrayFieldStart("rows"); + json.writeStartArray(); + writeValue(json, value); + json.writeEndArray(); + json.writeEndArray(); + json.writeNumberField("affected_row_count", 0); + json.writeStringField("last_insert_rowid", "0"); + json.writeEndObject(); + json.writeEndObject(); + json.writeEndObject(); + json.writeEndArray(); + json.writeEndObject(); + } + return out.toByteArray(); + } + + private static void writeValue(JsonGenerator json, Value value) throws IOException { + json.writeStartObject(); + switch (value) { + case Value.Null n -> json.writeStringField("type", "null"); + case Value.Integer i -> { + json.writeStringField("type", "integer"); + json.writeStringField("value", Long.toString(i.value())); + } + case Value.Float f -> { + json.writeStringField("type", "float"); + json.writeNumberField("value", f.value()); + } + case Value.Text t -> { + json.writeStringField("type", "text"); + json.writeStringField("value", t.value()); + } + case Value.Blob b -> { + json.writeStringField("type", "blob"); + json.writeStringField("base64", Base64.getEncoder().encodeToString(b.bytes())); + } + } + json.writeEndObject(); + } + + private static Value firstValue(PipelineRespBody response) { + StreamResult.Ok ok = (StreamResult.Ok) response.results().get(0); + StreamResponse.Execute execute = (StreamResponse.Execute) ok.response(); + return execute.result().rows().get(0).values().get(0); + } +} diff --git a/libsql-jdbc/pom.xml b/libsql-jdbc/pom.xml index 2dafd3d..92aa7f3 100644 --- a/libsql-jdbc/pom.xml +++ b/libsql-jdbc/pom.xml @@ -32,6 +32,11 @@ assertj-core test + + org.jetbrains + jetCheck + test + org.flywaydb flyway-core diff --git a/libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParser.java b/libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParser.java index d0231e1..ffa8611 100644 --- a/libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParser.java +++ b/libsql-jdbc/src/main/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParser.java @@ -21,7 +21,7 @@ public class LibsqlUrlParser { private static final Set CREDENTIAL_QUERY_PARAMS = Set.of("authToken", "jwt", "password", "token"); - private static final Pattern USER_INFO = Pattern.compile("^(jdbc:[a-zA-Z]+:[a-zA-Z]+://)[^/@]*@"); + private static final Pattern USER_INFO = Pattern.compile("^(jdbc:[a-zA-Z]+:[a-zA-Z]+://)[^/?#]*@"); public static boolean acceptsUrl(String url) { if (url == null) return false; @@ -145,16 +145,28 @@ private static LibsqlClientConfig.Encoding resolveEncoding( } private static Duration resolveDuration( - String name, Map queryParams, Properties info, Duration defaultValue) { + String name, + Map queryParams, + Properties info, + Duration defaultValue) throws SQLException { if (queryParams.containsKey(name)) { - return Duration.ofMillis(Long.parseLong(queryParams.get(name))); + return parseDuration(name, queryParams.get(name)); } if (info != null && info.containsKey(name)) { - return Duration.ofMillis(Long.parseLong(info.getProperty(name))); + return parseDuration(name, info.getProperty(name)); } return defaultValue; } + /** Keeps a bad timeout a JDBC error instead of leaking {@link NumberFormatException}. */ + private static Duration parseDuration(String name, String value) throws SQLException { + try { + return Duration.ofMillis(Long.parseLong(value)); + } catch (NumberFormatException e) { + throw new SQLException("Invalid " + name + " '" + value + "': expected milliseconds", e); + } + } + private static String buildEndpoint(URI uri, String scheme) { int port = uri.getPort(); String path = uri.getPath() != null ? uri.getPath() : ""; diff --git a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/ConnectionLifecyclePropertyTest.java b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/ConnectionLifecyclePropertyTest.java new file mode 100644 index 0000000..515ad32 --- /dev/null +++ b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/ConnectionLifecyclePropertyTest.java @@ -0,0 +1,311 @@ +package io.github.magnusp.libsql.jdbc; + +import io.github.magnusp.libsql.client.LibsqlClient; +import io.github.magnusp.libsql.client.LibsqlClientConfig; +import io.github.magnusp.libsql.hrana.codec.Batch; +import io.github.magnusp.libsql.hrana.codec.BatchResult; +import io.github.magnusp.libsql.hrana.codec.Stmt; +import io.github.magnusp.libsql.hrana.codec.StmtResult; +import java.sql.SQLException; +import java.sql.Savepoint; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.jetbrains.jetCheck.Generator; +import org.jetbrains.jetCheck.ImperativeCommand; +import org.jetbrains.jetCheck.IntDistribution; +import org.jetbrains.jetCheck.PropertyChecker; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Model-based property for the connection transaction lifecycle. + * + *

jetCheck's {@link ImperativeCommand} support generates and shrinks command sequences while + * replaying them against a fresh connection. The model below tracks auto-commit, the optional open + * transaction, and the savepoint stack. The fake client never rejects a statement, so a failure is + * a driver state-machine bug rather than a server error. + */ +class ConnectionLifecyclePropertyTest { + + private static final int ITERATIONS = 1_000; + private static final LibsqlClientConfig CONFIG = + new LibsqlClientConfig("http://localhost:1", null); + + @Test + void generatedLifecyclesMatchTheTransactionModel() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .checkScenarios(ConnectionScenario::new); + } + + private enum ActionKind { + SET_AUTO_COMMIT_TRUE, + SET_AUTO_COMMIT_FALSE, + COMMIT, + ROLLBACK, + SET_SAVEPOINT, + RELEASE_LATEST, + ROLLBACK_LATEST, + EXECUTE, + CLOSE + } + + private record Action(ActionKind kind, String savepointName) {} + + private record State( + boolean closed, + boolean autoCommit, + boolean transactionActive, + boolean transactionStartedBySavepoint, + List savepoints) { + + static State initial() { + return new State(false, true, false, false, List.of()); + } + + boolean allows(Action action) { + if (action.kind() == ActionKind.CLOSE) { + return true; + } + if (closed) { + return false; + } + return switch (action.kind()) { + case COMMIT, ROLLBACK -> !autoCommit; + case RELEASE_LATEST, ROLLBACK_LATEST -> !autoCommit && !savepoints.isEmpty(); + default -> true; + }; + } + + State after(Action action) { + return switch (action.kind()) { + case SET_AUTO_COMMIT_TRUE -> autoCommit + ? this + : new State(false, true, false, false, List.of()); + case SET_AUTO_COMMIT_FALSE -> + new State(false, false, transactionActive, transactionStartedBySavepoint, savepoints); + case COMMIT, ROLLBACK -> new State(false, false, false, false, List.of()); + case SET_SAVEPOINT -> autoCommit + ? new State(false, false, true, true, List.of(action.savepointName())) + : new State( + false, + false, + true, + transactionStartedBySavepoint, + append(savepoints, action.savepointName())); + case RELEASE_LATEST -> { + int index = savepoints.lastIndexOf(last(savepoints)); + boolean started = index == 0 && transactionStartedBySavepoint; + yield new State( + false, + false, + started ? false : transactionActive, + started ? false : transactionStartedBySavepoint, + List.copyOf(savepoints.subList(0, index))); + } + case ROLLBACK_LATEST -> { + int index = savepoints.lastIndexOf(last(savepoints)); + yield new State( + false, + false, + transactionActive, + transactionStartedBySavepoint, + List.copyOf(savepoints.subList(0, index + 1))); + } + case EXECUTE -> autoCommit + ? this + : new State(false, false, true, transactionStartedBySavepoint, savepoints); + case CLOSE -> new State(true, autoCommit, false, false, List.of()); + }; + } + + private static List append(List values, String value) { + List copy = new ArrayList<>(values); + copy.add(value); + return List.copyOf(copy); + } + + private static String last(List values) { + return values.get(values.size() - 1); + } + } + + private static final class ConnectionScenario implements ImperativeCommand { + + @Override + public void performCommand(ImperativeCommand.Environment env) { + Fixture fixture = new Fixture(); + try { + env.executeCommands(IntDistribution.uniform(0, 50), commands(fixture)); + fixture.assertInvariants(); + } finally { + try { + fixture.connection.close(); + } catch (SQLException e) { + throw new AssertionError("could not close the connection", e); + } + } + } + } + + private record Command(Fixture fixture, ActionKind kind, String savepointName) + implements ImperativeCommand { + + @Override + public void performCommand(ImperativeCommand.Environment env) { + env.logMessage(kind + (savepointName == null ? "" : "(" + savepointName + ")")); + fixture.apply(new Action(kind, savepointName)); + } + } + + private static final class Fixture { + + private final RecordingClient client = new RecordingClient(); + private final LibsqlConnection connection = new LibsqlConnection(CONFIG, client); + private final List savepoints = new ArrayList<>(); + private State state = State.initial(); + + void apply(Action action) { + if ((action.kind() == ActionKind.RELEASE_LATEST || action.kind() == ActionKind.ROLLBACK_LATEST) + && state.savepoints().isEmpty()) { + return; + } + + boolean expectedToSucceed = state.allows(action); + try { + execute(action); + assertThat(expectedToSucceed) + .as("%s should have failed in %s", action.kind(), state) + .isTrue(); + state = state.after(action); + } catch (SQLException expected) { + assertThat(expectedToSucceed) + .as("unexpected SQLException from %s in %s", action.kind(), state) + .isFalse(); + } catch (RuntimeException unexpected) { + throw new AssertionError( + "unexpected " + unexpected + " from " + action.kind() + " in " + state, unexpected); + } + + assertInvariants(); + } + + private void execute(Action action) throws SQLException { + switch (action.kind()) { + case SET_AUTO_COMMIT_TRUE -> connection.setAutoCommit(true); + case SET_AUTO_COMMIT_FALSE -> connection.setAutoCommit(false); + case COMMIT -> connection.commit(); + case ROLLBACK -> connection.rollback(); + case SET_SAVEPOINT -> savepoints.add(connection.setSavepoint(action.savepointName())); + case RELEASE_LATEST -> { + connection.releaseSavepoint(lastSavepoint()); + savepoints.remove(savepoints.size() - 1); + } + case ROLLBACK_LATEST -> connection.rollback(lastSavepoint()); + case EXECUTE -> connection.executeStatement(new Stmt("SELECT 1")); + case CLOSE -> connection.close(); + } + } + + private Savepoint lastSavepoint() { + return savepoints.get(savepoints.size() - 1); + } + + void assertInvariants() { + try { + assertThat(connection.isClosed()).isEqualTo(state.closed()); + if (state.closed()) { + assertThatThrownBy(connection::getAutoCommit).isInstanceOf(SQLException.class); + } else { + assertThat(connection.getAutoCommit()).isEqualTo(state.autoCommit()); + } + } catch (SQLException e) { + throw new AssertionError("could not read connection state", e); + } + assertThat(state.autoCommit() && (!state.savepoints().isEmpty() || state.transactionActive())) + .as("auto-commit implies no transaction state, was %s", state) + .isFalse(); + assertThat(state.transactionStartedBySavepoint() + && !(state.transactionActive() + && !state.autoCommit() + && !state.savepoints().isEmpty())) + .as("savepoint-owned transaction must be active, was %s", state) + .isFalse(); + } + } + + private static Generator commands(Fixture fixture) { + return Generator.sampledFrom( + new Command(fixture, ActionKind.SET_AUTO_COMMIT_TRUE, null), + new Command(fixture, ActionKind.SET_AUTO_COMMIT_FALSE, null), + new Command(fixture, ActionKind.COMMIT, null), + new Command(fixture, ActionKind.ROLLBACK, null), + new Command(fixture, ActionKind.SET_SAVEPOINT, "a"), + new Command(fixture, ActionKind.SET_SAVEPOINT, "b"), + new Command(fixture, ActionKind.SET_SAVEPOINT, "dup"), + new Command(fixture, ActionKind.RELEASE_LATEST, null), + new Command(fixture, ActionKind.ROLLBACK_LATEST, null), + new Command(fixture, ActionKind.EXECUTE, null), + new Command(fixture, ActionKind.CLOSE, null)); + } + + private static final class RecordingClient implements LibsqlClient { + + private boolean openStream; + + @Override + public StmtResult executeOneShot(Stmt stmt) { + return emptyResult(); + } + + @Override + public StmtResult executeOnStream(Stmt stmt) { + openStream = true; + return emptyResult(); + } + + @Override + public BatchResult executeBatchOneShot(Batch batch) { + return emptyBatch(batch); + } + + @Override + public BatchResult executeBatchOnStream(Batch batch) { + openStream = true; + return emptyBatch(batch); + } + + @Override + public boolean hasOpenStream() { + return openStream; + } + + @Override + public void closeStream() { + openStream = false; + } + + @Override + public void close() { + openStream = false; + } + + private static StmtResult emptyResult() { + return new StmtResult(List.of(), List.of(), 0, null); + } + + private static BatchResult emptyBatch(Batch batch) { + Map results = new LinkedHashMap<>(); + for (int i = 0; i < batch.steps().size(); i++) { + if (batch.steps().get(i).stmt() != null) { + results.put(i, emptyResult()); + } + } + return new BatchResult(results, Map.of()); + } + } +} diff --git a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserPropertyTest.java b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserPropertyTest.java new file mode 100644 index 0000000..6c9da6a --- /dev/null +++ b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserPropertyTest.java @@ -0,0 +1,118 @@ +package io.github.magnusp.libsql.jdbc; + +import java.sql.SQLException; +import java.util.Properties; + +import org.jetbrains.jetCheck.Generator; +import org.jetbrains.jetCheck.IntDistribution; +import org.jetbrains.jetCheck.PropertyChecker; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Properties for URL parsing. + * + *

{@link LibsqlUrlParser#parse} is a JDBC boundary: any input the driver accepts must produce a + * config or a {@link SQLException}. An unchecked exception escaping here is a driver bug, because + * the caller cannot distinguish it from a programming error. + */ +class LibsqlUrlParserPropertyTest { + + private static final int ITERATIONS = 500; + + @Test + void parseNeverThrowsAnUnexpectedException() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(timeoutUrls(), url -> { + try { + LibsqlUrlParser.parse(url, new Properties()); + } catch (SQLException expected) { + // A rejected timeout is a caller error, not a driver crash. + } catch (RuntimeException unexpected) { + throw new AssertionError("unexpected " + unexpected + " for " + url, unexpected); + } + return true; + }); + } + + @Test + void arbitraryUrlsNeverThrowAnUnexpectedException() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(arbitraryUrls().map(body -> body.startsWith("jdbc:libsql:") + ? body + : "jdbc:libsql:" + body), url -> { + try { + LibsqlUrlParser.parse(url, new Properties()); + } catch (SQLException expected) { + // A malformed URL is a caller error, not a driver crash. + } catch (RuntimeException unexpected) { + throw new AssertionError("unexpected " + unexpected + " for " + url, unexpected); + } + return true; + }); + } + + @Test + void parseNeverThrowsAnUnexpectedExceptionFromProperties() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(timeoutValues(), value -> { + Properties properties = new Properties(); + properties.setProperty("connectTimeout", value); + try { + LibsqlUrlParser.parse("jdbc:libsql:http://localhost:8080", properties); + } catch (SQLException expected) { + // A rejected timeout is a caller error, not a driver crash. + } catch (RuntimeException unexpected) { + throw new AssertionError("unexpected " + unexpected + " for " + value, unexpected); + } + return true; + }); + } + + @Test + void metadataUrlIsIdempotent() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(arbitraryUrls(), url -> { + String once = LibsqlUrlParser.metadataUrl(url); + return once.equals(LibsqlUrlParser.metadataUrl(once)); + }); + } + + private static Generator timeoutUrls() { + return timeoutValues().map(value -> "jdbc:libsql:http://localhost:8080/dev/tenant?connectTimeout=" + value); + } + + private static Generator timeoutValues() { + return Generator.anyOf( + Generator.stringsOf(IntDistribution.uniform(0, 20), Generator.digits()), + Generator.stringsOf(IntDistribution.uniform(0, 16), Generator.charsInRange((char) 0, (char) 127)), + Generator.constant(""), + Generator.constant("-1"), + Generator.constant("+5"), + Generator.constant("999999999999999999999999")); + } + + private static Generator arbitraryUrls() { + Generator fragments = Generator.anyOf( + Generator.constant("jdbc:libsql:"), + Generator.constant("http://"), + Generator.constant("localhost:8080"), + Generator.constant("/dev/tenant"), + Generator.constant("?"), + Generator.constant("&"), + Generator.constant("="), + Generator.constant("authToken"), + Generator.constant("token"), + Generator.constant("jwt"), + Generator.constant("connectTimeout"), + Generator.constant("user:pass@"), + Generator.stringsOf(IntDistribution.uniform(0, 8), Generator.charsInRange((char) 0, (char) 127))); + return Generator.listsOf(IntDistribution.uniform(0, 10), fragments) + .map(parts -> String.join("", parts)); + } +} diff --git a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserTest.java b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserTest.java index 04bd891..76ad967 100644 --- a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserTest.java +++ b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/LibsqlUrlParserTest.java @@ -54,6 +54,23 @@ void testConnectTimeoutStillWinsAlongsideSocketTimeout() throws SQLException { assertThat(config.connectTimeout().toMillis()).isEqualTo(2500); } + @Test + void testRejectsANonNumericTimeoutAsAJDBCError() { + assertThatThrownBy(() -> LibsqlUrlParser.parse( + "jdbc:libsql:http://localhost:8080/dev/tenant?connectTimeout=soon", + new Properties())) + .isInstanceOf(SQLException.class) + .hasMessageContaining("connectTimeout") + .hasCauseInstanceOf(NumberFormatException.class); + + Properties properties = new Properties(); + properties.setProperty("busyTimeout", "later"); + assertThatThrownBy(() -> LibsqlUrlParser.parse("jdbc:libsql:http://localhost:8080", properties)) + .isInstanceOf(SQLException.class) + .hasMessageContaining("busyTimeout") + .hasCauseInstanceOf(NumberFormatException.class); + } + @Test void testParseWebSocketUrlKeepsTheSchemeAndPath() throws SQLException { LibsqlClientConfig config = LibsqlUrlParser.parse( @@ -152,6 +169,18 @@ void testMetadataUrlRedactsUserInfo() { .isEqualTo("jdbc:libsql:http://localhost:8080/dev"); } + @Test + void testMetadataUrlRedactsRepeatedUserInfo() { + assertThat(LibsqlUrlParser.metadataUrl("jdbc:libsql:http://user:pass@user:pass@localhost:8080/dev")) + .isEqualTo("jdbc:libsql:http://localhost:8080/dev"); + } + + @Test + void testMetadataUrlDoesNotTreatAnAtSignInTheQueryAsUserInfo() { + String url = "jdbc:libsql:http://localhost:8080/dev?filter=a@b"; + assertThat(LibsqlUrlParser.metadataUrl(url)).isEqualTo(url); + } + @Test void testMetadataUrlLeavesANonCredentialUrlUnchanged() { String url = "jdbc:sqlite:http://localhost:8080/dev/tenant?connectTimeout=5000"; diff --git a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlMetadataParsingPropertyTest.java b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlMetadataParsingPropertyTest.java new file mode 100644 index 0000000..6fd4649 --- /dev/null +++ b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlMetadataParsingPropertyTest.java @@ -0,0 +1,69 @@ +package io.github.magnusp.libsql.jdbc; + +import io.github.magnusp.libsql.hrana.codec.Col; +import org.jetbrains.jetCheck.Generator; +import org.jetbrains.jetCheck.IntDistribution; +import org.jetbrains.jetCheck.PropertyChecker; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Properties for metadata text that originates in the database rather than in the driver. + * + *

SQLite lets a schema declare almost any type name. The generated domain is syntactically + * valid SQLite type text: an arbitrary string with no parentheses, or a parenthesized precision. + * A malformed precision such as {@code DECIMAL(abc)} is deliberately excluded: {@link + * LibsqlResultSetMetaDataTest} currently pins a {@link NumberFormatException} for that input. + */ +class SqlMetadataParsingPropertyTest { + + private static final int ITERATIONS = 500; + + @Test + void declaredTypeNeverThrowsOnValidDeclaredText() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(declaredTypes(), declType -> { + DeclaredType type = DeclaredType.of(new Col("c", declType)); + assertThat(type.declaredTypeName()).isNotNull(); + assertThat(type.typeName()).isNotNull(); + assertThat(type.precision()).isNotNegative(); + assertThat(type.scale()).isNotNegative(); + return true; + }); + } + + @Test + void autoIncrementDetectionNeverThrowsOnArbitraryCreateStatements() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(Generator.zipWith(arbitraryText(), arbitraryText(), Pair::new), pair -> { + SqlAutoIncrement.isAutoIncrementColumn(pair.createSql(), pair.columnName()); + return true; + }); + } + + private record Pair(String createSql, String columnName) {} + + private static Generator declaredTypes() { + return Generator.anyOf( + Generator.stringsOf( + IntDistribution.uniform(0, 24), Generator.charsInRange((char) 0x20, (char) 0x7E)) + .map(text -> text.replace('(', '_').replace(')', '_')), + Generator.constant("INTEGER"), + Generator.constant("DECIMAL(10,2)"), + Generator.constant("VARCHAR(255)"), + Generator.constant("BIG INT"), + Generator.constant(" NUMERIC "), + Generator.zipWith( + Generator.integers(0, 1_000_000), + Generator.integers(0, 1_000_000), + (precision, scale) -> "DECIMAL(" + precision + "," + scale + ")")); + } + + private static Generator arbitraryText() { + return Generator.stringsOf( + IntDistribution.uniform(0, 80), Generator.charsInRange((char) 0, Character.MAX_VALUE)); + } +} diff --git a/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlScriptPropertyTest.java b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlScriptPropertyTest.java new file mode 100644 index 0000000..86ea931 --- /dev/null +++ b/libsql-jdbc/src/test/java/io/github/magnusp/libsql/jdbc/SqlScriptPropertyTest.java @@ -0,0 +1,157 @@ +package io.github.magnusp.libsql.jdbc; + +import java.util.List; + +import org.jetbrains.jetCheck.Generator; +import org.jetbrains.jetCheck.IntDistribution; +import org.jetbrains.jetCheck.PropertyChecker; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Properties for the statement splitter and the local tokenizer. + * + *

The generated scripts contain the cases the hand-written tests enumerate: semicolons in + * quoted text, comments and trigger bodies. The point is coverage of combinations, not a + * replacement for the named examples in {@link SqlScriptTest}. + */ +class SqlScriptPropertyTest { + + private static final int ITERATIONS = 500; + + @Test + void splittingReproducesEveryGeneratedStatement() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(scripts(), script -> { + assertThat(SqlScript.split(script.sql())) + .as("split of %s", script.sql()) + .containsExactlyElementsOf(script.expected()); + return true; + }); + } + + @Test + void splittingNeverThrowsOnArbitraryText() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(arbitraryText(), sql -> SqlScript.split(sql) != null); + } + + @Test + void splittingNeverReturnsABlankStatement() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(arbitraryText(), sql -> { + assertThat(SqlScript.split(sql)).allSatisfy(statement -> assertThat(statement) + .isNotBlank()); + return true; + }); + } + + @Test + void everySplitStatementSplitsToItself() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(arbitraryText(), sql -> { + for (String statement : SqlScript.split(sql)) { + assertThat(SqlScript.split(statement)) + .as("second split of %s", statement) + .containsExactly(statement); + } + return true; + }); + } + + @Test + void placeholderCountIgnoresQuotedAndCommentedQuestionMarks() { + PropertyChecker.customized() + .withIterationCount(ITERATIONS) + .forAll(Generator.listsOf(IntDistribution.uniform(0, 12), tokenizerFragments()), fragments -> { + String sql = String.join(" ", fragments); + long expected = fragments.stream().filter("?"::equals).count(); + assertThat(SqlTokenizer.placeholderCount(sql)) + .as("placeholders in %s", sql) + .isEqualTo((int) expected); + return true; + }); + } + + private record Script(String sql, List expected) {} + + private static Generator